about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-03-27T11·11+0100
committerflokli <flokli@flokli.de>2024-03-28T07·02+0000
commit07a51c7dc9dd37205368eb0653e23e33cc04406f (patch)
treeb328028d3219579ac3c10b558eefa999d2fed262
parent93dc5df957129b48e95c92c5c31bca3da5488208 (diff)
feat(tvix/store): add rstest-based PathInfoService tests r/7788
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 <cbrewster@hey.com>
-rw-r--r--tvix/Cargo.lock2
-rw-r--r--tvix/Cargo.nix8
-rw-r--r--tvix/castore/src/blobservice/mod.rs2
-rw-r--r--tvix/castore/src/directoryservice/mod.rs2
-rw-r--r--tvix/store/Cargo.toml2
-rw-r--r--tvix/store/src/lib.rs6
-rw-r--r--tvix/store/src/pathinfoservice/mod.rs3
-rw-r--r--tvix/store/src/pathinfoservice/tests/mod.rs112
-rw-r--r--tvix/store/src/pathinfoservice/tests/utils.rs60
-rw-r--r--tvix/store/src/tests/fixtures.rs23
10 files changed, 216 insertions, 4 deletions
diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock
index 797cdd5d54..4e95d39b6a 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 001b63d526..e6920342d7 100644
--- a/tvix/Cargo.nix
+++ b/tvix/Cargo.nix
@@ -12989,6 +12989,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 e9cbd19062..4ba56a4af7 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 997b47e502..e9ac731739 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 2b93cd7a52..367e63c21e 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 2fa86ff6a4..8c32aaf885 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 1f52d20cae..c334c8dc56 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 0000000000..a3035d094d
--- /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<dyn BlobService>,
+    Arc<dyn DirectoryService>,
+    Box<dyn PathInfoService>,
+);
+
+/// 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<dyn BlobService> = tvix_castore::blobservice::from_addr("memory://")
+        .await
+        .unwrap()
+        .into();
+    let directory_service: Arc<dyn DirectoryService> =
+        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 0000000000..31ec57aade
--- /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<dyn PathInfoService> =
+                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 233f985915..500ac0aa5b 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<dyn BlobService> {
+    Arc::from(MemoryBlobService::default())
+}
+
+#[fixture]
+pub(crate) fn directory_service() -> Arc<dyn DirectoryService> {
+    Arc::from(MemoryDirectoryService::default())
+}