test(e2e): four of the five backlog rows, and why the fifth is not written

What

Four of the five rows lint:e2e-catalog-sync reports as backlog. The fifth is escalated rather than written; Not in this MR below says why.

Row Where
e2e.npm.publish.stalled-publish-cut e2e/instance/upload/npm_stalled_test.go
e2e.npm.publish.provenance-absent-without-flag e2e/bootstrap/npm/provenance_test.go
e2e.oci.discover.manifest-detail-via-api e2e/bootstrap/oci/manifest_detail_test.go
e2e.docker.discover.manifest-detail-via-api e2e/bootstrap/docker/manifest_detail_test.go

e2e-catalog check moves from 181 covered / 5 backlog to 185 covered / 1 backlog, measured at 10d34ed22 and at this branch's tip.

Reviewing it: three groups that share no test file

Past the 500 reviewable lines docs/dev/development-model.md gates at, so here is the split. The three groups are independent — take them one at a time.

Group Files Lines
0. Production internal/format/npm/publish_stream.go 7
1. npm stalled publish e2e/instance/{drain.go, stalled_payload_internal_test.go, upload_variant.go, upload_variant_internal_test.go, upload/npm_stalled_test.go}, e2e/internal/client/npm/npm.go ~330
2. npm provenance e2e/bootstrap/npm/provenance_test.go, e2e/internal/client/npm/npm.go ~140
3. container manifest detail e2e/bootstrap/{oci,docker}/manifest_detail_test.go, e2e/internal/registry/{registry.go, registry_test.go} ~1,180
shared patterns.go, limitations.md, four catalog Status cells, the caveat pin ~105

Group 3 is most of the diff, and two thirds of it is one 459-line test file written twice. internal/format/oci serves both catalogs, and a profile package may not hold a scenario helper, so the docker file is a near-copy rather than a shared body — the same duplication the catalogs themselves take deliberately. Reviewing the oci file reviews both; the docker one differs in its header, its scenario constant, its two function names, and one registry.Format.

The production change

One line of behaviour-preserving code: the npm publish stream-failure log message becomes MessagePublishStreamFailed instead of a literal. internal/format/maven already exports MessageArtifactStreamFailed for exactly this, so a rename is a compile error rather than an assertion that silently matches nothing. The e2e test imports it directly, which is what the Maven row does; e2e/internal/logs re-exports only the internal/storage messages, so nothing was added there.

What green does not attest

The manifest-detail cell says a remote repository's tags stops at the first 1,000 cached names. A cached tag row is written one per by-tag fill — upsertContainerRemoteTag is reached from a tag read and from nowhere else — so observing that window costs 1,001 reads through the ingress inside one test's five-minute budget. The rest of the remote arm is asserted; the window is recorded under Green does not attest: in limitations.md, under ## 1. Harness and CI, because the cap is correct behaviour and only its cost is out of reach.

e2e/internal/catalog/registry_test.go pins the caveat population, so the two new entries are added there deliberately. That guard caught them, as designed.

Not in this MR: e2e.npm.publish.reject-wrong-second-attachment

Its cell specifies that a second _attachments entry named anything other than {name}-{version}.sigstore is rejected. The service has no such name check. readAttachments (internal/format/npm/streampub/attachment.go) reads one key, bounds its length, streams its value, and then aborts publish_envelope_invalid on any second entry whatever it is called.

So a test written to that cell would pass against an implementation with no .sigstore handling at all, which is today's — a green result attesting a behaviour that does not exist. #1273 (priority::1, AR-Blocks::GA) is the open bug for that gap, and #1156 (closed) records the decision it implements. The row is left not started and in the backlog rather than narrowed, reclassified, or registered as unassertable — what to do with it is a decision for the catalog's owner.

Two discrepancies the catalog owner may want

  • annotations. ContainerManifestDetail in api/openapi/v1.yaml requires fourteen fields; the two cells enumerate twelve, omitting annotations and tags_preview on the detail. The tests assert what the cells name. The cells are not edited here.
  • The plan is out of tree by standing instruction, so this MR carries no (plan: N/M) marker — it would name nothing. Same as !2484–!2492.

Testing

Rig-verified. Both profiles played on this MR's own pipeline at the final commit, 2842610329:

Job Result
test:e2e:caproni:bootstrap 16460283151 DONE 368 tests in 87.923s, no failure count
test:e2e:caproni:instance 16460283153 DONE 114 tests in 727.468s, no failure count

All four rows pass, with durations that say they ran rather than skipped:

PASS e2e.oci.discover.manifest-detail-via-api                (10.61s)
PASS e2e.docker.discover.manifest-detail-via-api             (6.53s)
PASS e2e.npm.publish.provenance-absent-without-flag          (6.57s)
PASS e2e.npm.publish.stalled-publish-cut                     (46.50s)
PASS   └─ the_cut_is_told_from_a_client_that_vanished        (0.96s)

The marker subtest is the one worth naming: it is the clause asserting a cut answers 499 and that server.ErrInactivityTimeout's text rides the stream-failure line under the request's own correlation id.

A rig run is also what caught the fixture this MR started with. crane append's empty base names no architecture, so the platform triple was null where the cell says one value nulls alone, and both manifest-detail rows failed on it in job 16460052940. ociupstream.BuildImage renders a config naming amd64 and linux with no variant, which is the shape the clause needs.

Locally, at the final commit:

  • go build -tags=e2e ./e2e/..., go vet -tags=e2e ./e2e/... — clean. go build alone does not compile _test.go files, so vet is what type-checks them.
  • go test ./e2e/... — every package ok.
  • go test ./internal/format/npm/... — every package ok, for the production change.
  • golangci-lint run --build-tags=e2e --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./e2e/... — 0 issues., after fixing six it found.
  • The same on ./internal/format/npm/... untagged — 0 issues.
  • scripts/ci/check-comment-caps.sh --base origin/sahmed/e2e-caproni-authz — OK, after compressing three blocks it rejected.

Assertions watched fail against deliberately broken code, one per new seam: the stalled-payload lead (fill reverted to filler), the manifest-detail null distinction (HasTags weakened to a length check), and the one //nolint token these files carry (containedctx, removed and seen to fire).

lint:e2e-catalog-sync stays red at allow_failure: true, reporting disagreements 1 — the escalated row, down from 5. That is the honest state and is not suppressed.

Edited by Suleimi Ahmed

Merge request reports

Loading
Loading