From 8704f228f06ba831dae009c117ab76e04728c2cd Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 12 Jun 2026 16:38:08 +0200 Subject: [PATCH 1/2] Add Ninja snapshot test for command_available expansion (#310) PR #309 introduced the `command_available` stdlib predicate but no snapshot pinned the generated Ninja output for the path where a `when: command_available(...)` / `when: not command_available(...)` pair filters top-level actions before `ninja_gen::generate`. Add `command_available_manifest_ninja_snapshot`, modelled on `conditional_manifest_ninja_snapshot`: the manifest guards two complementary actions on a command name guaranteed absent (with `cwd_mode="never"` for determinism), asserts the fallback action is present and the preferred action absent, and snapshots the Ninja output to `tests/snapshots/ninja`. --- tests/ninja_snapshot_tests.rs | 47 +++++++++++++++++++ ...sts__command_available_manifest_ninja.snap | 14 ++++++ 2 files changed, 61 insertions(+) create mode 100644 tests/snapshots/ninja/ninja_snapshot_tests__command_available_manifest_ninja.snap diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index c477012b6..b26685a2b 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -131,6 +131,53 @@ fn conditional_manifest_ninja_snapshot() -> Result<()> { Ok(()) } +#[test] +fn command_available_manifest_ninja_snapshot() -> Result<()> { + // `cwd_mode="never"` plus a command name guaranteed absent keeps the + // expansion deterministic without any real binary on PATH. + let manifest_yaml = r#" + netsuke_version: "1.0.0" + actions: + - name: preferred-action + command: echo preferred + when: command_available("netsuke-command-that-should-not-exist", cwd_mode="never") + - name: fallback-action + command: echo fallback + when: not command_available("netsuke-command-that-should-not-exist", cwd_mode="never") + rules: + - name: touch + command: "touch $out" + targets: + - name: out/result + sources: in/source + rule: touch + "#; + + let manifest = manifest::from_str(manifest_yaml)?; + let ir = BuildGraph::from_manifest(&manifest)?; + let ninja_content = ninja_gen::generate(&ir)?; + + ensure!( + ninja_content.contains("fallback"), + "expected fallback action in Ninja output:\n{ninja_content}" + ); + ensure!( + !ninja_content.contains("preferred"), + "preferred action guarded by an absent command should not appear:\n{ninja_content}" + ); + + let mut settings = Settings::new(); + settings.set_snapshot_path(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/snapshots/ninja" + )); + settings.bind(|| { + assert_snapshot!("command_available_manifest_ninja", ninja_content); + }); + + Ok(()) +} + #[test] fn multi_command_manifest_ninja_snapshot() -> Result<()> { let fixture_dir = Dir::open_ambient_dir(env!("CARGO_MANIFEST_DIR"), ambient_authority()) diff --git a/tests/snapshots/ninja/ninja_snapshot_tests__command_available_manifest_ninja.snap b/tests/snapshots/ninja/ninja_snapshot_tests__command_available_manifest_ninja.snap new file mode 100644 index 000000000..4b03d996e --- /dev/null +++ b/tests/snapshots/ninja/ninja_snapshot_tests__command_available_manifest_ninja.snap @@ -0,0 +1,14 @@ +--- +source: tests/ninja_snapshot_tests.rs +assertion_line: 174 +expression: ninja_content +--- +rule 001cc863ffad2333bb5b4a22fde35146827965e79233821681254065519a1ce2 + command = touch out/result + +rule e8c3bab461990fead7f11b007334dcd68b89db712472c5a294c69609dd7302b5 + command = echo fallback + +build fallback-action: e8c3bab461990fead7f11b007334dcd68b89db712472c5a294c69609dd7302b5 + +build out/result: 001cc863ffad2333bb5b4a22fde35146827965e79233821681254065519a1ce2 in/source From 33153402b5abedff908fb6cfa867620334d29fad Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 14 Aug 2026 15:48:15 +0200 Subject: [PATCH 2/2] Make command_available snapshot test PATH-independent (#310) The fixture relied on a command name guaranteed absent, but `command_available` still searches every directory on the host PATH even with `cwd_mode="never"`; a developer or CI image carrying a binary named `netsuke-command-that-should-not-exist` would select the preferred action and break both the assertions and the snapshot. Add a public `manifest::from_str_with_env_and_config` entrypoint so a test can inject a full `StdlibConfig` alongside the existing `EnvReader` seam, then pin the resolver to an empty PATH with `StdlibConfig::from_current_dir()?.with_path_override("")`. With `cwd_mode="never"` the workspace fallback is disabled too, so the expansion is deterministic on every host without touching the process environment. The new entrypoint lives in `src/manifest/parse_with_config.rs` to keep `src/manifest/mod.rs` inside the 400-line Whitaker limit. Document the seam in the developers-guide injected-environment list. Co-Authored-By: Claude --- docs/developers-guide.md | 4 +- src/manifest/mod.rs | 2 + src/manifest/parse_with_config.rs | 73 +++++++++++++++++++++++++++++++ tests/ninja_snapshot_tests.rs | 17 ++++--- 4 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 src/manifest/parse_with_config.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 2da36302e..1bd99ad2c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -2250,8 +2250,8 @@ mutate_env_var(world, EnvVarKey::from("NETSUKE_EMOJI"), None)?; Production-facing unit and integration tests follow the same rule. Use the appropriate injected seam, such as `run_with_ninja_program`, -`from_path_with_policy_and_env`, `StdlibConfig::with_path_override`, -`StdlibConfig::with_home_override`, or +`from_path_with_policy_and_env`, `manifest::from_str_with_env_and_config`, +`StdlibConfig::with_path_override`, `StdlibConfig::with_home_override`, or `StdlibConfig::with_command_path_override`. End-to-end tests may call `env_clear()` and then apply values with `Command::env`, because the mutation is confined to the child. diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 04b986a20..8ea3d6954 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -44,6 +44,7 @@ mod expand; mod glob; mod hints; mod jinja_macros; +mod parse_with_config; mod render; /// JSON representation of a manifest node after YAML and Jinja evaluation. @@ -58,6 +59,7 @@ pub use env_reader::{EnvReadError, EnvReader, process_env_reader}; pub use glob::glob_paths; pub(crate) use expand::expand_foreach; +pub use parse_with_config::from_str_with_env_and_config; pub use render::render_manifest; use self::{env_reader::env_var_with, jinja_macros::register_manifest_macros}; diff --git a/src/manifest/parse_with_config.rs b/src/manifest/parse_with_config.rs new file mode 100644 index 000000000..36141d695 --- /dev/null +++ b/src/manifest/parse_with_config.rs @@ -0,0 +1,73 @@ +//! Manifest string parsing with explicit stdlib configuration. +//! +//! The `from_str`-family entrypoints in the parent module parse with the +//! default stdlib registration. This module hosts the variant that also +//! injects a caller-owned [`StdlibConfig`], so tests can pin behaviour such +//! as `command_available` resolution without mutating the process +//! environment. + +use super::{EnvReader, ManifestName, ManifestParse, from_str_named}; +use crate::{ast::NetsukeManifest, stdlib::StdlibConfig}; +use anyhow::Result; + +/// Parse a manifest string with an explicit environment reader and stdlib +/// configuration. +/// +/// Combines the [`EnvReader`] of +/// [`crate::manifest::from_str_with_env`] with a full +/// [`StdlibConfig`], so a caller — in practice a test — can pin stdlib +/// behaviour such as `command_available` resolution without touching the +/// process environment or `PATH`. For instance, +/// [`StdlibConfig::with_path_override`] substitutes the host `PATH` the +/// helper searches. +/// +/// # Errors +/// +/// Returns an error if YAML parsing or Jinja evaluation fails. +/// +/// # Examples +/// +/// ``` +/// use netsuke::{ +/// ast::Recipe, +/// manifest::{EnvReadError, EnvReader, from_str_with_env_and_config}, +/// stdlib::StdlibConfig, +/// }; +/// use std::sync::Arc; +/// +/// let reader: EnvReader = Arc::new(|name| match name { +/// "PROFILE" => Ok("release".to_owned()), +/// _ => Err(EnvReadError::NotPresent), +/// }); +/// let config = StdlibConfig::from_current_dir() +/// .expect("construct stdlib config") +/// .with_path_override(""); +/// let yaml = concat!( +/// "netsuke_version: 1.0.0\n", +/// "targets:\n", +/// " - name: build\n", +/// " command: echo {{ env('PROFILE') }}\n", +/// ); +/// let manifest = +/// from_str_with_env_and_config(yaml, &reader, config).expect("parse manifest"); +/// +/// assert!(matches!( +/// &manifest.targets[0].recipe, +/// Recipe::Command { command } if command.as_single() == Some("echo release") +/// )); +/// ``` +pub fn from_str_with_env_and_config( + yaml: &str, + env_reader: &EnvReader, + stdlib_config: StdlibConfig, +) -> Result { + from_str_named( + yaml, + ManifestParse { + name: &ManifestName::new("Netsukefile"), + stdlib_config: Some(stdlib_config), + env_reader, + }, + &mut None, + ) +} diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index b26685a2b..49de45e0d 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -4,11 +4,10 @@ //! output using `insta`, and validate it with the real `ninja` //! executable. The manifest uses a simple TOUCH rule so the build is //! fast and deterministic. - use anyhow::{Context, Result, ensure}; use cap_std::{ambient_authority, fs_utf8::Dir}; use insta::{Settings, assert_snapshot}; -use netsuke::{ir::BuildGraph, manifest, ninja_gen}; +use netsuke::{ir::BuildGraph, manifest, ninja_gen, stdlib::StdlibConfig}; use std::{fs, process::Command}; use tempfile::tempdir; use test_support::ensure_binaries_available; @@ -133,8 +132,11 @@ fn conditional_manifest_ninja_snapshot() -> Result<()> { #[test] fn command_available_manifest_ninja_snapshot() -> Result<()> { - // `cwd_mode="never"` plus a command name guaranteed absent keeps the - // expansion deterministic without any real binary on PATH. + // Pin the `command_available` resolver to an empty PATH through the + // stdlib configuration seam, so a host or CI image with a binary named + // like the fixture cannot flip the guard. `cwd_mode="never"` additionally + // excludes the workspace root from the search; the absent command name + // alone is not sufficient for determinism. let manifest_yaml = r#" netsuke_version: "1.0.0" actions: @@ -153,7 +155,12 @@ fn command_available_manifest_ninja_snapshot() -> Result<()> { rule: touch "#; - let manifest = manifest::from_str(manifest_yaml)?; + let config = StdlibConfig::from_current_dir()?.with_path_override(""); + let manifest = manifest::from_str_with_env_and_config( + manifest_yaml, + &manifest::process_env_reader(), + config, + )?; let ir = BuildGraph::from_manifest(&manifest)?; let ninja_content = ninja_gen::generate(&ir)?;