Skip to content
Merged
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
9 changes: 0 additions & 9 deletions src/openjd/model/v2023_09/_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -758,15 +758,6 @@ def _requires_oneof(cls, values: dict[str, Any], info: ValidationInfo) -> dict[s

on_enter = values.get("onEnter")
on_exit = values.get("onExit")
# Base 2023-09 (§3.5) requires onEnter whenever a script is present;
# RFC 0008 relaxes this to "at least one action" when the
# WRAP_ACTIONS extension is declared. The strict base rule is only
# applied at template decode (context present) — job-instantiation
# re-validation has no parsing context, matching the other extension
# gates in this module.
if context is not None and "WRAP_ACTIONS" not in extensions:
if on_enter is None:
raise ValueError("onEnter is required.")
if on_enter is None and on_exit is None:
raise ValueError("Must define one of: onEnter or onExit")
return values
Expand Down
40 changes: 31 additions & 9 deletions test/openjd/model_v0/v2023_09/test_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class TestEnvironmentActions:
"data",
(
pytest.param({"onEnter": {"command": "foo"}}, id="has onEnter"),
pytest.param({"onExit": {"command": "foo"}}, id="has onExit"),
# For making sure our pre-validator logic is correct
pytest.param(
{
Expand All @@ -187,26 +188,35 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
# THEN
# no exception was raised.

def test_parse_onexit_only_with_wrap_actions_extension(self) -> None:
# RFC 0008: with the WRAP_ACTIONS extension declared, an environment
# may define any single action without a standalone onEnter.
@pytest.mark.parametrize(
"supported_extensions",
(
pytest.param([], id="no extensions"),
pytest.param(["EXPR", "WRAP_ACTIONS"], id="EXPR and WRAP_ACTIONS"),
),
)
def test_parse_onexit_only(self, supported_extensions: list[str]) -> None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This relaxation is only covered on the pure-Python (v0) side. AGENTS.md ("Reference parity") asks that every reference test under test/openjd/model_v0/ have an equivalent under test/openjd/model_v1/ exercising the same behaviour through the Rust binding, and there is currently no v1 test that decodes an environment whose script.actions defines onExit only — the closest fixture (MINIMAL_ENV in test_rust_model_bindings.py:36) defines both actions.

That matters more than usual here because the constraint being removed was introduced in a2cc32f as part of the openjd-rs parity work. If openjd-model 0.5.2 still rejects onExit-only, this change flips the divergence rather than closing it: v0 would accept a template that decode_environment_template through the binding rejects, and nothing in the suite would catch it.

Suggest adding a v1 counterpart that decodes {"actions": {"onExit": {"command": "foo"}}} — if it passes, it is a regression test for the parity you are asserting; if it fails, it belongs in test/openjd/model_v1/test_known_gaps.py as an xfail so the gap is recorded.

# An environment may define onExit without onEnter; either ordinary
# action alone satisfies the one-of requirement, with or without an
# extension being declared.

# GIVEN
context = ModelParsingContext(supported_extensions=["EXPR", "WRAP_ACTIONS"])
context = ModelParsingContext(supported_extensions=supported_extensions)

# WHEN
_parse_model(model=EnvironmentActions, obj={"onExit": {"command": "foo"}}, context=context)
actions = _parse_model(
model=EnvironmentActions, obj={"onExit": {"command": "foo"}}, context=context
)

# THEN
# no exception was raised.
assert actions.onEnter is None
assert actions.onExit is not None
assert actions.onExit.command == "foo"

@pytest.mark.parametrize(
"data",
(
pytest.param({}, id="empty object"),
# §3.5: base 2023-09 requires onEnter whenever a script is
# present (the WRAP_ACTIONS extension relaxes this).
pytest.param({"onExit": {"command": "foo"}}, id="onExit only"),
pytest.param({"onEnter": {"command": "foo"}, "onUnknown": "blah"}, id="unknown field"),
),
)
Expand All @@ -219,3 +229,15 @@ def test_parse_fails(self, data: dict[str, Any]) -> None:

# THEN
assert len(excinfo.value.errors()) > 0

def test_parse_fails_no_actions_message(self) -> None:
# An actions object with neither ordinary action must be rejected with
# the one-of message; this is the only remaining constraint on which
# ordinary actions an environment defines.

# WHEN
with pytest.raises(ValidationError) as excinfo:
_parse_model(model=EnvironmentActions, obj={})

# THEN
assert "Must define one of: onEnter or onExit" in str(excinfo.value)
10 changes: 10 additions & 0 deletions test/openjd/model_v0/v2023_09/test_environment_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ class TestEnvironmentTemplate:
{"specificationVersion": "environment-2023-09", "environment": ENVIRONMENT},
id="minimum required",
),
pytest.param(
{
"specificationVersion": "environment-2023-09",
"environment": {
"name": "Foo",
"script": {"actions": {"onExit": {"command": "foo"}}},
},
},
id="script with onExit only",
),
pytest.param(
{
"specificationVersion": "environment-2023-09",
Expand Down
14 changes: 14 additions & 0 deletions test/openjd/model_v0/v2023_09/test_job_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,20 @@ class TestJobTemplate:
},
id="with least environments",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "Foo",
"steps": [STEP_TEMPLATE],
"jobEnvironments": [
{
"name": "Foo",
"script": {"actions": {"onExit": {"command": "foo"}}},
}
],
},
id="with environment defining onExit only",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
Expand Down
Loading