test: Add 13 extended-parameter-type conformance fixtures - #158
Conversation
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2 closed type union + EL §1.2.1 (list nesting max 2; LIST[LIST[LIST[INT]]] forbidden). Single defect; accept partner is mainline 2.16--list-list-int-param.yaml. New axis (nesting-depth rejection).
There was a problem hiding this comment.
Quorum verdict: GOOD (4/5) — spec: §2 intro / RFC 0007 (type names fully case-insensitive under EXPR, incl. inside LIST[...] brackets). Nits (coverage): marginal over mainline 2--type-case-insensitive.yaml (a case-folder passes both, a fixed-set matcher fails both), and it drops scalar PATH, the one type the sibling covers.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (4/5) — spec: §2.10 (default must be a valid IntRangeExpr) + EL §1.2.1 (int64). Uses exactly 2^63 — correct invalid value. Nits: (1) the header cites boundary pair 2.10--range-expr-endpoint-int64-max.yaml which is not on this branch — it's parked in PR #166's proposed/ (openjd-rs false-rejects it); say so, else the reference dangles; (2) the int64 bound comes from EL §1.2.1, not §3.4.1.1.1 (whose grammar says <Int> is "any integer value"); (3) until the accept twin is live, an int32-capped implementation passes this negative for the wrong reason.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.11 (default items are <string>; the dual int/intstring form is granted only to INT per §2.3). STRING was the only list type missing a wrong-item-type negative. Mirrors the existing 2.12 precedent.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.12 item.allowedValues (accept side; default ⊆ allowedValues). item.allowedValues for LIST[PATH] was wholly untested on mainline; pair complete intra-PR.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.12 item.allowedValues (reject side). Minimal one-element delta from its positive pair — good design; the only defect is the out-of-list element.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (4/5) — spec: §2.13 + EL §1.2.1 (element at exactly 2^63-1 accepted; pins full int64 range against int32 narrowing). Reject partner confirmed in PR #166 proposed/. Nits: only fixture in the set with no header comment (int64 fixtures particularly need one since the bound comes from the EL spec, not §2.13); "item-" in the name suggests an item: constraint block that isn't present.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.15 schema deliberately has no item property (parallel to §2.9's no-allowedValues note). Declaring item.allowedValues is the sole defect. Distinct validation path from the structural 2.15 negatives on mainline.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (4/5) — spec: §2.16 + EL §1.2.1 (inner elements at 2^63-1 / 2^63-2). Partner in PR #166. Same nits as the 2.13 sibling: no header comment; "inner-item" naming implies an item.item: block that isn't exercised; mechanical duplication of the 2.13 boundary (kept because nested-list is a declared defect axis).
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.9 note ("BOOL does not support allowedValues"). Comment quotes the normative text; sole defect is the property. Mainline 2.9 negatives were all wrong-default-type — unknown-property rejection is new coverage.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (4/5; 1 dissent NOT-GOOD) — spec: §2.12 Param.<name>[i] returns path + EL §2.3.1 properties; expected values verified against pathlib semantics. Dissent finding (real, cheap to fix): expected substring STEM0:alpha is contained in STEM0:alpha.exr, so an implementation whose .stem wrongly returns .name passes the whole fixture. Add a terminator (e.g. STEM0:alpha:END) or trailing sentinel to the STEM lines.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.15 accepted item spellings + EL §1.3.2 embedded interpolation (list → its string representation). Coercions correct. Nit (service-compat): pins [true, false, true] with ", " spacing — the spec's only spacing evidence is the §1.3.2 example; a compact-JSON implementation would fail on formatting alone. Coverage nit: overlaps the runtime sibling; could have merged.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one comment nit) — spec: §2.15 (all six supplied spellings coerce per the §2.9 value table; element access, len, not). Fills the only missing per-type runtime fixture (2.9-2.14 and 2.16 all have one). Nit: the header claims coverage of "each of the admitted element spellings" but float 1.0/0.0 and several string forms are untested — trim the claim.
|
Quorum review (5 independent agents). 13 fixtures; per-fixture verdicts posted as file comments. Net: 8 GOOD, 5 GOOD-WITH-NITS, 0 NOT-GOOD. All int64 boundaries use the exact spec values, all .invalid fixtures are single-defect, and the int64 reject partners are correctly parked in PR #166's proposed/. Actionable nits: a stale header reference to a boundary-pair file that lives in #166, two fixtures missing header comments, and one prefix-substring trap in the indexed-property test. |
13 fixtures from the RFC 0007 coverage audit: int64 accept twins for LIST[INT] and LIST[LIST[INT]] elements, weird mixed-case job parameter type spellings, first-ever LIST[BOOL] runtime coverage, three normative prohibitions (BOOL+allowedValues, item: on LIST[BOOL], triple-nested list), LIST[PATH] item.allowedValues pair, indexed-element property access Param.Paths[0].stem, RANGE_EXPR endpoint overflow reject, and a LIST[STRING] wrong-item-type negative. Suite: EXPR 350 -> 363 passing. Spec-correct fixtures that FAIL today are split to conformance-param-types-expected-failures. Review: quorum-review fixes — :END terminators close the stem-vs-name prefix trap in the indexed-property test; anyOf admits compact JSON list rendering; header comments added to the two int64-max accept fixtures (bound comes from the EXPR type table) and corrected on the 2.10 negative (its accept twin is parked in the expected-failures PR); the runtime fixture's coverage claim trimmed to the spellings actually exercised; scalar PATH added to the mixed-case fixture. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
fdb115b to
1b08917
Compare
|
Quorum-review fixes applied and pushed (rebased onto mainline 3069673):
Verified: 29 affected fixtures pass against openjd-rs upstream/main |
Summary
13 extended-parameter-type fixtures from a coverage audit of RFC 0007. Suite: EXPR 350 → 363 passing, 0 failing; base unchanged.
Areas covered
LIST[INT]element andLIST[LIST[INT]]inner element at 2^63−1 accepted — the data-axis boundary the audit found untested for every container type.LIST[BOOL]runtime coverage (the audit foundEXPR/jobs/had no2.15--*file at all): element values reaching the task, and interpolation rendering. Note: these two fixtures pass on openjd-rs and currently FAIL on the Python CLI (TypeError: List contains incompatible types: bool, string) — they are live divergence catchers, green against the reference implementation.LiSt[iNt]and friends accepted with EXPR (job parameters).BOOL+allowedValuesrejected;item:onLIST[BOOL]rejected;LIST[LIST[LIST[INT]]]triple nesting rejected — all stated in the schema, none previously tested.LIST[PATH]element constraints:item.allowedValuesaccept + violation pair (previously appeared in zero fixtures).{{Param.Paths[0].stem}}— the subscript route of the known property-access bug class; verified NOT affected.RANGE_EXPRendpoint overflow reject and aLIST[STRING]wrong-item-type negative (numeric elements rejected, no silent YAML coercion — verified on both implementations).Companion PR
Five spec-correct fixtures that FAIL today (list-element 2^63 acceptance in both implementations, the openjd-rs 2^62 endpoint cap, task-parameter case-insensitivity unimplemented in both, base lowercase type acceptance in openjd-rs) are in the expected-failures PR (branch
conformance-param-types-expected-failures).