diff --git a/tests/assert_cmd_tests.rs b/tests/assert_cmd_tests.rs index 22ba6591..97767222 100644 --- a/tests/assert_cmd_tests.rs +++ b/tests/assert_cmd_tests.rs @@ -8,6 +8,7 @@ use assert_cmd::Command; use std::fs; use tempfile::tempdir; +#[expect(unused, reason = "support module exports helpers unused in this test")] mod support; #[test] diff --git a/tests/cucumber.rs b/tests/cucumber.rs index b4b7c5c3..d512ec20 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -28,6 +28,7 @@ mod check_ninja; #[path = "support/env.rs"] mod env; mod steps; +#[expect(unused, reason = "support module exports helpers unused in this test")] mod support; #[tokio::main] diff --git a/tests/ninja_env_tests.rs b/tests/ninja_env_tests.rs new file mode 100644 index 00000000..661f9f3f --- /dev/null +++ b/tests/ninja_env_tests.rs @@ -0,0 +1,49 @@ +use mockable::MockEnv; +use netsuke::runner::NINJA_ENV; +use rstest::rstest; +use std::env::VarError; +use support::env_lock::EnvLock; +use support::ninja_env::override_ninja_env; + +#[expect( + unused, + reason = "support module exports helpers unused in these tests" +)] +mod support; + +#[rstest] +#[case(Some("orig"))] +#[case(None)] +#[case(Some(""))] +fn override_ninja_env_restores(#[case] original: Option<&'static str>) { + let mut env = MockEnv::new(); + match original { + Some(val) => { + let returned = val.to_string(); + env.expect_raw() + .withf(|k| k == NINJA_ENV) + .times(1) + .return_once(move |_| Ok(returned)); + } + None => { + env.expect_raw() + .withf(|k| k == NINJA_ENV) + .times(1) + .return_once(|_| Err(VarError::NotPresent)); + } + } + + { + let _guard = override_ninja_env(EnvLock::acquire(), &env, "new"); + assert_eq!(std::env::var(NINJA_ENV).as_deref(), Ok("new")); + } + + match original { + Some(val) => assert_eq!(std::env::var(NINJA_ENV).as_deref(), Ok(val)), + None => assert!(std::env::var(NINJA_ENV).is_err()), + } + + let _cleanup = EnvLock::acquire(); + // SAFETY: `EnvLock` serialises this mutation; see above for details. + unsafe { std::env::remove_var(NINJA_ENV) }; +} diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index abff32e9..c221a490 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -1,5 +1,6 @@ +use mockable::DefaultEnv; use netsuke::cli::{BuildArgs, Cli, Commands}; -use netsuke::runner::{BuildTargets, NINJA_ENV, run, run_ninja}; +use netsuke::runner::{BuildTargets, run, run_ninja}; use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; @@ -8,6 +9,7 @@ use std::path::{Path, PathBuf}; mod check_ninja; mod support; use support::env_lock::EnvLock; +use support::ninja_env::override_ninja_env; use support::path_guard::PathGuard; /// Fixture: Put a fake `ninja` (that checks for a build file) on PATH. @@ -220,10 +222,12 @@ fn run_manifest_subcommand_writes_file() { #[serial] fn run_respects_env_override_for_ninja() { let (temp_dir, ninja_path) = support::fake_ninja(0); - let original = std::env::var_os(NINJA_ENV); - unsafe { - std::env::set_var(NINJA_ENV, &ninja_path); - } + // `NINJA_ENV` expects UTF-8; lossy conversion is acceptable in this test. + let program = ninja_path.to_string_lossy().into_owned(); + let env = DefaultEnv::new(); + // `set_var` is `unsafe` on Rust 2024; the lock serialises the mutation and + // `override_ninja_env` restores the original value via its guard. + let _guard = override_ninja_env(EnvLock::acquire(), &env, &program); let temp = tempfile::tempdir().expect("temp dir"); let manifest_path = temp.path().join("Netsukefile"); @@ -241,14 +245,6 @@ fn run_respects_env_override_for_ninja() { let result = run(&cli); assert!(result.is_ok()); - - unsafe { - if let Some(val) = original { - std::env::set_var(NINJA_ENV, val); - } else { - std::env::remove_var(NINJA_ENV); - } - } drop(ninja_path); drop(temp_dir); } diff --git a/tests/support/env_lock.rs b/tests/support/env_lock.rs index cf22b3c2..333c7f0c 100644 --- a/tests/support/env_lock.rs +++ b/tests/support/env_lock.rs @@ -10,6 +10,7 @@ use std::sync::{Mutex, MutexGuard}; static ENV_LOCK: Mutex<()> = Mutex::new(()); #[allow(dead_code, reason = "only some tests mutate PATH")] +#[derive(Debug)] /// RAII guard that holds the global environment lock. pub struct EnvLock(MutexGuard<'static, ()>); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 0dae0142..d5e51161 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -4,6 +4,7 @@ //! logging utilities used in behavioural tests. pub mod env_lock; +pub mod ninja_env; pub mod path_guard; #[expect(unused_imports, reason = "re-export for selective test crates")] @@ -17,6 +18,13 @@ use tempfile::TempDir; /// Create a fake Ninja executable that exits with `exit_code`. /// /// Returns the temporary directory and the path to the executable. +#[cfg_attr( + not(test), + expect( + unused_code, + reason = "only some test crates spawn fake ninja binaries" + ) +)] 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/ninja_env.rs b/tests/support/ninja_env.rs new file mode 100644 index 00000000..8fcf00dc --- /dev/null +++ b/tests/support/ninja_env.rs @@ -0,0 +1,72 @@ +//! Override and restore [`NINJA_ENV`] for tests. +//! +//! Provides a helper that sets [`NINJA_ENV`] while ensuring it is restored +//! afterwards. This uses [`EnvLock`] to serialise mutations to the global +//! environment and captures the previous value through a `mockable::Env` +//! implementation so tests can inject their own state. + +use super::env_lock::EnvLock; +use mockable::Env; +use netsuke::runner::NINJA_ENV; + +/// Guard that resets `NINJA_ENV` on drop. +/// +/// Holding the guard keeps the environment override in place. Dropping it +/// restores the prior value while releasing the environment lock, cleaning up +/// global state even if a test panics. +#[must_use] +#[derive(Debug)] +pub struct NinjaEnvGuard { + _lock: EnvLock, + original: Option, +} + +/// Set [`NINJA_ENV`] to `value`, returning a guard that restores the previous +/// value when dropped. +/// +/// # Thread Safety +/// +/// This function is **not thread-safe**. Callers must supply an +/// [`EnvLock`](super::env_lock::EnvLock), which is stored in the returned guard +/// to serialise the mutation and ensure restoration occurs before the lock is +/// released. +/// +/// Drop order is enforced: dropping the guard restores [`NINJA_ENV`] and only +/// then releases the lock. +/// +/// # Examples +/// ```ignore +/// use mockable::DefaultEnv; +/// use crate::support::{env_lock::EnvLock, ninja_env::override_ninja_env}; +/// let env = DefaultEnv::new(); +/// let _guard = override_ninja_env(EnvLock::acquire(), &env, "/usr/bin/ninja"); +/// ``` +#[cfg_attr( + not(test), + expect(unused_code, reason = "only some tests override NINJA_ENV") +)] +pub fn override_ninja_env(lock: EnvLock, env: &impl Env, value: &str) -> NinjaEnvGuard { + let original = env.raw(NINJA_ENV).ok(); + // Safety: `EnvLock` serialises this mutation. `set_var` is `unsafe` on Rust + // 2024 and the guard restores the prior value on drop. + unsafe { std::env::set_var(NINJA_ENV, value) }; + NinjaEnvGuard { + _lock: lock, + original, + } +} + +impl Drop for NinjaEnvGuard { + fn drop(&mut self) { + // Safety: the guard holds [`EnvLock`] for its lifetime, so these + // `set_var`/`remove_var` calls are serialised. Both functions are + // `unsafe` on Rust 2024. + unsafe { + if let Some(ref val) = self.original { + std::env::set_var(NINJA_ENV, val); + } else { + std::env::remove_var(NINJA_ENV); + } + } + } +}