From bc42c355cf8d83120b16214f3a1a17f67851d157 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 17 May 2024 15:24:08 +0200 Subject: refactor(tvix/store/pathinfo): test with PathInfoService directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since cl/…, a PathInfoService doesn't need to implement `calculate_nar` anymore, so most of them don't actually have a handle to a {Blob,Directory}Service anymore. This means, we can simplify the construction of them for test cases a lot. Change-Id: I100e9e1c9b00a049b4d6136c57aad4cdb04461c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11691 Reviewed-by: Connor Brewster Tested-by: BuildkiteCI --- tvix/store/src/pathinfoservice/tests/mod.rs | 77 +++++++-------------------- tvix/store/src/pathinfoservice/tests/utils.rs | 35 +++++++----- 2 files changed, 41 insertions(+), 71 deletions(-) (limited to 'tvix/store/src/pathinfoservice/tests') diff --git a/tvix/store/src/pathinfoservice/tests/mod.rs b/tvix/store/src/pathinfoservice/tests/mod.rs index 26166d1b75ca..061655e4bafc 100644 --- a/tvix/store/src/pathinfoservice/tests/mod.rs +++ b/tvix/store/src/pathinfoservice/tests/mod.rs @@ -4,62 +4,30 @@ 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::pathinfoservice::MemoryPathInfoService; +use crate::pathinfoservice::SledPathInfoService; use crate::proto::PathInfo; use crate::tests::fixtures::DUMMY_PATH_DIGEST; +use tvix_castore::proto as castorepb; mod utils; pub 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(), - ) -} +#[cfg(all(feature = "cloud", feature = "integration"))] +use self::utils::make_bigtable_path_info_service; #[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)] -#[cfg_attr(all(feature = "cloud",feature="integration"), case::bigtable(make_path_info_service("bigtable://instance-1?project_id=project-1&table_name=table-1&family_name=cf1").await))] -pub fn path_info_services( - #[case] services: ( - impl BlobService, - impl DirectoryService, - impl PathInfoService, - ), -) { -} +#[case::memory(MemoryPathInfoService::default())] +#[case::grpc({ + let (_, _, svc) = make_grpc_path_info_service_client().await; + svc +})] +#[case::sled(SledPathInfoService::new_temporary().unwrap())] +#[cfg_attr(all(feature = "cloud",feature="integration"), case::bigtable(make_bigtable_path_info_service().await))] +pub fn path_info_services(#[case] svc: impl PathInfoService) {} // FUTUREWORK: add more tests rejecting invalid PathInfo messages. // A subset of them should also ensure references to other PathInfos, or @@ -68,9 +36,8 @@ pub fn path_info_services( /// 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 +async fn not_found(svc: impl PathInfoService) { + assert!(svc .get(DUMMY_PATH_DIGEST) .await .expect("must succeed") @@ -80,9 +47,7 @@ async fn not_found(services: BSDSPS) { /// 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; - +async fn put_get(svc: impl PathInfoService) { let path_info = PathInfo { node: Some(castorepb::Node { node: Some(castorepb::node::Node::Symlink(castorepb::SymlinkNode { @@ -94,20 +59,14 @@ async fn put_get(services: BSDSPS) { }; // insert - let resp = path_info_service - .put(path_info.clone()) - .await - .expect("must succeed"); + let resp = svc.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_PATH_DIGEST) - .await - .expect("must succeed"); + let resp = svc.get(DUMMY_PATH_DIGEST).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 index 30c5902b610f..ee170468d1d2 100644 --- a/tvix/store/src/pathinfoservice/tests/utils.rs +++ b/tvix/store/src/pathinfoservice/tests/utils.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use tonic::transport::{Endpoint, Server, Uri}; +use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService}; use crate::{ nar::{NarCalculationService, SimpleRenderer}, @@ -15,7 +16,8 @@ use crate::{ /// 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 { +pub async fn make_grpc_path_info_service_client( +) -> (impl BlobService, impl DirectoryService, GRPCPathInfoService) { let (left, right) = tokio::io::duplex(64); let blob_service = blob_service(); @@ -47,18 +49,27 @@ pub async fn make_grpc_path_info_service_client() -> super::BSDSPS { // 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(), - ), + let path_info_service = 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) } + +#[cfg(all(feature = "cloud", feature = "integration"))] +pub(crate) async fn make_bigtable_path_info_service( +) -> crate::pathinfoservice::BigtablePathInfoService { + use crate::pathinfoservice::bigtable::BigtableParameters; + use crate::pathinfoservice::BigtablePathInfoService; + + BigtablePathInfoService::connect(BigtableParameters::default_for_tests()) + .await + .unwrap() +} -- cgit 1.4.1