From 1e10a84330703c6141a9771ba7c2905d8a8ae652 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 12 Aug 2025 23:48:34 +0100 Subject: [PATCH 1/5] Promote test support to crate --- Cargo.lock | 10 ++ Cargo.toml | 1 + test_support/Cargo.toml | 9 ++ .../src}/check_ninja.rs | 0 {tests/support => test_support/src}/env.rs | 6 +- .../support => test_support/src}/env_lock.rs | 0 test_support/src/lib.rs | 36 +++++++ .../src}/path_guard.rs | 7 +- tests/assert_cmd_tests.rs | 5 +- tests/cucumber.rs | 8 +- tests/env_path_tests.rs | 10 +- tests/path_guard_tests.rs | 7 +- tests/runner_tests.rs | 18 ++-- tests/steps/process_steps.rs | 9 +- tests/support/mod.rs | 94 ------------------- 15 files changed, 78 insertions(+), 142 deletions(-) create mode 100644 test_support/Cargo.toml rename {tests/support => test_support/src}/check_ninja.rs (100%) rename {tests/support => test_support/src}/env.rs (92%) rename {tests/support => test_support/src}/env_lock.rs (100%) create mode 100644 test_support/src/lib.rs rename {tests/support => test_support/src}/path_guard.rs (87%) delete mode 100644 tests/support/mod.rs diff --git a/Cargo.lock b/Cargo.lock index bfab925c..65fca98a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -894,6 +894,7 @@ dependencies = [ "serial_test", "sha2", "tempfile", + "test_support", "thiserror", "tokio", "tracing", @@ -1524,6 +1525,15 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" +[[package]] +name = "test_support" +version = "0.1.0" +dependencies = [ + "mockable", + "rstest", + "tempfile", +] + [[package]] name = "textwrap" version = "0.16.2" diff --git a/Cargo.toml b/Cargo.toml index 8c760e58..fe343b0b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,6 +69,7 @@ assert_cmd = "2.0.17" mockable = { version = "0.3", features = ["mock"] } serial_test = "3" mockall = "0.11" +test_support = { path = "test_support" } [[test]] name = "cucumber" diff --git a/test_support/Cargo.toml b/test_support/Cargo.toml new file mode 100644 index 00000000..a414064c --- /dev/null +++ b/test_support/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "test_support" +version = "0.1.0" +edition = "2024" + +[dependencies] +tempfile = "3.8.0" +mockable = { version = "0.3", features = ["mock"] } +rstest = "0.18.0" diff --git a/tests/support/check_ninja.rs b/test_support/src/check_ninja.rs similarity index 100% rename from tests/support/check_ninja.rs rename to test_support/src/check_ninja.rs diff --git a/tests/support/env.rs b/test_support/src/env.rs similarity index 92% rename from tests/support/env.rs rename to test_support/src/env.rs index 7686663f..c908b287 100644 --- a/tests/support/env.rs +++ b/test_support/src/env.rs @@ -9,11 +9,9 @@ use std::ffi::{OsStr, OsString}; use std::io::{self, Write}; use std::path::Path; -use crate::support::env_lock::EnvLock; -use crate::support::path_guard::PathGuard; +use crate::{env_lock::EnvLock, path_guard::PathGuard}; /// Alias for the real process environment. -#[allow(dead_code, reason = "re-exported for tests")] pub type SystemEnv = DefaultEnv; /// Environment trait with mutation capabilities. @@ -57,7 +55,6 @@ pub fn mocked_path_env() -> MockEnv { /// Write a minimal manifest to `file`. /// /// The manifest declares a single `hello` target that prints a greeting. -#[allow(dead_code, reason = "used in Cucumber tests")] pub fn write_manifest(file: &mut impl Write) -> io::Result<()> { writeln!( file, @@ -77,7 +74,6 @@ pub fn write_manifest(file: &mut impl Write) -> io::Result<()> { /// Mutating `PATH` is `unsafe` in Rust 2024 because it alters process globals. /// `EnvLock` serialises access and `PathGuard` rolls back the change, keeping /// the unsafety scoped to a single test. -#[allow(dead_code, reason = "used in runner tests")] pub fn prepend_dir_to_path(env: &impl EnvMut, dir: &Path) -> PathGuard { let original = env.raw("PATH").ok(); let original_os = original.clone().map(OsString::from); diff --git a/tests/support/env_lock.rs b/test_support/src/env_lock.rs similarity index 100% rename from tests/support/env_lock.rs rename to test_support/src/env_lock.rs diff --git a/test_support/src/lib.rs b/test_support/src/lib.rs new file mode 100644 index 00000000..e79d7ac2 --- /dev/null +++ b/test_support/src/lib.rs @@ -0,0 +1,36 @@ +//! Test utilities for process management. +//! +//! This module provides helpers for creating fake executables along with +//! logging utilities used in behavioural tests. + +pub mod check_ninja; +pub mod env; +pub mod env_lock; +pub mod path_guard; + +pub use path_guard::PathGuard; + +use std::fs::{self, File}; +use std::io::Write; +use std::path::PathBuf; +use tempfile::TempDir; + +/// Create a fake Ninja executable that exits with `exit_code`. +/// +/// Returns the temporary directory and the path to the executable. +pub fn fake_ninja(exit_code: i32) -> (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, "#!/bin/sh\nexit {exit_code}").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) +} + +// Additional helpers can be added here as the test suite evolves. diff --git a/tests/support/path_guard.rs b/test_support/src/path_guard.rs similarity index 87% rename from tests/support/path_guard.rs rename to test_support/src/path_guard.rs index 662ad5c6..94a0221f 100644 --- a/tests/support/path_guard.rs +++ b/test_support/src/path_guard.rs @@ -5,7 +5,7 @@ use std::ffi::{OsStr, OsString}; -use super::env_lock::EnvLock; +use crate::env_lock::EnvLock; /// Environment abstraction for setting variables. pub trait Env { @@ -28,7 +28,6 @@ impl Env for StdEnv { } /// Original `PATH` state captured by `PathGuard`. -#[allow(dead_code, reason = "only some tests mutate PATH")] #[derive(Debug)] enum OriginalPath { Unset, @@ -38,7 +37,6 @@ enum OriginalPath { /// 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")] #[derive(Debug)] pub struct PathGuard { original: Option, @@ -49,7 +47,6 @@ impl PathGuard { /// Create a guard capturing the current `PATH` using the real environment. /// /// Returns a guard that restores the variable when dropped. - #[allow(dead_code, reason = "only some tests mutate PATH")] pub fn new(original: Option) -> Self { let state = original.map_or(OriginalPath::Unset, OriginalPath::Set); Self { @@ -61,7 +58,6 @@ impl PathGuard { impl PathGuard { /// Create a guard that uses `env` to restore `PATH`. - #[allow(dead_code, reason = "only some tests mutate PATH")] pub fn with_env(original: OsString, env: E) -> Self { Self { original: Some(OriginalPath::Set(original)), @@ -70,7 +66,6 @@ impl PathGuard { } /// Access the underlying environment. - #[allow(dead_code, reason = "only some tests mutate PATH")] pub fn env_mut(&mut self) -> &mut E { &mut self.env } diff --git a/tests/assert_cmd_tests.rs b/tests/assert_cmd_tests.rs index 22ba6591..9d201670 100644 --- a/tests/assert_cmd_tests.rs +++ b/tests/assert_cmd_tests.rs @@ -7,8 +7,7 @@ use assert_cmd::Command; use std::fs; use tempfile::tempdir; - -mod support; +use test_support::fake_ninja; #[test] fn manifest_subcommand_writes_file() { @@ -27,7 +26,7 @@ fn manifest_subcommand_writes_file() { #[test] fn build_with_emit_writes_file() { - let (ninja_dir, _ninja_path) = support::fake_ninja(0); + let (ninja_dir, _ninja_path) = fake_ninja(0); let temp = tempdir().expect("temp dir"); fs::copy("tests/data/minimal.yml", temp.path().join("Netsukefile")).expect("copy manifest"); let output = temp.path().join("emitted.ninja"); diff --git a/tests/cucumber.rs b/tests/cucumber.rs index b4b7c5c3..11c44d45 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -1,6 +1,7 @@ //! Cucumber test runner and world state. use cucumber::World; +use test_support::path_guard::PathGuard; /// Shared state for Cucumber scenarios. #[derive(Debug, Default, World)] @@ -20,15 +21,10 @@ pub struct CliWorld { /// Temporary directory handle for test isolation. pub temp: Option, /// Guard that restores `PATH` after each scenario. - pub path_guard: Option, + pub path_guard: Option, } -#[path = "support/check_ninja.rs"] -mod check_ninja; -#[path = "support/env.rs"] -mod env; mod steps; -mod support; #[tokio::main] async fn main() { diff --git a/tests/env_path_tests.rs b/tests/env_path_tests.rs index 6227164c..78ea2539 100644 --- a/tests/env_path_tests.rs +++ b/tests/env_path_tests.rs @@ -4,12 +4,10 @@ use mockable::Env; use rstest::rstest; use serial_test::serial; - -#[path = "support/env.rs"] -mod env; -mod support; -use env::{SystemEnv, mocked_path_env, prepend_dir_to_path}; -use support::env_lock::EnvLock; +use test_support::{ + env::{SystemEnv, mocked_path_env, prepend_dir_to_path}, + env_lock::EnvLock, +}; #[rstest] #[serial] diff --git a/tests/path_guard_tests.rs b/tests/path_guard_tests.rs index 2f3008b2..91fa7b70 100644 --- a/tests/path_guard_tests.rs +++ b/tests/path_guard_tests.rs @@ -3,14 +3,9 @@ //! 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; +use test_support::path_guard::{Env, PathGuard}; mock! { pub Env {} diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 50f54efd..25cd1163 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -3,14 +3,12 @@ use netsuke::runner::{BuildTargets, NINJA_ENV, run, run_ninja}; use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; - -#[path = "support/check_ninja.rs"] -mod check_ninja; -#[path = "support/env.rs"] -mod env; -mod support; -use env::{SystemEnv, prepend_dir_to_path}; -use support::path_guard::PathGuard; +use test_support::{ + check_ninja, + env::{SystemEnv, prepend_dir_to_path}, + fake_ninja, + path_guard::PathGuard, +}; /// Fixture: Put a fake `ninja` (that checks for a build file) on `PATH`. /// @@ -36,7 +34,7 @@ fn ninja_in_path() -> (tempfile::TempDir, PathBuf, PathGuard) { /// Returns: (tempdir holding ninja, `ninja_path`, PATH guard) #[fixture] fn ninja_with_exit_code(#[default(0)] exit_code: i32) -> (tempfile::TempDir, PathBuf, PathGuard) { - let (ninja_dir, ninja_path) = support::fake_ninja(exit_code); + let (ninja_dir, ninja_path) = fake_ninja(exit_code); let env = SystemEnv::new(); let guard = prepend_dir_to_path(&env, ninja_dir.path()); (ninja_dir, ninja_path, guard) @@ -209,7 +207,7 @@ fn run_manifest_subcommand_writes_file() { #[test] #[serial] fn run_respects_env_override_for_ninja() { - let (temp_dir, ninja_path) = support::fake_ninja(0); + let (temp_dir, ninja_path) = fake_ninja(0); let original = std::env::var_os(NINJA_ENV); unsafe { std::env::set_var(NINJA_ENV, &ninja_path); diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 4e96c71e..4dc3d474 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -1,15 +1,12 @@ //! Step definitions for Ninja process execution. -use crate::{ - CliWorld, check_ninja, - env::{self, EnvMut}, - support, -}; +use crate::CliWorld; use cucumber::{given, then, when}; use netsuke::runner::{self, BuildTargets, NINJA_PROGRAM}; use std::fs; use std::path::{Path, PathBuf}; use tempfile::{NamedTempFile, TempDir}; +use test_support::{check_ninja, env::{self, EnvMut}}; /// Installs a test-specific ninja binary and updates the `PATH`. #[expect( @@ -26,7 +23,7 @@ fn install_test_ninja(env: &impl EnvMut, world: &mut CliWorld, dir: TempDir, nin /// Creates a fake ninja executable that exits with the given status code. #[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); + let (dir, path) = test_support::fake_ninja(code); let env = env::mocked_path_env(); install_test_ninja(&env, world, dir, path); } diff --git a/tests/support/mod.rs b/tests/support/mod.rs deleted file mode 100644 index 402d3235..00000000 --- a/tests/support/mod.rs +++ /dev/null @@ -1,94 +0,0 @@ -//! Test utilities for process management. -//! -//! This module provides helpers for creating fake executables along with -//! logging utilities used in behavioural tests. - -pub mod env_lock; -pub mod path_guard; - -#[expect(unused_imports, reason = "re-export for selective test crates")] -pub use path_guard::PathGuard; - -use std::fs::{self, File}; -use std::io::{self, Write}; -use std::path::{Path, PathBuf}; -use tempfile::TempDir; - -/// Create a fake Ninja executable that exits with `exit_code`. -/// -/// Returns the temporary directory and the path to the executable. -#[allow(dead_code, reason = "used in other test crates")] -pub fn fake_ninja(exit_code: i32) -> (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, "#!/bin/sh\nexit {exit_code}").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) -} - -/// Set up `env` so `PATH` resolves only to `dir`. -/// -/// Build a valid `PATH` string that contains exactly one entry pointing to -/// `dir` and configure the mock to return it. This avoids lossy conversions -/// and makes the UTF-8 requirement explicit to callers. -/// -/// Note: `MockEnv::raw` returns a `String`, so callers must accept UTF-8. This -/// helper returns an error if the constructed `PATH` cannot be represented as -/// UTF-8. -/// -/// # Examples -/// ```ignore -/// let (dir, _) = fake_ninja(0); -/// let mut env = MockEnv::new(); -/// mock_path_to(&mut env, dir.path()); -/// ``` -#[expect(dead_code, reason = "used in PATH tests")] -pub fn mock_path_to(env: &mut mockable::MockEnv, dir: &Path) -> io::Result<()> { - // Join using the platform-appropriate separator while ensuring exactly one - // element is present in the PATH value. - let joined = std::env::join_paths([dir.as_os_str()]) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; - - // `MockEnv::raw` expects a `String`. Propagate if the single-entry PATH is - // not valid UTF-8 to keep the contract explicit. - let path = joined.into_string().map_err(|os| { - io::Error::new( - io::ErrorKind::InvalidData, - format!("non-UTF-8 PATH entry: {}", os.to_string_lossy()), - ) - })?; - - env.expect_raw() - .withf(|key| key == "PATH") - .returning(move |_| Ok(path.clone())); - - Ok(()) -} - -/// Create a fake Ninja executable that writes its current directory to the file -/// specified as the first argument. -/// -/// Returns the temporary directory and the path to the executable. -#[expect(dead_code, reason = "used only in directory tests")] -pub fn fake_ninja_pwd() -> (TempDir, PathBuf) { - let dir = TempDir::new().expect("temp dir"); - let path = dir.path().join("ninja"); - let mut file = File::create(&path).expect("script"); - // The script writes its working directory to the provided file and exits. - writeln!(file, "#!/bin/sh\npwd > \"$1\"").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) -} From 9a6a4dad603a3f3f55c1a2179a54713de9a83958 Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 13 Aug 2025 06:28:43 +0100 Subject: [PATCH 2/5] Format process steps imports --- tests/steps/process_steps.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 4dc3d474..f8cd53f0 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -6,7 +6,10 @@ use netsuke::runner::{self, BuildTargets, NINJA_PROGRAM}; use std::fs; use std::path::{Path, PathBuf}; use tempfile::{NamedTempFile, TempDir}; -use test_support::{check_ninja, env::{self, EnvMut}}; +use test_support::{ + check_ninja, + env::{self, EnvMut}, +}; /// Installs a test-specific ninja binary and updates the `PATH`. #[expect( From 24b6f2846e16e47e248c1a44773d960717da1d6f Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 13 Aug 2025 19:15:25 +0100 Subject: [PATCH 3/5] Harden test_support crate and update tests --- Cargo.lock | 1 - test_support/Cargo.toml | 3 +++ test_support/src/env.rs | 2 -- test_support/src/lib.rs | 31 +++++++++++++++++++++++-------- tests/cucumber.rs | 2 +- tests/path_guard_tests.rs | 2 +- tests/runner_tests.rs | 3 +-- tests/steps/process_steps.rs | 5 +++-- 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65fca98a..b9bcdd47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1530,7 +1530,6 @@ name = "test_support" version = "0.1.0" dependencies = [ "mockable", - "rstest", "tempfile", ] diff --git a/test_support/Cargo.toml b/test_support/Cargo.toml index a414064c..443271cd 100644 --- a/test_support/Cargo.toml +++ b/test_support/Cargo.toml @@ -2,8 +2,11 @@ name = "test_support" version = "0.1.0" edition = "2024" +publish = false [dependencies] tempfile = "3.8.0" mockable = { version = "0.3", features = ["mock"] } + +[dev-dependencies] rstest = "0.18.0" diff --git a/test_support/src/env.rs b/test_support/src/env.rs index c908b287..ae1cae9b 100644 --- a/test_support/src/env.rs +++ b/test_support/src/env.rs @@ -4,7 +4,6 @@ //! manifests. use mockable::{DefaultEnv, Env, MockEnv}; -use rstest::fixture; use std::ffi::{OsStr, OsString}; use std::io::{self, Write}; use std::path::Path; @@ -42,7 +41,6 @@ impl EnvMut for MockEnv { /// 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(); diff --git a/test_support/src/lib.rs b/test_support/src/lib.rs index e79d7ac2..0cb5c7ca 100644 --- a/test_support/src/lib.rs +++ b/test_support/src/lib.rs @@ -1,7 +1,8 @@ -//! Test utilities for process management. +//! Test-only utilities for integration and unit tests. //! -//! This module provides helpers for creating fake executables along with -//! logging utilities used in behavioural tests. +//! This crate offers helpers for crafting fake executables, manipulating the +//! environment, and guarding global state so tests can exercise process +//! interactions without touching the host system. pub mod check_ninja; pub mod env; @@ -19,17 +20,31 @@ use tempfile::TempDir; /// /// Returns the temporary directory and the path to the executable. pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { - let dir = TempDir::new().expect("temp dir"); + let dir = TempDir::new().expect("failed to create temporary directory"); + + #[cfg(unix)] let path = dir.path().join("ninja"); - let mut file = File::create(&path).expect("script"); - writeln!(file, "#!/bin/sh\nexit {exit_code}").expect("write script"); + #[cfg(windows)] + let path = dir.path().join("ninja.cmd"); + #[cfg(unix)] { + let mut file = File::create(&path).expect("failed to create script"); + writeln!(file, "#!/bin/sh\nexit {}", exit_code).expect("failed to write script"); use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&path).expect("meta").permissions(); + let mut perms = fs::metadata(&path) + .expect("failed to read script metadata") + .permissions(); perms.set_mode(0o755); - fs::set_permissions(&path, perms).expect("perms"); + fs::set_permissions(&path, perms).expect("failed to set script permissions"); } + + #[cfg(windows)] + { + let mut file = File::create(&path).expect("failed to create batch file"); + writeln!(file, "@echo off\r\nexit /B {}", exit_code).expect("failed to write batch file"); + } + (dir, path) } diff --git a/tests/cucumber.rs b/tests/cucumber.rs index 11c44d45..c830bcdc 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -1,7 +1,7 @@ //! Cucumber test runner and world state. use cucumber::World; -use test_support::path_guard::PathGuard; +use test_support::PathGuard; /// Shared state for Cucumber scenarios. #[derive(Debug, Default, World)] diff --git a/tests/path_guard_tests.rs b/tests/path_guard_tests.rs index 91fa7b70..45eb0b84 100644 --- a/tests/path_guard_tests.rs +++ b/tests/path_guard_tests.rs @@ -5,7 +5,7 @@ use mockall::{Sequence, mock}; use std::ffi::OsStr; -use test_support::path_guard::{Env, PathGuard}; +use test_support::{PathGuard, path_guard::Env}; mock! { pub Env {} diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 25cd1163..b724a5e4 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -4,10 +4,9 @@ use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; use test_support::{ - check_ninja, + PathGuard, check_ninja, env::{SystemEnv, prepend_dir_to_path}, fake_ninja, - path_guard::PathGuard, }; /// Fixture: Put a fake `ninja` (that checks for a build file) on `PATH`. diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index f8cd53f0..75b6e287 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -9,6 +9,7 @@ use tempfile::{NamedTempFile, TempDir}; use test_support::{ check_ninja, env::{self, EnvMut}, + fake_ninja, }; /// Installs a test-specific ninja binary and updates the `PATH`. @@ -25,8 +26,8 @@ fn install_test_ninja(env: &impl EnvMut, world: &mut CliWorld, dir: TempDir, nin /// Creates a fake ninja executable that exits with the given status code. #[given(expr = "a fake ninja executable that exits with {int}")] -fn fake_ninja(world: &mut CliWorld, code: i32) { - let (dir, path) = test_support::fake_ninja(code); +fn install_fake_ninja(world: &mut CliWorld, code: i32) { + let (dir, path) = fake_ninja(code); let env = env::mocked_path_env(); install_test_ninja(&env, world, dir, path); } From d1ef40257b71e553df1feb1cf48a352100fc368a Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 13 Aug 2025 20:19:13 +0100 Subject: [PATCH 4/5] Refine fake_ninja helper and update call sites --- test_support/src/lib.rs | 73 ++++++++++++++++++++++++++++++------ tests/assert_cmd_tests.rs | 2 +- tests/runner_tests.rs | 4 +- tests/steps/process_steps.rs | 4 +- 4 files changed, 67 insertions(+), 16 deletions(-) diff --git a/test_support/src/lib.rs b/test_support/src/lib.rs index 0cb5c7ca..52b80d96 100644 --- a/test_support/src/lib.rs +++ b/test_support/src/lib.rs @@ -1,8 +1,14 @@ -//! Test-only utilities for integration and unit tests. +//! Test-support crate for Netsuke. //! -//! This crate offers helpers for crafting fake executables, manipulating the -//! environment, and guarding global state so tests can exercise process -//! interactions without touching the host system. +//! This crate provides test-only utilities for: +//! - creating fake executables for process-related tests +//! - manipulating PATH safely (PathGuard) +//! - serialising environment mutation across tests (EnvLock) +//! +//! All items are intended for use in tests within this workspace; avoid using +//! them in production code. +//! +//! Platform notes: fake executables are implemented for Unix and Windows. pub mod check_ninja; pub mod env; @@ -19,8 +25,24 @@ use tempfile::TempDir; /// Create a fake Ninja executable that exits with `exit_code`. /// /// Returns the temporary directory and the path to the executable. -pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { - let dir = TempDir::new().expect("failed to create temporary directory"); +/// +/// The returned [`TempDir`] must be kept alive for the executable to remain on +/// disk. +/// +/// # Example +/// +/// ```rust,ignore +/// use test_support::fake_ninja; +/// +/// // Create a fake `ninja` that exits with code 1 +/// let (dir, ninja_path) = fake_ninja(1u8); +/// +/// // Prepend `dir.path()` to PATH via your env helper, then spawn `ninja`. +/// // When `dir` is dropped, the fake executable is removed. +/// ``` +pub fn fake_ninja(exit_code: u8) -> (TempDir, PathBuf) { + let dir = TempDir::new() + .unwrap_or_else(|e| panic!("fake_ninja: failed to create temporary directory: {e}")); #[cfg(unix)] let path = dir.path().join("ninja"); @@ -29,20 +51,47 @@ pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { #[cfg(unix)] { - let mut file = File::create(&path).expect("failed to create script"); - writeln!(file, "#!/bin/sh\nexit {}", exit_code).expect("failed to write script"); + let mut file = File::create(&path).unwrap_or_else(|e| { + panic!( + "fake_ninja: failed to create script {}: {e}", + path.display() + ) + }); + writeln!(file, "#!/bin/sh\nexit {}", exit_code).unwrap_or_else(|e| { + panic!("fake_ninja: failed to write script {}: {e}", path.display()) + }); use std::os::unix::fs::PermissionsExt; let mut perms = fs::metadata(&path) - .expect("failed to read script metadata") + .unwrap_or_else(|e| { + panic!( + "fake_ninja: failed to read metadata {}: {e}", + path.display() + ) + }) .permissions(); perms.set_mode(0o755); - fs::set_permissions(&path, perms).expect("failed to set script permissions"); + fs::set_permissions(&path, perms).unwrap_or_else(|e| { + panic!( + "fake_ninja: failed to set permissions {}: {e}", + path.display() + ) + }); } #[cfg(windows)] { - let mut file = File::create(&path).expect("failed to create batch file"); - writeln!(file, "@echo off\r\nexit /B {}", exit_code).expect("failed to write batch file"); + let mut file = File::create(&path).unwrap_or_else(|e| { + panic!( + "fake_ninja: failed to create batch file {}: {e}", + path.display() + ) + }); + writeln!(file, "@echo off\r\nexit /B {}", exit_code).unwrap_or_else(|e| { + panic!( + "fake_ninja: failed to write batch file {}: {e}", + path.display() + ) + }); } (dir, path) diff --git a/tests/assert_cmd_tests.rs b/tests/assert_cmd_tests.rs index 9d201670..34d04b2a 100644 --- a/tests/assert_cmd_tests.rs +++ b/tests/assert_cmd_tests.rs @@ -26,7 +26,7 @@ fn manifest_subcommand_writes_file() { #[test] fn build_with_emit_writes_file() { - let (ninja_dir, _ninja_path) = fake_ninja(0); + let (ninja_dir, _ninja_path) = fake_ninja(0u8); let temp = tempdir().expect("temp dir"); fs::copy("tests/data/minimal.yml", temp.path().join("Netsukefile")).expect("copy manifest"); let output = temp.path().join("emitted.ninja"); diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index b724a5e4..69dc9df1 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -32,7 +32,7 @@ fn ninja_in_path() -> (tempfile::TempDir, PathBuf, PathGuard) { /// /// Returns: (tempdir holding ninja, `ninja_path`, PATH guard) #[fixture] -fn ninja_with_exit_code(#[default(0)] exit_code: i32) -> (tempfile::TempDir, PathBuf, PathGuard) { +fn ninja_with_exit_code(#[default(0u8)] exit_code: u8) -> (tempfile::TempDir, PathBuf, PathGuard) { let (ninja_dir, ninja_path) = fake_ninja(exit_code); let env = SystemEnv::new(); let guard = prepend_dir_to_path(&env, ninja_dir.path()); @@ -206,7 +206,7 @@ fn run_manifest_subcommand_writes_file() { #[test] #[serial] fn run_respects_env_override_for_ninja() { - let (temp_dir, ninja_path) = fake_ninja(0); + let (temp_dir, ninja_path) = fake_ninja(0u8); let original = std::env::var_os(NINJA_ENV); unsafe { std::env::set_var(NINJA_ENV, &ninja_path); diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 75b6e287..41b4df70 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -27,7 +27,9 @@ fn install_test_ninja(env: &impl EnvMut, world: &mut CliWorld, dir: TempDir, nin /// Creates a fake ninja executable that exits with the given status code. #[given(expr = "a fake ninja executable that exits with {int}")] fn install_fake_ninja(world: &mut CliWorld, code: i32) { - let (dir, path) = fake_ninja(code); + let (dir, path) = fake_ninja( + u8::try_from(code).expect("exit code must be between 0 and 255"), + ); let env = env::mocked_path_env(); install_test_ninja(&env, world, dir, path); } From da4ff8102384da8d0550bccd188bc222f6c90839 Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 13 Aug 2025 22:47:39 +0100 Subject: [PATCH 5/5] Serialise NINJA_ENV edits and document PathGuard --- test_support/Cargo.toml | 1 + test_support/src/lib.rs | 2 +- tests/runner_tests.rs | 7 ++++++- tests/steps/process_steps.rs | 4 +--- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test_support/Cargo.toml b/test_support/Cargo.toml index 443271cd..a6a8032c 100644 --- a/test_support/Cargo.toml +++ b/test_support/Cargo.toml @@ -2,6 +2,7 @@ name = "test_support" version = "0.1.0" edition = "2024" +rust-version = "1.85.0" publish = false [dependencies] diff --git a/test_support/src/lib.rs b/test_support/src/lib.rs index 52b80d96..aff63b25 100644 --- a/test_support/src/lib.rs +++ b/test_support/src/lib.rs @@ -14,7 +14,7 @@ pub mod check_ninja; pub mod env; pub mod env_lock; pub mod path_guard; - +/// Re-export of [`PathGuard`] for crate-level ergonomics in tests. pub use path_guard::PathGuard; use std::fs::{self, File}; diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 69dc9df1..76c8ddfc 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -4,9 +4,11 @@ use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; use test_support::{ - PathGuard, check_ninja, + check_ninja, env::{SystemEnv, prepend_dir_to_path}, + env_lock::EnvLock, fake_ninja, + path_guard::PathGuard, }; /// Fixture: Put a fake `ninja` (that checks for a build file) on `PATH`. @@ -208,6 +210,8 @@ fn run_manifest_subcommand_writes_file() { fn run_respects_env_override_for_ninja() { let (temp_dir, ninja_path) = fake_ninja(0u8); let original = std::env::var_os(NINJA_ENV); + let _lock = EnvLock::acquire(); + // SAFETY: `EnvLock` serialises access to process-global state. unsafe { std::env::set_var(NINJA_ENV, &ninja_path); } @@ -229,6 +233,7 @@ fn run_respects_env_override_for_ninja() { let result = run(&cli); assert!(result.is_ok()); + // SAFETY: `EnvLock` ensures exclusive access while the variable is reset. unsafe { if let Some(val) = original { std::env::set_var(NINJA_ENV, val); diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 41b4df70..4a9d4cf7 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -27,9 +27,7 @@ fn install_test_ninja(env: &impl EnvMut, world: &mut CliWorld, dir: TempDir, nin /// Creates a fake ninja executable that exits with the given status code. #[given(expr = "a fake ninja executable that exits with {int}")] fn install_fake_ninja(world: &mut CliWorld, code: i32) { - let (dir, path) = fake_ninja( - u8::try_from(code).expect("exit code must be between 0 and 255"), - ); + let (dir, path) = fake_ninja(u8::try_from(code).expect("exit code must be between 0 and 255")); let env = env::mocked_path_env(); install_test_ninja(&env, world, dir, path); }