From a020755c581e2f6bde9c0e69a61574e87abfc3dd Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 19 Apr 2024 13:52:50 +0300 Subject: chore(tvix/store): migrate import.rs and tests/pathinfo.rs to rstest 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 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster --- tvix/store/src/import.rs | 19 +++-- tvix/store/src/pathinfoservice/grpc.rs | 2 +- tvix/store/src/pathinfoservice/tests/mod.rs | 6 +- tvix/store/src/proto/tests/pathinfo.rs | 122 ++++++++++++---------------- tvix/store/src/tests/fixtures.rs | 10 +-- 5 files changed, 72 insertions(+), 87 deletions(-) diff --git a/tvix/store/src/import.rs b/tvix/store/src/import.rs index 69f68d46a2fa..e6d451834c1f 100644 --- a/tvix/store/src/import.rs +++ b/tvix/store/src/import.rs @@ -156,21 +156,22 @@ mod tests { use std::{ffi::OsStr, path::PathBuf}; use crate::import::path_to_name; - use test_case::test_case; + use rstest::rstest; - #[test_case("a/b/c", "c"; "simple path")] - #[test_case("a/b/../c", "c"; "simple path containing ..")] - #[test_case("a/b/../c/d/../e", "e"; "path containing multiple ..")] + #[rstest] + #[case::simple_path("a/b/c", "c")] + #[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(); assert_eq!(path_to_name(&path).expect("must succeed"), expected_name); } - #[test_case(b"a/b/.."; "path ending in ..")] - #[test_case(b"\xf8\xa1\xa1\xa1\xa1"; "non unicode path")] - - fn test_invalid_path_to_name(invalid_path: &[u8]) { + #[rstest] + #[case::path_ending_in_dotdot(b"a/b/..")] + #[case::non_unicode_path(b"\xf8\xa1\xa1\xa1\xa1")] + fn test_invalid_path_to_name(#[case] invalid_path: &[u8]) { let path: PathBuf = unsafe { OsStr::from_encoded_bytes_unchecked(invalid_path) }.into(); path_to_name(&path).expect_err("must fail"); } diff --git a/tvix/store/src/pathinfoservice/grpc.rs b/tvix/store/src/pathinfoservice/grpc.rs index 02e0cb590b3e..1138ebdc198b 100644 --- a/tvix/store/src/pathinfoservice/grpc.rs +++ b/tvix/store/src/pathinfoservice/grpc.rs @@ -207,7 +207,7 @@ mod tests { }; let path_info = grpc_client - .get(fixtures::DUMMY_OUTPUT_HASH) + .get(fixtures::DUMMY_PATH_DIGEST) .await .expect("must not be error"); diff --git a/tvix/store/src/pathinfoservice/tests/mod.rs b/tvix/store/src/pathinfoservice/tests/mod.rs index da9ad11cabfe..c9b9a0637767 100644 --- a/tvix/store/src/pathinfoservice/tests/mod.rs +++ b/tvix/store/src/pathinfoservice/tests/mod.rs @@ -10,7 +10,7 @@ use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService} use super::PathInfoService; use crate::proto::PathInfo; -use crate::tests::fixtures::DUMMY_OUTPUT_HASH; +use crate::tests::fixtures::DUMMY_PATH_DIGEST; mod utils; use self::utils::make_grpc_path_info_service_client; @@ -71,7 +71,7 @@ pub fn path_info_services( async fn not_found(services: BSDSPS) { let (_, _, path_info_service) = services; assert!(path_info_service - .get(DUMMY_OUTPUT_HASH) + .get(DUMMY_PATH_DIGEST) .await .expect("must succeed") .is_none()); @@ -105,7 +105,7 @@ async fn put_get(services: BSDSPS) { // get it back let resp = path_info_service - .get(DUMMY_OUTPUT_HASH) + .get(DUMMY_PATH_DIGEST) .await .expect("must succeed"); diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs index 36c4f33933fb..4d0834878d7c 100644 --- a/tvix/store/src/proto/tests/pathinfo.rs +++ b/tvix/store/src/proto/tests/pathinfo.rs @@ -4,95 +4,81 @@ use bytes::Bytes; use data_encoding::BASE64; use nix_compat::nixbase32; use nix_compat::store_path::{self, StorePathRef}; -use test_case::test_case; +use rstest::rstest; use tvix_castore::proto as castorepb; -#[test_case( - None, - Err(ValidatePathInfoError::NoNodePresent) ; - "No node" -)] -#[test_case( - Some(castorepb::Node { node: None }), - Err(ValidatePathInfoError::NoNodePresent); - "No node 2" -)] -fn validate_no_node( - t_node: Option, - t_result: Result, +#[rstest] +#[case::no_node(None, Err(ValidatePathInfoError::NoNodePresent))] +#[case::no_node_2(Some(castorepb::Node { node: None}), Err(ValidatePathInfoError::NoNodePresent))] + +fn validate_pathinfo( + #[case] node: Option, + #[case] exp_result: Result, ) { // construct the PathInfo object let p = PathInfo { - node: t_node, + node, ..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( - castorepb::DirectoryNode { - name: DUMMY_NAME.into(), +#[rstest] +#[case::ok(castorepb::DirectoryNode { + name: DUMMY_PATH.into(), digest: DUMMY_DIGEST.clone().into(), size: 0, - }, - Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); - "ok" -)] -#[test_case( - castorepb::DirectoryNode { - name: DUMMY_NAME.into(), +}, Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap()))] +#[case::invalid_digest_length(castorepb::DirectoryNode { + name: DUMMY_PATH.into(), digest: Bytes::new(), size: 0, - }, - Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))); - "invalid digest length" -)] -#[test_case( - castorepb::DirectoryNode { +}, Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))))] +#[case::invalid_node_name_no_storepath(castorepb::DirectoryNode { name: "invalid".into(), digest: DUMMY_DIGEST.clone().into(), size: 0, - }, - Err(ValidatePathInfoError::InvalidNodeName( +}, Err(ValidatePathInfoError::InvalidNodeName( "invalid".into(), store_path::Error::InvalidLength - )); - "invalid node name" -)] +)))] fn validate_directory( - t_directory_node: castorepb::DirectoryNode, - t_result: Result, + #[case] directory_node: castorepb::DirectoryNode, + #[case] exp_result: Result, ) { // construct the PathInfo object let p = PathInfo { node: Some(castorepb::Node { - node: Some(castorepb::node::Node::Directory(t_directory_node)), + node: Some(castorepb::node::Node::Directory(directory_node)), }), ..Default::default() }; - assert_eq!(t_result, p.validate()); + assert_eq!(exp_result, p.validate()); } -#[test_case( +#[rstest] +#[case::ok( castorepb::FileNode { - name: DUMMY_NAME.into(), + name: DUMMY_PATH.into(), digest: DUMMY_DIGEST.clone().into(), size: 0, executable: false, }, - Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); - "ok" + Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap()) )] -#[test_case( +#[case::invalid_digest_len( castorepb::FileNode { - name: DUMMY_NAME.into(), + name: DUMMY_PATH.into(), digest: Bytes::new(), ..Default::default() }, - Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))); - "invalid digest length" + Err(ValidatePathInfoError::InvalidRootNode(castorepb::ValidateNodeError::InvalidDigestLen(0))) )] -#[test_case( +#[case::invalid_node_name( castorepb::FileNode { name: "invalid".into(), digest: DUMMY_DIGEST.clone().into(), @@ -101,32 +87,31 @@ fn validate_directory( Err(ValidatePathInfoError::InvalidNodeName( "invalid".into(), store_path::Error::InvalidLength - )); - "invalid node name" + )) )] fn validate_file( - t_file_node: castorepb::FileNode, - t_result: Result, + #[case] file_node: castorepb::FileNode, + #[case] exp_result: Result, ) { // construct the PathInfo object let p = PathInfo { node: Some(castorepb::Node { - node: Some(castorepb::node::Node::File(t_file_node)), + node: Some(castorepb::node::Node::File(file_node)), }), ..Default::default() }; - assert_eq!(t_result, p.validate()); + assert_eq!(exp_result, p.validate()); } -#[test_case( +#[rstest] +#[case::ok( castorepb::SymlinkNode { - name: DUMMY_NAME.into(), + name: DUMMY_PATH.into(), target: "foo".into(), }, - Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed")); - "ok" + Ok(StorePathRef::from_bytes(DUMMY_PATH.as_bytes()).unwrap()) )] -#[test_case( +#[case::invalid_node_name( castorepb::SymlinkNode { name: "invalid".into(), target: "foo".into(), @@ -134,21 +119,20 @@ fn validate_file( Err(ValidatePathInfoError::InvalidNodeName( "invalid".into(), store_path::Error::InvalidLength - )); - "invalid node name" + )) )] fn validate_symlink( - t_symlink_node: castorepb::SymlinkNode, - t_result: Result, + #[case] symlink_node: castorepb::SymlinkNode, + #[case] exp_result: Result, ) { // construct the PathInfo object let p = PathInfo { node: Some(castorepb::Node { - node: Some(castorepb::node::Node::Symlink(t_symlink_node)), + node: Some(castorepb::node::Node::Symlink(symlink_node)), }), ..Default::default() }; - assert_eq!(t_result, p.validate()); + assert_eq!(exp_result, p.validate()); } /// 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") { ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(0, e_expected, e_actual) => { 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), } @@ -273,7 +257,7 @@ fn validate_valid_deriver() { let narinfo = path_info.narinfo.as_mut().unwrap(); narinfo.deriver = Some(crate::proto::StorePath { 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"); diff --git a/tvix/store/src/tests/fixtures.rs b/tvix/store/src/tests/fixtures.rs index 500ac0aa5bc2..1c8359a2c0c7 100644 --- a/tvix/store/src/tests/fixtures.rs +++ b/tvix/store/src/tests/fixtures.rs @@ -13,8 +13,8 @@ use crate::proto::{ NarInfo, PathInfo, }; -pub const DUMMY_NAME: &str = "00000000000000000000000000000000-dummy"; -pub const DUMMY_OUTPUT_HASH: [u8; 20] = [0; 20]; +pub const DUMMY_PATH: &str = "00000000000000000000000000000000-dummy"; +pub const DUMMY_PATH_DIGEST: [u8; 20] = [0; 20]; lazy_static! { /// 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 { node: Some(castorepb::Node { node: Some(castorepb::node::Node::Directory(castorepb::DirectoryNode { - name: DUMMY_NAME.into(), + name: DUMMY_PATH.into(), digest: DUMMY_DIGEST.clone().into(), size: 0, })), }), - references: vec![DUMMY_OUTPUT_HASH.as_slice().into()], + references: vec![DUMMY_PATH_DIGEST.as_slice().into()], narinfo: None, }; @@ -123,7 +123,7 @@ lazy_static! { nar_size: 0, nar_sha256: DUMMY_DIGEST.clone().into(), signatures: vec![], - reference_names: vec![DUMMY_NAME.to_string()], + reference_names: vec![DUMMY_PATH.to_string()], deriver: None, ca: Some(Ca { r#type: ca::Hash::NarSha256.into(), digest: DUMMY_DIGEST.clone().into() }) }), -- cgit 1.4.1