diff --git a/Cargo.lock b/Cargo.lock index d7f8b449..bfab925c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -884,6 +884,7 @@ dependencies = [ "itoa", "minijinja", "mockable", + "mockall", "rstest", "semver", "serde", diff --git a/Cargo.toml b/Cargo.toml index 048b393d..8c760e58 100644 --- a/Cargo.toml +++ b/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" diff --git a/tests/path_guard_tests.rs b/tests/path_guard_tests.rs new file mode 100644 index 00000000..2f3008b2 --- /dev/null +++ b/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")); + } + } +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 0dae0142..e2646b1f 100644 --- a/tests/support/mod.rs +++ b/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"); diff --git a/tests/support/path_guard.rs b/tests/support/path_guard.rs index 96c54dcd..9c7e40d3 100644 --- a/tests/support/path_guard.rs +++ b/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 { + env: E, original_path: Option, } 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 PathGuard { + /// 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 Drop for PathGuard { + fn drop(&mut self) { + self.restore(); + } +}