Skip to content

test: repr_py control-character conformance tests - #162

Merged
leongdl merged 3 commits into
OpenJobDescription:mainlinefrom
leongdl:conformance-wrap-actions-expected-failures
Sep 11, 2026
Merged

test: repr_py control-character conformance tests#162
leongdl merged 3 commits into
OpenJobDescription:mainlinefrom
leongdl:conformance-wrap-actions-expected-failures

Conversation

@leongdl

@leongdl leongdl commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Two fixtures pinning Expression Language §2.2.6's requirement that repr_py escape control characters, promoted out of proposed/ now that the fix has landed.

TLDR. Both were parked here as expected failures. openjd-rs#374 merged as 5b04959, so both pass and become the regression tests the proposed/README.md promised. Verified against a pre-fix binary as the control: both fail before the fix and pass after, so they pin what they claim.

The two fixtures

Fixture What it pins
EXPR/jobs/expr2.2.6--repr-py-newline-roundtrip repr_py("a\nb") fed to ast.literal_eval round-trips to the input
WRAP_ACTIONS/jobs/wrap-repr-py-escapes-newline-in-wrapped-args the same rule through RFC 0008's repr_py(WrappedAction.Args) forwarding pattern, with a two-line python -c body

The pair is deliberate. The EXPR one isolates the rule; the WRAP_ACTIONS one exercises the end-to-end path a template author actually writes, where the value crosses two process boundaries before being parsed.

Measured

Built openjd from openjd-rs main at 5b04959, and separately from a pre-fix commit as the control:

Fixture pre-fix merged
expr2.2.6--repr-py-newline-roundtrip FAIL PASS
wrap-repr-py-escapes-newline-in-wrapped-args FAIL PASS

Both pre-fix failures are SyntaxError: unterminated string literal with the expected output missing, which is exactly the defect: repr_py quoted its output without escaping, so a value carrying a newline emitted it raw and CPython would not parse the result.

Run through the runner's directory scan from the new locations, not single-file mode, since discoverability is the point of leaving proposed/:

2023-09/EXPR/jobs:
  ✓ expr2.2.6--repr-py-newline-roundtrip
2023-09/WRAP_ACTIONS/jobs:
  ✓ wrap-repr-py-escapes-newline-in-wrapped-args

No regression in either category: WRAP_ACTIONS 73 passed, 0 failed and EXPR 352 passed, 0 failed.

Notes for review

The headers are rewritten rather than moved unchanged. Both described the parked state and asserted that the implementations fail, which is no longer true. They now carry spec, test and expectation in three paragraphs, name the paired fixture, and sit inside the comment-line budget.

proposed/README.md and the proposed/ directory are removed. The fixture it described was its only entry.

The EXPR fixture originates on the func-lib expected-failures branch behind #165, which should drop its copy so the two do not collide on merge.

One thing this does not resolve, recorded so it is not lost: Template Schemas §5.2 restricts <ArgString> to characters outside the Cc category, which excludes the newlines both of these fixtures rely on. Neither implementation enforces that, and 150 of 1,191 merged fixtures carry a newline in an args entry, so the multi-line python -c convention is load-bearing suite-wide. Either §5.2 is relaxed to permit U+000A and U+000D, or that convention is invalid. That is a spec decision and a separate PR; these fixtures are correct under the reading both implementations already follow.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Quorum verdict: GOOD-WITH-NITS (4 reviewers flagged variants of the same two issues) — spec: EL §2.2.6 repr_py (newline must be escaped — spec gives the exact worked example) via RFC 0008 WrappedAction.Args forwarding.

(1) Spec self-tension, correctly disclosed but under-weighted: the fixture's own onRun args contain a literal U+000A, which §5.2's ArgString Cc-exclusion arguably forbids — as written the template may be spec-invalid, so promotion is contingent on resolving §5.2 vs the multi-line python -c convention (which the Deadline Docker container env also relies on; if §5.2 is enforced, that breaks service-wide). Resolve the spec question first.
(2) Attribution: README says "implementation bug (openjd-rs)" but the 2026-08-12 sweep shows it fails BOTH implementations — state Python's status.
(3) Partial overlap with PR #165's expr2.2.6--repr-py-newline-roundtrip (same root defect); the end-to-end wrap path justifies keeping both, but cross-reference them.

@leongdl

leongdl commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Quorum review (5 independent agents). 1 fixture + README. Verdict: GOOD-WITH-NITS. The repr_py newline defect is real (fails BOTH implementations per the 2026-08-12 sweep) and the end-to-end wrap-forwarding angle adds value over PR #165's unit-level twin — but the fixture is hostage to an unresolved spec tension its own README flags, and the README under-attributes the failure. Cross-PR: this branch uses component-level WRAP_ACTIONS/proposed/ while PR #166 uses kind-level job_templates/proposed/ — the five expected-failures PRs should standardize on one placement (kind-level routes promotion mechanically).

@leongdl
leongdl force-pushed the conformance-wrap-actions-expected-failures branch from 8ad36ab to cf644fb Compare August 15, 2026 19:26
@leongdl

leongdl commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Quorum-review fixes applied and pushed (rebased onto mainline 3069673):

  • Restructured to kind-level parking: WRAP_ACTIONS/jobs/proposed/ (promotion = move up one directory).
  • README corrected: the repr_py newline defect fails both implementations (2026-08-12 sweep), not just openjd-rs.
  • The §5.2 ArgString/newline conflict is now the stated promotion gate rather than a footnote — as written the fixture's own args are arguably spec-invalid until that's resolved (and the resolution also decides the fate of the multi-line python -c convention suite-wide).
  • Cross-referenced the unit-level twin in the func-lib expected-failures PR (test: [expected failures] function-library fixtures pending spec decisions #165).

Comment thread conformance-tests/2023-09/WRAP_ACTIONS/jobs/proposed/README.md Outdated
@leongdl leongdl changed the title test: [expected failures] WRAP_ACTIONS fixtures needing implementation fixes test: Promote the repr_py control-character fixtures out of proposed/ Sep 10, 2026
leongdl added a commit to leongdl/openjd-specifications that referenced this pull request Sep 10, 2026
openjd-rs#374 landed the repr_py escaping fix, so
expr2.2.6--repr-py-newline-roundtrip passes and no longer belongs in
proposed/. It is promoted to EXPR/jobs/ on OpenJobDescription#162 alongside its
WRAP_ACTIONS twin, where the two land together and the control run
against a pre-fix binary is recorded.

Removing it here so the two copies do not collide when both merge. Its
README section goes with it.

The other two fixtures on this branch are untouched and still parked:
both are spec decisions, not implementation bugs.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
@leongdl leongdl changed the title test: Promote the repr_py control-character fixtures out of proposed/ test: repr_py control-character conformance tests Sep 10, 2026
Spec-correct fixtures that FAIL against current reference implementations, parked in a proposed/ directory the conformance runner does not discover, so merging keeps the suite green. Each entry in proposed/README.md records the observed output, spec citation, and classification. Promote each fixture up one directory unchanged when its implementation fix lands. Companion to conformance-wrap-actions-gaps.

Review: quorum-review fixes — kind-level proposed/ placement
(jobs/proposed/); README corrected: the defect fails BOTH implementations
(2026-08-12 sweep), the §5.2 ArgString conflict is now the stated
promotion gate rather than a footnote (the fixture's own args are
arguably spec-invalid until that is resolved), and the unit-level repr_py
twin in the func-lib expected-failures PR is cross-referenced.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
Both were parked as expected failures. openjd-rs#374 landed the fix, so
they pass and become the regression tests the proposed/README promised.

Measured against a binary built from openjd-rs main at 5b04959, and
against a pre-fix binary as the control:

  fixture                                        pre-fix   merged
  EXPR/jobs/expr2.2.6--repr-py-newline-roundtrip  FAIL      PASS
  WRAP_ACTIONS/.../wrap-repr-py-escapes-newline   FAIL      PASS

Both pre-fix failures are "SyntaxError: unterminated string literal" with
the expected output missing, which is the defect they were written for:
repr_py quoted its output without escaping, so a value carrying a newline
emitted it raw and CPython would not parse the result.

Discovered by the runner's directory scan from their new locations, not
just single-file mode, which is what leaving proposed/ was for. No
regression in either category: WRAP_ACTIONS 73 passed 0 failed, EXPR 352
passed 0 failed.

The EXPR fixture comes from the func-lib expected-failures branch (OpenJobDescription#165),
which should drop its copy so the two do not collide on merge.

Headers rewritten, since both described the parked state and claimed the
implementations fail. Now spec, test, expectation in three paragraphs each,
naming the paired fixture, within the comment-line budget.

proposed/README.md and the proposed/ directory are removed: the fixture it
described was its only entry.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
@leongdl
leongdl force-pushed the conformance-wrap-actions-expected-failures branch from 8b8851f to 7189e46 Compare September 10, 2026 17:37
windows-latest failed the Python lane on this fixture and ubuntu-latest
passed the same installed CLI. Not an implementation difference: the
fixture matched its own forbidden marker.

The harness scans a job's whole output for forbidden substrings, and on
Windows openjd-sessions-for-python echoes the full child command line at
INFO (_subprocess.py logs list2cmdline(self._args)); the POSIX path logs
only the temp .sh path. This fixture's script source contained the literal
FAIL, and FAIL is its forbidden entry, so the echo matched even though
every assertion passed.

The run log discriminates the two cases. It reports "Found forbidden
output: FAIL" with no "Missing expected output" line, so NEWLINE:PASS was
printed and the escaping worked on Windows too. A genuine failure of this
fixture looks different: the pre-fix control run reported the missing
NEWLINE:PASS line alongside a SyntaxError.

Split the marker across two adjacent literals, the convention this repo
already uses for the same hazard (20c1a84, and the timeout fixture's
'SHOULD_NOT' + '_PRINT'). Python concatenates them at compile time, so the
run-time value is unchanged at 'FAIL got=' while the source no longer
carries the literal. Verified both halves: the value still compares equal,
and the script body no longer contains FAIL.

Verified the marker still catches a real failure: comparing against a
wrong value makes the runner report 0 passed, 1 failed, and restoring the
fixture byte-identically returns it to 1 passed, 0 failed.

The Rust lane was never affected, because it prints only COMMAND_OUTPUT
records while the command echo is tagged FILE_PATH|PROCESS_CONTROL. The
WRAP_ACTIONS fixture is unaffected too: it declares no forbidden entries.

Windows itself is unverified from here.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
@leongdl
leongdl marked this pull request as ready for review September 11, 2026 01:58
@leongdl
leongdl requested a review from a team as a code owner September 11, 2026 01:58
@leongdl
leongdl merged commit 2c63e8a into OpenJobDescription:mainline Sep 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants