diff --git a/src/ir.rs b/src/ir.rs index 6df41511..846b39ce 100644 --- a/src/ir.rs +++ b/src/ir.rs @@ -62,7 +62,7 @@ pub struct BuildEdge { pub implicit_outputs: Vec, /// Order-only dependencies that do not trigger rebuilds (Ninja `||`). pub order_only_deps: Vec, - /// Always run the command even if the output exists. + /// Output does not correspond to a real file. pub phony: bool, /// Run the command on every invocation regardless of timestamps. pub always: bool, diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index 44a5a099..3163a883 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -145,12 +145,7 @@ impl Display for DisplayEdge<'_> { if !self.edge.implicit_outputs.is_empty() { write!(f, " | {}", join(&self.edge.implicit_outputs))?; } - let rule = if self.edge.phony { - "phony" - } else { - &self.edge.action_id - }; - write!(f, ": {rule}")?; + write!(f, ": {}", self.edge.action_id)?; if !self.edge.inputs.is_empty() { write!(f, " {}", join(&self.edge.inputs))?; } diff --git a/tests/features/ninja.feature b/tests/features/ninja.feature index 1f98fd0b..af28c6ae 100644 --- a/tests/features/ninja.feature +++ b/tests/features/ninja.feature @@ -6,7 +6,8 @@ Feature: Ninja file generation Then the ninja file contains "rule" And the ninja file contains "build hello.o:" - Scenario: Phony target rule + Scenario: Phony target runs its command When the manifest file "tests/data/phony.yml" is compiled to IR And the ninja file is generated - Then the ninja file contains "build clean: phony" + Then the ninja file contains "build clean:" + And the ninja file contains "rm -rf build" diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs index cf9a3eb7..cf38c2cb 100644 --- a/tests/ninja_gen_tests.rs +++ b/tests/ninja_gen_tests.rs @@ -9,9 +9,9 @@ use insta::{Settings, assert_snapshot}; use netsuke::ast::Recipe; use netsuke::ir::{Action, BuildEdge, BuildGraph}; use netsuke::ninja_gen::generate; -use rstest::rstest; +use rstest::{fixture, rstest}; use std::{fs, path::PathBuf, process::Command}; -use tempfile::tempdir; +use tempfile::{TempDir, tempdir}; fn skip_if_ninja_unavailable() -> bool { match Command::new("ninja").arg("--version").output() { @@ -27,8 +27,26 @@ fn skip_if_ninja_unavailable() -> bool { } } +/// Define how the integration test should assert Ninja's behaviour. +#[derive(Debug)] +enum AssertionType { + FileContent(String), + FileExists, + StatusSuccess, +} + +/// Provide a temporary directory when Ninja is available, skipping otherwise. +#[fixture] +fn ninja_integration_setup() -> Option { + if skip_if_ninja_unavailable() { + None + } else { + Some(tempdir().expect("temp dir")) + } +} + #[rstest] -#[case::phony( +#[case::phony_target_runs_command( Action { recipe: Recipe::Command { command: "true".into() }, description: None, @@ -50,7 +68,7 @@ fn skip_if_ninja_unavailable() -> bool { concat!( "rule a\n", " command = true\n\n", - "build out: phony in\n\n", + "build out: a in\n\n", ), )] #[case::standard_build( @@ -169,72 +187,40 @@ fn generate_multiline_script_snapshot() { ); } -/// Ensure a multi-line script produces a Ninja manifest that Ninja accepts. -#[rstest] -#[ignore = "requires Ninja"] -fn integration_multiline_script_valid() { - if skip_if_ninja_unavailable() { - return; - } - - let mut graph = BuildGraph::default(); - graph.actions.insert( - "script".into(), - Action { - recipe: Recipe::Script { - script: "echo one\necho two".into(), - }, - description: None, - depfile: None, - deps_format: None, - pool: None, - restat: false, - }, - ); - graph.targets.insert( - PathBuf::from("out"), - BuildEdge { - action_id: "script".into(), - inputs: Vec::new(), - explicit_outputs: vec![PathBuf::from("out")], - implicit_outputs: Vec::new(), - order_only_deps: Vec::new(), - phony: false, - always: false, - }, - ); - graph.default_targets.push(PathBuf::from("out")); - - let ninja = generate(&graph); - let dir = tempdir().expect("temp dir"); - fs::write(dir.path().join("build.ninja"), &ninja).expect("write ninja"); - let status = Command::new("ninja") - .arg("-n") - .current_dir(dir.path()) - .status() - .expect("run ninja"); - assert!(status.success()); -} - -/// Test that scripts containing percent signs execute correctly. +/// Integration scenarios to confirm Ninja executes commands correctly. #[rstest] -#[ignore = "requires Ninja"] -fn generate_script_with_percent() { - if skip_if_ninja_unavailable() { - return; - } - - let action = Action { - recipe: Recipe::Script { - script: "echo 100% > out".into(), - }, +#[case::multiline_script_valid( + Action { + recipe: Recipe::Script { script: "echo one\necho two".into() }, description: None, depfile: None, deps_format: None, pool: None, restat: false, - }; - let edge = BuildEdge { + }, + BuildEdge { + action_id: "script".into(), + inputs: Vec::new(), + explicit_outputs: vec![PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }, + PathBuf::from("out"), + vec!["-n"], + AssertionType::StatusSuccess, +)] +#[case::script_with_percent( + Action { + recipe: Recipe::Script { script: "echo 100% > out".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { action_id: "percent".into(), inputs: Vec::new(), explicit_outputs: vec![PathBuf::from("out")], @@ -242,43 +228,21 @@ fn generate_script_with_percent() { order_only_deps: Vec::new(), phony: false, always: false, - }; - let mut graph = BuildGraph::default(); - graph.actions.insert("percent".into(), action); - graph.targets.insert(PathBuf::from("out"), edge); - graph.default_targets.push(PathBuf::from("out")); - - let ninja = generate(&graph); - let dir = tempdir().expect("temp dir"); - fs::write(dir.path().join("build.ninja"), &ninja).expect("write ninja"); - let status = Command::new("ninja") - .arg("out") - .current_dir(dir.path()) - .status() - .expect("run ninja"); - assert!(status.success()); - let content = fs::read_to_string(dir.path().join("out")).expect("read out"); - assert_eq!(content.trim(), "100%"); -} - -#[rstest] -#[ignore = "requires Ninja"] -fn generate_script_with_backtick() { - if skip_if_ninja_unavailable() { - return; - } - - let action = Action { - recipe: Recipe::Script { - script: "echo `echo hi` > out".into(), - }, + }, + PathBuf::from("out"), + vec!["out"], + AssertionType::FileContent("100%".into()), +)] +#[case::script_with_backtick( + Action { + recipe: Recipe::Script { script: "echo `echo hi` > out".into() }, description: None, depfile: None, deps_format: None, pool: None, restat: false, - }; - let edge = BuildEdge { + }, + BuildEdge { action_id: "tick".into(), inputs: Vec::new(), explicit_outputs: vec![PathBuf::from("out")], @@ -286,21 +250,74 @@ fn generate_script_with_backtick() { order_only_deps: Vec::new(), phony: false, always: false, + }, + PathBuf::from("out"), + vec!["out"], + AssertionType::FileContent("hi".into()), +)] +#[case::phony_action_executes_command( + Action { + recipe: Recipe::Command { command: "touch action-called.txt".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { + action_id: "hello".into(), + inputs: Vec::new(), + explicit_outputs: vec![PathBuf::from("say-hello")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: true, + always: false, + }, + PathBuf::from("action-called.txt"), + vec!["say-hello"], + AssertionType::FileExists, +)] +fn ninja_integration_tests( + ninja_integration_setup: Option, + #[case] action: Action, + #[case] edge: BuildEdge, + #[case] target_name: PathBuf, + #[case] ninja_args: Vec<&str>, + #[case] assertion: AssertionType, +) { + let Some(dir) = ninja_integration_setup else { + return; }; + + let output = edge + .explicit_outputs + .first() + .expect("explicit output") + .clone(); let mut graph = BuildGraph::default(); - graph.actions.insert("tick".into(), action); - graph.targets.insert(PathBuf::from("out"), edge); - graph.default_targets.push(PathBuf::from("out")); + graph.actions.insert(edge.action_id.clone(), action); + graph.targets.insert(output.clone(), edge); + graph.default_targets.push(output); let ninja = generate(&graph); - let dir = tempdir().expect("temp dir"); fs::write(dir.path().join("build.ninja"), &ninja).expect("write ninja"); let status = Command::new("ninja") - .arg("out") + .args(&ninja_args) .current_dir(dir.path()) .status() .expect("run ninja"); - assert!(status.success()); - let content = fs::read_to_string(dir.path().join("out")).expect("read out"); - assert_eq!(content.trim(), "hi"); + + match assertion { + AssertionType::StatusSuccess => assert!(status.success()), + AssertionType::FileExists => { + assert!(status.success()); + assert!(dir.path().join(target_name).exists()); + } + AssertionType::FileContent(expected) => { + assert!(status.success()); + let content = + fs::read_to_string(dir.path().join(target_name)).expect("read target file"); + assert_eq!(content.trim(), expected); + } + } }