feat(conformance): ask AC #36 whether a row seeded, not what Status it reported
Summary
AC #36 (closed) reports that a run established nothing when its fixture is demonstrably
on the upstream, and exits 2, which S04 §Exit codes reserves for a tool error,
against a registry the OCI specification calls conformant.
Related to #47 (closed). Found by the S08 remote goal run's reference validation and recorded as the plan's §Spec-amendment candidates Filed 5.
Targets main directly. Deliberately kept out of that run's row MRs
(!271 (merged), !272 (merged), !273 (merged)): it moves an acceptance criterion's predicate and adds a
constructor to S04's result-type vocabulary, which is a spec decision rather
than a row one, and mixing it in would blur what a reviewer is being asked to
approve.
What is wrong
AC #36 (closed) named its condition correctly all along, no seeding row completed, but
operationalised it as "every selected NeedsUpstream descriptor reported
StatusSkip".
A row can seed and then skip. S07 §Pluggable behavior gives ten hosted
catalog rows an arm that does exactly that, across six rows of its table, and
the S08 remote band inherits two (oci.remote.blob-range,
oci.remote.referrers-proxy, both S08 §Per-format rows).
Measured against a live Artifact Registry with the Range header stripped in
flight, so the relay behaved as a registry that does not implement it:
| Selection | Exit |
|---|---|
--filter=oci.remote.blob-range |
2, "no seeding row completed" |
--filter=oci.remote.*blob* |
0 |
The seeding completed in both. The proxy log for the first shows the seeding
PUT answering 201 and the read-back 200. The diagnosis sends the operator
to --run-id reuse and the credential's write scope on --upstream-url, and
neither is implicated.
One passing seeding row satisfied the old check, so a full run hid this and a narrowed run showed it. A narrowed run is the ordinary shape of a CI gate.
oci.remote.referrers-proxy reaches the same arm against GitLab Container
Registry, which implements v1.0 only
(container-registry#1205),
so a named real target takes it.
Why the two obvious fixes do not work
Keying on TestCase.SetupFailure looks right, since only NewSeedFailureCase
sets it. It sets it only when SeedError.RunAttributable is true. The
row-attributable residue, a refusal that skips this row and lets later rows
run, carries a nil SetupFailure exactly like a pluggable skip does.
SetupFailure's operative meaning is not "could not seed" but "end the run":
runDescriptors short-circuits on it, and that is AC #33.
Keying on the absence of a recorded seeding failure fails the other way,
and worse. A row that skips before it writes anything records no failure, so a
run of nothing but those rows exits 0 having relayed nothing, which is the
state this acceptance criterion exists to make loud. requireCrane's driver
gate in pkg/conformance/oci is exactly that shape, and S07 states its
pluggable arm as probe-then-skip without ordering the probe after the seed.
A test disproves each wrong fix rather than an argument, which is why the two
controls in run_pluggable_skip_test.go are not optional.
What this changes
TestCase carries an unexported affirmative marker, and the new
SkipAfterSeeding constructor is the only thing that sets it. The runner asks
whether any selected seeding row got the fixture onto the upstream.
SetupFailure's semantics are untouched.
The marker is affirmative because the two mistakes cost differently. A row that
seeded and failed to say so exits 2: loud, on a named row, fixed once. A row
that never seeded and was counted anyway exits 0 on no evidence, and nothing
else in the report contradicts it, because a skip moves no exit code. So the
unset marker means "established nothing", and a module that forgets the
constructor fails toward the verdict rather than away from it. S08 §Preflight
records the choice.
The first branch's diagnosis widens with it. It named the refusing status as
though every row reaching the branch had one; it now sends the operator to the
row reasons, where each row's own cause lives. The verdict also logs its two
inputs, seeding_rows_selected and seeding_rows_established, because the
marker reaches no renderer and an exit-0 remote run otherwise records nothing
about what it established.
Note for library consumers
seeded is the first unexported field on TestCase, so a keyed composite
literal becomes the one form that compiles outside pkg/conformance, and
cmp.Diff over a TestCase or a Report panics unless given
cmpopts.IgnoreUnexported(conformance.TestCase{}) or an Exporter. It does not
degrade to a wrong answer: the panic is cannot handle unexported field at {conformance.TestCase}.seeded, so a consumer's passing test becomes a
panicking one. reflect.DeepEqual does not panic but changes answer:
DeepEqual(Skip(n, r), SkipAfterSeeding(n, r)) is now false.
Nothing in this repo is affected: the full suite passes, and a cmp.Diff over
either type would panic rather than fail, so no call site compares one. Every
TestCase literal in the tree is keyed.
Nothing in the Artifact Registry is affected either, for a structural reason
rather than a lucky one: it consumes this project as a container image, not as
a Go module. Its go.mod carries no registry-conformance requirement, and
scripts/conformance/maven_setup.sh runs inside the tool image. The
constructors S04 already directs authors to are unaffected.
The spec changes with the code
The predicate is normative, so its sites are amended here rather than in a follow-up:
- S08 §Preflight: the mechanism paragraph, the pinned branch diagnosis, and a new paragraph pinning the constructor rule and why the marker is affirmative.
- S08 §Acceptance Criteria, AC #36 (closed): what "completed" means, and the upstream qualifier's justification, which reads against the seeding test now.
- S08 §Error Cases: the AC #36 (closed) row.
- S08 §S04 amendments: the !231 (merged) row's status-test clause marked
superseded by !275, and two new rows for the S04 edits below. - S04 §Entry points and §Execution order inside
RunModulestep 9: the sentinel's doc block and the step-9 predicate. - S04 §
TestDescriptor,TestCase,Status,Detail:SkipAfterSeedingin the constructor list.
One fixture changed, and it is not a weakened test
run_remote_test.go's skipResult now builds through NewSeedFailureCase.
Its own doc already called it "a row-attributable seeding refusal", and it was
standing that up with a plain Skip because the old predicate could not tell
the two apart. Under the new one a plain Skip means a row that never wrote,
so the fixture had to become what it already claimed to be. No assertion was
dropped; the test that consumes it is renamed to what it asserts
(TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded).
Spec coverage
Spec: docs/specs/S08-remote-contracts.md
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC #36 (closed) | A selected seeding row that seeded and then skipped on a pluggable-behavior arm completed, so the run exits 0 |
TestRunModule_PluggableSkipIsNotNothingEstablished, TestRunModule_PluggableSkipVerdictDoesNotDependOnWhatElseWasSelected |
| AC #36 (closed) | A selected seeding row that could not seed did not complete, so the run exits 2 |
TestRunModule_RowAttributableSeedFailureIsStillNothingEstablished, TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded |
| AC #36 (closed) | A selected seeding row that skipped before it wrote did not complete, so the run exits 2 |
TestRunModule_SkipBeforeSeedingIsStillNothingEstablished |
| AC #36 (closed) | The verdict does not depend on what else the selection held | TestRunModule_PluggableSkipVerdictDoesNotDependOnWhatElseWasSelected |
| AC #33 | SetupFailure's short-circuit still preempts the AC #36 (closed) verdict |
TestRunModule_SetupFailureShortCircuitPreemptsNothingEstablished (unchanged) |
Error cases
| # | Condition | Tests |
|---|---|---|
| Runner | No selected NeedsUpstream descriptor got the fixture onto the upstream: exit 2, ErrNothingEstablished wrapped in the row-reasons diagnosis |
TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded (asserts the widened message verbatim) |
| Runner | m.TestCatalog(cfg) holds no seeding row: unchanged branch and message |
TestRunModule_NothingEstablishedWhenCatalogHoldsNoSeedingRow (unchanged) |
S04 vocabulary
| # | Addition | Tests |
|---|---|---|
§TestDescriptor, TestCase, Status, Detail |
SkipAfterSeeding returns Skip's shape in every exported field, plus the marker |
TestSkipAfterSeeding_ShapeMatchesSkipInEveryExportedField |
Security considerations
| # | Concern | Tests |
|---|---|---|
| S08 §Security Considerations | None reached. The marker is a bool, carries no upstream bytes, and reaches no output channel; NewSeedFailureCase's scrub and detailBodyCap bound are untouched |
n/a |
Test plan
-
go test -race ./...green. -
pre-commit run --all-filesclean. - Reverting the predicate to the status test kills both pluggable-skip tests and nothing else.
- Counting every case as seeded kills both controls and the branch-1 table, and nothing else.
- The row-attributable control passes in both directions, which is what keeps the fix from being "never fire AC #36 (closed) on a skip".
Three mutations, because each isolates one direction of the verdict and the middle one is the property whose absence let the first attempt at this fix through.