Inject mock env for Ninja tests

tests/cucumber.rs

@@ -19,21 +19,14 @@ pub struct CliWorld {
     pub run_error: Option<String>,
     /// Temporary directory handle for test isolation.
     pub temp: Option<tempfile::TempDir>,
-    /// Original `PATH` value restored after each scenario.
-    pub original_path: Option<std::ffi::OsString>,
-}
-
-impl Drop for CliWorld {
-    fn drop(&mut self) {
-        if let Some(path) = self.original_path.take() {
-            // SAFETY: nightly marks `set_var` as unsafe; restore path for isolation.
-            unsafe {
-                std::env::set_var("PATH", path);
-            }
-        }
-    }
+    /// Guard that restores `PATH` after each scenario.
+    pub path_guard: Option<support::path_guard::PathGuard>,
 }
 
+#[path = "support/check_ninja.rs"]
+mod check_ninja;
+#[path = "support/env.rs"]
+mod env;
 mod steps;
 mod support;
 

tests/runner_tests.rs

@@ -2,44 +2,27 @@ use netsuke::cli::{BuildArgs, Cli, Commands};
 use netsuke::runner::{BuildTargets, NINJA_ENV, run, run_ninja};
 use rstest::{fixture, rstest};
 use serial_test::serial;
-use std::ffi::OsString;
 use std::path::{Path, PathBuf};
 
+#[path = "support/check_ninja.rs"]
+mod check_ninja;
 mod support;
-
-/// Guard that restores PATH to its original value when dropped.
-///
-/// Using a simple guard avoids heap allocation and guarantees teardown on
-/// early returns or panics.
-struct PathGuard {
-    original: OsString,
-}
-
-impl PathGuard {
-    fn new(original: OsString) -> Self {
-        Self { original }
-    }
-}
-
-impl Drop for PathGuard {
-    fn drop(&mut self) {
-        // Nightly marks set_var unsafe.
-        unsafe { std::env::set_var("PATH", &self.original) };
-    }
-}
+use support::env_lock::EnvLock;
+use support::path_guard::PathGuard;
 
 /// Fixture: Put a fake `ninja` (that checks for a build file) on PATH.
 ///
 /// Returns: (tempdir holding ninja, `ninja_path`, PATH guard)
 #[fixture]
 fn ninja_in_path() -> (tempfile::TempDir, PathBuf, PathGuard) {
-    let (ninja_dir, ninja_path) = support::fake_ninja_check_build_file();
+    let (ninja_dir, ninja_path) = check_ninja::fake_ninja_check_build_file();
 
     // Save PATH and prepend our fake ninja directory.
     let original_path = std::env::var_os("PATH").unwrap_or_default();
     let mut paths: Vec<_> = std::env::split_paths(&original_path).collect();
     paths.insert(0, ninja_dir.path().to_path_buf());
     let new_path = std::env::join_paths(paths).expect("join paths");
+    let _lock = EnvLock::acquire();
     // Nightly marks set_var unsafe.
     unsafe { std::env::set_var("PATH", &new_path) };
 
@@ -61,6 +44,7 @@ fn ninja_with_exit_code(#[default(0)] exit_code: i32) -> (tempfile::TempDir, Pat
     let mut paths: Vec<_> = std::env::split_paths(&original_path).collect();
     paths.insert(0, ninja_dir.path().to_path_buf());
     let new_path = std::env::join_paths(paths).expect("join paths");
+    let _lock = EnvLock::acquire();
     // Nightly marks set_var unsafe.
     unsafe { std::env::set_var("PATH", &new_path) };
 

tests/steps/process_steps.rs

@@ -1,28 +1,32 @@
 //! Step definitions for Ninja process execution.
 
-use crate::{CliWorld, support};
+use crate::{CliWorld, check_ninja, env, support};
 use cucumber::{given, then, when};
+use mockable::Env;
 use netsuke::runner::{self, BuildTargets, NINJA_PROGRAM};
 use std::fs;
 use std::path::{Path, PathBuf};
+use support::env_lock::EnvLock;
+use support::path_guard::PathGuard;
 use tempfile::{NamedTempFile, TempDir};
 
 /// Installs a test-specific ninja binary and updates the `PATH`.
 #[expect(
     clippy::needless_pass_by_value,
     reason = "helper owns path for simplicity"
 )]
-fn install_test_ninja(world: &mut CliWorld, dir: TempDir, ninja_path: PathBuf) {
-    let original = world
-        .original_path
-        .get_or_insert_with(|| std::env::var_os("PATH").unwrap_or_default());
-
-    let new_path = format!("{}:{}", dir.path().display(), original.to_string_lossy());
-    // SAFETY: nightly marks `set_var` as unsafe; override path for test isolation.
+fn install_test_ninja(env: &impl Env, world: &mut CliWorld, dir: TempDir, ninja_path: PathBuf) {
+    let original = env.raw("PATH").unwrap_or_default();
+    let guard = PathGuard::new(original.clone().into());
+    let new_path = format!("{}:{}", dir.path().display(), original);
+    // SAFETY: `std::env::set_var` is `unsafe` in Rust 2024 due to global state.
+    // `EnvLock` serialises mutations and `PathGuard` restores the prior value.
+    let _lock = EnvLock::acquire();
     unsafe {
         std::env::set_var("PATH", &new_path);
     }
 
+    world.path_guard = Some(guard);
     world.ninja = Some(ninja_path.to_string_lossy().into_owned());
     world.temp = Some(dir);
 }
@@ -31,14 +35,16 @@ fn install_test_ninja(world: &mut CliWorld, dir: TempDir, ninja_path: PathBuf) {
 #[given(expr = "a fake ninja executable that exits with {int}")]
 fn fake_ninja(world: &mut CliWorld, code: i32) {
     let (dir, path) = support::fake_ninja(code);
-    install_test_ninja(world, dir, path);
+    let env = env::mocked_path_env();
+    install_test_ninja(&env, world, dir, path);
 }
 
 /// Creates a fake ninja executable that validates the build file path.
 #[given("a fake ninja executable that checks for the build file")]
 fn fake_ninja_check(world: &mut CliWorld) {
-    let (dir, path) = support::fake_ninja_check_build_file();
-    install_test_ninja(world, dir, path);
+    let (dir, path) = check_ninja::fake_ninja_check_build_file();
+    let env = env::mocked_path_env();
+    install_test_ninja(&env, world, dir, path);
 }
 
 /// Sets up a scenario where no ninja executable is available.
@@ -50,7 +56,8 @@ fn fake_ninja_check(world: &mut CliWorld) {
 fn no_ninja(world: &mut CliWorld) {
     let dir = TempDir::new().expect("temp dir");
     let path = dir.path().join("ninja");
-    install_test_ninja(world, dir, path);
+    let env = env::mocked_path_env();
+    install_test_ninja(&env, world, dir, path);
 }
 
 /// Updates the CLI to use the temporary directory created for the fake ninja.
@@ -92,7 +99,7 @@ fn run(world: &mut CliWorld) {
     if !manifest_path.exists() {
         let mut file =
             NamedTempFile::new_in(dir.path()).expect("Failed to create temporary manifest file");
-        support::write_manifest(&mut file).expect("Failed to write manifest content");
+        env::write_manifest(&mut file).expect("Failed to write manifest content");
         file.persist(&manifest_path)
             .expect("Failed to persist manifest file");
     }

tests/support/check_ninja.rs

@@ -0,0 +1,36 @@
+//! Helpers for validating build file paths via fake Ninja binaries.
+
+use std::fs::{self, File};
+use std::io::Write;
+use std::path::PathBuf;
+use tempfile::TempDir;
+
+/// Create a fake Ninja that validates the build file path provided via `-f`.
+///
+/// The script exits with status `1` if the file is missing or not a regular
+/// file, otherwise `0`.
+pub fn fake_ninja_check_build_file() -> (TempDir, PathBuf) {
+    let dir = TempDir::new().expect("temp dir");
+    let path = dir.path().join("ninja");
+    let mut file = File::create(&path).expect("script");
+    writeln!(
+        file,
+        concat!(
+            "#!/bin/sh\n",
+            "if [ \"$1\" = \"-f\" ] && [ ! -f \"$2\" ]; then\n",
+            "  echo 'missing build file: $2' >&2\n",
+            "  exit 1\n",
+            "fi\n",
+            "exit 0"
+        ),
+    )
+    .expect("write script");
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        let mut perms = fs::metadata(&path).expect("meta").permissions();
+        perms.set_mode(0o755);
+        fs::set_permissions(&path, perms).expect("perms");
+    }
+    (dir, path)
+}

tests/support/env.rs

@@ -0,0 +1,40 @@
+//! Helpers for environment manipulation in process tests.
+//!
+//! Provides fixtures and utilities for managing `PATH` and writing minimal
+//! manifests.
+
+use mockable::MockEnv;
+use rstest::fixture;
+use std::io::{self, Write};
+
+/// Fixture: capture the original `PATH` via a mocked environment.
+///
+/// Returns a `MockEnv` that yields the current `PATH` when queried. Tests can
+/// modify the real environment while the mock continues to expose the initial
+/// value.
+#[fixture]
+pub fn mocked_path_env() -> MockEnv {
+    let original = std::env::var("PATH").unwrap_or_default();
+    let mut env = MockEnv::new();
+    env.expect_raw()
+        .withf(|k| k == "PATH")
+        .returning(move |_| Ok(original.clone()));
+    env
+}
+
+/// Write a minimal manifest to `file`.
+///
+/// The manifest declares a single `hello` target that prints a greeting.
+pub fn write_manifest(file: &mut impl Write) -> io::Result<()> {
+    writeln!(
+        file,
+        concat!(
+            "netsuke_version: \"1.0.0\"\n",
+            "targets:\n",
+            "  - name: hello\n",
+            "    recipe:\n",
+            "      kind: command\n",
+            "      command: \"echo hi\"\n"
+        ),
+    )
+}

tests/support/env_lock.rs

@@ -0,0 +1,22 @@
+//! Serialise environment mutations across tests.
+//!
+//! The `EnvLock` guard ensures that changes to global state like `PATH` are
+//! synchronised, preventing interference between concurrently running tests.
+
+use std::sync::{Mutex, MutexGuard};
+
+#[allow(dead_code, reason = "only some tests mutate PATH")]
+/// Global mutex protecting environment changes.
+static ENV_LOCK: Mutex<()> = Mutex::new(());
+
+#[allow(dead_code, reason = "only some tests mutate PATH")]
+/// RAII guard that holds the global environment lock.
+pub struct EnvLock(MutexGuard<'static, ()>);
+
+impl EnvLock {
+    #[allow(dead_code, reason = "only some tests mutate PATH")]
+    /// Acquire the global lock serialising environment mutations.
+    pub fn acquire() -> Self {
+        Self(ENV_LOCK.lock().expect("env lock"))
+    }
+}