From cea688aacde5ed4c29ed49288de597072d6552a7 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 12 Jun 2026 15:58:09 +0200 Subject: [PATCH 1/5] Extend expand_foreach test coverage (#296) Strengthen the conditional-expansion suite per the review follow-up: - assert `foreach` keys are stripped from expanded targets and from `when`-filtered targets and actions; - cover `foreach: []` producing no entries; - cover non-object (bare string) entries passing through unexpanded; - pin the precedence rule that iteration-injected `item` overrides an entry's own `item` var while unrelated vars survive; - exercise a Jinja filter in `name` (`{{ item | upper }}`) through the full `manifest::from_str` pipeline, where name rendering occurs. No production code changes; existing tests untouched. --- .../action_condition_cases.rs | 1 + .../expand_test_cases/condition_cases.rs | 112 ++++++++++++++++++ src/manifest/expand_tests.rs | 15 +++ 3 files changed, 128 insertions(+) diff --git a/src/manifest/expand_test_cases/action_condition_cases.rs b/src/manifest/expand_test_cases/action_condition_cases.rs index e851ae816..6a811b14a 100644 --- a/src/manifest/expand_test_cases/action_condition_cases.rs +++ b/src/manifest/expand_test_cases/action_condition_cases.rs @@ -44,6 +44,7 @@ fn expand_foreach_applies_action_when_expression() -> Result<()> { let actions = actions(&doc)?; anyhow::ensure!(actions.len() == 2, "expected filtered actions"); anyhow::ensure!(indexes(actions, "action")? == vec![1, 2], "wrong indexes"); + ensure_foreach_removed(actions, "filtered action")?; Ok(()) } diff --git a/src/manifest/expand_test_cases/condition_cases.rs b/src/manifest/expand_test_cases/condition_cases.rs index 2d826f032..29f73e697 100644 --- a/src/manifest/expand_test_cases/condition_cases.rs +++ b/src/manifest/expand_test_cases/condition_cases.rs @@ -158,6 +158,10 @@ fn expand_foreach_expands_sequence_values() -> Result<()> { anyhow::ensure!(targets.len() == 2, "expected two targets"); for (idx, target) in targets.iter().enumerate() { let map = target.as_object().context("target map")?; + anyhow::ensure!( + !map.contains_key("foreach"), + "foreach should be removed after target expansion" + ); let vars = map .get("vars") .and_then(|v| v.as_object()) @@ -200,6 +204,114 @@ fn expand_foreach_applies_when_expression() -> Result<()> { "unexpected filtered indexes: {:?}", indexes ); + for target in targets { + let map = target.as_object().context("target map")?; + anyhow::ensure!( + !map.contains_key("foreach"), + "foreach should be removed from filtered targets" + ); + } + Ok(()) +} + +#[test] +fn expand_foreach_empty_foreach_produces_no_entries() -> Result<()> { + let env = Environment::new(); + let mut doc: ManifestValue = serde_saphyr::from_str( + "targets: + - name: literal + foreach: [] + command: echo hi", + )?; + expand_foreach(&mut doc, &env)?; + let targets = targets(&doc)?; + anyhow::ensure!( + targets.is_empty(), + "empty foreach should expand to no targets: {targets:?}" + ); + Ok(()) +} + +#[test] +fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { + let env = Environment::new(); + let mut doc: ManifestValue = serde_saphyr::from_str( + "targets: + - just-a-string + - name: real + command: echo hi", + )?; + expand_foreach(&mut doc, &env)?; + let targets = targets(&doc)?; + anyhow::ensure!(targets.len() == 2, "expected both entries to survive"); + anyhow::ensure!( + targets.first().and_then(ManifestValue::as_str) == Some("just-a-string"), + "bare string entry should pass through unexpanded: {:?}", + targets.first() + ); + Ok(()) +} + +#[test] +fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars() -> Result<()> { + let env = Environment::new(); + let mut doc: ManifestValue = serde_saphyr::from_str( + "targets: + - name: literal + foreach: + - from-iteration + vars: + item: from-entry + other: untouched", + )?; + expand_foreach(&mut doc, &env)?; + let targets = targets(&doc)?; + anyhow::ensure!(targets.len() == 1, "expected one expanded target"); + let vars = targets + .first() + .and_then(ManifestValue::as_object) + .and_then(|map| map.get("vars")) + .and_then(ManifestValue::as_object) + .context("vars map")?; + // `inject_iteration_vars` inserts `item`/`index` after cloning the entry, + // so the iteration-injected value takes precedence over the entry's own + // `item` var while unrelated vars survive. + anyhow::ensure!( + vars.get("item").and_then(ManifestValue::as_str) == Some("from-iteration"), + "iteration item should override the entry's own item var: {vars:?}" + ); + anyhow::ensure!( + vars.get("other").and_then(ManifestValue::as_str) == Some("untouched"), + "unrelated entry vars should survive expansion: {vars:?}" + ); + Ok(()) +} + +#[test] +fn expand_foreach_jinja_filter_in_name() -> Result<()> { + // Name rendering happens in the full pipeline (render_manifest), so this + // test drives manifest::from_str rather than expand_foreach directly. + let manifest = crate::manifest::from_str( + "netsuke_version: \"1.0.0\" +targets: + - name: '{{ item | upper }}' + foreach: + - alpha + - beta + command: echo hi", + )?; + let names: Vec<&str> = manifest + .targets + .iter() + .map(|t| match &t.name { + crate::ast::StringOrList::String(s) => Ok(s.as_str()), + other => Err(anyhow::anyhow!("expected string name, got {other:?}")), + }) + .collect::>()?; + anyhow::ensure!( + names == ["ALPHA", "BETA"], + "expected uppercased names from Jinja filter: {names:?}" + ); Ok(()) } diff --git a/src/manifest/expand_tests.rs b/src/manifest/expand_tests.rs index edacb57ad..f19c26c97 100644 --- a/src/manifest/expand_tests.rs +++ b/src/manifest/expand_tests.rs @@ -29,6 +29,21 @@ pub(super) fn actions(doc: &ManifestValue) -> Result<&[ManifestValue]> { .context("actions sequence missing") } +pub(super) fn ensure_foreach_removed( + entries: &[ManifestValue], + section: &str, +) -> Result<()> { + for entry in entries { + let map = entry + .as_object() + .with_context(|| format!("{section} entry map"))?; + anyhow::ensure!( + !map.contains_key("foreach"), + "foreach should be removed after {section} expansion" + ); + } + Ok(()) +} pub(super) fn section_entries<'a>( doc: &'a ManifestValue, section: &str, From c183b0cc894d295adeabb3cf40a9f33969c29bbb Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 05:21:09 +0200 Subject: [PATCH 2/5] Keep foreach coverage within test-module limit (#296) Reuse the foreach-removal assertion so the reviewed coverage stays readable and satisfies the module-size policy. --- .../expand_test_cases/condition_cases.rs | 25 +++---------------- src/manifest/expand_tests.rs | 5 +--- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/src/manifest/expand_test_cases/condition_cases.rs b/src/manifest/expand_test_cases/condition_cases.rs index 29f73e697..afeed3c9d 100644 --- a/src/manifest/expand_test_cases/condition_cases.rs +++ b/src/manifest/expand_test_cases/condition_cases.rs @@ -156,12 +156,9 @@ fn expand_foreach_expands_sequence_values() -> Result<()> { expand_foreach(&mut doc, &env)?; let targets = targets(&doc)?; anyhow::ensure!(targets.len() == 2, "expected two targets"); + ensure_foreach_removed(targets, "target")?; for (idx, target) in targets.iter().enumerate() { let map = target.as_object().context("target map")?; - anyhow::ensure!( - !map.contains_key("foreach"), - "foreach should be removed after target expansion" - ); let vars = map .get("vars") .and_then(|v| v.as_object()) @@ -198,19 +195,8 @@ fn expand_foreach_applies_when_expression() -> Result<()> { expand_foreach(&mut doc, &env)?; let targets = targets(&doc)?; anyhow::ensure!(targets.len() == 2, "expected filtered targets"); - let indexes = indexes(targets, "target")?; - anyhow::ensure!( - indexes == vec![1, 2], - "unexpected filtered indexes: {:?}", - indexes - ); - for target in targets { - let map = target.as_object().context("target map")?; - anyhow::ensure!( - !map.contains_key("foreach"), - "foreach should be removed from filtered targets" - ); - } + anyhow::ensure!(indexes(targets, "target")? == vec![1, 2], "wrong indexes"); + ensure_foreach_removed(targets, "filtered target")?; Ok(()) } @@ -225,10 +211,7 @@ fn expand_foreach_empty_foreach_produces_no_entries() -> Result<()> { )?; expand_foreach(&mut doc, &env)?; let targets = targets(&doc)?; - anyhow::ensure!( - targets.is_empty(), - "empty foreach should expand to no targets: {targets:?}" - ); + anyhow::ensure!(targets.is_empty(), "empty foreach must produce no targets"); Ok(()) } diff --git a/src/manifest/expand_tests.rs b/src/manifest/expand_tests.rs index f19c26c97..3930eee96 100644 --- a/src/manifest/expand_tests.rs +++ b/src/manifest/expand_tests.rs @@ -29,10 +29,7 @@ pub(super) fn actions(doc: &ManifestValue) -> Result<&[ManifestValue]> { .context("actions sequence missing") } -pub(super) fn ensure_foreach_removed( - entries: &[ManifestValue], - section: &str, -) -> Result<()> { +pub(super) fn ensure_foreach_removed(entries: &[ManifestValue], section: &str) -> Result<()> { for entry in entries { let map = entry .as_object() From 86d81e160f35f1ddef57634169ea42952eb5eb21 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 19:45:48 +0200 Subject: [PATCH 3/5] Strengthen foreach pass-through test (#296) Assert that the object following a bare string retains its name. Move the independent key-order test into its structural module to keep the condition cases below the enforced module-size limit. --- .../expand_test_cases/condition_cases.rs | 37 ++++--------------- .../expand_test_cases/structure_cases.rs | 34 +++++++++++++++++ src/manifest/expand_tests.rs | 3 ++ 3 files changed, 45 insertions(+), 29 deletions(-) create mode 100644 src/manifest/expand_test_cases/structure_cases.rs diff --git a/src/manifest/expand_test_cases/condition_cases.rs b/src/manifest/expand_test_cases/condition_cases.rs index afeed3c9d..af7e9aeda 100644 --- a/src/manifest/expand_test_cases/condition_cases.rs +++ b/src/manifest/expand_test_cases/condition_cases.rs @@ -232,6 +232,14 @@ fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { "bare string entry should pass through unexpanded: {:?}", targets.first() ); + let second_target = targets + .get(1) + .and_then(ManifestValue::as_object) + .context("second target object")?; + anyhow::ensure!( + second_target.get("name").and_then(ManifestValue::as_str) == Some("real"), + "second target should remain object named real: {second_target:?}" + ); Ok(()) } @@ -298,35 +306,6 @@ targets: Ok(()) } -#[test] -fn expand_foreach_preserves_object_key_order() -> Result<()> { - let env = Environment::new(); - let yaml = r"targets: - - name: literal - vars: - existing: keep - foreach: - - 1 - - 2 - when: 'true' - after: done -"; - let mut doc: ManifestValue = serde_saphyr::from_str(yaml)?; - expand_foreach(&mut doc, &env)?; - let targets = targets(&doc)?; - anyhow::ensure!(targets.len() == 2, "expected expanded targets"); - for target in targets { - let map = target.as_object().context("target object")?; - let keys: Vec<&str> = map.keys().map(String::as_str).collect(); - anyhow::ensure!( - keys == ["name", "vars", "after"], - "key order should remain stable: {:?}", - keys - ); - } - Ok(()) -} - #[rstest] #[case("false", 0, "expression false drops target")] #[case("0", 0, "expression 0 drops target")] diff --git a/src/manifest/expand_test_cases/structure_cases.rs b/src/manifest/expand_test_cases/structure_cases.rs new file mode 100644 index 000000000..164d76009 --- /dev/null +++ b/src/manifest/expand_test_cases/structure_cases.rs @@ -0,0 +1,34 @@ +//! Structural preservation cases for manifest foreach expansion. + +use super::*; +use anyhow::{Context, Result}; +use minijinja::Environment; + +#[test] +fn expand_foreach_preserves_object_key_order() -> Result<()> { + let env = Environment::new(); + let yaml = r"targets: + - name: literal + vars: + existing: keep + foreach: + - 1 + - 2 + when: 'true' + after: done +"; + let mut doc: ManifestValue = serde_saphyr::from_str(yaml)?; + expand_foreach(&mut doc, &env)?; + let targets = targets(&doc)?; + anyhow::ensure!(targets.len() == 2, "expected expanded targets"); + for target in targets { + let map = target.as_object().context("target object")?; + let keys: Vec<&str> = map.keys().map(String::as_str).collect(); + anyhow::ensure!( + keys == ["name", "vars", "after"], + "key order should remain stable: {:?}", + keys + ); + } + Ok(()) +} diff --git a/src/manifest/expand_tests.rs b/src/manifest/expand_tests.rs index 3930eee96..02ea6b77c 100644 --- a/src/manifest/expand_tests.rs +++ b/src/manifest/expand_tests.rs @@ -10,6 +10,9 @@ mod action_condition_cases; #[path = "expand_test_cases/condition_cases.rs"] mod condition_cases; +#[path = "expand_test_cases/structure_cases.rs"] +mod structure_cases; + #[path = "expand_test_cases/property_cases.rs"] mod property_cases; #[path = "expand_test_cases/target_command_available_cases.rs"] From 1b7c2c931bed948423ea4785efc58a78b1aaf07d Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 10 Aug 2026 16:02:22 +0200 Subject: [PATCH 4/5] Add foreach expansion property coverage (#296) Exercise generated sequences, filtering, key order, and variable precedence while ensuring mixed entries prove real expansion occurs. --- .../expand_test_cases/condition_cases.rs | 18 +++ .../foreach_property_cases.rs | 150 ++++++++++++++++++ src/manifest/expand_tests.rs | 3 + 3 files changed, 171 insertions(+) create mode 100644 src/manifest/expand_test_cases/foreach_property_cases.rs diff --git a/src/manifest/expand_test_cases/condition_cases.rs b/src/manifest/expand_test_cases/condition_cases.rs index af7e9aeda..4b08ce48a 100644 --- a/src/manifest/expand_test_cases/condition_cases.rs +++ b/src/manifest/expand_test_cases/condition_cases.rs @@ -222,6 +222,8 @@ fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { "targets: - just-a-string - name: real + foreach: + - expanded command: echo hi", )?; expand_foreach(&mut doc, &env)?; @@ -240,6 +242,22 @@ fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { second_target.get("name").and_then(ManifestValue::as_str) == Some("real"), "second target should remain object named real: {second_target:?}" ); + anyhow::ensure!( + !second_target.contains_key("foreach"), + "expanded target should no longer contain foreach: {second_target:?}" + ); + let vars = second_target + .get("vars") + .and_then(ManifestValue::as_object) + .context("second target vars")?; + anyhow::ensure!( + vars.get("item").and_then(ManifestValue::as_str) == Some("expanded"), + "second target should retain the iteration item: {vars:?}" + ); + anyhow::ensure!( + vars.get("index").and_then(ManifestValue::as_u64) == Some(0), + "second target should retain the iteration index: {vars:?}" + ); Ok(()) } diff --git a/src/manifest/expand_test_cases/foreach_property_cases.rs b/src/manifest/expand_test_cases/foreach_property_cases.rs new file mode 100644 index 000000000..73ba90a6d --- /dev/null +++ b/src/manifest/expand_test_cases/foreach_property_cases.rs @@ -0,0 +1,150 @@ +//! Generated invariants for manifest foreach expansion. +//! +//! The fixed cases pin specific regressions, while these properties vary +//! sequence values, filtering, variable collisions, and source key order. + +use super::*; +use minijinja::Environment; +use proptest::prelude::*; + +proptest! { + /// Expansion preserves a leading non-object entry and expands each value. + #[test] + fn foreach_expands_generated_sequence_values(values in proptest::collection::vec(-100_i16..100, 1..8)) { + let env = Environment::new(); + let yaml = format!( + "targets:\n - bare-string\n - name: generated\n foreach: {values:?}\n command: echo {{{{ item }}}}" + ); + let mut doc: ManifestValue = serde_saphyr::from_str(&yaml) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + expand_foreach(&mut doc, &env) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let expanded = targets(&doc).map_err(|error| TestCaseError::fail(error.to_string()))?; + + prop_assert_eq!(expanded.len(), values.len() + 1); + prop_assert_eq!(expanded.first().and_then(ManifestValue::as_str), Some("bare-string")); + for (index, (entry, value)) in expanded.iter().skip(1).zip(&values).enumerate() { + let map = entry.as_object().ok_or_else(|| { + TestCaseError::fail(format!("expanded entry {index} should be an object: {entry:?}")) + })?; + prop_assert!(!map.contains_key("foreach")); + prop_assert_eq!(map.get("name").and_then(ManifestValue::as_str), Some("generated")); + let vars = map.get("vars").and_then(ManifestValue::as_object).ok_or_else(|| { + TestCaseError::fail(format!("expanded entry {index} should contain vars: {map:?}")) + })?; + prop_assert_eq!(vars.get("item").and_then(ManifestValue::as_i64), Some(i64::from(*value))); + prop_assert_eq!(vars.get("index").and_then(ManifestValue::as_u64), Some(index as u64)); + } + } + + /// Iteration values override colliding entry vars while other vars survive. + #[test] + fn foreach_generated_iteration_values_override_entry_vars( + values in proptest::collection::vec("[a-z]{1,6}", 1..8), + entry_item in "[a-z]{1,6}", + other_value in "[a-z]{1,6}", + ) { + let env = Environment::new(); + let foreach = serde_json::to_string(&values) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let entry_item_json = serde_json::to_string(&entry_item) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let other = serde_json::to_string(&other_value) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let yaml = format!( + "targets:\n - name: variables\n foreach: {foreach}\n vars:\n item: {entry_item_json}\n other: {other}" + ); + let mut doc: ManifestValue = serde_saphyr::from_str(&yaml) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + expand_foreach(&mut doc, &env) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let expanded = targets(&doc).map_err(|error| TestCaseError::fail(error.to_string()))?; + + prop_assert_eq!(expanded.len(), values.len()); + for (index, (entry, value)) in expanded.iter().zip(&values).enumerate() { + let map = entry.as_object().ok_or_else(|| { + TestCaseError::fail(format!("expanded entry {index} should be an object: {entry:?}")) + })?; + let vars = map.get("vars").and_then(ManifestValue::as_object).ok_or_else(|| { + TestCaseError::fail(format!("expanded entry {index} should contain vars: {map:?}")) + })?; + prop_assert_eq!(vars.get("item").and_then(ManifestValue::as_str), Some(value.as_str())); + prop_assert_eq!(vars.get("other").and_then(ManifestValue::as_str), Some(other_value.as_str())); + prop_assert_eq!(vars.get("index").and_then(ManifestValue::as_u64), Some(index as u64)); + } + } + + /// Filtering preserves original indexes for every generated input sequence. + #[test] + fn foreach_filters_generated_sequence_values( + values in proptest::collection::vec(-20_i16..21, 0..8), + threshold in -20_i16..21, + ) { + let env = Environment::new(); + let yaml = format!( + "targets:\n - name: filtered\n foreach: {values:?}\n when: 'item > {threshold}'" + ); + let mut doc: ManifestValue = serde_saphyr::from_str(&yaml) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + expand_foreach(&mut doc, &env) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let expanded = targets(&doc).map_err(|error| TestCaseError::fail(error.to_string()))?; + let expected: Vec<_> = values + .iter() + .enumerate() + .filter(|(_, value)| **value > threshold) + .collect(); + + prop_assert_eq!(expanded.len(), expected.len()); + for (entry, (index, value)) in expanded.iter().zip(expected) { + let map = entry.as_object().ok_or_else(|| { + TestCaseError::fail(format!("filtered entry should be an object: {entry:?}")) + })?; + prop_assert!(!map.contains_key("foreach")); + let vars = map.get("vars").and_then(ManifestValue::as_object).ok_or_else(|| { + TestCaseError::fail(format!("filtered entry should contain vars: {map:?}")) + })?; + prop_assert_eq!(vars.get("item").and_then(ManifestValue::as_i64), Some(i64::from(*value))); + prop_assert_eq!(vars.get("index").and_then(ManifestValue::as_u64), Some(index as u64)); + } + } + + /// Expansion removes `foreach` without reordering user-specified map keys. + #[test] + fn foreach_preserves_generated_source_key_order(order in prop_oneof![ + Just(vec!["name", "vars", "after"]), + Just(vec!["name", "after", "vars"]), + Just(vec!["vars", "name", "after"]), + Just(vec!["vars", "after", "name"]), + Just(vec!["after", "name", "vars"]), + Just(vec!["after", "vars", "name"]), + ]) { + let env = Environment::new(); + let mut yaml = String::from("targets:\n"); + for (index, key) in order.iter().enumerate() { + yaml.push_str(if index == 0 { " - " } else { " " }); + match *key { + "name" => yaml.push_str("name: ordered\n"), + "vars" => yaml.push_str("vars:\n static: keep\n"), + "after" => yaml.push_str("after: done\n"), + _ => return Err(TestCaseError::fail("property strategy produced an unknown key".to_owned())), + } + } + yaml.push_str(" foreach: [value]"); + let mut doc: ManifestValue = serde_saphyr::from_str(&yaml) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + expand_foreach(&mut doc, &env) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let expanded = targets(&doc).map_err(|error| TestCaseError::fail(error.to_string()))?; + + prop_assert_eq!(expanded.len(), 1); + let entry = expanded.first().ok_or_else(|| { + TestCaseError::fail("expected one expanded target after length check".to_owned()) + })?; + let map = entry.as_object().ok_or_else(|| { + TestCaseError::fail(format!("expanded target should be an object: {entry:?}")) + })?; + let keys: Vec<_> = map.keys().map(String::as_str).collect(); + prop_assert_eq!(keys, order); + } +} diff --git a/src/manifest/expand_tests.rs b/src/manifest/expand_tests.rs index 02ea6b77c..016d58495 100644 --- a/src/manifest/expand_tests.rs +++ b/src/manifest/expand_tests.rs @@ -10,6 +10,9 @@ mod action_condition_cases; #[path = "expand_test_cases/condition_cases.rs"] mod condition_cases; +#[path = "expand_test_cases/foreach_property_cases.rs"] +mod foreach_property_cases; + #[path = "expand_test_cases/structure_cases.rs"] mod structure_cases; From c5699b1cd33f636d791ba15333c63cf9797ef539 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 14 Aug 2026 02:14:41 +0200 Subject: [PATCH 5/5] Share foreach test environments (#296) Inject a fresh module-local environment fixture into the related conditional expansion tests to remove repeated setup. --- .../expand_test_cases/condition_cases.rs | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/manifest/expand_test_cases/condition_cases.rs b/src/manifest/expand_test_cases/condition_cases.rs index 4b08ce48a..051caf597 100644 --- a/src/manifest/expand_test_cases/condition_cases.rs +++ b/src/manifest/expand_test_cases/condition_cases.rs @@ -1,10 +1,14 @@ -//! Conditional expansion cases for manifest entries; action-only cases live -//! in `action_condition_cases`. +//! Conditional expansion cases; action-only cases live in `action_condition_cases`. use super::*; use anyhow::{Context, Result}; use minijinja::Environment; -use rstest::rstest; +use rstest::{fixture, rstest}; + +#[fixture] +fn environment() -> Environment<'static> { + Environment::new() +} #[rstest] #[case::targets("targets")] @@ -200,24 +204,26 @@ fn expand_foreach_applies_when_expression() -> Result<()> { Ok(()) } -#[test] -fn expand_foreach_empty_foreach_produces_no_entries() -> Result<()> { - let env = Environment::new(); +#[rstest] +fn expand_foreach_empty_foreach_produces_no_entries( + environment: Environment<'static>, +) -> Result<()> { let mut doc: ManifestValue = serde_saphyr::from_str( "targets: - name: literal foreach: [] command: echo hi", )?; - expand_foreach(&mut doc, &env)?; + expand_foreach(&mut doc, &environment)?; let targets = targets(&doc)?; anyhow::ensure!(targets.is_empty(), "empty foreach must produce no targets"); Ok(()) } -#[test] -fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { - let env = Environment::new(); +#[rstest] +fn expand_foreach_non_object_entry_is_passed_through( + environment: Environment<'static>, +) -> Result<()> { let mut doc: ManifestValue = serde_saphyr::from_str( "targets: - just-a-string @@ -226,7 +232,7 @@ fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { - expanded command: echo hi", )?; - expand_foreach(&mut doc, &env)?; + expand_foreach(&mut doc, &environment)?; let targets = targets(&doc)?; anyhow::ensure!(targets.len() == 2, "expected both entries to survive"); anyhow::ensure!( @@ -261,9 +267,10 @@ fn expand_foreach_non_object_entry_is_passed_through() -> Result<()> { Ok(()) } -#[test] -fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars() -> Result<()> { - let env = Environment::new(); +#[rstest] +fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars( + environment: Environment<'static>, +) -> Result<()> { let mut doc: ManifestValue = serde_saphyr::from_str( "targets: - name: literal @@ -273,7 +280,7 @@ fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars() -> Resul item: from-entry other: untouched", )?; - expand_foreach(&mut doc, &env)?; + expand_foreach(&mut doc, &environment)?; let targets = targets(&doc)?; anyhow::ensure!(targets.len() == 1, "expected one expanded target"); let vars = targets @@ -282,9 +289,7 @@ fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars() -> Resul .and_then(|map| map.get("vars")) .and_then(ManifestValue::as_object) .context("vars map")?; - // `inject_iteration_vars` inserts `item`/`index` after cloning the entry, - // so the iteration-injected value takes precedence over the entry's own - // `item` var while unrelated vars survive. + // Iteration vars override colliding entry vars while unrelated vars survive. anyhow::ensure!( vars.get("item").and_then(ManifestValue::as_str) == Some("from-iteration"), "iteration item should override the entry's own item var: {vars:?}" @@ -298,8 +303,7 @@ fn expand_foreach_iteration_vars_do_not_get_overwritten_by_entry_vars() -> Resul #[test] fn expand_foreach_jinja_filter_in_name() -> Result<()> { - // Name rendering happens in the full pipeline (render_manifest), so this - // test drives manifest::from_str rather than expand_foreach directly. + // Name rendering happens in `render_manifest`, so drive the full parsing pipeline. let manifest = crate::manifest::from_str( "netsuke_version: \"1.0.0\" targets: