Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ thiserror = "1"
sha2 = "0.10"
itoa = "1"
itertools = "0.12"
tempfile = "3"

[lints.clippy]
pedantic = { level = "warn", priority = -1 }
Expand Down Expand Up @@ -59,7 +60,6 @@ rstest = "0.18.0"
cucumber = "0.20.0"
tokio = { version = "1", features = ["macros", "rt-multi-thread"], default-features = false }
insta = { version = "1", features = ["yaml"] }
tempfile = "3"

[[test]]
name = "cucumber"
Expand Down
55 changes: 52 additions & 3 deletions src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
//! handles command execution. It now delegates build requests to the Ninja
//! subprocess, streaming its output back to the user.

use crate::cli::{Cli, Commands};
use crate::{
cli::{Cli, Commands},
ir, manifest, ninja_gen,
};
use std::io::{self, BufRead, BufReader, Write};
use std::path::Path;
use std::process::{Command, Stdio};
use std::thread;
use tempfile::NamedTempFile;

/// Execute the parsed [`Cli`] commands.
///
Expand All @@ -35,8 +39,10 @@ pub fn run(cli: &Cli) -> io::Result<()> {

/// Invoke the Ninja executable with the provided CLI settings.
///
/// The function forwards the job count and working directory to Ninja and
/// streams its standard output and error back to the user.
/// This loads the `Netsukefile`, converts it to an intermediate representation,
/// generates a temporary Ninja build script, and executes Ninja with this
/// script. Job count and working directory options are forwarded to Ninja, and
/// the child's standard output and error are streamed back to the user.
///
/// # Errors
///
Expand All @@ -47,13 +53,18 @@ pub fn run(cli: &Cli) -> io::Result<()> {
///
/// Panics if the child's output streams cannot be captured.
pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()> {
// Keep the file handle alive so the temporary script outlives the child
// process.
let build_file = manifest_to_build_file(&cli.file)?;

let mut cmd = Command::new(program);
if let Some(dir) = &cli.directory {
cmd.current_dir(dir).arg("-C").arg(dir);
}
if let Some(jobs) = cli.jobs {
cmd.arg("-j").arg(jobs.to_string());
}
cmd.arg("-f").arg(build_file.path());
cmd.args(targets);
cmd.stdout(Stdio::piped());
cmd.stderr(Stdio::piped());
Expand Down Expand Up @@ -94,3 +105,41 @@ pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()
))
}
}

/// Generate a temporary Ninja build file from a manifest.
///
/// # Examples
///
/// ```ignore
/// # use std::path::Path;
/// # fn demo() -> std::io::Result<()> {
/// let file = netsuke::runner::manifest_to_build_file(Path::new("tests/data/rules.yml"))?;
/// assert!(file.path().exists());
/// # Ok(())
/// # }
/// ```
fn manifest_to_build_file(path: &Path) -> io::Result<NamedTempFile> {
let manifest = manifest::from_path(path).map_err(|e| {
io::Error::new(
io::ErrorKind::InvalidData,
format!("Failed to load manifest from {}: {e}", path.display()),
)
})?;
let graph = ir::BuildGraph::from_manifest(&manifest).map_err(|e| {
io::Error::new(
io::ErrorKind::InvalidData,
format!("Failed to convert manifest to build graph: {e}"),
)
})?;
let ninja_script = ninja_gen::generate(&graph);

let mut build_file = NamedTempFile::new()
.map_err(|e| io::Error::other(format!("Failed to create temporary build file: {e}")))?;
build_file
.write_all(ninja_script.as_bytes())
.map_err(|e| io::Error::other(format!("Failed to write build file: {e}")))?;
build_file
.flush()
.map_err(|e| io::Error::other(format!("Failed to flush build file: {e}")))?;
Ok(build_file)
}
48 changes: 44 additions & 4 deletions tests/runner_tests.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
//! Unit tests for Ninja process invocation.
//!
//! These tests verify that the runner can translate a manifest into a Ninja
//! build script and invoke the Ninja process with appropriate arguments.

use netsuke::cli::{Cli, Commands};
use netsuke::runner;
use rstest::rstest;
use std::fs;
use std::path::{Path, PathBuf};
use tempfile::NamedTempFile;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Creates a default CLI configuration for testing Ninja invocation.
fn test_cli() -> Cli {
fn cli_with_manifest(file: PathBuf) -> Cli {
Cli {
file: PathBuf::from("Netsukefile"),
file,
directory: None,
jobs: None,
command: Some(Commands::Build {
Expand All @@ -24,15 +29,50 @@ mod support;
#[case(1, false)]
fn run_ninja_status(#[case] code: i32, #[case] succeeds: bool) {
let (_dir, path) = support::fake_ninja(code);
let cli = test_cli();
let mut manifest = NamedTempFile::new().expect("manifest");
support::write_manifest(&mut manifest);
let cli = cli_with_manifest(manifest.path().to_path_buf());
let result = runner::run_ninja(&path, &cli, &[]);
assert_eq!(result.is_ok(), succeeds);
}

#[rstest]
fn run_ninja_not_found() {
let cli = test_cli();
let mut manifest = NamedTempFile::new().expect("manifest");
support::write_manifest(&mut manifest);
let cli = cli_with_manifest(manifest.path().to_path_buf());
let err =
runner::run_ninja(Path::new("does-not-exist"), &cli, &[]).expect_err("process should fail");
assert_eq!(err.kind(), std::io::ErrorKind::NotFound);
}

#[rstest]
fn run_pipeline_generates_ninja() {
use netsuke::ast::Recipe;
use netsuke::hasher::ActionHasher;
use netsuke::ir::Action;

let (_dir, path, capture) = support::fake_ninja_capture();
let mut manifest = NamedTempFile::new().expect("manifest");
support::write_manifest(&mut manifest);
let cli = cli_with_manifest(manifest.path().to_path_buf());

runner::run_ninja(&path, &cli, &[]).expect("run ninja");

let generated = fs::read_to_string(&capture).expect("captured build");

let action = Action {
recipe: Recipe::Command {
command: "echo hi".into(),
},
description: None,
depfile: None,
deps_format: None,
pool: None,
restat: false,
};
let hash = ActionHasher::hash(&action);
let expected =
format!("rule {hash}\n command = echo hi\n\nbuild out: {hash}\n\ndefault out\n");
assert_eq!(generated, expected);
}
40 changes: 30 additions & 10 deletions tests/steps/process_steps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
use crate::{CliWorld, support};
use cucumber::{given, then, when};
use netsuke::runner;
use std::path::PathBuf;
use tempfile::TempDir;
use std::fs;
use std::path::{Path, PathBuf};
use tempfile::{NamedTempFile, TempDir};

/// Installs a test-specific ninja binary and updates the `PATH`.
#[expect(
Expand Down Expand Up @@ -50,18 +51,32 @@ fn no_ninja(world: &mut CliWorld) {
/// This step runs the `ninja` executable using the CLI configuration stored in
/// the world, then updates the world's `run_status` and `run_error` fields based
/// on the execution outcome.
#[expect(
clippy::option_if_let_else,
reason = "explicit conditional is clearer than map_or_else"
)]
#[when("the ninja process is run")]
fn run(world: &mut CliWorld) {
let cli = world.cli.as_ref().expect("cli");
let program = if let Some(ninja) = &world.ninja {
std::path::Path::new(ninja)
// Touch the capture variant so the support module's helpers remain used.
let _ = support::fake_ninja_capture as fn() -> (TempDir, PathBuf, PathBuf);
Comment on lines +56 to +57
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove the workaround for unused function reference.

The comment indicates this is keeping fake_ninja_capture artificially referenced. Instead, use #[expect(dead_code)] on the function in the support module if it's genuinely unused in some contexts.

-    // Touch the capture variant so the support module's helpers remain used.
-    let _ = support::fake_ninja_capture as fn() -> (TempDir, PathBuf, PathBuf);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Touch the capture variant so the support module's helpers remain used.
let _ = support::fake_ninja_capture as fn() -> (TempDir, PathBuf, PathBuf);
🤖 Prompt for AI Agents
In tests/steps/process_steps.rs around lines 56 to 57, remove the line that
assigns `support::fake_ninja_capture` to an unused variable as a workaround to
keep the function referenced. Instead, go to the support module where
`fake_ninja_capture` is defined and add the attribute `#[expect(dead_code)]` to
that function to suppress unused code warnings properly.

let cli = world.cli.as_mut().expect("cli");

// Ensure a manifest exists at the path expected by the CLI.
let dir = world.temp.as_ref().expect("temp dir");
let manifest_path = if cli.file.is_absolute() {
cli.file.clone()
} else {
std::path::Path::new("ninja")
dir.path().join(&cli.file)
};
if !manifest_path.exists() {
let mut file = NamedTempFile::new_in(dir.path()).expect("manifest");
support::write_manifest(&mut file);
// Persist the temporary file to the desired manifest path.
file.persist(&manifest_path).expect("persist manifest");
}
cli.file.clone_from(&manifest_path);

let program = world
.ninja
.as_ref()
.map_or_else(|| Path::new("ninja"), Path::new);

match runner::run_ninja(program, cli, &[]) {
Ok(()) => {
world.run_status = Some(true);
Expand All @@ -72,6 +87,11 @@ fn run(world: &mut CliWorld) {
world.run_error = Some(e.to_string());
}
}

// Clean up any manifest left outside the temporary directory.
if !manifest_path.starts_with(dir.path()) {
let _ = fs::remove_file(manifest_path);
}
}

/// Asserts that the command succeeds.
Expand Down
61 changes: 54 additions & 7 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,69 @@
use std::fs::{self, File};
use std::io::Write;
use std::path::PathBuf;
use tempfile::TempDir;
use tempfile::{NamedTempFile, 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");
fn write_script(dir: &TempDir, body: &str) -> PathBuf {
let path = dir.path().join("ninja");
let mut file = File::create(&path).expect("script");
writeln!(file, "#!/bin/sh\nexit {exit_code}").expect("write script");
file.write_all(body.as_bytes()).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");
}
path
}

/// 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 body = format!("#!/bin/sh\nexit {exit_code}\n");
let path = write_script(&dir, &body);
(dir, path)
}

/// Create a fake Ninja that copies the provided build file to a capture path.
///
/// Returns the temporary directory, path to the executable and the capture file
/// location.
pub fn fake_ninja_capture() -> (TempDir, PathBuf, PathBuf) {
let dir = TempDir::new().expect("temp dir");
let capture = dir.path().join("captured.ninja");
let body = format!(
concat!(
"#!/bin/sh\n",
"while [ $# -gt 0 ]; do\n",
" if [ \"$1\" = \"-f\" ] && [ $# -gt 1 ]; then\n",
" shift\n",
" cat \"$1\" > \"{}\"\n",
" shift\n",
" else\n",
" shift\n",
" fi\n",
"done\n",
),
capture.display(),
);
let path = write_script(&dir, &body);
(dir, path, capture)
}

/// Write a minimal Netsukefile to the provided temporary file.
pub fn write_manifest(file: &mut NamedTempFile) {
let manifest = concat!(
"netsuke_version: \"1.0.0\"\n",
"targets:\n",
" - name: out\n",
" recipe:\n",
" kind: command\n",
" command: echo hi\n",
"defaults:\n",
" - out\n",
);
file.write_all(manifest.as_bytes()).expect("write manifest");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading