From 093d248e4bdbb7db05efbca14bc4c884de88494d Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 21 May 2026 09:25:22 -0500 Subject: [PATCH] address review: cross-platform tests, preserve iteration-0 history - Rewrite shell scripts in tests to use Python via script files instead of POSIX syntax, so they pass on Windows CI. - Snapshot iteration-0 nested-step results under a namespaced key (parent:child:0) before the first copy-back overwrite, preserving complete per-iteration history for debugging. --- src/specify_cli/workflows/engine.py | 9 ++++ tests/test_workflows.py | 69 ++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 0e5223ad1..2feaaef70 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -672,6 +672,15 @@ class WorkflowEngine: for _loop_iter in range(max_iters - 1): if not evaluate_condition(condition, context): break + # Snapshot iteration-0 results under a + # namespaced key before the first overwrite. + if _loop_iter == 0: + for ns in result.next_steps: + orig = ns.get("id") + if orig and orig in context.steps: + ns_key = f"{step_id}:{orig}:0" + context.steps[ns_key] = context.steps[orig] + state.step_results[ns_key] = context.steps[orig] # Namespace nested step IDs per iteration iter_steps = [] original_ids = {} diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 616a681ee..d8b2304ae 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1907,8 +1907,18 @@ steps: # Iteration 1: counter=2, echoes "done" → condition false → stop # Without the fix, condition always reads iteration-0 stdout, # so the loop runs all max_iterations. + import sys + counter_file = project_dir / ".counter" counter_file.write_text("0", encoding="utf-8") + py = sys.executable + script_file = project_dir / "_tick.py" + script_file.write_text( + f"import pathlib; p = pathlib.Path(r'{counter_file}')\n" + "n = int(p.read_text()) + 1; p.write_text(str(n))\n" + "print('done' if n >= 2 else str(n), end='')\n", + encoding="utf-8", + ) yaml_str = f""" schema_version: "1.0" @@ -1924,11 +1934,7 @@ steps: steps: - id: attempt type: shell - run: | - n=$(cat {counter_file}) - n=$((n + 1)) - printf '%s' $n > {counter_file} - if [ "$n" -ge 2 ]; then printf done; else printf $n; fi + run: {py} {script_file} """ definition = WorkflowDefinition.from_string(yaml_str) engine = WorkflowEngine(project_dir) @@ -1939,6 +1945,8 @@ steps: assert state.step_results["attempt"]["output"]["stdout"] == "done" # Namespaced iteration-1 result should also exist. assert "retry-loop:attempt:1" in state.step_results + # Iteration-0 history preserved under namespaced key. + assert "retry-loop:attempt:0" in state.step_results # Counter should be 2 (iteration 0 + iteration 1), not 5. assert counter_file.read_text(encoding="utf-8").strip() == "2" @@ -1950,8 +1958,18 @@ steps: from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition from specify_cli.workflows.base import RunStatus + import sys + counter_file = project_dir / ".counter" counter_file.write_text("0", encoding="utf-8") + py = sys.executable + script_file = project_dir / "_tick.py" + script_file.write_text( + f"import pathlib; p = pathlib.Path(r'{counter_file}')\n" + "n = int(p.read_text()) + 1; p.write_text(str(n))\n" + "print('done' if n >= 2 else str(n), end='')\n", + encoding="utf-8", + ) yaml_str = f""" schema_version: "1.0" @@ -1967,11 +1985,7 @@ steps: steps: - id: attempt type: shell - run: | - n=$(cat {counter_file}) - n=$((n + 1)) - printf '%s' $n > {counter_file} - if [ "$n" -ge 2 ]; then printf done; else printf $n; fi + run: {py} {script_file} """ definition = WorkflowDefinition.from_string(yaml_str) engine = WorkflowEngine(project_dir) @@ -1990,8 +2004,18 @@ steps: from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition from specify_cli.workflows.base import RunStatus + import sys + counter_file = project_dir / ".counter" counter_file.write_text("0", encoding="utf-8") + py = sys.executable + script_file = project_dir / "_tick.py" + script_file.write_text( + f"import pathlib; p = pathlib.Path(r'{counter_file}')\n" + "n = int(p.read_text()) + 1; p.write_text(str(n))\n" + "print('pending', end='')\n", + encoding="utf-8", + ) yaml_str = f""" schema_version: "1.0" @@ -2007,11 +2031,7 @@ steps: steps: - id: tick type: shell - run: | - n=$(cat {counter_file}) - n=$((n + 1)) - printf '%s' $n > {counter_file} - printf 'pending' + run: {py} {script_file} """ definition = WorkflowDefinition.from_string(yaml_str) engine = WorkflowEngine(project_dir) @@ -2022,7 +2042,8 @@ steps: assert counter_file.read_text(encoding="utf-8").strip() == "3" # Unprefixed key holds the last iteration's result. assert state.step_results["tick"]["output"]["stdout"] == "pending" - # Namespaced keys for loop iterations 1 and 2 exist. + # Namespaced keys for all iterations exist. + assert "retry-loop:tick:0" in state.step_results assert "retry-loop:tick:1" in state.step_results assert "retry-loop:tick:2" in state.step_results @@ -2035,8 +2056,18 @@ steps: from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition from specify_cli.workflows.base import RunStatus + import sys + counter_file = project_dir / ".counter" counter_file.write_text("0", encoding="utf-8") + py = sys.executable + script_file = project_dir / "_tick.py" + script_file.write_text( + f"import pathlib; p = pathlib.Path(r'{counter_file}')\n" + "n = int(p.read_text()) + 1; p.write_text(str(n))\n" + "print('pending', end='')\n", + encoding="utf-8", + ) yaml_str = f""" schema_version: "1.0" @@ -2052,11 +2083,7 @@ steps: steps: - id: tick type: shell - run: | - n=$(cat {counter_file}) - n=$((n + 1)) - printf '%s' $n > {counter_file} - printf 'pending' + run: {py} {script_file} """ definition = WorkflowDefinition.from_string(yaml_str) engine = WorkflowEngine(project_dir)