From d2ced1c0da71cdb331f4693bfaf625f887b84d07 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 28 Aug 2026 16:38:51 +0200 Subject: [PATCH 1/5] Add executable locator property tests (#533) Cover candidate generation and first-match resolution across generated UTF-8 layouts, optional target directories, and presence masks. Extend the diagnostic JSON snapshot contract to retain unrelated version fields, and document why its finite filter example is complete. --- ...snapshot-testing-in-netsuke-using-insta.md | 84 ++++++------ src/diagnostic_json_tests.rs | 11 ++ test_support/src/netsuke/locator.rs | 3 + .../locator/tests/locator_property_tests.rs | 123 ++++++++++++++++++ 4 files changed, 178 insertions(+), 43 deletions(-) create mode 100644 test_support/src/netsuke/locator/tests/locator_property_tests.rs diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md index 2cd3011c1..9a333f13d 100644 --- a/docs/snapshot-testing-in-netsuke-using-insta.md +++ b/docs/snapshot-testing-in-netsuke-using-insta.md @@ -68,7 +68,7 @@ use netsuke::NetsukeManifest; // assumed struct for parsed manifest use netsuke::ir::BuildGraph; // assumed IR data structure # fn main() -> Result<(), Box> { - // Example Netsuke manifest in YAML (string literal for test) + // (Re-use the same manifest YAML as before) let manifest_yaml = r#" netsuke_version: "0.1" rules: @@ -79,65 +79,58 @@ use netsuke::ir::BuildGraph; // assumed IR data structure deps: ["hello.c"] rule: "compile" "#; - - // 1. Parse manifest YAML into AST/manifest struct let manifest = NetsukeManifest::from_yaml_str(manifest_yaml)?; - - // 2. Generate the IR (BuildGraph) from the manifest let build_graph = BuildGraph::from_manifest(&manifest)?; - // 3. Convert IR to a deterministic string representation - // For example, use Debug trait or implement a custom Display/serialization - let ir_pretty = format!("{:#?}", build_graph); + // Generate Ninja file content from the IR + // `generate` returns `Result`; handle errors + let ninja_file = ninja_gen::generate(&build_graph)?; - // 4. Assert snapshot, storing output in tests/snapshots/ir/ + // The output is a multi-line Ninja build script (as a String) + // Ensure the output is deterministic + // (e.g., consistent ordering of rules/targets) Settings::new() - .set_snapshot_path("tests/snapshots/ir") + .set_snapshot_path("tests/snapshots/ninja") .bind(|| { - assert_snapshot!("simple_manifest_ir", ir_pretty); + assert_snapshot!("simple_manifest_ninja", ninja_file); }); Ok(()) # } ``` -This test involves: - -- Construct a **deterministic** input (a small manifest with a known rule and - target). - -- Run the IR generation (`BuildGraph::from_manifest`). This function should - produce the intermediate build graph. +The match explicitly handles the `Result` from `generate` so any formatting or +missing action errors surface during tests. Production code should propagate +the error and report it with `miette` rather than panicking. -- Format the IR consistently for comparison. Pretty-printed debug output - (`{:#?}`) can be used, but for more complex structures implement `Display` or - use `assert_yaml_snapshot!` to serialize the IR to YAML/JSON for clarity. +Key points for Ninja snapshot tests: -- Use `Settings::new().set_snapshot_path("tests/snapshots/ir")` to direct the - snapshot file to the IR snapshot directory. Call `assert_snapshot!` with a - snapshot name (`"simple_manifest_ir"`) and the IR output string. On first run, - `insta` will record this output as the reference snapshot. **Determinism in - IR Output:** To ensure consistent snapshots, the IR output must be - **deterministic**. This means that given the same manifest input, the IR’s - printed form should not vary between test runs or across machines. Pay - attention to ordering and ephemeral data: +- Use a known manifest input and first derive the IR. An IR can also be + constructed directly for tests, but using the manifest→IR pipeline ensures + realistic coverage. -- **Ordering:** If `BuildGraph` contains collections (e.g. sets of targets - or rules), iterate or sort them in a fixed order before printing. Using - `BTreeMap` or sorting vectors of targets by name can help. This avoids - nondeterministic ordering from hash maps. +- Call the Ninja generation function (`ninja_gen::generate`), which + yields a `Result`. This function traverses the IR and + outputs rules and build statements in Ninja syntax, returning an error if any + build edge references an undefined action. -- **Stable Identifiers:** If IR includes IDs or memory addresses, prefer stable - identifiers. For example, when generating rule IDs, assign them in insertion - order so they are consistent, or omit details that can change. +- As with IR, **determinism is crucial**. The Ninja output should list rules, + targets, and dependencies in a consistent order. For example, if the IR does + not preserve order, targets may need to be sorted by name or hashing and + deduplication must avoid randomness. The design’s approach of consolidating + rules by a hash of their properties should still produce the same ordering + given the same input, as long as iteration over hashmaps is avoided or + stabilized. -- **No timestamps or environment-specific data:** The IR should not include - timestamps, random values, or absolute file system paths. If such data is - unavoidable, use `insta` redactions or post-process the output to replace - them with placeholders (e.g., ``). +- Use `Settings::set_snapshot_path` to store these snapshots in a separate + `tests/snapshots/ninja` directory. The snapshot name + `"simple_manifest_ninja"` identifies this particular scenario. -By making the IR snapshot output stable, the snapshot tests will reliably catch -regressions. If the IR generation logic changes intentionally (e.g., new fields -added), the snapshot will change predictably, prompting a review. +With this setup, IR tests and Ninja tests have distinct snapshot files. For +example, after the first test run (see next section), expected snapshot files +include `tests/snapshots/ir/simple_manifest_ir.snap` and +`tests/snapshots/ninja/simple_manifest_ninja.snap` (or combined snapshot files +per test module). These snapshot files contain the expected IR debug output and +Ninja file text respectively. ## Writing Snapshot Tests for Ninja File Output @@ -301,6 +294,11 @@ field named `version` remains visible in snapshot diffs. The generator name and `schema_version` are deliberately excluded from this redaction and remain asserted structurally. +The fixed diagnostic example exercises every structural filter path because the +regular expression has no data-dependent branches. The +`snapshot_test_support` property test separately varies valid `SemVer` +generator versions. + ## Running and Updating Snapshot Tests > In this repository the canonical runner is cargo-nextest: `make test`, or diff --git a/src/diagnostic_json_tests.rs b/src/diagnostic_json_tests.rs index 5ad66f4e4..5230046a1 100644 --- a/src/diagnostic_json_tests.rs +++ b/src/diagnostic_json_tests.rs @@ -1,4 +1,9 @@ //! Tests for Netsuke's JSON diagnostics schema. +//! +//! The diagnostic snapshot filter is one anchored regular expression with no +//! data-dependent branching. A fixed example that places unrelated `version` +//! fields on both sides of the matching generator block therefore covers every +//! filter path; property generation would not exercise a distinct behaviour. use super::{render_diagnostic_json, render_error_json}; use crate::ir::IrGenError; @@ -28,6 +33,9 @@ fn parse_json_value(document: &str) -> Result { fn snapshot_filter_preserves_versions_outside_the_generator_block() { let rendered = concat!( "{\n", + " \"schema\": {\n", + " \"version\": \"3.4.5\"\n", + " },\n", " \"generator\": {\n", " \"name\": \"netsuke\",\n", " \"version\": \"9.9.9\"\n", @@ -42,6 +50,9 @@ fn snapshot_filter_preserves_versions_outside_the_generator_block() { snapshot_settings().bind(|| { assert_snapshot!(rendered, @r#" { + "schema": { + "version": "3.4.5" + }, "generator": { "name": "netsuke", "version": "[version]" diff --git a/test_support/src/netsuke/locator.rs b/test_support/src/netsuke/locator.rs index 720f68b68..3dcda1c2d 100644 --- a/test_support/src/netsuke/locator.rs +++ b/test_support/src/netsuke/locator.rs @@ -119,6 +119,9 @@ mod tests { use mockable::MockEnv; use rstest::{fixture, rstest}; + #[path = "locator_property_tests.rs"] + mod locator_property_tests; + fn utf8_root(temp: &tempfile::TempDir) -> Result { Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) .map_err(|path| anyhow::anyhow!("temp dir {} is not UTF-8", path.display())) diff --git a/test_support/src/netsuke/locator/tests/locator_property_tests.rs b/test_support/src/netsuke/locator/tests/locator_property_tests.rs new file mode 100644 index 000000000..9233e9d1c --- /dev/null +++ b/test_support/src/netsuke/locator/tests/locator_property_tests.rs @@ -0,0 +1,123 @@ +//! Property tests for generated Netsuke executable locator layouts. +//! +//! The table tests in `locator.rs` pin Cargo's named layouts and exhaust every +//! three-candidate presence mask. These properties complement them by stating +//! the same candidate ordering and selection invariants over arbitrary valid +//! UTF-8 root components, profiles, target triples, and target directories. + +use super::super::{candidate_paths, netsuke_executable_from}; +use super::{binary_name, env_with_target_dir, touch, utf8_root}; +use proptest::prelude::*; +use proptest::test_runner::TestCaseError; + +/// Generate a valid UTF-8 path component for a temporary-root child. +fn root_component() -> impl Strategy { + "[a-z][a-z0-9_-]{0,8}" +} + +/// Generate a valid Cargo profile component distinct from `deps`. +fn profile_component() -> impl Strategy { + "[a-z][a-z0-9_-]{0,8}".prop_filter("profile component must not be `deps`", |component| { + component != "deps" + }) +} + +/// Generate a valid target-triple component distinct from `deps`. +fn target_triple() -> impl Strategy { + "[a-z][a-z0-9_-]{0,8}".prop_filter("target triple must not be `deps`", |component| { + component != "deps" + }) +} + +/// Generate an optional valid UTF-8 `CARGO_TARGET_DIR` component. +fn target_dir_component() -> impl Strategy> { + proptest::option::of(root_component()) +} + +/// Build an absolute UTF-8 root under a newly allocated temporary directory. +fn generated_root( + root_component: String, +) -> Result<(tempfile::TempDir, camino::Utf8PathBuf), TestCaseError> { + let temp = tempfile::tempdir().map_err(|error| TestCaseError::fail(error.to_string()))?; + let root = utf8_root(&temp).map_err(|error| TestCaseError::fail(error.to_string()))?; + Ok((temp, root.join(root_component))) +} + +proptest! { + /// Keep candidate contents and order stable for every generated layout. + #[test] + fn candidate_paths_match_the_documented_order( + root_component in root_component(), + profile in profile_component(), + triple in target_triple(), + target_dir_component in target_dir_component(), + ) { + let (_temp, root) = generated_root(root_component)?; + let exe_dir = root.join("build").join(&triple).join(&profile); + let target_dir = target_dir_component + .as_deref() + .map(|component| root.join(component)); + let env = env_with_target_dir(target_dir.as_deref()); + let binary = binary_name(); + + let candidates = candidate_paths(&env, &exe_dir, &binary); + let mut expected = vec![exe_dir.join(&binary)]; + if let Some(target_root) = &target_dir { + expected.push(target_root.join(&profile).join(&binary)); + expected.push(target_root.join(&triple).join(&profile).join(&binary)); + } + + prop_assert_eq!(&candidates, &expected); + if target_dir.is_none() { + prop_assert_eq!(candidates.len(), 1); + } + } + + /// Resolve the first staged candidate and report every missing path. + #[test] + fn executable_lookup_honours_generated_candidate_order( + root_component in root_component(), + profile in profile_component(), + triple in target_triple(), + target_dir_component in target_dir_component(), + presence in 0u8..8, + ) { + let (_temp, root) = generated_root(root_component)?; + let exe_dir = root.join("build").join(&triple).join(&profile); + let executable = exe_dir.join("deps").join("test-exe"); + touch(&executable).map_err(|error| TestCaseError::fail(error.to_string()))?; + + let target_dir = target_dir_component + .as_deref() + .map(|component| root.join(component)); + let env = env_with_target_dir(target_dir.as_deref()); + let binary = binary_name(); + let candidates = candidate_paths(&env, &exe_dir, &binary); + for (slot, candidate) in candidates.iter().enumerate() { + if presence & (1 << slot) != 0 { + touch(candidate).map_err(|error| TestCaseError::fail(error.to_string()))?; + } + } + + let located = netsuke_executable_from(&env, &executable); + let first_present = candidates + .iter() + .enumerate() + .find(|(slot, _)| presence & (1 << slot) != 0); + if let Some((_, expected)) = first_present { + let resolved = located.map_err(|error| TestCaseError::fail(error.to_string()))?; + prop_assert_eq!(resolved.as_path(), expected.as_path()); + } else { + let error = located + .err() + .ok_or_else(|| TestCaseError::fail("missing candidates should fail"))?; + let message = error.to_string(); + for candidate in candidates { + prop_assert!( + message.contains(candidate.as_str()), + "missing-candidate diagnostic should list {candidate}; got: {message}" + ); + } + } + } +} From a375cc92f0e0b5c4cd0cc911a86546960ab21d0b Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 28 Aug 2026 19:23:15 +0200 Subject: [PATCH 2/5] Document executable locator test flow (#533) Add an accessible sequence diagram for generated executable-candidate resolution, including first-match selection and missing diagnostics. --- docs/netsuke-design.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index c00699286..c4cbc8984 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2308,6 +2308,42 @@ rule, so its view of the environment always matches the child's. The developers' guide documents the module layout and the `PATH` composition helper under "Module: `runner::process::command_env`". +Integration-test support finds the already-built `netsuke` executable before +spawning it. Its locator derives an ordered candidate list from the test +executable's layout and injected `CARGO_TARGET_DIR`, then selects the first +existing candidate. The property tests construct the filesystem layout and the +expected result independently, so they validate both successful selection and +the diagnostic that records every attempted candidate. + +For screen readers: The following sequence shows a property test creating +candidate paths, asking the locator to inspect them in order, and comparing the +result with its independently reconstructed expectation. When a candidate +exists, the locator returns the first matching path. When none exists, it +returns a diagnostic listing every candidate it checked. + +```mermaid +sequenceDiagram + participant PropertyTest + participant Locator + participant Filesystem + participant Diagnostic + + PropertyTest->>Filesystem: create candidate paths from generated layout + PropertyTest->>Locator: locate_executable() + Locator->>Filesystem: check candidates in lookup order + Filesystem-->>Locator: candidate presence + alt executable candidate exists + Locator-->>PropertyTest: first matching path + else no candidate exists + Locator->>Diagnostic: build missing-candidate diagnostic + Diagnostic-->>PropertyTest: missing diagnostic + end + PropertyTest->>PropertyTest: verify independently reconstructed result +``` + +Figure: Property-based test support resolves the first executable candidate or +reports every missing candidate. + ### 6.2 The Criticality of Shell Escaping A primary security responsibility for Netsuke is the prevention of command From 22d758dda8f430e1b5bf5fae9a8dd3c4749e026e Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 28 Aug 2026 20:24:20 +0200 Subject: [PATCH 3/5] Document executable locator property tests (#533) Describe the private property-test child module and its split responsibility from the locator's fixed-layout regression tests. --- docs/developers-guide.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6ce538f72..48aac23c3 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -3610,6 +3610,14 @@ The locator's unit tests live beside it in executable layouts, an `out` directory that is *not* the Cargo build layout, both fallback paths, and the missing-binary case. +The private, test-only child module +`test_support/src/netsuke/locator/tests/locator_property_tests.rs` owns the +generated property coverage. `locator.rs` retains the fixed Cargo-layout and +named presence-mask regression tests, while the child verifies candidate-list +content and order, first-present lookup selection across optional +`CARGO_TARGET_DIR`/profile/target-triple layouts, and missing-candidate +diagnostics. + ## Digest rendering `src/hex.rs` (`netsuke::hex`) is the single owner of lowercase hexadecimal From d18712e0f0cf4d836f2527e0fd64cb34dc2792bb Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 28 Aug 2026 20:34:19 +0200 Subject: [PATCH 4/5] Harden locator property fixtures (#533) Exclude Windows-reserved components and the sole presence-mask alias.\n\nAlign the design diagram with the locator API exercised by the\nproperty test. --- docs/netsuke-design.md | 2 +- .../locator/tests/locator_property_tests.rs | 35 ++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index c4cbc8984..3130b31a7 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2329,7 +2329,7 @@ sequenceDiagram participant Diagnostic PropertyTest->>Filesystem: create candidate paths from generated layout - PropertyTest->>Locator: locate_executable() + PropertyTest->>Locator: netsuke_executable_from() Locator->>Filesystem: check candidates in lookup order Filesystem-->>Locator: candidate presence alt executable candidate exists diff --git a/test_support/src/netsuke/locator/tests/locator_property_tests.rs b/test_support/src/netsuke/locator/tests/locator_property_tests.rs index 9233e9d1c..b8e51d702 100644 --- a/test_support/src/netsuke/locator/tests/locator_property_tests.rs +++ b/test_support/src/netsuke/locator/tests/locator_property_tests.rs @@ -10,21 +10,41 @@ use super::{binary_name, env_with_target_dir, touch, utf8_root}; use proptest::prelude::*; use proptest::test_runner::TestCaseError; +/// List the DOS device names that cannot form Windows path components. +const WINDOWS_RESERVED_DEVICE_NAMES: &[&str] = &[ + "con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8", + "com9", "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", +]; + +/// Determine whether `component` is reserved as a Windows device name. +fn is_windows_reserved_device_name(component: &str) -> bool { + WINDOWS_RESERVED_DEVICE_NAMES + .iter() + .any(|name| component.eq_ignore_ascii_case(name)) +} + +/// Generate a valid UTF-8 path component that is safe on Windows. +fn safe_component() -> impl Strategy { + "[a-z][a-z0-9_-]{0,8}".prop_filter("component must not be a Windows device name", |component| { + !is_windows_reserved_device_name(component) + }) +} + /// Generate a valid UTF-8 path component for a temporary-root child. fn root_component() -> impl Strategy { - "[a-z][a-z0-9_-]{0,8}" + safe_component() } /// Generate a valid Cargo profile component distinct from `deps`. fn profile_component() -> impl Strategy { - "[a-z][a-z0-9_-]{0,8}".prop_filter("profile component must not be `deps`", |component| { + safe_component().prop_filter("profile component must not be `deps`", |component| { component != "deps" }) } /// Generate a valid target-triple component distinct from `deps`. fn target_triple() -> impl Strategy { - "[a-z][a-z0-9_-]{0,8}".prop_filter("target triple must not be `deps`", |component| { + safe_component().prop_filter("target triple must not be `deps`", |component| { component != "deps" }) } @@ -34,6 +54,13 @@ fn target_dir_component() -> impl Strategy> { proptest::option::of(root_component()) } +/// Generate target directories that cannot alias the primary candidate. +fn lookup_target_dir_component() -> impl Strategy> { + target_dir_component().prop_filter("target directory must not be `build`", |component| { + component.as_deref() != Some("build") + }) +} + /// Build an absolute UTF-8 root under a newly allocated temporary directory. fn generated_root( root_component: String, @@ -79,7 +106,7 @@ proptest! { root_component in root_component(), profile in profile_component(), triple in target_triple(), - target_dir_component in target_dir_component(), + target_dir_component in lookup_target_dir_component(), presence in 0u8..8, ) { let (_temp, root) = generated_root(root_component)?; From 4f782e6a6f6e8c39fe8a3a0a9c2a473effe8ddc0 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 29 Aug 2026 00:17:07 +0200 Subject: [PATCH 5/5] Preserve snapshot guide on rebase Retain main's updated snapshot guidance while preserving issue 533's\nfinite-control-flow rationale. --- ...snapshot-testing-in-netsuke-using-insta.md | 79 ++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md index 9a333f13d..d03d5c8b9 100644 --- a/docs/snapshot-testing-in-netsuke-using-insta.md +++ b/docs/snapshot-testing-in-netsuke-using-insta.md @@ -68,7 +68,7 @@ use netsuke::NetsukeManifest; // assumed struct for parsed manifest use netsuke::ir::BuildGraph; // assumed IR data structure # fn main() -> Result<(), Box> { - // (Re-use the same manifest YAML as before) + // Example Netsuke manifest in YAML (string literal for test) let manifest_yaml = r#" netsuke_version: "0.1" rules: @@ -79,58 +79,65 @@ use netsuke::ir::BuildGraph; // assumed IR data structure deps: ["hello.c"] rule: "compile" "#; + + // 1. Parse manifest YAML into AST/manifest struct let manifest = NetsukeManifest::from_yaml_str(manifest_yaml)?; + + // 2. Generate the IR (BuildGraph) from the manifest let build_graph = BuildGraph::from_manifest(&manifest)?; - // Generate Ninja file content from the IR - // `generate` returns `Result`; handle errors - let ninja_file = ninja_gen::generate(&build_graph)?; + // 3. Convert IR to a deterministic string representation + // For example, use Debug trait or implement a custom Display/serialization + let ir_pretty = format!("{:#?}", build_graph); - // The output is a multi-line Ninja build script (as a String) - // Ensure the output is deterministic - // (e.g., consistent ordering of rules/targets) + // 4. Assert snapshot, storing output in tests/snapshots/ir/ Settings::new() - .set_snapshot_path("tests/snapshots/ninja") + .set_snapshot_path("tests/snapshots/ir") .bind(|| { - assert_snapshot!("simple_manifest_ninja", ninja_file); + assert_snapshot!("simple_manifest_ir", ir_pretty); }); Ok(()) # } ``` -The match explicitly handles the `Result` from `generate` so any formatting or -missing action errors surface during tests. Production code should propagate -the error and report it with `miette` rather than panicking. +This test involves: -Key points for Ninja snapshot tests: +- Construct a **deterministic** input (a small manifest with a known rule and + target). -- Use a known manifest input and first derive the IR. An IR can also be - constructed directly for tests, but using the manifest→IR pipeline ensures - realistic coverage. +- Run the IR generation (`BuildGraph::from_manifest`). This function should + produce the intermediate build graph. -- Call the Ninja generation function (`ninja_gen::generate`), which - yields a `Result`. This function traverses the IR and - outputs rules and build statements in Ninja syntax, returning an error if any - build edge references an undefined action. +- Format the IR consistently for comparison. Pretty-printed debug output + (`{:#?}`) can be used, but for more complex structures implement `Display` or + use `assert_yaml_snapshot!` to serialize the IR to YAML/JSON for clarity. -- As with IR, **determinism is crucial**. The Ninja output should list rules, - targets, and dependencies in a consistent order. For example, if the IR does - not preserve order, targets may need to be sorted by name or hashing and - deduplication must avoid randomness. The design’s approach of consolidating - rules by a hash of their properties should still produce the same ordering - given the same input, as long as iteration over hashmaps is avoided or - stabilized. +- Use `Settings::new().set_snapshot_path("tests/snapshots/ir")` to direct the + snapshot file to the IR snapshot directory. Call `assert_snapshot!` with a + snapshot name (`"simple_manifest_ir"`) and the IR output string. On first run, + `insta` will record this output as the reference snapshot. **Determinism in + IR Output:** To ensure consistent snapshots, the IR output must be + **deterministic**. This means that given the same manifest input, the IR’s + printed form should not vary between test runs or across machines. Pay + attention to ordering and ephemeral data: -- Use `Settings::set_snapshot_path` to store these snapshots in a separate - `tests/snapshots/ninja` directory. The snapshot name - `"simple_manifest_ninja"` identifies this particular scenario. +- **Ordering:** If `BuildGraph` contains collections (e.g. sets of targets + or rules), iterate or sort them in a fixed order before printing. Using + `BTreeMap` or sorting vectors of targets by name can help. This avoids + nondeterministic ordering from hash maps. -With this setup, IR tests and Ninja tests have distinct snapshot files. For -example, after the first test run (see next section), expected snapshot files -include `tests/snapshots/ir/simple_manifest_ir.snap` and -`tests/snapshots/ninja/simple_manifest_ninja.snap` (or combined snapshot files -per test module). These snapshot files contain the expected IR debug output and -Ninja file text respectively. +- **Stable Identifiers:** If IR includes IDs or memory addresses, prefer stable + identifiers. For example, when generating rule IDs, assign them in insertion + order so they are consistent, or omit details that can change. + +- **No timestamps or environment-specific data:** The IR should not include + timestamps, random values, or absolute file system paths. If such data is + unavoidable, use `insta` redactions or post-process the output to replace + them with placeholders (e.g., ``). + +By making the IR snapshot output stable, the snapshot tests will reliably catch +regressions. If the IR generation logic changes intentionally (e.g., new fields +added), the snapshot will change predictably, prompting a review. ## Writing Snapshot Tests for Ninja File Output