Skip to content

test: adapt StepScript de-sugaring tests to openjd-model 0.11.9 #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seant-aws merged 1 commit into from
Sep 9, 2026
Merged

test: adapt StepScript de-sugaring tests to openjd-model 0.11.9 #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ dependencies = [
# StepTemplate.resolve_syntax_sugar() (run_step_task.py, for FEATURE_BUNDLE_1
# simple-action steps) and openjd.expr.SerializedSymbolTable (the runtime
# adapters). resolve_syntax_sugar() first shipped in openjd-model 0.9.0, well
# below the floor here, so no floor change is needed for it to exist. It is
# recorded because the fold order it produces,
# [*step lets, *simple-action lets], is model behaviour this package's tests
# assert rather than behaviour this package controls.
# below the floor here, so no floor change is needed for it to exist. Its
# fold behaviour changed in openjd-model 0.11.9: the produced `script.let` no
# longer includes step-scope `let`, which now travels exclusively through the
# service-served resolvedSymbolTable. This package's tests have been adapted
# to that contract.
"openjd-model >= 0.11.4, < 0.12",

@github-actions github-actions Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency floor should be raised to >= 0.11.9 to match what this PR now relies on.

The comment directly above states that resolve_syntax_sugar()'s fold behaviour changed in openjd-model 0.11.9, and both the docstring in run_step_task.py and the assertions in test_run_step_task.py / test_step_scope_let_end_to_end.py now encode the post-0.11.9 contract. But the specifier still admits 0.11.4 through 0.11.8, where the fold still emits [*step lets, *simple-action lets]. Against any of those versions:

  • test_start_de_sugars_a_bash_step fails outright (step_script.let would be ["base = 'from step'", "msg = base"], not ["msg = base"]).
  • More importantly at runtime, step-scope names arrive on both channels again — resolved by the service in resolvedSymbolTable and as source expressions re-evaluated by the session — which is exactly the unverified double-declaration hazard the removed docstring paragraph described. The new docstring asserts "those names no longer arrive twice", which is false for a resolvable version in the current range.

Note this is the same shape of coupling the openjd-sessions == 0.12.0 pin just above documents ("this pin and the code change in this PR must ship together"). Suggest "openjd-model >= 0.11.9, < 0.12", and updating the comment to say the floor was raised for this reason rather than that no floor change is needed.

# tomli became tomllib in standard library in Python 3.11
"tomli == 2.0.* ; python_version<'3.11'",
Expand Down
20 changes: 10 additions & 10 deletions src/deadline_worker_agent/sessions/actions/run_step_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ def _resolve_step_script(step_template: StepTemplate) -> StepScript:
A FEATURE_BUNDLE_1 simple-action template (`bash:`, `cmd:`, `node:`,
`powershell:`, `python:`) has no `script` at all. The service serves the
sugar as authored and the worker never instantiates a job, so nothing
de-sugars it. `resolve_syntax_sugar()` does that here, returning a new
template whose script carries `[*step lets, *simple-action lets]`.
de-sugars it. `resolve_syntax_sugar()` does that here, synthesizing a
runnable `script` from the sugar. Under openjd-model >=0.11.9 the produced
`script.let` carries only the simple-action's own `let`; step-scope `let` is
no longer folded in.

Step-scope `let` values reach the session through the resolved symbol table
the service serves. For a `script:` template that is the only channel, and
the source expressions are never re-evaluated here.

That is not true of the sugar path: the fold above re-declares every
step-scope binding in the produced `script.let`, so on a sugar template
those names arrive twice -- once resolved by the service in the table, and
once as a source expression the session re-evaluates on top of it. No test
covers a sugar template together with a populated resolvedSymbolTable, so
whether the two channels can disagree (a binding that re-evaluates to a
different value, or fails to re-evaluate against the per-action table) is
currently unverified rather than known-safe.
The same is now true of the sugar path: step-scope bindings travel
exclusively through the service-served `resolvedSymbolTable`, not through
the fold, so those names no longer arrive twice. The fold synthesizes the
script structure and carries only the simple-action `let`, which leaves the
resolved symbol table as the single channel for step-scope bindings on both
paths.
"""
script = step_template.script
if script is not None:
Expand Down
15 changes: 9 additions & 6 deletions test/unit/sessions/actions/test_run_step_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,13 @@ class TestRunStepTaskActionSimpleActionSugar:
"""

def test_start_de_sugars_a_bash_step(self, mock_session, mock_executor):
"""The folded script goes out, carrying both `let` scopes in order.
"""The folded script goes out, carrying the simple-action ``let``.

``resolve_syntax_sugar()`` folds step-scope ``let`` into the script's own
``let`` as ``[*step lets, *simple-action lets]``. Without that fold the
action would have no script at all to send.
``resolve_syntax_sugar()`` still synthesizes a runnable script from the
sugar, but under openjd-model >=0.11.9 it no longer folds step-scope
``let`` into the produced ``script.let`` -- only the simple-action's own
``let`` remains. Step-scope bindings travel via ``resolvedSymbolTable``
instead. Without the fold the action would have no script at all to send.
"""
details = _step_details_from_template(
{
Expand All @@ -149,8 +151,9 @@ def test_start_de_sugars_a_bash_step(self, mock_session, mock_executor):
call_kwargs = mock_session.run_task.call_args.kwargs
step_script = call_kwargs["step_script"]
assert step_script is not None
# Both scopes present exactly once, step bindings first.
assert step_script.let == ["base = 'from step'", "msg = base"]
# Only the simple-action `let` folds into the script now; the
# step-scope `let` travels via resolvedSymbolTable.
assert step_script.let == ["msg = base"]
assert step_script.actions.onRun.command == "bash"

def test_start_does_not_mutate_the_served_template(self, mock_session, mock_executor):
Expand Down
18 changes: 10 additions & 8 deletions test/unit/sessions/test_step_scope_let_end_to_end.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,22 +406,24 @@ def test_a_bash_sugar_step_runs_and_resolves_its_own_let(
assert state == ActionState.SUCCESS
assert any("BASH:hello from bash" in m for m in caplog.messages)

def test_step_and_sugar_scope_let_both_resolve_through_the_fold(
def test_step_and_sugar_scope_let_both_resolve(
self, tmp_path: Path, caplog: pytest.LogCaptureFixture
) -> None:
"""Both scopes of a sugar template resolve, via the fold alone.

``resolve_syntax_sugar()`` folds the step's ``let`` into the script's own
as ``[*step lets, *simple-action lets]``, so the de-sugared script
carries both scopes in RFC 0005 order and a sugar-scope binding can
reference a step-scope one. This is the only thing that resolves the
step's scope on this path -- nothing else re-applies it.
"""Both scopes of a sugar template resolve, via two channels.

Under openjd-model >=0.11.9 the step's ``let`` no longer folds into the
de-sugared ``script.let``; it travels via ``resolvedSymbolTable``
instead. ``resolve_syntax_sugar()`` still synthesizes the script
structure and folds in the sugar-scope ``let``, so a sugar-scope binding
(``out``) can reference a step-scope one (``base``) once ``base`` is
served in the resolved table.
"""
caplog.set_level(logging.INFO)
details = _bash_sugar_step_details(
step_let=["base = 'from step'"],
sugar_let=["out = base + '|sugar'"],
script_body='echo "OUT:{{ out }}"',
resolved_symbol_table=[{"name": "base", "type": "string", "value": "from step"}],

@github-actions github-actions Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding resolved_symbol_table here removes the only coverage of a sugar template that has step-scope let but no served resolvedSymbolTable.

After this change, in TestSimpleActionSugarEndToEnd:

  • test_a_bash_sugar_step_runs_and_resolves_its_own_let passes step_let=None,
  • this test passes step_let=[...] plus a matching resolved table.

So nothing exercises step_let=["base = ...] with resolved_symbol_table=None. That combination used to work — the fold re-declared base in script.let — and under >=0.11.9 it now fails to resolve, because neither channel supplies base. The behaviour change is intentional per the docstring, but it means correctness on the sugar path is now entirely contingent on the service always populating resolvedSymbolTable whenever a step declares let. That assumption is worth pinning: either a negative test asserting the action fails cleanly in that case, or a note in the docstring that a served-table-less sugar template with step-scope let is out of contract.

For comparison, TestResolvedSymbolTableEndToEnd.test_a_step_without_resolved_table_still_runs deliberately keeps the no-table control for the script: path; the sugar path no longer has an equivalent for the let-bearing case.

)
assert details.step_template.script is None

Expand Down
Loading