refactor(tvix/castore): move gRPC directorysvc tests off tonic_mock

The tests were using the gRPC Service/Server interface, not the gRPC
clients.

Use our new gen_directorysvc_grpc_client() function to spin up a server
in a separate thread, and test via that client, rather than *just* the
server implementation.

This is more correct, because we're now really exercising the gRPC
stack, and some of the stream implementations are also only implemented
on the client.

Change-Id: I99434a2399856a44c9e6988d0b720f7a429d0ad1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9560
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This commit is contained in:
Florian Klink 2023-10-07 10:26:36 +02:00 committed by flokli
parent a77914db73
commit 3b620e6d0c

View file

@ -1,26 +1,25 @@
use crate::fixtures::{DIRECTORY_A, DIRECTORY_B, DIRECTORY_C}; use crate::fixtures::{DIRECTORY_A, DIRECTORY_B, DIRECTORY_C};
use crate::proto::directory_service_server::DirectoryService as GRPCDirectoryService; use crate::proto::directory_service_client::DirectoryServiceClient;
use crate::proto::get_directory_request::ByWhat; use crate::proto::get_directory_request::ByWhat;
use crate::proto::GetDirectoryRequest;
use crate::proto::{Directory, DirectoryNode, SymlinkNode}; use crate::proto::{Directory, DirectoryNode, SymlinkNode};
use crate::proto::{GRPCDirectoryServiceWrapper, GetDirectoryRequest}; use crate::utils::gen_directorysvc_grpc_client;
use crate::utils::gen_directory_service; use tempfile::TempDir;
use tokio_stream::StreamExt; use tokio_stream::StreamExt;
use tonic::transport::Channel;
use tonic::Status; use tonic::Status;
fn gen_grpc_service() -> GRPCDirectoryServiceWrapper {
let directory_service = gen_directory_service();
GRPCDirectoryServiceWrapper::from(directory_service)
}
/// Send the specified GetDirectoryRequest. /// Send the specified GetDirectoryRequest.
/// Returns an error in the case of an error response, or an error in one of /// Returns an error in the case of an error response, or an error in one of
// the items in the stream, or a Vec<Directory> in the case of a successful // the items in the stream, or a Vec<Directory> in the case of a successful
/// request. /// request.
async fn get_directories<S: GRPCDirectoryService>( async fn get_directories(
svc: &S, grpc_client: &mut DirectoryServiceClient<Channel>,
get_directory_request: GetDirectoryRequest, get_directory_request: GetDirectoryRequest,
) -> Result<Vec<Directory>, Status> { ) -> Result<Vec<Directory>, Status> {
let resp = svc.get(tonic::Request::new(get_directory_request)).await; let resp = grpc_client
.get(tonic::Request::new(get_directory_request))
.await;
// if the response is an error itself, return the error, otherwise unpack // if the response is an error itself, return the error, otherwise unpack
let stream = match resp { let stream = match resp {
@ -38,47 +37,53 @@ async fn get_directories<S: GRPCDirectoryService>(
/// Trying to get a non-existent Directory should return a not found error. /// Trying to get a non-existent Directory should return a not found error.
#[tokio::test] #[tokio::test]
async fn not_found() { async fn not_found() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
let resp = service let resp = grpc_client
.get(tonic::Request::new(GetDirectoryRequest { .get(tonic::Request::new(GetDirectoryRequest {
by_what: Some(ByWhat::Digest(DIRECTORY_A.digest().into())), by_what: Some(ByWhat::Digest(DIRECTORY_A.digest().into())),
..Default::default() ..Default::default()
})) }))
.await; .await;
let mut rx = resp.expect("must succeed").into_inner().into_inner(); let stream = resp.expect("must succeed").into_inner();
let items: Vec<_> = stream.collect().await;
// The stream should contain one element, an error with Code::NotFound. // The stream should contain one element, an error with Code::NotFound.
let item = rx assert_eq!(1, items.len());
.recv() let item = items[0].clone();
.await
.expect("must be some")
.expect_err("must be err");
assert_eq!(item.code(), tonic::Code::NotFound);
// … and nothing else assert!(item.is_err(), "must be err");
assert!(rx.recv().await.is_none()); assert_eq!(
tonic::Code::NotFound,
item.unwrap_err().code(),
"must be err"
);
} }
/// Put a Directory into the store, get it back. /// Put a Directory into the store, get it back.
#[tokio::test] #[tokio::test]
async fn put_get() { async fn put_get() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
let streaming_request = tonic_mock::streaming_request(vec![DIRECTORY_A.clone()]); // send directory A.
let put_resp = service let put_resp = {
.put(streaming_request) grpc_client
.await .put(tokio_stream::once(DIRECTORY_A.clone()))
.expect("must succeed") .await
.into_inner(); .expect("must succeed")
.into_inner()
};
// the sent root_digest should match the calculated digest // the sent root_digest should match the calculated digest
assert_eq!(put_resp.root_digest, DIRECTORY_A.digest().to_vec()); assert_eq!(put_resp.root_digest, DIRECTORY_A.digest().to_vec());
// get it back // get it back
let items = get_directories( let items = get_directories(
&service, &mut grpc_client,
GetDirectoryRequest { GetDirectoryRequest {
by_what: Some(ByWhat::Digest(DIRECTORY_A.digest().into())), by_what: Some(ByWhat::Digest(DIRECTORY_A.digest().into())),
..Default::default() ..Default::default()
@ -93,33 +98,36 @@ async fn put_get() {
/// Put multiple Directories into the store, and get them back /// Put multiple Directories into the store, and get them back
#[tokio::test] #[tokio::test]
async fn put_get_multiple() { async fn put_get_multiple() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
// sending "b" (which refers to "a") without sending "a" first should fail. // sending "b" (which refers to "a") without sending "a" first should fail.
let put_resp = service let put_resp = {
.put(tonic_mock::streaming_request(vec![DIRECTORY_B.clone()])) grpc_client
.await .put(tokio_stream::once(DIRECTORY_B.clone()))
.expect_err("must fail"); .await
.expect_err("must fail")
};
assert_eq!(tonic::Code::InvalidArgument, put_resp.code()); assert_eq!(tonic::Code::InvalidArgument, put_resp.code());
// sending "a", then "b" should succeed, and the response should contain the digest of b. // sending "a", then "b" should succeed, and the response should contain the digest of b.
let put_resp = service let put_resp = {
.put(tonic_mock::streaming_request(vec![ grpc_client
DIRECTORY_A.clone(), .put(tokio_stream::iter(vec![
DIRECTORY_B.clone(), DIRECTORY_A.clone(),
])) DIRECTORY_B.clone(),
.await ]))
.expect("must succeed"); .await
.expect("must succeed")
.into_inner()
};
assert_eq!( assert_eq!(DIRECTORY_B.digest().to_vec(), put_resp.root_digest);
DIRECTORY_B.digest().to_vec(),
put_resp.into_inner().root_digest
);
// now, request b, first in non-recursive mode. // now, request b, first in non-recursive mode.
let items = get_directories( let items = get_directories(
&service, &mut grpc_client,
GetDirectoryRequest { GetDirectoryRequest {
recursive: false, recursive: false,
by_what: Some(ByWhat::Digest(DIRECTORY_B.digest().into())), by_what: Some(ByWhat::Digest(DIRECTORY_B.digest().into())),
@ -133,7 +141,7 @@ async fn put_get_multiple() {
// now, request b, but in recursive mode. // now, request b, but in recursive mode.
let items = get_directories( let items = get_directories(
&service, &mut grpc_client,
GetDirectoryRequest { GetDirectoryRequest {
recursive: true, recursive: true,
by_what: Some(ByWhat::Digest(DIRECTORY_B.digest().into())), by_what: Some(ByWhat::Digest(DIRECTORY_B.digest().into())),
@ -149,18 +157,21 @@ async fn put_get_multiple() {
/// Put multiple Directories into the store, and omit duplicates. /// Put multiple Directories into the store, and omit duplicates.
#[tokio::test] #[tokio::test]
async fn put_get_dedup() { async fn put_get_dedup() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
// Send "A", then "C", which refers to "A" two times // Send "A", then "C", which refers to "A" two times
// Pretend we're a dumb client sending A twice. // Pretend we're a dumb client sending A twice.
let put_resp = service let put_resp = {
.put(tonic_mock::streaming_request(vec![ grpc_client
DIRECTORY_A.clone(), .put(tokio_stream::iter(vec![
DIRECTORY_A.clone(), DIRECTORY_A.clone(),
DIRECTORY_C.clone(), DIRECTORY_A.clone(),
])) DIRECTORY_C.clone(),
.await ]))
.expect("must succeed"); .await
.expect("must succeed")
};
assert_eq!( assert_eq!(
DIRECTORY_C.digest().to_vec(), DIRECTORY_C.digest().to_vec(),
@ -169,7 +180,7 @@ async fn put_get_dedup() {
// Ask for "C" recursively. We expect to only get "A" once, as there's no point sending it twice. // Ask for "C" recursively. We expect to only get "A" once, as there's no point sending it twice.
let items = get_directories( let items = get_directories(
&service, &mut grpc_client,
GetDirectoryRequest { GetDirectoryRequest {
recursive: true, recursive: true,
by_what: Some(ByWhat::Digest(DIRECTORY_C.digest().into())), by_what: Some(ByWhat::Digest(DIRECTORY_C.digest().into())),
@ -185,7 +196,8 @@ async fn put_get_dedup() {
/// Trying to upload a Directory failing validation should fail. /// Trying to upload a Directory failing validation should fail.
#[tokio::test] #[tokio::test]
async fn put_reject_failed_validation() { async fn put_reject_failed_validation() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
// construct a broken Directory message that fails validation // construct a broken Directory message that fails validation
let broken_directory = Directory { let broken_directory = Directory {
@ -198,10 +210,12 @@ async fn put_reject_failed_validation() {
assert!(broken_directory.validate().is_err()); assert!(broken_directory.validate().is_err());
// send it over, it must fail // send it over, it must fail
let put_resp = service let put_resp = {
.put(tonic_mock::streaming_request(vec![broken_directory])) grpc_client
.await .put(tokio_stream::once(broken_directory))
.expect_err("must fail"); .await
.expect_err("must fail")
};
assert_eq!(put_resp.code(), tonic::Code::InvalidArgument); assert_eq!(put_resp.code(), tonic::Code::InvalidArgument);
} }
@ -209,7 +223,8 @@ async fn put_reject_failed_validation() {
/// Trying to upload a Directory with wrong size should fail. /// Trying to upload a Directory with wrong size should fail.
#[tokio::test] #[tokio::test]
async fn put_reject_wrong_size() { async fn put_reject_wrong_size() {
let service = gen_grpc_service(); let tempdir = TempDir::new().expect("must succeed");
let mut grpc_client = gen_directorysvc_grpc_client(tempdir.path()).await;
// Construct a directory referring to DIRECTORY_A, but with wrong size. // Construct a directory referring to DIRECTORY_A, but with wrong size.
let broken_parent_directory = Directory { let broken_parent_directory = Directory {
@ -227,13 +242,14 @@ async fn put_reject_wrong_size() {
); );
// now upload both (first A, then the broken parent). This must fail. // now upload both (first A, then the broken parent). This must fail.
let put_resp = service let put_resp = {
.put(tonic_mock::streaming_request(vec![ grpc_client
DIRECTORY_A.clone(), .put(tokio_stream::iter(vec![
broken_parent_directory, DIRECTORY_A.clone(),
])) broken_parent_directory,
.await ]))
.expect_err("must fail"); .await
.expect_err("must fail")
};
assert_eq!(put_resp.code(), tonic::Code::InvalidArgument); assert_eq!(put_resp.code(), tonic::Code::InvalidArgument);
} }