Refactor PathGuard for injectable env and add mock test

Cargo.toml

@@ -68,6 +68,7 @@ insta = { version = "1", features = ["yaml"] }
 assert_cmd = "2.0.17"
 mockable = { version = "0.3", features = ["mock"] }
 serial_test = "3"
+mockall = "0.11"
 
 [[test]]
 name = "cucumber"

tests/path_guard_tests.rs

@@ -0,0 +1,42 @@
+//! Tests for PATH restoration behaviour using mock environments.
+//!
+//! Verifies that `PathGuard` restores `PATH` without mutating the real
+//! process environment.
+
+#[path = "support/env_lock.rs"]
+mod env_lock;
+#[path = "support/path_guard.rs"]
+mod path_guard;
+
+use mockall::{Sequence, mock};
+use path_guard::{Env, PathGuard};
+use std::ffi::OsStr;
+
+mock! {
+    pub Env {}
+    impl Env for Env {
+        unsafe fn set_var(&mut self, key: &str, val: &OsStr);
+    }
+}
+
+#[test]
+fn restores_path_without_touching_real_env() {
+    let mut env = MockEnv::new();
+    let mut seq = Sequence::new();
+    env.expect_set_var()
+        .withf(|k, v| k == "PATH" && v == OsStr::new("/tmp"))
+        .times(1)
+        .in_sequence(&mut seq)
+        .return_const(());
+    env.expect_set_var()
+        .withf(|k, v| k == "PATH" && v == OsStr::new("/orig"))
+        .times(1)
+        .in_sequence(&mut seq)
+        .return_const(());
+    {
+        let mut guard = PathGuard::with_env("/orig".into(), env);
+        unsafe {
+            guard.env_mut().set_var("PATH", OsStr::new("/tmp"));
+        }
+    }
+}

tests/support/mod.rs

@@ -3,6 +3,8 @@
 //! This module provides helpers for creating fake executables along with
 //! logging utilities used in behavioural tests.
 
+#![allow(unexpected_cfgs, reason = "test utilities use custom cfg")]
+
 pub mod env_lock;
 pub mod path_guard;
 
@@ -17,6 +19,7 @@ use tempfile::TempDir;
 /// Create a fake Ninja executable that exits with `exit_code`.
 ///
 /// Returns the temporary directory and the path to the executable.
+#[cfg(test)]
 pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) {
     let dir = TempDir::new().expect("temp dir");
     let path = dir.path().join("ninja");

tests/support/path_guard.rs

@@ -3,35 +3,89 @@
 //! Provides a guard that resets the environment variable on drop so tests do
 //! not pollute global state.
 
-use std::ffi::OsString;
+#![expect(unexpected_cfgs, reason = "test utilities use custom cfg")]
+
+use std::ffi::{OsStr, OsString};
 
 use super::env_lock::EnvLock;
 
+/// Environment interface allowing `PATH` mutation.
+#[cfg(test)]
+pub trait Env {
+    /// Set an environment variable.
+    ///
+    /// # Safety
+    ///
+    /// Mutating process-wide state is `unsafe` in Rust 2024.
+    unsafe fn set_var(&mut self, key: &str, val: &OsStr);
+}
+
+/// Real environment implementation.
+#[cfg(test)]
+#[derive(Debug, Default)]
+pub struct RealEnv;
+
+impl Env for RealEnv {
+    #[expect(unsafe_op_in_unsafe_fn, reason = "delegates to std::env")]
+    unsafe fn set_var(&mut self, key: &str, val: &OsStr) {
+        std::env::set_var(key, val);
+    }
+}
+
 /// Guard that restores `PATH` to its original value when dropped.
 ///
 /// This uses RAII to ensure the environment is reset even if a test panics.
-#[allow(dead_code, reason = "only some tests mutate PATH")]
+#[cfg_attr(expect, expect(dead_code, reason = "only some tests mutate PATH"))]
+#[cfg_attr(not(expect), allow(dead_code, reason = "only some tests mutate PATH"))]
 #[derive(Debug)]
-pub struct PathGuard {
+pub struct PathGuard<E: Env = RealEnv> {
+    env: E,
     original_path: Option<OsString>,
 }
 
 impl PathGuard {
-    #[allow(dead_code, reason = "only some tests mutate PATH")]
-    /// Create a guard capturing the current `PATH`.
+    /// Create a guard capturing the current `PATH` using the real environment.
+    #[cfg_attr(expect, expect(dead_code, reason = "only some tests mutate PATH"))]
+    #[cfg_attr(not(expect), allow(dead_code, reason = "only some tests mutate PATH"))]
     pub fn new(original: OsString) -> Self {
+        Self::with_env(original, RealEnv)
+    }
+}
+
+impl<E: Env> PathGuard<E> {
+    /// Create a guard for `PATH` using a provided environment.
+    #[cfg_attr(expect, expect(dead_code, reason = "only some tests mutate PATH"))]
+    #[cfg_attr(not(expect), allow(dead_code, reason = "only some tests mutate PATH"))]
+    pub fn with_env(original: OsString, env: E) -> Self {
         Self {
+            env,
             original_path: Some(original),
         }
     }
-}
 
-impl Drop for PathGuard {
-    fn drop(&mut self) {
+    /// Access the underlying environment for mutation during a test.
+    #[cfg_attr(expect, expect(dead_code, reason = "used in env injection tests"))]
+    #[cfg_attr(not(expect), allow(dead_code, reason = "used in env injection tests"))]
+    pub fn env_mut(&mut self) -> &mut E {
+        &mut self.env
+    }
+
+    #[cfg_attr(expect, expect(dead_code, reason = "only used via Drop impl"))]
+    #[cfg_attr(not(expect), allow(dead_code, reason = "only used via Drop impl"))]
+    fn restore(&mut self) {
         let _lock = EnvLock::acquire();
         if let Some(path) = self.original_path.take() {
-            // Nightly marks `set_var` unsafe; restoring `PATH` cleans up global state.
-            unsafe { std::env::set_var("PATH", path) };
+            // SAFETY: `std::env::set_var` is `unsafe` in Rust 2024 because it
+            // mutates process-wide state. `EnvLock` serialises mutations and
+            // this guard's RAII drop restores the prior `PATH`, mitigating the
+            // unsafety.
+            unsafe { self.env.set_var("PATH", &path) };
         }
     }
 }
+
+impl<E: Env> Drop for PathGuard<E> {
+    fn drop(&mut self) {
+        self.restore();
+    }
+}