From d1c1bb7630283ecf76c27c11608d3b3ef1d654ac Mon Sep 17 00:00:00 2001 From: Leynos Date: Thu, 31 Jul 2025 23:42:50 +0100 Subject: [PATCH 1/3] Extend manifest parsing cucumber tests Adds new Gherkin scenarios verifying manifest parsing and updates step definitions. --- tests/features/manifest.feature | 51 +++++++++++++++---------- tests/steps/manifest_steps.rs | 67 +++++++++++++++++++++++++++++---- 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index a4d6ec22..86f0ea0b 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -1,36 +1,47 @@ -Feature: Manifest parsing - - Scenario: Parse minimal manifest - When the manifest file "tests/data/minimal.yml" is parsed +Feature: Manifest Parsing + As a user, + I want to define my build in a YAML manifest, + So that Netsuke can understand and execute it. + + Scenario: Parsing a minimal valid manifest + Given the manifest file "tests/data/minimal.yml" is parsed + When the manifest version is checked Then the manifest version is "1.0.0" And the first target name is "hello" - Scenario: Parse phony and always flags - When the manifest file "tests/data/phony.yml" is parsed + Scenario: Parsing a manifest with phony and always flags + Given the manifest file "tests/data/phony.yml" is parsed + When the target flags are checked Then the first target is phony And the first target is always rebuilt - Scenario: Actions are always treated as phony - When the manifest file "tests/data/actions.yml" is parsed + Scenario: A target in the 'actions' block is implicitly phony + Given the manifest file "tests/data/actions.yml" is parsed + When the action flags are checked Then the first action is phony - Scenario: Invalid action fails to parse - When the manifest file "tests/data/action_invalid.yml" is parsed - Then parsing the manifest fails - - Scenario: Manifest with rules parses correctly - When the manifest file "tests/data/rules.yml" is parsed + Scenario: Parsing a manifest with rules + Given the manifest file "tests/data/rules.yml" is parsed + When the manifest contents are checked Then the first rule name is "compile" And the first target name is "hello.o" - Scenario: Unknown field fails to parse - When the manifest file "tests/data/unknown_field.yml" is parsed + Scenario: Parsing fails for a manifest with an unknown top-level field + Given the manifest file "tests/data/unknown_field.yml" is parsed + When the parsing result is checked + Then parsing the manifest fails + + Scenario: Parsing fails for a manifest with an invalid version string + Given the manifest file "tests/data/invalid_version.yml" is parsed + When the parsing result is checked Then parsing the manifest fails - Scenario: Invalid version fails to parse - When the manifest file "tests/data/invalid_version.yml" is parsed + Scenario: Parsing fails for a target that is missing a recipe + Given the manifest file "tests/data/missing_recipe.yml" is parsed + When the parsing result is checked Then parsing the manifest fails - Scenario: Missing recipe fails to parse - When the manifest file "tests/data/missing_recipe.yml" is parsed + Scenario: Parsing fails for an action that is missing a recipe + Given the manifest file "tests/data/action_invalid.yml" is parsed + When the parsing result is checked Then parsing the manifest fails diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index 5f2ae08a..deb55a7b 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -1,16 +1,11 @@ //! Step definitions for manifest parsing scenarios. use crate::CliWorld; -use cucumber::{then, when}; +use cucumber::{given, then, when}; use netsuke::{ast::StringOrList, manifest}; -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] -#[when(expr = "the manifest file {string} is parsed")] -fn parse_manifest(world: &mut CliWorld, path: String) { - match manifest::from_path(&path) { +fn parse_manifest_inner(world: &mut CliWorld, path: &str) { + match manifest::from_path(path) { Ok(manifest) => { world.manifest = Some(manifest); world.manifest_error = None; @@ -22,6 +17,62 @@ fn parse_manifest(world: &mut CliWorld, path: String) { } } +fn assert_manifest(world: &CliWorld) { + assert!( + world.manifest.is_some(), + "manifest should have been parsed successfully" + ); +} + +fn assert_parsed(world: &CliWorld) { + assert!( + world.manifest.is_some() || world.manifest_error.is_some(), + "manifest should have been parsed" + ); +} +#[expect( + clippy::needless_pass_by_value, + reason = "Cucumber requires owned String arguments" +)] +#[given(expr = "the manifest file {string} is parsed")] +fn given_parse_manifest(world: &mut CliWorld, path: String) { + parse_manifest_inner(world, &path); +} + +#[expect( + clippy::needless_pass_by_value, + reason = "Cucumber requires owned String arguments" +)] +#[when(expr = "the manifest file {string} is parsed")] +fn parse_manifest(world: &mut CliWorld, path: String) { + parse_manifest_inner(world, &path); +} + +#[when("the manifest version is checked")] +fn when_manifest_version_checked(world: &mut CliWorld) { + assert_manifest(world); +} + +#[when("the target flags are checked")] +fn when_target_flags_checked(world: &mut CliWorld) { + assert_manifest(world); +} + +#[when("the action flags are checked")] +fn when_action_flags_checked(world: &mut CliWorld) { + assert_manifest(world); +} + +#[when("the manifest contents are checked")] +fn when_manifest_contents_checked(world: &mut CliWorld) { + assert_manifest(world); +} + +#[when("the parsing result is checked")] +fn when_parsing_result_checked(world: &mut CliWorld) { + assert_parsed(world); +} + #[expect( clippy::needless_pass_by_value, reason = "Cucumber requires owned String arguments" From a9f28faea31134d20e4067a330a05ff5b5d60162 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 1 Aug 2025 00:21:13 +0100 Subject: [PATCH 2/3] Consolidate manifest check steps --- tests/steps/manifest_steps.rs | 53 +++++++---------------------------- 1 file changed, 10 insertions(+), 43 deletions(-) diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index deb55a7b..c2716f78 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -1,4 +1,8 @@ //! Step definitions for manifest parsing scenarios. +#![expect( + clippy::needless_pass_by_value, + reason = "Cucumber requires owned String arguments" +)] use crate::CliWorld; use cucumber::{given, then, when}; @@ -30,63 +34,30 @@ fn assert_parsed(world: &CliWorld) { "manifest should have been parsed" ); } -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] #[given(expr = "the manifest file {string} is parsed")] fn given_parse_manifest(world: &mut CliWorld, path: String) { parse_manifest_inner(world, &path); } -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] #[when(expr = "the manifest file {string} is parsed")] fn parse_manifest(world: &mut CliWorld, path: String) { parse_manifest_inner(world, &path); } -#[when("the manifest version is checked")] -fn when_manifest_version_checked(world: &mut CliWorld) { - assert_manifest(world); -} - -#[when("the target flags are checked")] -fn when_target_flags_checked(world: &mut CliWorld) { - assert_manifest(world); -} - -#[when("the action flags are checked")] -fn when_action_flags_checked(world: &mut CliWorld) { - assert_manifest(world); -} - -#[when("the manifest contents are checked")] -fn when_manifest_contents_checked(world: &mut CliWorld) { - assert_manifest(world); -} - -#[when("the parsing result is checked")] -fn when_parsing_result_checked(world: &mut CliWorld) { - assert_parsed(world); +#[when(regex = r"^the (?P[a-z ]+) (?:is|are) checked$")] +fn when_item_checked(world: &mut CliWorld, item: String) { + match item.as_str() { + "parsing result" => assert_parsed(world), + _ => assert_manifest(world), + } } -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] #[then(expr = "the manifest version is {string}")] fn manifest_version(world: &mut CliWorld, version: String) { let manifest = world.manifest.as_ref().expect("manifest"); assert_eq!(manifest.netsuke_version.to_string(), version); } -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] #[then(expr = "the first target name is {string}")] fn first_target_name(world: &mut CliWorld, name: String) { let manifest = world.manifest.as_ref().expect("manifest"); @@ -123,10 +94,6 @@ fn manifest_parse_error(world: &mut CliWorld) { assert!(world.manifest_error.is_some(), "expected parse error"); } -#[expect( - clippy::needless_pass_by_value, - reason = "Cucumber requires owned String arguments" -)] #[then(expr = "the first rule name is {string}")] fn first_rule_name(world: &mut CliWorld, name: String) { let manifest = world.manifest.as_ref().expect("manifest"); From db19be57607e63d64544f7fd27cc8e9003fe3319 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 1 Aug 2025 10:54:24 +0100 Subject: [PATCH 3/3] Make manifest checks explicit --- tests/features/manifest.feature | 8 ++++---- tests/steps/manifest_steps.rs | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index 86f0ea0b..dcab19c3 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -5,24 +5,24 @@ Feature: Manifest Parsing Scenario: Parsing a minimal valid manifest Given the manifest file "tests/data/minimal.yml" is parsed - When the manifest version is checked + When the version is checked Then the manifest version is "1.0.0" And the first target name is "hello" Scenario: Parsing a manifest with phony and always flags Given the manifest file "tests/data/phony.yml" is parsed - When the target flags are checked + When the flags are checked Then the first target is phony And the first target is always rebuilt Scenario: A target in the 'actions' block is implicitly phony Given the manifest file "tests/data/actions.yml" is parsed - When the action flags are checked + When the flags are checked Then the first action is phony Scenario: Parsing a manifest with rules Given the manifest file "tests/data/rules.yml" is parsed - When the manifest contents are checked + When the rules are checked Then the first rule name is "compile" And the first target name is "hello.o" diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index c2716f78..686b090f 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -48,7 +48,8 @@ fn parse_manifest(world: &mut CliWorld, path: String) { fn when_item_checked(world: &mut CliWorld, item: String) { match item.as_str() { "parsing result" => assert_parsed(world), - _ => assert_manifest(world), + "manifest" | "version" | "flags" | "rules" => assert_manifest(world), + unexpected => panic!("Unexpected item checked: '{unexpected}'"), } }