test: Add 34 base 2023-09 conformance fixtures, strengthen 6 - #161
test: Add 34 base 2023-09 conformance fixtures, strengthen 6#161leongdl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Quorum verdict: NOT-GOOD (3/5: spec-literalist, service-compat, coverage) — the base 2023-09 spec never bounds <integer> (int64 appears only in the EXPR doc), and the service-compat reviewer verified empirically that openjd-model 0.11.2 ACCEPTS 2^63 (create_job succeeds), so this fixture fails against the reference implementation today. It is exactly the invalid partner that PR #163 parks in proposed/ for this reason — it should join that family (or land only after a base-spec int-bounds erratum). The int64-max accept twin in this PR is fine and should stay.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §1.1.1 (128-char limit measured post-resolution, inclusive; base = no FB1). Character-exact (1+127); verified accepted by openjd-model 0.11.2. No boundary-accept existed before.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §1.1.1 (>128 post-resolution rejected without FB1). Strongest-constructed fixture in the PR: the short literal + long default means only a post-resolution check fires, and the one-character-delta twin eliminates wrong-reason rejection. Verified: openjd-model 0.11.2's context-aware validator rejects at 128 without FB1 (the 512 TypeAdapter cap is supplemented), so no model conflict — and it matches the Deadline service's manual 128 check.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (3 reviewers same nit) — spec: §2 closed parameter-definition schemas. Matches the reference implementation (pydantic extra=forbid) and suite precedent, but the spec never explicitly states unknown keys must be rejected — a one-sentence spec clarification would put this whole family on solid ground. Distinct validation site from the root-level unknown-field fixture.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.1 (minValue is not a STRING-definition field). Stronger than a generic unknown-key test: minValue is a known key globally, so single-class-with-optional-fields implementations pass the unknown-key test but fail this. Genuinely distinct path.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.3 (minLength is not an INT-definition field). Mirror of minvalue-on-string in the other direction; together they cover both directions of the shared-fields modeling hazard.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.1 maxLength inclusive. Completes the off-by-one pair with the existing over-limit reject; catches exclusive-bound implementations.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.1 minLength inclusive. Symmetric partner; not redundant with the global value-length-cap fixture.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, shared nit) — spec: §2.3 default at exactly 2^63-1, accepted (verified against openjd-model 0.11.2). Family nit: the int64 premise isn't normative in base — as accepts these are the safe direction, but the family deserves a base-spec int-bounds erratum. Reject partner correctly parked in PR #163.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.3 default at exactly -2^63; only fixture exercising the negative extreme (catches abs-value/unsigned-parse bugs). Partner in #163.
There was a problem hiding this comment.
Quorum verdict: GOOD (4/5, mild family redundancy noted) — spec: §2.3 minValue at the bound. Per-field coverage defensible (separate parse paths); partner in #163.
There was a problem hiding this comment.
Quorum verdict: GOOD (4/5) — spec: §2.3 maxValue at the bound. Same family notes as minValue sibling; partner in #163.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.3 allowedValues entry at the bound; completes the 4-field accept matrix. Partner in #163.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.3 supplied value at 2^63-1, resolved exactly. The exact-output assertion catches silent float round-tripping (would print ...5808). First large-int runtime test in the jobs suite; safe accept direction.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.3 min/maxValue inclusive for supplied values. First 2.3 fixture in the jobs suite — supplied-value-at-boundary is a distinct job-creation path from template-default checks.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §2.4 (largest finite double accepted; verified against the model). Positive control for the 1e309 twin; guards clamp-or-reject parsers.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (3/5; 1 NOT-GOOD dissent) — the base spec never states binary64 for <float> (64-bit IEEE lives in the EXPR doc), so the MUST-reject rests on the same class of spec-silent premise that pushed the int64 rejects into #163's proposed/ — the dissent wants it moved there for consistency. Mitigations the majority accepted: YAML parsers materialize 1e309 as inf, merging it into the (defensible) non-finite rejection, and openjd-model rejects it today ("Input should be a finite number"). Recommend: keep, but pair with a §2.4 spec erratum pinning finite binary64.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.4 <float>/<floatstring> "base-10 value" wording; .inf has no base-10 numeral and JSON templates can't express it. Model rejects (verified). Nit: the spec doesn't explicitly exclude non-finite values — codify.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.4, same non-finite reading; NaN additionally breaks min/max/allowedValues comparability (a different hazard from inf, justifying the pair). Same codify-in-spec nit.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §2.4 min/maxValue inclusive for supplied values; 0.5/99.5 are exactly representable (good choice). Nit (adversarial): the full-line substrings pin decimal rendering — an implementation printing 0.50 honors inclusivity yet fails; base spec fixes no canonical float rendering. Consider comparison-based assertions.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §3.6 (let gated on EXPR). Every rejection path IS the gating rule (unknown key without the extension); model rejects with a targeted error. StepScript attachment point previously uncovered.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one craft nit) — spec: §3.6 at the StepTemplate level; with the env-template fixture this covers all three let attachment points. Nit: template body is byte-identical to EXPR/job_templates/3.6--let-requires-expr.invalid.yaml — defensible for base-only runners, but document the duplication.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §5.3 (mode discriminates the CancelationMethod union; required in both forms). Catches mode-defaulting single-object models. Distinct from the existing bad-mode-value fixture.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §5.3 (fmtstring mode is FB1-only; base admits only the two literals). The unused-looking Mode parameter is load-bearing (makes the fmtstring resolvable, isolating the gating rule) — nice construction. Targets FB1-capable implementations forgetting the gate, which the bad-mode-value fixture can't catch.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §5.3.1 (CancelationMethodTerminate is {mode: TERMINATE} only; notifyPeriodInSeconds exists only on §5.3.2). Catches the lenient one-struct model. Service check done: Deadline templates only pair notifyPeriod with NOTIFY_THEN_TERMINATE — no conflict.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §7.3/§7.3.1. Note the failure kind: {{MyParam}} is grammatically a valid single-Identifier Name; invalidity is semantic (no such value in base scope) — the comment says so honestly. Nits: an implementation deferring symbol resolution to job creation would pass openjd check (spec doesn't mandate check-time resolution); the rejection differs in kind from the grammar-error siblings.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, overlap nit) — spec: §7.1/§7.3 (Identifier charset). Any tokenization is a grammar rejection of the same expression. Partial path overlap with digit-start (same production), but they catch different regex mistakes — acceptable.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §7.1 (Identifier can't start with a digit) inside a reference segment — distinct from the existing name-level digit-start fixture.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §7.3 Name production (no empty segment). Inner empty segment is the classic split-on-dot hazard; distinct from the existing empty-reference fixture.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §7.3 (trailing dot = empty final segment). Different tokenizer position from inner-empty (some languages drop trailing empties in split), so the pair is justified.
There was a problem hiding this comment.
Quorum verdict: GOOD (4/5, two nits) — spec: §8 (SimpleAction sugar is FB1-only). Both rejection paths (unknown bash key, missing script) are the same fact, so no wrong-reason pass. Nits: the fixture is doubly invalid so it can't isolate the FB1 cause; conceptually near the existing FB1 8--simple-action-without-extension.invalid.yaml (bash vs python key). Also note the pre-existing §8-vs-§3.5 spec inconsistency on where sugar keys nest — worth a spec fix.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §8 bash desugaring (bash <generated .sh> <args...>). EXT_IS_SH:True can only come from the script executing with $0 set — not satisfiable by CLI echo; FIRST_ARG pins ordering. posix-gated correctly. First argv-contract test in the FB1 suite.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: §8 python desugaring. Cross-language twin, same non-vacuous construction; ungated matches the python-baseline convention.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — §3.4.3 zip semantics. The strengthening is genuine: old forbidden entries were near-vacuous out-of-range values; the new ones are exactly the mispaired combos a cross-product bug emits. Active on today's runner (forbidden is a mainline feature; no inert keys added).
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — §3.4.3 three-way zip. Strengthening kills full-product and pairwise-zip errors. Residual escape (adversarial reviewer): an implementation associating (A,C) while producting B emits (1,y,True) and (2,x,False) — neither forbidden — and still passes. Two more forbidden lines would close it.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — §3.4.3 (A,B)*C. All four product-only combos now forbidden — the product-instead-of-zip failure mode is fully foreclosed.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — §3.4.3 ((A,B),C). New forbidden entries catch both full-product and the plausible (A,B)*C misparse.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one note) — §3.4.3 (AB, CD). Forbidden entries pin off-by-one alignment, iteration-order divergence, and reversed zip; the column-major escape still drops expected lines so it fails anyway. Note: the zip result depends on product element order, which only the §3.4.3 example table establishes — a normative ordering sentence would help.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — §3.4.3 A*(B,C). Nit: forbids only 2 of 4 product-only combos, asymmetric with the mixed twin which forbids all four — cheap to match. Realistic failure modes are still caught (any mispaired (B,C) emits a forbidden line with some A).
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (2 reviewers same nit) — spec: §4.3 (onExit executes). Real gap: no jobs fixture asserted onExit runs at all. Nit: the header claims "runs when the session ends" but the mainline runner's unordered matching can't pin timing — an implementation running onExit before the task passes. Reword the comment to "runs"; the ordering claim becomes assertable only with the harness branch's outputSequence.
|
Quorum review (5 independent agents). 34 new + 6 strengthened + 2 FB1 fixtures; per-fixture verdicts posted as file comments. Net: one NOT-GOOD that should move to PR #163's proposed/ (2.3--int-value-above-int64-max — openjd-model 0.11.2 empirically ACCEPTS 2^63, and the base spec states no int64 bound), a cluster of GOOD-WITH-NITS resting on implicit spec readings that deserve errata (closed schemas, non-finite floats, int64 in base), and a large majority of clean, well-paired boundary fixtures. The 128/129 job-name pair is the strongest-constructed item (one-character-delta twins; verified against openjd-model 0.11.2 both directions). The six 3.4.3 forbidden-strengthenings are genuine and active on today's runner (two residual escapes noted). |
34 new fixtures plus 6 strengthened: INT/FLOAT numeric boundary accepts, inclusive-boundary accepts, onExit-runs, cancelation defaults, SimpleAction argv desugaring, reference-grammar negatives, cross-type field misuse, EXPR/FEATURE_BUNDLE_1 gating from the base side, post-resolution name length, and off-diagonal forbidden lines added to six associative combination fixtures. Suite: 655 -> 687 passing. Spec-correct fixtures that FAIL against current implementations (the int64 acceptance family, type-name case gate, merge widening) are split to conformance-base-expected-failures. Review: quorum-review fixes — moved 2.3--int-value-above-int64-max .invalid.test to the expected-failures PR's proposed/ (the base spec states no int64 bound and openjd-model 0.11.2 accepts 2^63, so the fixture fails the reference implementation; it documents a defect, not settled spec); closed the two remaining partial-association escapes with extra forbidden lines in 3.4.3--associative-three-parameters and 3.4.3--product-with-associative; onexit header no longer claims a timing the runner cannot assert; float-boundary assertions accept both 0.5 and 0.50 renderings via anyOf; documented the deliberate duplication of the base let-gating template. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
52c3869 to
e55f96a
Compare
|
Quorum-review fixes applied and pushed (rebased onto mainline 3069673):
Verified: full base suite (687 fixtures incl. this PR's) passes 687/687 against openjd-rs upstream/main |
1 participant
Summary
34 new base 2023-09 fixtures plus 6 strengthened, from a systematic coverage audit of the base spec. Full suite goes 655 → 687 passing, 0 failing (plus 2 FEATURE_BUNDLE_1 fixtures, 57 → 57 incl. the new ones).
Areas covered
default/minValue/maxValue/allowedValuesaccepted at 2^63−1 and −2^63; FLOAT accepted at 1e308, rejected at 1e309/.inf/.nan; supplied INT value at 2^63−1 runs, at 2^63 rejected at job creation.>for>=passed the old suite.onExitruns: verified there was previously no base job fixture containing anonExitaction at all; marker-based fixture added.TERMINATE+notifyPeriodInSecondsrejected;cancelationwith nomoderejected; format-stringmoderejected without FEATURE_BUNDLE_1.{{MyParam}}and four malformed dotted segments rejected — the inner-segment parser was previously untested.minValueon STRING,minLengthon INT, unknown key in a parameter definition.letrejected in base step template and step script;bash:SimpleAction key rejected without FEATURE_BUNDLE_1.3.4.3associative-combination job fixtures gained off-diagonalforbidden:lines. Before this, an implementation computing a full cross product where the spec requires a zip passed all six.Deliberately excluded
Cases blocked on spec defects recorded in the audit (descending-range semantics where the spec's own example table contradicts both implementations and four existing fixtures;
<posintstring>gating;maxLength: 0) — those need spec decisions first, not fixtures.Companion PR
Spec-correct fixtures that FAIL against current implementations (the int64 template-validation acceptance family, the base type-name case gate, §1.2.1 merge widening) are in the expected-failures PR (branch
conformance-base-expected-failures).