test: Add a §2 task parameter type case-insensitivity fixture - #166
test: Add a §2 task parameter type case-insensitivity fixture#166leongdl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.10 + §3.4.1.1.1 + EL §1.2.1 (2^63-1 endpoints must validate). Pins openjd-rs's undocumented 2^62 endpoint cap (rs-only per sweep); fix widens acceptance only. The README's note that the companion negative (PR #158's 2.10--range-expr-endpoint-int64-overflow) passes for the wrong reason until this accept is green is exactly right — promote as a pair. Nit: the quoted validation error looks pydantic-style (Python) for an rs-attributed bug — double-check which CLI produced it, and state that Python passes.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.13 + EL §1.2.1 (LIST[INT] element bound). Matches sweep (fails BOTH — say so; README says 'the current openjd CLI' without attribution). Nits: only fixture family in the five PRs without in-file header comments — add one (the bound comes from the EL type table, not §2.13); carry the same 'pending restoration of the RFC 0005 overflow text' hedge the expr-lang README uses for the identical bound. Accept twin: PR #158's 2.13--list-int-item-int64-max.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.16 (inner element, one container deeper — a declared sweep axis, kept despite likely sharing 2.13's recursive code path). Same nits as 2.13: no header comment, no per-implementation attribution, needs the spec-restoration hedge. Accept twin: PR #158's 2.16--list-list-int-inner-item-int64-max.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one attribution fix) — spec: §2 intro (with EXPR, job AND task parameter type names are case-insensitive — the sentence explicitly covers task types). Acceptance-widening only; queue environments unaffected (they carry job params). Promotion-ready. Fix: the README quotes a serde/Rust-style error and diagnoses rs-specifically, but the sweep shows BOTH implementations fail — add Python's observation. Inventory note: the reject twin (task-param lowercase WITHOUT EXPR) doesn't exist anywhere — consider adding it.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (dedupe action) — spec: §2 (base type names case-sensitive; the gate is otherwise undetectable suite-wide — good justification). Matches sweep (rs-only; Python correctly rejects — README omits this, its PR #163 sibling states it). Action: duplicates PR #163's base/proposed/2--type-lowercase.invalid.yaml — same construct, same defect, two different proposed/ paths; if both PRs land the same pin exists twice. Keep one or differentiate the type token.
|
Quorum review (5 independent agents). 5 fixtures + 2 READMEs. Net: 3 GOOD, 2 GOOD-WITH-NITS. This branch's kind-level placement (EXPR/job_templates/proposed/, base/job_templates/proposed/) is the better convention of the two in use — promotion is 'move up one directory unchanged' — and the quorum recommends the other four expected-failures branches adopt it. Actions: dedupe 2--type-lowercase-string against PR #163's identical fixture; add per-implementation attribution to the README (sweep shows several entries fail BOTH or rs-only where the README is vague); add the missing header comments to the two list-overflow fixtures and carry the same spec-restoration hedge the expr-lang README uses for the identical int64 bound. |
04735b7 to
2cb3fca
Compare
|
Quorum-review fixes applied and pushed (rebased onto mainline 3069673):
|
| command: python | ||
| args: | ||
| - "-c" | ||
| - "print()" |
There was a problem hiding this comment.
Lets print this and assert the output.
There was a problem hiding this comment.
Do we have a negative - case as well, try to group them into one test set.
| parameterDefinitions: | ||
| - name: Matrix | ||
| type: LIST[LIST[INT]] | ||
| default: [[9223372036854775808]] |
There was a problem hiding this comment.
Also add a negative cases for coverage.
| @@ -0,0 +1,87 @@ | |||
| # Proposed fixtures (extended parameter types) — job_templates | |||
| @@ -0,0 +1,19 @@ | |||
| # Section 2.10 / 3.4.1.1.1: RANGE_EXPR endpoints at the int64 boundary. | |||
There was a problem hiding this comment.
The four steps
-
Merge PR 350 — the Python code fix. Two blockers right now: it's BEHIND mainline so it needs updating, and REVIEW_REQUIRED with no approval. Checks pass.
-
Release openjd-model to PyPI. This is the step people forget. The specs Python lane runs pip install openjd-cli, so it tests whatever is published. Release fires on a push to mainline touching CHANGELOG.md, produced by release_bump.yml which is workflow_dispatch only — a human has to trigger it. Until then the lane still resolves openjd-model 0.11.9, which has the bug.
-
Release openjd-rs. The Rust lane runs cargo install openjd-cli, and #364 is merged but unreleased. The good news: release PR #365 already contains it — I verified 361b2c5 is an ancestor of that branch's 40c29ad. Merging #365 publishes the fix.
-
Merge PR 166 and promote the fixture. Merging 166 lands the file on mainline still inside proposed/, where the non-recursive glob can't see it. PR 166's own README states the rule: "promotion is a move up one directory unchanged". So promotion is a separate one-line move from EXPR/job_templates/proposed/ to EXPR/job_templates/.
Template Schemas §2 makes job and task parameter type names case-insensitive when EXPR is enabled. The case was implemented for job parameter types only, so a task parameter spelled `type: int` was rejected. Both implementations have since merged the fix: openjd-rs #364 and openjd-model-for-python #350. The fixture runs as an ordinary conformance test rather than being parked. It covers the four task parameter types of §3.4.1 in mixed case. It fails on both lanes until those fixes are released, because the runner installs published artifacts: `pip install openjd-cli` resolves openjd-model 0.11.9, which predates #350, and `cargo install openjd-cli` resolves 0.1.14, which predates #364. Measured against both. Neither release is queued. Also drops the three fixtures this branch parked in proposed/, and that directory's README. They pinned int64 bounds in list elements and a range endpoint, all still failing in one or both implementations, and are better raised on their own once those bugs are fixed than carried here. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
2cb3fca to
5c64d8d
Compare
leongdl
changed the title
leongdl
changed the title
1 participant
Adds one conformance fixture:
2023-09/EXPR/job_templates/3.4.1--task-param-type-case-insensitive.yaml.What it pins
Template Schemas §2 makes job and task parameter type names case-insensitive when the EXPR extension is enabled. The case was implemented for job parameter types only, so a task parameter spelled
type: intwas rejected with EXPR declared:The fixture covers the four task parameter types of §3.4.1 —
int,Float,sTrInG,pAtH— and must validate.Status: red until two releases ship
Both implementations have merged the fix, and neither has released it:
pip install openjd-cli→ openjd-model 0.11.9cargo install openjd-cli→ 0.1.14Measured, not inferred: the released Python CLI rejects the fixture, and a build of openjd-rs
mainaccepts it.Neither release is queued, and each needs a deliberate act:
release_bump.yml(manualworkflow_dispatch), which lands a CHANGELOG bump on mainline and triggers publish.Once both are on PyPI and crates.io, this goes green with no further change here.
Changes from the previous revision
This branch previously parked four fixtures under
job_templates/proposed/, which the runner does not scan — collection uses a non-recursivedirectory.glob("*.yaml")per kind directory, so nothing inproposed/ever ran.The case-insensitivity fixture is now an ordinary conformance test, and
proposed/plus its README are gone. The branch is also rebased onto mainline, which clears an unrelated failure: it carried the pre-rename3.5--env-script-onexit-only.invalid.yaml, corrected upstream in 563c6ec.The other three parked fixtures were dropped rather than promoted, because each still fails:
main2.10--range-expr-endpoint-int64-max2.13--list-int-item-int64-overflow.invalid2.16--list-list-int-inner-item-int64-overflow.invalidThey pin genuine int64-bound defects — bounds are enforced for expression literals but not for values arriving as YAML data — and the two
.invalidones additionally depend on restoring the overflow-is-an-error text that was dropped from the published spec. Better raised on their own once those are addressed than carried in a PR that is otherwise ready.Not covered, and worth adding
Nothing in the suite pins the other half of §2: that a lowercase task parameter type is still rejected without EXPR. Both implementations gate the fold on the extension, so a regression dropping that gate would pass the whole suite.