Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/specify_cli/workflows/steps/if_then/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ def validate(self, config: dict[str, Any]) -> list[str]:
errors.append(
f"If step {config.get('id', '?')!r}: 'then' must be a list of steps."
)
else_branch = config.get("else", [])
if else_branch and not isinstance(else_branch, list):
else_branch = config.get("else")
if else_branch is not None and not isinstance(else_branch, list):
errors.append(
f"If step {config.get('id', '?')!r}: 'else' must be a list of steps."
)
Expand Down
40 changes: 40 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,46 @@ def test_validate_missing_condition(self):
errors = step.validate({"id": "test", "then": []})
assert any("missing 'condition'" in e for e in errors)

@pytest.mark.parametrize("bad_else", [False, 0, "", {}, 42])
def test_validate_rejects_non_list_else(self, bad_else):
"""A non-list 'else' must be rejected even when it is falsy.

The original guard used ``if else_branch and ...`` which
short-circuits for falsy non-list values (False/0/''/{}), letting a
malformed else-branch pass validation only to be silently skipped at
runtime. ``then`` is already strictly validated; ``else`` must match.
"""
from specify_cli.workflows.steps.if_then import IfThenStep

step = IfThenStep()
errors = step.validate(
{"id": "i", "condition": "true", "then": [], "else": bad_else}
)
assert any("'else' must be a list of steps" in e for e in errors)

@pytest.mark.parametrize("ok_else", [None, [], [{"id": "x", "command": "/y"}]])
def test_validate_accepts_valid_else(self, ok_else):
"""An explicit 'else' of None or a list stays valid.

``else`` is set explicitly here (including ``else: None``) so the
explicit-None case is exercised, not just the missing-key case.
"""
from specify_cli.workflows.steps.if_then import IfThenStep

step = IfThenStep()
errors = step.validate(
{"id": "i", "condition": "true", "then": [], "else": ok_else}
)
assert not any("'else'" in e for e in errors)

def test_validate_accepts_missing_else(self):
"""A missing 'else' key stays valid (no else branch)."""
from specify_cli.workflows.steps.if_then import IfThenStep

step = IfThenStep()
errors = step.validate({"id": "i", "condition": "true", "then": []})
assert not any("'else'" in e for e in errors)


class TestSwitchStep:
"""Test the switch step type."""
Expand Down
Loading