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
5 changes: 4 additions & 1 deletion src/specify_cli/workflows/steps/do_while/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ def validate(self, config: dict[str, Any]) -> list[str]:
)
max_iter = config.get("max_iterations")
if max_iter is not None:
if not isinstance(max_iter, int) or max_iter < 1:
# bool is a subclass of int, so isinstance(True, int) is True and
# True < 1 is False; reject bools explicitly so `max_iterations: true`
# is a type error rather than a silent single iteration.
if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1:
errors.append(
f"Do-while step {config.get('id', '?')!r}: "
f"'max_iterations' must be an integer >= 1."
Expand Down
5 changes: 4 additions & 1 deletion src/specify_cli/workflows/steps/while_loop/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def validate(self, config: dict[str, Any]) -> list[str]:
)
max_iter = config.get("max_iterations")
if max_iter is not None:
if not isinstance(max_iter, int) or max_iter < 1:
# bool is a subclass of int, so isinstance(True, int) is True and
# True < 1 is False; reject bools explicitly so `max_iterations: true`
# is a type error rather than a silent single iteration.
if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1:
errors.append(
f"While step {config.get('id', '?')!r}: "
f"'max_iterations' must be an integer >= 1."
Expand Down
21 changes: 21 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -1784,6 +1784,12 @@ def test_validate_invalid_max_iterations(self):
step = WhileStep()
errors = step.validate({"id": "test", "condition": "{{ true }}", "max_iterations": 0, "steps": []})
assert any("must be an integer >= 1" in e for e in errors)
# bool is an int subclass; `max_iterations: true` must be rejected, not
# silently treated as a single iteration.
bool_errors = step.validate(
{"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []}
)
assert any("must be an integer >= 1" in e for e in bool_errors)


class TestDoWhileStep:
Expand Down Expand Up @@ -1823,6 +1829,21 @@ def test_execute_with_true_condition(self):
assert len(result.next_steps) == 1
assert result.output["max_iterations"] == 5

def test_validate_rejects_bool_max_iterations(self):
from specify_cli.workflows.steps.do_while import DoWhileStep

step = DoWhileStep()
# bool is an int subclass; `max_iterations: true` must be rejected.
errors = step.validate(
{"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []}
)
assert any("must be an integer >= 1" in e for e in errors)
# a real positive integer is still accepted.
ok = step.validate(
{"id": "test", "condition": "{{ true }}", "max_iterations": 3, "steps": []}
)
assert not any("max_iterations" in e for e in ok)
Comment on lines +1842 to +1845

def test_execute_empty_steps(self):
from specify_cli.workflows.steps.do_while import DoWhileStep
from specify_cli.workflows.base import StepContext
Expand Down