From 07a51c7dc9dd37205368eb0653e23e33cc04406f Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Wed, 27 Mar 2024 12:11:39 +0100 Subject: feat(tvix/store): add rstest-based PathInfoService tests This introduces rstest-based tests. We also add fixtures for creating some BlobService / DirectoryService out of thin air. To test a PathInfoService, we don't really care too much about its internal storage - ensuring they work is up to the castore tests. Change-Id: Ia62af076ef9c9fbfcf8b020a781454ad299d972e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11272 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster --- tvix/Cargo.lock | 2 + tvix/Cargo.nix | 8 ++ tvix/castore/src/blobservice/mod.rs | 2 +- tvix/castore/src/directoryservice/mod.rs | 2 +- tvix/store/Cargo.toml | 2 + tvix/store/src/lib.rs | 6 ++ tvix/store/src/pathinfoservice/mod.rs | 3 + tvix/store/src/pathinfoservice/tests/mod.rs | 112 ++++++++++++++++++++++++++ tvix/store/src/pathinfoservice/tests/utils.rs | 60 ++++++++++++++ tvix/store/src/tests/fixtures.rs | 23 +++++- 10 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 tvix/store/src/pathinfoservice/tests/mod.rs create mode 100644 tvix/store/src/pathinfoservice/tests/utils.rs diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index 797cdd5d54dd..4e95d39b6a98 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -4014,6 +4014,8 @@ dependencies = [ "prost 0.12.3", "prost-build", "reqwest", + "rstest", + "rstest_reuse", "sha2", "sled", "tempfile", diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 001b63d526fb..e6920342d735 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -12988,6 +12988,14 @@ rec { } ]; devDependencies = [ + { + name = "rstest"; + packageId = "rstest"; + } + { + name = "rstest_reuse"; + packageId = "rstest_reuse"; + } { name = "tempfile"; packageId = "tempfile"; diff --git a/tvix/castore/src/blobservice/mod.rs b/tvix/castore/src/blobservice/mod.rs index e9cbd190623a..4ba56a4af731 100644 --- a/tvix/castore/src/blobservice/mod.rs +++ b/tvix/castore/src/blobservice/mod.rs @@ -14,7 +14,7 @@ mod object_store; mod sled; #[cfg(test)] -mod tests; +pub mod tests; pub use self::chunked_reader::ChunkedReader; pub use self::combinator::CombinedBlobService; diff --git a/tvix/castore/src/directoryservice/mod.rs b/tvix/castore/src/directoryservice/mod.rs index 997b47e502e4..e9ac73173988 100644 --- a/tvix/castore/src/directoryservice/mod.rs +++ b/tvix/castore/src/directoryservice/mod.rs @@ -9,7 +9,7 @@ mod memory; mod simple_putter; mod sled; #[cfg(test)] -mod tests; +pub mod tests; mod traverse; mod utils; diff --git a/tvix/store/Cargo.toml b/tvix/store/Cargo.toml index 2b93cd7a528c..367e63c21e9f 100644 --- a/tvix/store/Cargo.toml +++ b/tvix/store/Cargo.toml @@ -48,6 +48,8 @@ prost-build = "0.12.1" tonic-build = "0.11.0" [dev-dependencies] +rstest = "0.18.2" +rstest_reuse = "0.6.0" test-case = "3.3.1" tempfile = "3.3.0" tokio-retry = "0.3.0" diff --git a/tvix/store/src/lib.rs b/tvix/store/src/lib.rs index 2fa86ff6a468..8c32aaf885e8 100644 --- a/tvix/store/src/lib.rs +++ b/tvix/store/src/lib.rs @@ -6,3 +6,9 @@ pub mod utils; #[cfg(test)] mod tests; + +// That's what the rstest_reuse README asks us do, and fails about being unable +// to find rstest_reuse in crate root. +#[cfg(test)] +#[allow(clippy::single_component_path_imports)] +use rstest_reuse; diff --git a/tvix/store/src/pathinfoservice/mod.rs b/tvix/store/src/pathinfoservice/mod.rs index 1f52d20caed7..c334c8dc5631 100644 --- a/tvix/store/src/pathinfoservice/mod.rs +++ b/tvix/store/src/pathinfoservice/mod.rs @@ -7,6 +7,9 @@ mod sled; #[cfg(any(feature = "fuse", feature = "virtiofs"))] mod fs; +#[cfg(test)] +mod tests; + use futures::stream::BoxStream; use tonic::async_trait; use tvix_castore::proto as castorepb; diff --git a/tvix/store/src/pathinfoservice/tests/mod.rs b/tvix/store/src/pathinfoservice/tests/mod.rs new file mode 100644 index 000000000000..a3035d094d1e --- /dev/null +++ b/tvix/store/src/pathinfoservice/tests/mod.rs @@ -0,0 +1,112 @@ +//! This contains test scenarios that a given [PathInfoService] needs to pass. +//! We use [rstest] and [rstest_reuse] to provide all services we want to test +//! against, and then apply this template to all test functions. + +use rstest::*; +use rstest_reuse::{self, *}; +use std::sync::Arc; +use tvix_castore::proto as castorepb; +use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService}; + +use super::PathInfoService; +use crate::proto::PathInfo; +use crate::tests::fixtures::DUMMY_OUTPUT_HASH; + +mod utils; +use self::utils::make_grpc_path_info_service_client; + +/// Convenience type alias batching all three servives together. +#[allow(clippy::upper_case_acronyms)] +type BSDSPS = ( + Arc, + Arc, + Box, +); + +/// Creates a PathInfoService using a new Memory{Blob,Directory}Service. +/// We return a 3-tuple containing all of them, as some tests want to interact +/// with all three. +pub async fn make_path_info_service(uri: &str) -> BSDSPS { + let blob_service: Arc = tvix_castore::blobservice::from_addr("memory://") + .await + .unwrap() + .into(); + let directory_service: Arc = + tvix_castore::directoryservice::from_addr("memory://") + .await + .unwrap() + .into(); + + ( + blob_service.clone(), + directory_service.clone(), + crate::pathinfoservice::from_addr(uri, blob_service, directory_service) + .await + .unwrap(), + ) +} + +#[template] +#[rstest] +#[case::memory(make_path_info_service("memory://").await)] +#[case::grpc(make_grpc_path_info_service_client().await)] +#[case::sled(make_path_info_service("sled://").await)] +pub fn path_info_services( + #[case] services: ( + impl BlobService, + impl DirectoryService, + impl PathInfoService, + ), +) { +} + +// FUTUREWORK: add more tests rejecting invalid PathInfo messages. +// A subset of them should also ensure references to other PathInfos, or +// elements in {Blob,Directory}Service do exist. + +/// Trying to get a non-existent PathInfo should return Ok(None). +#[apply(path_info_services)] +#[tokio::test] +async fn not_found(services: BSDSPS) { + let (_, _, path_info_service) = services; + assert!(path_info_service + .get(DUMMY_OUTPUT_HASH) + .await + .expect("must succeed") + .is_none()); +} + +/// Put a PathInfo into the store, get it back. +#[apply(path_info_services)] +#[tokio::test] +async fn put_get(services: BSDSPS) { + let (_, _, path_info_service) = services; + + let path_info = PathInfo { + node: Some(castorepb::Node { + node: Some(castorepb::node::Node::Symlink(castorepb::SymlinkNode { + name: "00000000000000000000000000000000-foo".into(), + target: "doesntmatter".into(), + })), + }), + ..Default::default() + }; + + // insert + let resp = path_info_service + .put(path_info.clone()) + .await + .expect("must succeed"); + + // expect the returned PathInfo to be equal (for now) + // in the future, some stores might add additional fields/signatures. + assert_eq!(path_info, resp); + + // get it back + let resp = path_info_service + .get(DUMMY_OUTPUT_HASH) + .await + .expect("must succeed"); + + assert_eq!(Some(path_info), resp); +} diff --git a/tvix/store/src/pathinfoservice/tests/utils.rs b/tvix/store/src/pathinfoservice/tests/utils.rs new file mode 100644 index 000000000000..31ec57aade73 --- /dev/null +++ b/tvix/store/src/pathinfoservice/tests/utils.rs @@ -0,0 +1,60 @@ +use std::sync::Arc; + +use tonic::transport::{Endpoint, Server, Uri}; + +use crate::{ + pathinfoservice::{GRPCPathInfoService, MemoryPathInfoService, PathInfoService}, + proto::{ + path_info_service_client::PathInfoServiceClient, + path_info_service_server::PathInfoServiceServer, GRPCPathInfoServiceWrapper, + }, + tests::fixtures::{blob_service, directory_service}, +}; + +/// Constructs and returns a gRPC PathInfoService. +/// We also return memory-based {Blob,Directory}Service, +/// as the consumer of this function accepts a 3-tuple. +pub async fn make_grpc_path_info_service_client() -> super::BSDSPS { + let (left, right) = tokio::io::duplex(64); + + let blob_service = blob_service(); + let directory_service = directory_service(); + + // spin up a server, which will only connect once, to the left side. + tokio::spawn({ + let blob_service = blob_service.clone(); + let directory_service = directory_service.clone(); + async move { + let path_info_service: Arc = + Arc::from(MemoryPathInfoService::new(blob_service, directory_service)); + + // spin up a new DirectoryService + let mut server = Server::builder(); + let router = server.add_service(PathInfoServiceServer::new( + GRPCPathInfoServiceWrapper::new(path_info_service), + )); + + router + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(left))) + .await + } + }); + + // Create a client, connecting to the right side. The URI is unused. + let mut maybe_right = Some(right); + + let path_info_service = Box::new(GRPCPathInfoService::from_client( + PathInfoServiceClient::new( + Endpoint::try_from("http://[::]:50051") + .unwrap() + .connect_with_connector(tower::service_fn(move |_: Uri| { + let right = maybe_right.take().unwrap(); + async move { Ok::<_, std::io::Error>(right) } + })) + .await + .unwrap(), + ), + )); + + (blob_service, directory_service, path_info_service) +} diff --git a/tvix/store/src/tests/fixtures.rs b/tvix/store/src/tests/fixtures.rs index 233f98591528..500ac0aa5bc2 100644 --- a/tvix/store/src/tests/fixtures.rs +++ b/tvix/store/src/tests/fixtures.rs @@ -1,8 +1,17 @@ use lazy_static::lazy_static; +use rstest::*; +use std::sync::Arc; pub use tvix_castore::fixtures::*; -use tvix_castore::proto as castorepb; +use tvix_castore::{ + blobservice::{BlobService, MemoryBlobService}, + directoryservice::{DirectoryService, MemoryDirectoryService}, + proto as castorepb, +}; -use crate::proto::{nar_info::ca, nar_info::Ca, NarInfo, PathInfo}; +use crate::proto::{ + nar_info::{ca, Ca}, + NarInfo, PathInfo, +}; pub const DUMMY_NAME: &str = "00000000000000000000000000000000-dummy"; pub const DUMMY_OUTPUT_HASH: [u8; 20] = [0; 20]; @@ -121,3 +130,13 @@ lazy_static! { ..PATH_INFO_WITHOUT_NARINFO.clone() }; } + +#[fixture] +pub(crate) fn blob_service() -> Arc { + Arc::from(MemoryBlobService::default()) +} + +#[fixture] +pub(crate) fn directory_service() -> Arc { + Arc::from(MemoryDirectoryService::default()) +} -- cgit 1.4.1