mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

chore(tvix/store): migrate import.rs and tests/pathinfo.rs to rstest

Browse source 
Also, rename the DUMMY_NAME constant in the fixtures to DUMMY_PATH,
which aligns more with the ToString representation and from_bytes
conversions we have on StorePath[Ref].

Change-Id: I39763c9dfa84c5d86f2fd0171b3a4d36fd72f267
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11464
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This commit is contained in:
Florian Klink 2024-04-19 13:52:50 +03:00 committed by clbot
parent 6b5d664930
commit a020755c58
5 changed files with 72 additions and 87 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
tvix/store/src/import.rs
View file

@ -156,21 +156,22 @@ mod tests {
use std::{ffi::OsStr, path::PathBuf}; use std::{ffi::OsStr, path::PathBuf};
use crate::import::path_to_name; use crate::import::path_to_name;
use test_case::test_case; use rstest::rstest;
#[test_case("a/b/c", "c"; "simple path")] #[rstest]
#[test_case("a/b/../c", "c"; "simple path containing ..")] #[case::simple_path("a/b/c", "c")]
#[test_case("a/b/../c/d/../e", "e"; "path containing multiple ..")] #[case::simple_path_containing_dotdot("a/b/../c", "c")]
#[case::path_containing_multiple_dotdot("a/b/../c/d/../e", "e")]
fn test_path_to_name(path: &str, expected_name: &str) { fn test_path_to_name(#[case] path: &str, #[case] expected_name: &str) {
let path: PathBuf = path.into(); let path: PathBuf = path.into();
assert_eq!(path_to_name(&path).expect("must succeed"), expected_name); assert_eq!(path_to_name(&path).expect("must succeed"), expected_name);
} }
#[test_case(b"a/b/.."; "path ending in ..")] #[rstest]
#[test_case(b"\xf8\xa1\xa1\xa1\xa1"; "non unicode path")] #[case::path_ending_in_dotdot(b"a/b/..")]
#[case::non_unicode_path(b"\xf8\xa1\xa1\xa1\xa1")]
fn test_invalid_path_to_name(invalid_path: &[u8]) { fn test_invalid_path_to_name(#[case] invalid_path: &[u8]) {
let path: PathBuf = unsafe { OsStr::from_encoded_bytes_unchecked(invalid_path) }.into(); let path: PathBuf = unsafe { OsStr::from_encoded_bytes_unchecked(invalid_path) }.into();
path_to_name(&path).expect_err("must fail"); path_to_name(&path).expect_err("must fail");
} }

2
tvix/store/src/pathinfoservice/grpc.rs
View file

@ -207,7 +207,7 @@ mod tests {
}; };
let path_info = grpc_client let path_info = grpc_client
.get(fixtures::DUMMY_OUTPUT_HASH) .get(fixtures::DUMMY_PATH_DIGEST)
.await .await
.expect("must not be error"); .expect("must not be error");

6
tvix/store/src/pathinfoservice/tests/mod.rs
View file

@ -10,7 +10,7 @@ use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService}
use super::PathInfoService; use super::PathInfoService;
use crate::proto::PathInfo; use crate::proto::PathInfo;
use crate::tests::fixtures::DUMMY_OUTPUT_HASH; use crate::tests::fixtures::DUMMY_PATH_DIGEST;
mod utils; mod utils;
use self::utils::make_grpc_path_info_service_client; use self::utils::make_grpc_path_info_service_client;
@ -71,7 +71,7 @@ pub fn path_info_services(
async fn not_found(services: BSDSPS) { async fn not_found(services: BSDSPS) {
let (_, _, path_info_service) = services; let (_, _, path_info_service) = services;
assert!(path_info_service assert!(path_info_service
.get(DUMMY_OUTPUT_HASH) .get(DUMMY_PATH_DIGEST)
.await .await
.expect("must succeed") .expect("must succeed")
.is_none()); .is_none());
@ -105,7 +105,7 @@ async fn put_get(services: BSDSPS) {
// get it back // get it back
let resp = path_info_service let resp = path_info_service
.get(DUMMY_OUTPUT_HASH) .get(DUMMY_PATH_DIGEST)
.await .await
.expect("must succeed"); .expect("must succeed");

122
tvix/store/src/proto/tests/pathinfo.rs
View file

@ -4,95 +4,81 @@ use bytes::Bytes;
use data_encoding::BASE64; use data_encoding::BASE64;
use nix_compat::nixbase32; use nix_compat::nixbase32;
use nix_compat::store_path::{self, StorePathRef}; use nix_compat::store_path::{self, StorePathRef};
use test_case::test_case; use rstest::rstest;
use tvix_castore::proto as castorepb; use tvix_castore::proto as castorepb;
#[test_case( #[rstest]
None, #[case::no_node(None, Err(ValidatePathInfoError::NoNodePresent))]
Err(ValidatePathInfoError::NoNodePresent) ; #[case::no_node_2(Some(castorepb::Node { node: None}), Err(ValidatePathInfoError::NoNodePresent))]
"No node"
)] fn validate_pathinfo(
#[test_case( #[case] node: Option<castorepb::Node>,
Some(castorepb::Node { node: None }), #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
Err(ValidatePathInfoError::NoNodePresent);
"No node 2"
)]
fn validate_no_node(
t_node: Option<castorepb::Node>,
t_result: Result<StorePathRef, ValidatePathInfoError>,
) { ) {
// construct the PathInfo object // construct the PathInfo object
let p = PathInfo { let p = PathInfo {
node: t_node, node,
..Default::default() ..Default::default()
}; };
assert_eq!(t_result, p.validate());
assert_eq!(exp_result, p.validate());
let err = p.validate().expect_err("validation should fail");
assert!(matches!(err, ValidatePathInfoError::NoNodePresent));
} }
#[test_case( #[rstest]
castorepb::DirectoryNode { #[case::ok(castorepb::DirectoryNode {
name: DUMMY_NAME.into(), name: DUMMY_PATH.into(),
digest: DUMMY_DIGEST.clone().into(), digest: DUMMY_DIGEST.clone().into(),
size: 0, size: 0,
}, }, Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap()))]
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); #[case::invalid_digest_length(castorepb::DirectoryNode {
"ok" name: DUMMY_PATH.into(),
)]
#[test_case(
castorepb::DirectoryNode {
name: DUMMY_NAME.into(),
digest: Bytes::new(), digest: Bytes::new(),
size: 0, size: 0,
}, }, Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))))]
Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))); #[case::invalid_node_name_no_storepath(castorepb::DirectoryNode {
"invalid digest length"
)]
#[test_case(
castorepb::DirectoryNode {
name: "invalid".into(), name: "invalid".into(),
digest: DUMMY_DIGEST.clone().into(), digest: DUMMY_DIGEST.clone().into(),
size: 0, size: 0,
}, }, Err(ValidatePathInfoError::InvalidNodeName(
Err(ValidatePathInfoError::InvalidNodeName(
"invalid".into(), "invalid".into(),
store_path::Error::InvalidLength store_path::Error::InvalidLength
)); )))]
"invalid node name"
)]
fn validate_directory( fn validate_directory(
t_directory_node: castorepb::DirectoryNode, #[case] directory_node: castorepb::DirectoryNode,
t_result: Result<StorePathRef, ValidatePathInfoError>, #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
) { ) {
// construct the PathInfo object // construct the PathInfo object
let p = PathInfo { let p = PathInfo {
node: Some(castorepb::Node { node: Some(castorepb::Node {
node: Some(castorepb::node::Node::Directory(t_directory_node)), node: Some(castorepb::node::Node::Directory(directory_node)),
}), }),
..Default::default() ..Default::default()
}; };
assert_eq!(t_result, p.validate()); assert_eq!(exp_result, p.validate());
} }
#[test_case( #[rstest]
#[case::ok(
castorepb::FileNode { castorepb::FileNode {
name: DUMMY_NAME.into(), name: DUMMY_PATH.into(),
digest: DUMMY_DIGEST.clone().into(), digest: DUMMY_DIGEST.clone().into(),
size: 0, size: 0,
executable: false, executable: false,
}, },
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap())
"ok"
)] )]
#[test_case( #[case::invalid_digest_len(
castorepb::FileNode { castorepb::FileNode {
name: DUMMY_NAME.into(), name: DUMMY_PATH.into(),
digest: Bytes::new(), digest: Bytes::new(),
..Default::default() ..Default::default()
}, },
Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))); Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0)))
"invalid digest length"
)] )]
#[test_case( #[case::invalid_node_name(
castorepb::FileNode { castorepb::FileNode {
name: "invalid".into(), name: "invalid".into(),
digest: DUMMY_DIGEST.clone().into(), digest: DUMMY_DIGEST.clone().into(),
@ -101,32 +87,31 @@ fn validate_directory(
Err(ValidatePathInfoError::InvalidNodeName( Err(ValidatePathInfoError::InvalidNodeName(
"invalid".into(), "invalid".into(),
store_path::Error::InvalidLength store_path::Error::InvalidLength
)); ))
"invalid node name"
)] )]
fn validate_file( fn validate_file(
t_file_node: castorepb::FileNode, #[case] file_node: castorepb::FileNode,
t_result: Result<StorePathRef, ValidatePathInfoError>, #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
) { ) {
// construct the PathInfo object // construct the PathInfo object
let p = PathInfo { let p = PathInfo {
node: Some(castorepb::Node { node: Some(castorepb::Node {
node: Some(castorepb::node::Node::File(t_file_node)), node: Some(castorepb::node::Node::File(file_node)),
}), }),
..Default::default() ..Default::default()
}; };
assert_eq!(t_result, p.validate()); assert_eq!(exp_result, p.validate());
} }
#[test_case( #[rstest]
#[case::ok(
castorepb::SymlinkNode { castorepb::SymlinkNode {
name: DUMMY_NAME.into(), name: DUMMY_PATH.into(),
target: "foo".into(), target: "foo".into(),
}, },
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap())
"ok"
)] )]
#[test_case( #[case::invalid_node_name(
castorepb::SymlinkNode { castorepb::SymlinkNode {
name: "invalid".into(), name: "invalid".into(),
target: "foo".into(), target: "foo".into(),
@ -134,21 +119,20 @@ fn validate_file(
Err(ValidatePathInfoError::InvalidNodeName( Err(ValidatePathInfoError::InvalidNodeName(
"invalid".into(), "invalid".into(),
store_path::Error::InvalidLength store_path::Error::InvalidLength
)); ))
"invalid node name"
)] )]
fn validate_symlink( fn validate_symlink(
t_symlink_node: castorepb::SymlinkNode, #[case] symlink_node: castorepb::SymlinkNode,
t_result: Result<StorePathRef, ValidatePathInfoError>, #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
) { ) {
// construct the PathInfo object // construct the PathInfo object
let p = PathInfo { let p = PathInfo {
node: Some(castorepb::Node { node: Some(castorepb::Node {
node: Some(castorepb::node::Node::Symlink(t_symlink_node)), node: Some(castorepb::node::Node::Symlink(symlink_node)),
}), }),
..Default::default() ..Default::default()
}; };
assert_eq!(t_result, p.validate()); assert_eq!(exp_result, p.validate());
} }
/// Ensure parsing a correct PathInfo without narinfo populated succeeds. /// Ensure parsing a correct PathInfo without narinfo populated succeeds.
@ -235,7 +219,7 @@ fn validate_inconsistent_narinfo_reference_name_digest() {
match path_info.validate().expect_err("must fail") { match path_info.validate().expect_err("must fail") {
ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(0, e_expected, e_actual) => { ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(0, e_expected, e_actual) => {
assert_eq!(path_info.references[0][..], e_expected[..]); assert_eq!(path_info.references[0][..], e_expected[..]);
assert_eq!(DUMMY_OUTPUT_HASH, e_actual); assert_eq!(DUMMY_PATH_DIGEST, e_actual);
} }
e => panic!("unexpected error: {:?}", e), e => panic!("unexpected error: {:?}", e),
} }
@ -273,7 +257,7 @@ fn validate_valid_deriver() {
let narinfo = path_info.narinfo.as_mut().unwrap(); let narinfo = path_info.narinfo.as_mut().unwrap();
narinfo.deriver = Some(crate::proto::StorePath { narinfo.deriver = Some(crate::proto::StorePath {
name: "foo".to_string(), name: "foo".to_string(),
digest: Bytes::from(DUMMY_OUTPUT_HASH.as_slice()), digest: Bytes::from(DUMMY_PATH_DIGEST.as_slice()),
}); });
path_info.validate().expect("must validate"); path_info.validate().expect("must validate");

10
tvix/store/src/tests/fixtures.rs
View file

@ -13,8 +13,8 @@ use crate::proto::{
NarInfo, PathInfo, NarInfo, PathInfo,
}; };
pub const DUMMY_NAME: &str = "00000000000000000000000000000000-dummy"; pub const DUMMY_PATH: &str = "00000000000000000000000000000000-dummy";
pub const DUMMY_OUTPUT_HASH: [u8; 20] = [0; 20]; pub const DUMMY_PATH_DIGEST: [u8; 20] = [0; 20];
lazy_static! { lazy_static! {
/// The NAR representation of a symlink pointing to `/nix/store/somewhereelse` /// The NAR representation of a symlink pointing to `/nix/store/somewhereelse`
@ -106,12 +106,12 @@ lazy_static! {
pub static ref PATH_INFO_WITHOUT_NARINFO : PathInfo = PathInfo { pub static ref PATH_INFO_WITHOUT_NARINFO : PathInfo = PathInfo {
node: Some(castorepb::Node { node: Some(castorepb::Node {
node: Some(castorepb::node::Node::Directory(castorepb::DirectoryNode { node: Some(castorepb::node::Node::Directory(castorepb::DirectoryNode {
name: DUMMY_NAME.into(), name: DUMMY_PATH.into(),
digest: DUMMY_DIGEST.clone().into(), digest: DUMMY_DIGEST.clone().into(),
size: 0, size: 0,
})), })),
}), }),
references: vec![DUMMY_OUTPUT_HASH.as_slice().into()], references: vec![DUMMY_PATH_DIGEST.as_slice().into()],
narinfo: None, narinfo: None,
}; };
@ -123,7 +123,7 @@ lazy_static! {
nar_size: 0, nar_size: 0,
nar_sha256: DUMMY_DIGEST.clone().into(), nar_sha256: DUMMY_DIGEST.clone().into(),
signatures: vec![], signatures: vec![],
reference_names: vec![DUMMY_NAME.to_string()], reference_names: vec![DUMMY_PATH.to_string()],
deriver: None, deriver: None,
ca: Some(Ca { r#type: ca::Hash::NarSha256.into(), digest: DUMMY_DIGEST.clone().into() }) ca: Some(Ca { r#type: ca::Hash::NarSha256.into(), digest: DUMMY_DIGEST.clone().into() })
}), }),