From 6fbfacf50ded6308e530b59fdf37455664aeb3eb Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 06:50:16 -0300 Subject: [PATCH] nvx-rs: check the golden tests' fixture sections with one function The golden tests of host/python/golden.rs and host/sandbox/golden.rs compare the ports with what Python recorded in 19 sections of their fixtures, and each section repeated one frame around its checks: Mismatches::new(SECTION), a loop over cases(&fixture, SECTION), and mismatches.finish(). Each section named itself twice, and a section that left out finish() would compare nothing and still pass. check_section(fixture, section, check_case) now holds that frame: it starts the section's Mismatches, passes it to check_case with each case, and finishes it. Each section names itself once. Mismatches::new and finish become private to python/golden.rs, so the sandbox tests can no longer build a Mismatches that never finishes. The sections run the same checks on the same cases in the same order, and fail with the same messages. With finish() printing each section's counts, BASE and this tree compare the same values in each of the 19 sections, 7,796 in all, without a mismatch. Two new tests check what finish() reports through check_section: the cases that differ, and a section that compares nothing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/python/golden.rs | 78 +++++++++++++++++++------------ nvx-rs/src/host/sandbox/golden.rs | 74 ++++++++++------------------- 2 files changed, 73 insertions(+), 79 deletions(-) diff --git a/nvx-rs/src/host/python/golden.rs b/nvx-rs/src/host/python/golden.rs index fafee68de..1ce67955c 100644 --- a/nvx-rs/src/host/python/golden.rs +++ b/nvx-rs/src/host/python/golden.rs @@ -74,7 +74,7 @@ pub(crate) struct Mismatches { impl Mismatches { /// Starts checking a section. - pub(crate) fn new(section: &str) -> Self { + fn new(section: &str) -> Self { Self { section: section.to_owned(), checked: 0, @@ -93,7 +93,7 @@ impl Mismatches { } /// Fails with the mismatches, if any. - pub(crate) fn finish(self) { + fn finish(self) { assert!(self.checked > 0, "{} checked no cases", self.section); assert!( self.found.is_empty(), @@ -111,6 +111,38 @@ impl Mismatches { } } +/// Checks every case of a fixture's `section` with `check_case`, which compares the port's values +/// with Python's through [`Mismatches::check`], then fails with the cases on which they differ. +pub(crate) fn check_section( + fixture: &Value, + section: &str, + mut check_case: impl FnMut(&mut Mismatches, &Value), +) { + let mut mismatches = Mismatches::new(section); + for case in cases(fixture, section) { + check_case(&mut mismatches, case); + } + mismatches.finish(); +} + +/// Checks that a section fails with the cases on which the port and Python differ. +#[test] +#[should_panic(expected = "1 of 2 letters cases differ from Python")] +fn sections_fail_with_the_cases_that_differ() { + let fixture = json!({"letters": [{"text": "a"}, {"text": "b"}]}); + check_section(&fixture, "letters", |mismatches, case| { + mismatches.check(case, "a", text(case, "text")); + }); +} + +/// Checks that a section fails when it compares nothing. +#[test] +#[should_panic(expected = "letters checked no cases")] +fn sections_that_check_nothing_fail() { + let fixture = json!({"letters": [{"text": "a"}]}); + check_section(&fixture, "letters", |_, _| {}); +} + /// Returns the ranges of code points from `start` that satisfy `predicate`, as the generator /// lists them. fn ranges(start: u32, predicate: impl Fn(char) -> bool) -> Vec<(u32, u32)> { @@ -147,8 +179,7 @@ fn unicode_tables_match_python() { #[test] fn strings_integers_and_uuids_match_python() { let fixture = fixture("python.json"); - let mut mismatches = Mismatches::new("strings"); - for case in cases(&fixture, "strings") { + check_section(&fixture, "strings", |mismatches, case| { let value = text(case, "text"); mismatches.check(case, text(case, "repr"), repr(value).as_str()); mismatches.check( @@ -170,15 +201,13 @@ fn strings_integers_and_uuids_match_python() { .map(|int| int.to_string()) .map_err(|error| error.message(value)); mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); } #[test] fn pure_paths_match_pathlib() { let fixture = fixture("python.json"); - let mut mismatches = Mismatches::new("paths"); - for case in cases(&fixture, "paths") { + check_section(&fixture, "paths", |mismatches, case| { let path = PurePath::new(flavor(case), text(case, "text")); let parents: Vec = path .parents() @@ -201,33 +230,26 @@ fn pure_paths_match_pathlib() { parents, ), ); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("joins"); - for case in cases(&fixture, "joins") { + check_section(&fixture, "joins", |mismatches, case| { let path = PurePath::new(flavor(case), text(case, "left")).join(text(case, "right")); mismatches.check(case, text(case, "str"), path.as_str()); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("equality"); - for case in cases(&fixture, "equality") { + check_section(&fixture, "equality", |mismatches, case| { let left = PurePath::new(Flavor::Windows, text(case, "left")); let right = PurePath::new(Flavor::Windows, text(case, "right")); mismatches.check(case, case["equal"].as_bool().unwrap(), left == right); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("normpath"); - for case in cases(&fixture, "normpath") { + check_section(&fixture, "normpath", |mismatches, case| { mismatches.check( case, text(case, "normpath"), windows_normpath(text(case, "text")).as_str(), ); - } - mismatches.finish(); + }); } /// Returns a string in the generator's tagged form. @@ -281,8 +303,7 @@ fn tagged(document: &Document, id: NodeId) -> Value { #[test] fn json_documents_decode_as_python_decodes_them() { let fixture = fixture("python.json"); - let mut mismatches = Mismatches::new("json"); - for case in cases(&fixture, "json") { + check_section(&fixture, "json", |mismatches, case| { let expected = if let Some(value) = case.get("value") { Ok(value.clone()) } else if let Some(message) = case.get("decode_error") { @@ -299,16 +320,13 @@ fn json_documents_decode_as_python_decodes_them() { Err(ReadError::Hook(error)) => Err(("value", error.message(source))), }; mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("utf8"); - for case in cases(&fixture, "utf8") { + check_section(&fixture, "utf8", |mismatches, case| { mismatches.check( case, case["error"].as_str().map(str::to_owned), decode_utf8(&hex(text(case, "bytes"))).err(), ); - } - mismatches.finish(); + }); } diff --git a/nvx-rs/src/host/sandbox/golden.rs b/nvx-rs/src/host/sandbox/golden.rs index 12c476244..f4f27ebac 100644 --- a/nvx-rs/src/host/sandbox/golden.rs +++ b/nvx-rs/src/host/sandbox/golden.rs @@ -19,7 +19,7 @@ use super::options::{ validate_exec_environment, validate_run_share_targets, validate_sandbox_systemd_policy, }; use super::*; -use crate::host::python::golden::{Mismatches, cases, fixture, flavor, hex, text, texts}; +use crate::host::python::golden::{check_section, fixture, flavor, hex, text, texts}; use crate::host::python::uuid::parse_uuid; /// Returns the message of a result's error, or `None` if it succeeded. @@ -96,17 +96,14 @@ fn recorded_layer(flavor: Flavor, value: &str) -> SandboxLayer { #[test] fn identities_and_layers_parse_as_python_parses_them() { let fixture = fixture("sandbox.json"); - let mut mismatches = Mismatches::new("identities"); - for case in cases(&fixture, "identities") { + check_section(&fixture, "identities", |mismatches, case| { let actual = parse_workload_identity(text(case, "value")) .map(|(uid, gid)| json!([uid, gid])) .map_err(|error| error.to_string()); mismatches.check(case, outcome(case, "identity"), actual); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("layers"); - for case in cases(&fixture, "layers") { + check_section(&fixture, "layers", |mismatches, case| { let expected = outcome(case, "role").map(|_| json!([case["role"], case["path"], case["uuid"]])); let actual = SandboxLayer::parse_in(flavor(case), text(case, "value")) @@ -119,51 +116,43 @@ fn identities_and_layers_parse_as_python_parses_them() { }) .map_err(|error| error.to_string()); mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); } #[test] fn mount_values_targets_and_child_names_match_python() { let fixture = fixture("sandbox.json"); - let mut mismatches = Mismatches::new("mount_values"); - for case in cases(&fixture, "mount_values") { + check_section(&fixture, "mount_values", |mismatches, case| { let expected = outcome(case, "target") .map(|_| json!([case["target"], case["host_path"], case["access"]])); let actual = parse_mount_value(text(case, "value")) .map(|(target, host_path, access)| json!([target, host_path, access.as_str()])) .map_err(|error| error.to_string()); mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("mount_targets"); - for case in cases(&fixture, "mount_targets") { + check_section(&fixture, "mount_targets", |mismatches, case| { mismatches.check( case, case["error"].as_str().map(str::to_owned), error(&validate_mount_target(text(case, "target"))), ); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("child_names"); - for case in cases(&fixture, "child_names") { + check_section(&fixture, "child_names", |mismatches, case| { let index = usize::try_from(case["index"].as_u64().unwrap()).unwrap(); mismatches.check( case, text(case, "name"), aggregate_child_name(index, text(case, "target")).as_str(), ); - } - mismatches.finish(); + }); } #[test] fn mounts_lower_to_the_arguments_that_python_gives() { let fixture = fixture("sandbox.json"); - let mut mismatches = Mismatches::new("mounts"); - for case in cases(&fixture, "mounts") { + check_section(&fixture, "mounts", |mismatches, case| { let flavor = flavor(case); let cwd = cwd(&fixture, flavor); let expected = outcome(case, "mount").map(|_| { @@ -190,15 +179,13 @@ fn mounts_lower_to_the_arguments_that_python_gives() { ])) }); mismatches.check(case, expected, actual.map_err(|error| error.to_string())); - } - mismatches.finish(); + }); } #[test] fn launches_give_the_arguments_and_command_lines_that_python_gives() { let fixture = fixture("sandbox.json"); - let mut mismatches = Mismatches::new("launches"); - for case in cases(&fixture, "launches") { + check_section(&fixture, "launches", |mismatches, case| { let flavor = flavor(case); let layers = texts(case, "layers") .iter() @@ -243,15 +230,13 @@ fn launches_give_the_arguments_and_command_lines_that_python_gives() { }) .map_err(|error| error.to_string()); mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); } #[test] fn sandbox_option_helpers_match_nvx_py() { let fixture = fixture("sandbox_options.json"); - let mut mismatches = Mismatches::new("run_share_targets"); - for case in cases(&fixture, "run_share_targets") { + check_section(&fixture, "run_share_targets", |mismatches, case| { let targets = texts(case, "targets"); let targets: Vec<&str> = targets.iter().map(String::as_str).collect(); mismatches.check( @@ -259,11 +244,9 @@ fn sandbox_option_helpers_match_nvx_py() { case["error"].as_str().map(str::to_owned), error(&validate_run_share_targets(&targets)), ); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("sandbox_mounts"); - for case in cases(&fixture, "sandbox_mounts") { + check_section(&fixture, "sandbox_mounts", |mismatches, case| { let files = texts(case, "files"); let policy_paths: Vec = case["policy"] .as_array() @@ -302,18 +285,15 @@ fn sandbox_option_helpers_match_nvx_py() { .map(|mounts| Value::Array(mounts.iter().map(mount_fields).collect())) .map_err(|error| error.to_string()); mismatches.check(case, outcome(case, "result"), actual); - } - mismatches.finish(); + }); - let mut mismatches = Mismatches::new("environments"); - for case in cases(&fixture, "environments") { + check_section(&fixture, "environments", |mismatches, case| { mismatches.check( case, case["error"].as_str().map(str::to_owned), error(&validate_exec_environment(&texts(case, "entries"))), ); - } - mismatches.finish(); + }); } /// Returns the bytes of a document of the generator: its text, its bytes in hex, or a number @@ -336,8 +316,7 @@ fn environment_files_and_distro_manifests_read_as_python_reads_them() { let path = directory.path().join("environment.json"); let path = path.to_str().unwrap(); let shown = PurePath::native(path).as_str().to_owned(); - let mut mismatches = Mismatches::new("environment_files"); - for case in cases(&fixture, "environment_files") { + check_section(&fixture, "environment_files", |mismatches, case| { std::fs::write(path, document(case)).unwrap(); let mut options = SandboxOptions::new(SandboxOperation::Exec); options.environment_file = Some(path.to_owned()); @@ -350,8 +329,7 @@ fn environment_files_and_distro_manifests_read_as_python_reads_them() { .map(|environment| json!(environment)) .map_err(|error| error.to_string().replace(&shown, "{path}")); mismatches.check(case, expected, actual); - } - mismatches.finish(); + }); let layer = directory.path().join("distro.erofs"); std::fs::write(&layer, text(&fixture, "distro_layer")).unwrap(); @@ -370,14 +348,12 @@ fn environment_files_and_distro_manifests_read_as_python_reads_them() { }, ) .unwrap(); - let mut mismatches = Mismatches::new("distro_manifests"); - for case in cases(&fixture, "distro_manifests") { + check_section(&fixture, "distro_manifests", |mismatches, case| { std::fs::write(&manifest, document(&case["manifest"])).unwrap(); mismatches.check( case, case["error"].as_str().map(str::to_owned), error(&validate_sandbox_systemd_policy(&launch)), ); - } - mismatches.finish(); + }); }