about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-04-19T10·52+0300
committerclbot <clbot@tvl.fyi>2024-04-19T19·22+0000
commita020755c581e2f6bde9c0e69a61574e87abfc3dd (patch)
treef878a3bc8cebd07c0c21307fe6eb7299b3f48eab
parent6b5d664930b8f87f4b709e309a4445557b4bed48 (diff)
chore(tvix/store): migrate import.rs and tests/pathinfo.rs to rstest r/7965
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>
-rw-r--r--tvix/store/src/import.rs19
-rw-r--r--tvix/store/src/pathinfoservice/grpc.rs2
-rw-r--r--tvix/store/src/pathinfoservice/tests/mod.rs6
-rw-r--r--tvix/store/src/proto/tests/pathinfo.rs122
-rw-r--r--tvix/store/src/tests/fixtures.rs10
5 files changed, 72 insertions, 87 deletions
diff --git a/tvix/store/src/import.rs b/tvix/store/src/import.rs
index 69f68d46a2..e6d451834c 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 02e0cb590b..1138ebdc19 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 da9ad11cab..c9b9a06377 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 36c4f33933..4d0834878d 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<castorepb::Node>,
-    t_result: Result<StorePathRef, ValidatePathInfoError>,
+#[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<castorepb::Node>,
+    #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
 ) {
     // 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<StorePathRef, ValidatePathInfoError>,
+    #[case] directory_node: castorepb::DirectoryNode,
+    #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
 ) {
     // 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<StorePathRef, ValidatePathInfoError>,
+    #[case] file_node: castorepb::FileNode,
+    #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
 ) {
     // 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<StorePathRef, ValidatePathInfoError>,
+    #[case] symlink_node: castorepb::SymlinkNode,
+    #[case] exp_result: Result<StorePathRef, ValidatePathInfoError>,
 ) {
     // 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 500ac0aa5b..1c8359a2c0 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() })
         }),