Skip to content

Commit 1048686

Browse files
test(workflows): cover switch non-list branch execute paths
Copilot flagged the new switch branch guards as untested: coverage stopped at a non-mapping `cases` container. Add SwitchStep.execute tests for a matched case with a non-list body and a non-list default (dict/str/int), asserting FAILED, the branch-specific error, empty next_steps, and preserved expression_value. Also add explicit `default: null` / `else: null` normalization tests so the validator-approved empty-branch contract cannot regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3edf77d commit 1048686

1 file changed

Lines changed: 103 additions & 0 deletions

File tree

tests/test_workflows.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,6 +2111,24 @@ def test_execute_non_list_else_fails_loudly(self, bad_branch):
21112111
assert "'else' must be a list of steps" in (result.error or "")
21122112
assert result.next_steps == []
21132113

2114+
def test_execute_none_else_stays_empty(self):
2115+
"""An explicit ``else: null`` selected at runtime stays an empty branch.
2116+
2117+
``validate`` deliberately accepts ``else: None``; the execute guard must
2118+
normalize it to an empty branch (COMPLETED) rather than failing a
2119+
validator-approved workflow when the condition is false.
2120+
"""
2121+
from specify_cli.workflows.steps.if_then import IfThenStep
2122+
from specify_cli.workflows.base import StepContext, StepStatus
2123+
2124+
step = IfThenStep()
2125+
ctx = StepContext(inputs={})
2126+
result = step.execute(
2127+
{"id": "branch", "condition": "false", "then": [], "else": None}, ctx
2128+
)
2129+
assert result.status == StepStatus.COMPLETED
2130+
assert result.next_steps == []
2131+
21142132
@pytest.mark.parametrize("bad_else", [False, 0, "", {}, 42])
21152133
def test_validate_rejects_non_list_else(self, bad_else):
21162134
"""A non-list 'else' must be rejected even when it is falsy.
@@ -2244,6 +2262,91 @@ def test_execute_non_dict_cases_fails_loudly(self):
22442262
# expression is still evaluated, so its value is surfaced for context.
22452263
assert result.output["expression_value"] == "approve"
22462264

2265+
@pytest.mark.parametrize("bad_branch", [{"id": "x"}, "oops", 5])
2266+
def test_execute_non_list_matched_case_fails_loudly(self, bad_branch):
2267+
"""A matched case with a non-list body must fail the step, not crash.
2268+
2269+
``validate`` rejects a non-list case body, but the engine does not
2270+
auto-validate (see ``WorkflowEngine.load_workflow``) and feeds the
2271+
selected branch straight into ``_execute_steps``, which iterates it as
2272+
step mappings. A non-list body (a single mapping or scalar authoring
2273+
mistake) would be iterated element-wise and raise AttributeError on
2274+
``.get()``, taking down the whole run. Mirrors the non-mapping
2275+
``cases`` guard.
2276+
"""
2277+
from specify_cli.workflows.steps.switch import SwitchStep
2278+
from specify_cli.workflows.base import StepContext, StepStatus
2279+
2280+
step = SwitchStep()
2281+
ctx = StepContext(steps={"review": {"output": {"choice": "approve"}}})
2282+
result = step.execute(
2283+
{
2284+
"id": "route",
2285+
"expression": "{{ steps.review.output.choice }}",
2286+
"cases": {"approve": bad_branch},
2287+
},
2288+
ctx,
2289+
)
2290+
assert result.status == StepStatus.FAILED
2291+
assert "case 'approve' must be a list of steps" in (result.error or "")
2292+
assert result.next_steps == []
2293+
# expression is still evaluated, so its value is surfaced for context.
2294+
assert result.output["expression_value"] == "approve"
2295+
2296+
@pytest.mark.parametrize("bad_branch", [{"id": "x"}, "oops", 5])
2297+
def test_execute_non_list_default_fails_loudly(self, bad_branch):
2298+
"""A non-list ``default`` reached at runtime must fail, not crash.
2299+
2300+
Same asymmetry as the case body: ``default`` is only selected when no
2301+
case matches, so a non-list ``default`` reaches ``next_steps`` and would
2302+
crash the engine's step iteration on an unvalidated run.
2303+
"""
2304+
from specify_cli.workflows.steps.switch import SwitchStep
2305+
from specify_cli.workflows.base import StepContext, StepStatus
2306+
2307+
step = SwitchStep()
2308+
ctx = StepContext(steps={"review": {"output": {"choice": "other"}}})
2309+
result = step.execute(
2310+
{
2311+
"id": "route",
2312+
"expression": "{{ steps.review.output.choice }}",
2313+
"cases": {"approve": [{"id": "plan", "command": "speckit.plan"}]},
2314+
"default": bad_branch,
2315+
},
2316+
ctx,
2317+
)
2318+
assert result.status == StepStatus.FAILED
2319+
assert "'default' must be a list of steps" in (result.error or "")
2320+
assert result.next_steps == []
2321+
# expression is still evaluated, so its value is surfaced for context.
2322+
assert result.output["expression_value"] == "other"
2323+
2324+
@pytest.mark.parametrize("ok_default", [None, [], [{"id": "x", "command": "/y"}]])
2325+
def test_execute_none_default_stays_empty(self, ok_default):
2326+
"""An explicit ``default: null`` or a list default stays valid.
2327+
2328+
``validate`` deliberately accepts ``default: None``; the execute guard
2329+
must normalize it to an empty branch (COMPLETED) rather than failing a
2330+
validator-approved workflow.
2331+
"""
2332+
from specify_cli.workflows.steps.switch import SwitchStep
2333+
from specify_cli.workflows.base import StepContext, StepStatus
2334+
2335+
step = SwitchStep()
2336+
ctx = StepContext(steps={"review": {"output": {"choice": "other"}}})
2337+
result = step.execute(
2338+
{
2339+
"id": "route",
2340+
"expression": "{{ steps.review.output.choice }}",
2341+
"cases": {"approve": [{"id": "plan", "command": "speckit.plan"}]},
2342+
"default": ok_default,
2343+
},
2344+
ctx,
2345+
)
2346+
assert result.status == StepStatus.COMPLETED
2347+
assert result.output["matched_case"] == "__default__"
2348+
assert result.next_steps == (ok_default or [])
2349+
22472350
def test_validate_missing_expression(self):
22482351
from specify_cli.workflows.steps.switch import SwitchStep
22492352

0 commit comments

Comments
 (0)