From 3ce4b9b8cb8e6279937d9eaf295b507e62b59a74 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 22:44:39 +0100 Subject: [PATCH 1/5] Handle manifest path in process tests Ensure manifests exist before running ninja and provide helper to generate a default manifest for tests. --- tests/steps/process_steps.rs | 25 ++++++++++++++++++++++++- tests/support/mod.rs | 24 +++++++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 6a34153a..aa1530b9 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -4,7 +4,7 @@ use crate::{CliWorld, support}; use cucumber::{given, then, when}; use netsuke::runner; use std::path::PathBuf; -use tempfile::TempDir; +use tempfile::{NamedTempFile, TempDir}; /// Installs a test-specific ninja binary and updates the `PATH`. #[expect( @@ -56,6 +56,29 @@ fn no_ninja(world: &mut CliWorld) { )] #[when("the ninja process is run")] fn run(world: &mut CliWorld) { + let dir = world.temp.as_ref().expect("temp dir"); + let manifest_path = { + let cli = world.cli.as_ref().expect("cli"); + if cli.file.is_absolute() { + cli.file.clone() + } else { + dir.path().join(&cli.file) + } + }; + + if !manifest_path.exists() { + let mut file = + NamedTempFile::new_in(dir.path()).expect("Failed to create temporary manifest file"); + support::write_manifest(&mut file); + file.persist(&manifest_path) + .expect("Failed to persist manifest file"); + } + + { + let cli = world.cli.as_mut().expect("cli"); + cli.file.clone_from(&manifest_path); + } + let cli = world.cli.as_ref().expect("cli"); let program = if let Some(ninja) = &world.ninja { std::path::Path::new(ninja) diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 0c39c9eb..f3080e77 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -1,4 +1,7 @@ -//! Test utilities for process management and log capture. +//! Test utilities for process management. +//! +//! This module provides helpers for creating fake executables and +//! generating minimal manifests used in behavioural tests. use std::fs::{self, File}; use std::io::{self, Write}; @@ -90,3 +93,22 @@ pub fn fake_ninja_pwd() -> (TempDir, PathBuf) { } (dir, path) } + +/// Write a minimal manifest to `file`. +/// +/// The manifest declares a single `hello` target that prints a greeting. +#[allow(dead_code, reason = "shared test utility not used in all crates")] +pub fn write_manifest(file: &mut impl Write) { + writeln!( + file, + concat!( + "netsuke_version: \"1.0.0\"\n", + "targets:\n", + " - name: hello\n", + " recipe:\n", + " kind: command\n", + " command: \"echo hi\"\n" + ), + ) + .expect("write manifest content"); +} From d483a9c3b5a0544a6b8b81a2cc1f05a2ee36a0c8 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 23:59:26 +0100 Subject: [PATCH 2/5] Replace #[allow] with #[expect] to comply with coding guidelines. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- tests/support/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/support/mod.rs b/tests/support/mod.rs index f3080e77..452408cd 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -97,7 +97,7 @@ pub fn fake_ninja_pwd() -> (TempDir, PathBuf) { /// Write a minimal manifest to `file`. /// /// The manifest declares a single `hello` target that prints a greeting. -#[allow(dead_code, reason = "shared test utility not used in all crates")] +#[expect(dead_code, reason = "shared test utility not used in all crates")] pub fn write_manifest(file: &mut impl Write) { writeln!( file, From f030702161baabbe03b9299b4448104697486325 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Wed, 6 Aug 2025 00:12:41 +0100 Subject: [PATCH 3/5] Revises lint attributes in test support Switches from #[allow] to #[expect] where appropriate to make unused item warnings explicit. Keeps one #[allow] on a shared test helper whose usage varies by crate, preventing stray "unfulfilled expect" warnings in some builds. --- tests/support/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 452408cd..b59d7a88 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -29,7 +29,6 @@ pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { (dir, path) } -#[allow(dead_code, reason = "compiled as its own crate during linting")] #[derive(Clone)] struct BufferWriter { buf: Arc>>, @@ -54,7 +53,7 @@ impl Write for BufferWriter { /// let output = capture_logs(Level::INFO, || tracing::info!("hello")); /// assert!(output.contains("hello")); /// ``` -#[allow(dead_code, reason = "compiled as its own crate during linting")] +#[expect(dead_code, reason = "compiled as its own crate during linting")] pub fn capture_logs(level: Level, f: F) -> String where F: FnOnce(), @@ -77,7 +76,7 @@ where /// specified as the first argument. /// /// Returns the temporary directory and the path to the executable. -#[allow(dead_code, reason = "used only in directory tests")] +#[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"); @@ -97,7 +96,9 @@ pub fn fake_ninja_pwd() -> (TempDir, PathBuf) { /// Write a minimal manifest to `file`. /// /// The manifest declares a single `hello` target that prints a greeting. -#[expect(dead_code, reason = "shared test utility not used in all crates")] +/// This must be `allow` as `expect` will trigger an unfulfilled warning +/// despite the lint violation arising. +#[allow(dead_code, reason = "shared test utility not used in all crates")] pub fn write_manifest(file: &mut impl Write) { writeln!( file, From 9dae0621e4580a9e5032a58683d2b20818efeac2 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Wed, 6 Aug 2025 00:32:39 +0100 Subject: [PATCH 4/5] Propagates write_manifest errors explicitly Changes write_manifest from unwrapping to returning io::Result, letting callers decide how to handle failures and simplifying test diagnostics. The step that generates the temporary manifest now wraps the result with an expect message for clearer panic context. --- tests/steps/process_steps.rs | 2 +- tests/support/mod.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index aa1530b9..7c9adee9 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -69,7 +69,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); + support::write_manifest(&mut file).expect("Failed to write manifest content"); file.persist(&manifest_path) .expect("Failed to persist manifest file"); } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index b59d7a88..e526c1bc 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -99,7 +99,7 @@ pub fn fake_ninja_pwd() -> (TempDir, PathBuf) { /// This must be `allow` as `expect` will trigger an unfulfilled warning /// despite the lint violation arising. #[allow(dead_code, reason = "shared test utility not used in all crates")] -pub fn write_manifest(file: &mut impl Write) { +pub fn write_manifest(file: &mut impl Write) -> io::Result<()> { writeln!( file, concat!( @@ -111,5 +111,4 @@ pub fn write_manifest(file: &mut impl Write) { " command: \"echo hi\"\n" ), ) - .expect("write manifest content"); } From 4151697dee7b8c577fc663468ecd8312f585df7c Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Wed, 6 Aug 2025 00:49:41 +0100 Subject: [PATCH 5/5] Adds NamedTempFile support for test utilities --- tests/steps/process_steps.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index e8103e4c..7edbc961 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -5,7 +5,7 @@ use cucumber::{given, then, when}; use netsuke::runner::{self, BuildTargets}; use std::fs; use std::path::{Path, PathBuf}; -use tempfile::TempDir; +use tempfile::{NamedTempFile, TempDir}; /// Installs a test-specific ninja binary and updates the `PATH`. #[expect(