diff --git a/src/ir.rs b/src/ir.rs index ca6bba19..9fb84c35 100644 --- a/src/ir.rs +++ b/src/ir.rs @@ -322,10 +322,11 @@ fn find_cycle(targets: &HashMap) -> Option> { stack.push(node.clone()); - if let Some(edge) = targets.get(node) - && let Some(cycle) = visit_dependencies(targets, &edge.inputs, stack, states) - { - return Some(cycle); + if let Some(edge) = targets.get(node) { + let deps_result = visit_dependencies(targets, &edge.inputs, stack, states); + if let Some(cycle) = deps_result { + return Some(cycle); + } } stack.pop(); @@ -340,10 +341,11 @@ fn find_cycle(targets: &HashMap) -> Option> { states: &mut HashMap, ) -> Option> { for dep in deps { - if targets.contains_key(dep) - && let Some(cycle) = visit(targets, dep, stack, states) - { - return Some(cycle); + if targets.contains_key(dep) { + let visit_result = visit(targets, dep, stack, states); + if let Some(cycle) = visit_result { + return Some(cycle); + } } } None @@ -353,9 +355,10 @@ fn find_cycle(targets: &HashMap) -> Option> { let mut stack = Vec::new(); for node in targets.keys() { - if !states.contains_key(node) - && let Some(cycle) = visit(targets, node, &mut stack, &mut states) - { + if states.contains_key(node) { + continue; + } + if let Some(cycle) = visit(targets, node, &mut stack, &mut states) { return Some(cycle); } } diff --git a/tests/features/ir_generation.feature b/tests/features/ir_generation.feature new file mode 100644 index 00000000..7b731453 --- /dev/null +++ b/tests/features/ir_generation.feature @@ -0,0 +1,43 @@ +Feature: Intermediate Representation (IR) Generation + As a developer, + I want to compile a manifest into a valid build graph, + So that I can detect configuration errors before execution. + + Scenario: A new, empty BuildGraph has no content + Given a new BuildGraph is created + When its contents are checked + Then the graph has 0 actions + And the graph has 0 targets + And the graph has 0 default targets + + Scenario: Compiling a valid manifest with one rule and one target + Given the manifest file "tests/data/rules.yml" is compiled to IR + When the graph contents are checked + Then the graph has 1 actions + And the graph has 1 targets + + Scenario: Identical rules are deduplicated during IR generation + Given the manifest file "tests/data/duplicate_rules.yml" is compiled to IR + When the graph contents are checked + Then the graph has 1 actions + And the graph has 2 targets + + Scenario: IR generation fails if a target references a rule that does not exist + Given the manifest file "tests/data/missing_rule.yml" is compiled to IR + When the generation result is checked + Then IR generation fails + + Scenario: IR generation fails if a target specifies multiple rules + Given the manifest file "tests/data/multiple_rules_per_target.yml" is compiled to IR + When the generation result is checked + Then IR generation fails + + Scenario: IR generation fails if multiple targets produce the same output file + Given the manifest file "tests/data/duplicate_outputs.yml" is compiled to IR + When the generation result is checked + Then IR generation fails + + Scenario: IR generation fails if there is a circular dependency between targets + Given the manifest file "tests/data/circular.yml" is compiled to IR + When the generation result is checked + Then IR generation fails diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs index 53533182..a74d3ce9 100644 --- a/tests/steps/ir_steps.rs +++ b/tests/steps/ir_steps.rs @@ -1,10 +1,25 @@ //! Step definitions for `BuildGraph` scenarios. use crate::CliWorld; -use cucumber::{then, when}; +use cucumber::{given, then, when}; use netsuke::ir::BuildGraph; -#[when("a new BuildGraph is created")] +fn assert_graph(world: &CliWorld) { + assert!( + world.build_graph.is_some(), + "build graph should have been generated", + ); +} + +fn assert_generation_attempted(world: &CliWorld) { + match (world.build_graph.is_some(), world.manifest_error.is_some()) { + (true, false) | (false, true) => (), + (true, true) => panic!("unexpected: graph and error present"), + (false, false) => panic!("IR generation not attempted"), + } +} + +#[given("a new BuildGraph is created")] fn create_graph(world: &mut CliWorld) { world.build_graph = Some(BuildGraph::default()); } @@ -31,6 +46,7 @@ fn graph_defaults(world: &mut CliWorld, count: usize) { clippy::needless_pass_by_value, reason = "Cucumber requires owned String arguments" )] +#[given(expr = "the manifest file {string} is compiled to IR")] #[when(expr = "the manifest file {string} is compiled to IR")] fn compile_manifest(world: &mut CliWorld, path: String) { match netsuke::manifest::from_path(&path) @@ -47,6 +63,21 @@ fn compile_manifest(world: &mut CliWorld, path: String) { } } +#[when("its contents are checked")] +fn graph_checked(world: &mut CliWorld) { + assert_graph(world); +} + +#[when("the graph contents are checked")] +fn graph_contents_checked(world: &mut CliWorld) { + assert_graph(world); +} + +#[when("the generation result is checked")] +fn generation_result_checked(world: &mut CliWorld) { + assert_generation_attempted(world); +} + #[then("IR generation fails")] fn ir_generation_fails(world: &mut CliWorld) { assert!( diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index 686b090f..1494ebd2 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -34,6 +34,7 @@ fn assert_parsed(world: &CliWorld) { "manifest should have been parsed" ); } + #[given(expr = "the manifest file {string} is parsed")] fn given_parse_manifest(world: &mut CliWorld, path: String) { parse_manifest_inner(world, &path); @@ -44,7 +45,7 @@ fn parse_manifest(world: &mut CliWorld, path: String) { parse_manifest_inner(world, &path); } -#[when(regex = r"^the (?P[a-z ]+) (?:is|are) checked$")] +#[when(regex = r"^the (?Pparsing result|manifest|version|flags|rules) (?:is|are) checked$")] fn when_item_checked(world: &mut CliWorld, item: String) { match item.as_str() { "parsing result" => assert_parsed(world),