Skip to content

Commit 2384490

Browse files
authored
fix: map WRAP_ACTIONS in the ModelExtension binding (#322)
PyModelExtension was missing WRAP_ACTIONS, so the catch-all arm in From<ModelExtension> for PyModelExtension silently reported it as EXPR. A profile built with WRAP_ACTIONS enabled EXPR instead, and templates using onWrapEnvEnter/onWrapTaskRun/onWrapEnvExit were rejected with an error that did not name the cause. The catch-all cannot be removed because openjd_model::ModelExtension is non_exhaustive, so TestModelExtension guards the mapping instead: it fails when the binding enum drifts from ExtensionName, or when any spec name parses back to a different member. The pickle round-trip list also gains the new member. Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
1 parent 8eb4aa8 commit 2384490

4 files changed

Lines changed: 65 additions & 0 deletions

File tree

rust-bindings/src/model/profile.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ pub(crate) enum PyModelExtension {
184184
REDACTED_ENV_VARS = 1,
185185
FEATURE_BUNDLE_1 = 2,
186186
EXPR = 3,
187+
WRAP_ACTIONS = 4,
187188
}
188189

189190
#[cfg_attr(feature = "stub-gen", gen_stub_pymethods)]
@@ -238,6 +239,7 @@ impl From<PyModelExtension> for ModelExtension {
238239
PyModelExtension::REDACTED_ENV_VARS => ModelExtension::RedactedEnvVars,
239240
PyModelExtension::FEATURE_BUNDLE_1 => ModelExtension::FeatureBundle1,
240241
PyModelExtension::EXPR => ModelExtension::Expr,
242+
PyModelExtension::WRAP_ACTIONS => ModelExtension::WrapActions,
241243
}
242244
}
243245
}
@@ -249,6 +251,20 @@ impl From<ModelExtension> for PyModelExtension {
249251
ModelExtension::RedactedEnvVars => PyModelExtension::REDACTED_ENV_VARS,
250252
ModelExtension::FeatureBundle1 => PyModelExtension::FEATURE_BUNDLE_1,
251253
ModelExtension::Expr => PyModelExtension::EXPR,
254+
ModelExtension::WrapActions => PyModelExtension::WRAP_ACTIONS,
255+
// `ModelExtension` is `#[non_exhaustive]`, so downstream
256+
// crates cannot write an exhaustive match. This arm is
257+
// mandatory, and the compiler will not warn when the Rust
258+
// crate gains a variant that has no arm above. The fallback
259+
// therefore MISREPRESENTS any such variant as `EXPR`.
260+
//
261+
// A new upstream variant requires three edits: a new
262+
// `PyModelExtension` member, an arm here, and an arm in
263+
// `From<PyModelExtension> for ModelExtension`. The guard
264+
// against forgetting is
265+
// `test/openjd/model_v1/test_version_enums.py::TestModelExtension`,
266+
// which fails when this enum drifts from
267+
// `openjd.model.v2023_09.ExtensionName`.
252268
#[allow(unreachable_patterns)]
253269
_ => PyModelExtension::EXPR,
254270
}

src/openjd/_openjd_rs.pyi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3138,6 +3138,7 @@ class ModelExtension(enum.Enum):
31383138
REDACTED_ENV_VARS = ...
31393139
FEATURE_BUNDLE_1 = ...
31403140
EXPR = ...
3141+
WRAP_ACTIONS = ...
31413142

31423143
@property
31433144
def name(self) -> builtins.str:

test/openjd/model_v1/test_pickle.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"openjd._openjd_rs.ModelExtension.REDACTED_ENV_VARS",
5353
"openjd._openjd_rs.ModelExtension.FEATURE_BUNDLE_1",
5454
"openjd._openjd_rs.ModelExtension.EXPR",
55+
"openjd._openjd_rs.ModelExtension.WRAP_ACTIONS",
5556
"openjd._openjd_rs.TemplateSpecificationVersion.JOBTEMPLATE_v2023_09",
5657
"openjd._openjd_rs.TemplateSpecificationVersion.ENVIRONMENT_v2023_09",
5758
],

test/openjd/model_v1/test_version_enums.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import pytest
44

55
from openjd.model._v1 import TemplateSpecificationVersion, decode_job_template
6+
from openjd.model._v1.types import ModelExtension
7+
from openjd.model.v2023_09 import ExtensionName
68

79
# All known variants. Add new ones here as the spec evolves; the test
810
# ensures every variant is classified as exactly job-template OR
@@ -56,3 +58,48 @@ def test_template_specification_version_returned_from_decode(self) -> None:
5658
# like behaviour without being a str subclass).
5759
assert t.specification_version == "jobtemplate-2023-09"
5860
assert t.specification_version.value == "jobtemplate-2023-09"
61+
62+
63+
def _rust_extension_members() -> set[str]:
64+
# `ModelExtension` is a PyO3 pyclass enum, not a real `enum.Enum`, so
65+
# it is not iterable. Discover members by identity instead of relying
66+
# on the enum protocol or a hardcoded list.
67+
return {
68+
name
69+
for name in dir(ModelExtension)
70+
if isinstance(getattr(ModelExtension, name), ModelExtension)
71+
}
72+
73+
74+
class TestModelExtension:
75+
"""Drift guard for the `ModelExtension` PyO3 binding.
76+
77+
`openjd_model::types::ModelExtension` is `#[non_exhaustive]`, so
78+
`From<ModelExtension> for PyModelExtension` must carry a catch-all
79+
arm and the compiler cannot warn when the Rust crate gains a variant
80+
the binding does not map. These tests are that warning.
81+
82+
Regression: `WRAP_ACTIONS` landed in the Rust crate six days after
83+
the binding was written; the binding was never updated, so the
84+
catch-all silently mapped it to `EXPR`. A profile built with
85+
`WRAP_ACTIONS` therefore enabled `EXPR` instead, and templates using
86+
`onWrapEnvEnter`/`onWrapTaskRun`/`onWrapEnvExit` were rejected with
87+
an error that did not name the real cause.
88+
"""
89+
90+
def test_binding_members_match_python_extension_names(self) -> None:
91+
# The binding enum and the Python model layer describe the same
92+
# set of 2023-09 extensions. Divergence in either direction is a
93+
# bug: a missing member means the catch-all aliases it to the
94+
# wrong extension; an extra member means Python cannot name it.
95+
assert _rust_extension_members() == {e.value for e in ExtensionName}
96+
97+
@pytest.mark.parametrize("name", [e.value for e in ExtensionName])
98+
def test_name_round_trips_to_its_own_member(self, name: str) -> None:
99+
# Catches aliasing directly, independent of member-set equality:
100+
# every spec name must parse back to the member of the same name,
101+
# never to a different extension.
102+
parsed = ModelExtension.from_str(name)
103+
assert parsed is not None, f"{name} is not recognized by the Rust binding"
104+
assert parsed.name == name
105+
assert parsed.as_str() == name

0 commit comments

Comments
 (0)