test: [expected failures] base fixtures needing implementation fixes - #163
test: [expected failures] base fixtures needing implementation fixes#163leongdl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (5/5 with the same caveat) — spec: §1.2.1 ("each constraint must become more constrained", Job Template processed last). The literal reading supports rejection and the README honestly offers the alternative (intersection-merge) reading — this is closer to a spec decision than a clean mandate; the fixture may be invalidated by a spec revision rather than an implementation fix. Service note: adoption flips CreateJob accept→reject for queue-environment parameter merging (BealineDataPlane grafts queue-env param definitions onto customer templates), and the un-scoped constraint-OMISSION case is the real-world hazard — gate promotion on the spec decision plus a service deprecation path. Matches sweep (fails both). Accept twin: mainline 1.2.1--constraint-narrowing-int-range.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: §1.2.1, STRING minLength axis. Same narrow-vs-intersection ambiguity and same queue-env merge hazard as the int-range twin; good isolation (default satisfies both ranges). Accept twin: mainline 1.2.1--constraint-narrowing-minlength.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one dedupe action) — spec: §2 (base type names case-sensitive; case-insensitivity is EXPR-gated per RFC 0007 — the BNF literal plus the §2 sentence's negative implication is solid). Matches sweep (openjd-rs accepts, Python rejects) and the README states both. Action: this duplicates PR #166's base/job_templates/proposed/2--type-lowercase-string.invalid.yaml — same construct, same defect, two paths; keep one (or differentiate on the type token).
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (majority; 1 NOT-GOOD dissent) — the defect is real (both implementations accept 2^63; matches sweep) and rejection is a safe data-integrity adoption (values that can't round-trip int64 storage). Dissent (spec-literalist): the base 2023-09 spec never bounds — int64 exists only in the EXPR doc — so a bignum implementation is arguably conformant and this is a SPEC QUESTION, not an implementation bug. Consensus action: keep parked, but reword the README classification to 'pending base-spec integer-bounds erratum' and make that erratum the promotion gate. Also: README cites accept twin jobs/2.3--int-value-above-int64-max.invalid.test.yaml as 'in the main suite' — it's on PR #161's branch (and per that PR's review, it should move HERE).
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — same family, negative bound (-2^63-1). Matches sweep; accept twin 2.3--int-default-int64-min.yaml in PR #161. Same spec-erratum caveat as the family.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — same family, minValue axis; per-field decomposition justified (separate parse paths). Matches sweep; twin in PR #161. Inventory note: negative-side rejects (minValue/maxValue/allowedValues below -2^63) are uncovered — likely same code path, but note it.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — same family, maxValue axis. Service note: likeliest of the five to hit real templates (maxValue=2^63 as an infinity sentinel) — flag in any service change notice. Matches sweep; twin in PR #161.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — same family, allowedValues axis; an unrepresentable allowedValue is dead weight, low blast radius. Matches sweep; twin in PR #161.
|
Quorum review (5 independent agents). 8 fixtures + README. Net: 2 clean GOOD (type-lowercase, and the widening pair is GOOD-WITH-NITS), 6 GOOD-WITH-NITS. The central contested point: the five int64 fixtures document behavior BOTH implementations exhibit (matches the sweep) and are safe data-integrity adoptions — but the base 2023-09 spec text never bounds (int64 lives only in the EXPR doc), so the spec-literalist reviewer voted all five NOT-GOOD as 'spec question misclassified as implementation bug'. Majority keeps them parked here, with the classification updated and a base-spec integer-bounds erratum filed as the promotion gate. Also: 2--type-lowercase duplicates PR #166's 2--type-lowercase-string (same construct, same defect) — dedupe across the two PRs. README quality is the best of the five expected-failures branches (genuine dual-implementation attribution). |
Spec-correct fixtures that FAIL against current reference implementations: the int64 acceptance family (default/minValue/maxValue/allowedValues at 2^63, default at -2^63-1), the base type-name case-sensitivity gate, and 1.2.1 merge-widening. Parked in proposed/ (not discovered by the runner) so merging keeps the suite green; per-fixture output, spec citation, and classification in proposed/README.md. Promote each fixture unchanged when its fix lands. Companion to conformance-base-gaps. Review: quorum-review fixes — restructured to kind-level proposed/ directories (<component>/<kind>/proposed/) so promotion is a mechanical one-directory move; README split per kind and corrected: the int64 family is now classified as a spec question gated on a base-spec integer-bounds erratum (the base doc states no bound; a bignum implementation is arguably conformant), with genuine per-implementation attribution; adopted jobs/proposed/2.3--int-value-above-int64-max.invalid.test.yaml from the base-gaps PR (openjd-rs rejects the supplied 2^63 but openjd-model 0.11.x accepts it — verified both this session), replacing the stale claim that it 'lives in the main suite'; noted the widening fixtures' scheduler-merge blast radius and that the duplicate type-lowercase pin in the param-types PR was dropped in favor of this branch's copy. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
1c1d3d1 to
f5837f2
Compare
|
Quorum-review fixes applied and pushed (rebased onto mainline 3069673):
|
| @@ -0,0 +1,32 @@ | |||
| # Spec 1.2.1: each constraint must become MORE constrained in template processing order. | |||
| # The job template (processed last) widens the environment template's INT range, so the | |||
| # merge must be rejected. The default satisfies both ranges, isolating the widening rule. | |||
There was a problem hiding this comment.
This sounds wrong, the result of the merge should be the intersection and give minValue 10, maxValue 100, not a failure.
but each constraint must become more constrained as a subsequent definition is merged with the previous ones in processing order
This means that the resulting constraint we produce becomes more constrained. It does not mean that each subsequent constraint we take must be more constrained.
Important
Expected-failure fixtures — do not expect these to pass. Every fixture in this PR is believed spec-correct and FAILS against at least one current reference implementation. They are parked in
base/proposed/, which the conformance runner does not discover, so merging keeps the suite green. Per-fixture observed output, spec citation, and classification are inproposed/README.md. When a fix lands, promote the fixture up one directory unchanged.Contents (8 fixtures)
The int64 template-validation acceptance family (5) — both implementations accept all of these at
openjd checkdespite INT being a 64-bit signed integer:default: 9223372036854775808(2^63)minValue/maxValue/allowedValuesentry at 2^63default: -9223372036854775809(−2^63−1)Note the supplied-value branch is already correct in openjd-rs (and a green fixture for it ships in the companion PR); this family covers the template-validation axes, where both implementations still accept out-of-range integers.
Type-name case-sensitivity gate (1) —
type: stringwith noextensions:must be rejected (case-insensitivity is EXPR-gated per schema §2). openjd-rs accepts it (unconditionally case-insensitive parsing); the Python CLI correctly rejects. Implementations diverge; Rust-side bug.§1.2.1 merge widening (2) — a job template that widens an environment template's constraint is accepted by both implementations; Python's merge returns the intersection instead of rejecting. The spec states "each constraint must become more constrained." Either enforce the rule or respecify intersection semantics.
Companion to the green-fixture PR from branch
conformance-base-gaps.