test(storage): promote driver testsuites to narrowed interface (S06 Step 15)
What
S06 Step 15 — promote the generic storage-driver conformance suite out of the Go-toolchain-ignored internal/storage/_imported/ quarantine into its final home internal/storage/driver/testsuites/, adapted to AR's narrowed StorageDriver interface.
This is a port
The bulk of this MR is a faithful lift of the Container Registry storage-driver conformance suite (registry/storage/driver/testsuites) and its test helpers (testutil/rand.go, testutil/blobber.go, the random path/filename generators). The deliberate goal is to keep the suite structurally identical to CR so the changes stay easy to diff and review. Every deviation from CR is limited to what AR's narrowed interface and Go toolchain actually require — nothing was reworded, reordered, or "cleaned up" gratuitously.
How to review
Because this is a move-with-adaptations, a plain diff looks enormous (~1.7k added / ~2.7k removed) but most of it is the same code relocated from _imported/ to its final path. To see only the real changes, review with rename detection + word-diff so the move collapses and only the adaptations remain:
git show -M10% --word-diff(-M10% lets git treat the _imported deletion + new file as a rename even though the bodies were adapted; --word-diff then surfaces the intra-line edits.)
What we changed/added on top of the verbatim port
The port itself, adapted to compile against AR's interface:
- Re-pointed from CR's
storagedriver.StorageDrivertointernal/storage.StorageDriver. - Removed all
docker/distributionandlibtrustimports (neither is ingo.mod). - Dropped the tests for the narrowed-away surface (
Stat/Walk/List/recursiveDelete). - Replaced the
List-basedTearDownTestwith explicit per-test path tracking + non-recursiveDelete(the narrowed interface cannot enumerate the backend). - Replaced the s3/azure driver-specific chunk-size constants with a suite-level
MinChunkSizefield, so the package has no driver-package imports and compiles in isolation.
New test-helper files (CR ports):
internal/testutil/blobber.go— pseudo-random blob generator (CRtestutil/rand.go+testutil/blobber.go), reimplemented on stdlib (math/rand/v2).internal/testutil/storagepath.go— random path/filename generators.
Interface & error-surface additions (small, only to support the promoted suite + align with the spec):
internal/storage/driver.go:PutContentgains ametadata map[string]stringparameter — S06StoreBlobrequires metadata to be written with the object, but the interface lacked it. The S06 spec is updated to match.internal/storage/driver.go: documented that a negative offset onReader/WriterreturnsErrInvalidOffset; documented theFileWriterterminal-state contract.internal/storage/errors.go: addedErrAlreadyClosed/ErrAlreadyCommited/ErrAlreadyCanceledsentinels (needed by the writer-lifecycle tests) and theInvalidPathErrortyped error.internal/storage/driver/s3/limits.go:MinChunkSizeconst (the S3 5 MiB multipart minimum).
Tests restored/added beyond the Stat-dropped baseline:
TestExists(AC #7 (closed)) andTestWriterCancel(AC #15) — generically-testable criteria the original promotion had dropped along withStat.- Writer-lifecycle robustness (
TestWriterDoubleClose,TestWriterDoubleCommit,TestWriterCommitAfterCancel/Close,TestWriterCancelAfter*,TestWriterWriteAfter*) — restored from CR, enabled by the newErrAlready*sentinels. TestWriterOffsetDivergence(AC #6 (closed)) — now exercised generically.TestPutContentWithMetadata— thePutContentmetadata variant; metadata is verified viaURLFor(as isTestMoveWithMetadata).
Config & docs:
.golangci.yaml: lint/format exclusions are now scoped to_importedonly — the promoted files lint clean with no carve-out.docs/specs/S06-storage-layer.md,docs/plans/2026-05-15-storage-layer.md:PutContentmetadata signature; removed the offset-divergence deferral notes now that AC #6 (closed) is covered.
Could this be split?
The interface/error additions (driver.go, errors.go, s3/limits.go) and the spec/plan doc edits could in principle land as a small prep MR ahead of the test promotion. We chose not to: those additions total roughly ~70 lines and exist solely to satisfy the promoted suite, so peeling them out would not meaningfully shrink this MR — the weight is the ~1.7k-line suite relocation — while adding a second review + pipeline cycle. Hence the all-in-one. If a reviewer would still prefer the split, say so and it's an easy carve-out.
Spec coverage
| # | S06 StorageDriver criterion |
Retained test(s) | Notes |
|---|---|---|---|
| 1 | PutContent→GetContent round-trip | TestPutContentMultipleTimes, TestPutContentZeroSizeFile, TestTruncate, TestPutContentWithMetadata |
metadata variant verified via URLFor |
| 2 | Writer(0)→Commit→Reader(0) | TestWriteRead1-4, TestWriteReadStreams1-4, …NonUTF8, …SmallStream, …LargeStreams |
|
| 3 | Writer(offset>0) append | TestContinueStreamAppend{Large,Small}, TestContinueAppendZeroSizeBlob, TestOverwriteAppendBlob, TestOverwriteBlockBlob |
|
| 4 | Writer(0) errors if staging exists | TestNewWriterExistingStaging |
|
| 5 | Writer(offset>0) → *PathNotFoundError |
TestAppendInexistentBlob, TestReadNonexistent{,Stream} |
|
| 6 | Writer(offset>0) → *OffsetDivergenceError |
TestWriterOffsetDivergence |
|
| 7 | Exists true / false |
TestExists |
(false,err) transient channel needs fault injection → driver-specific (16-17) |
| 8 | Move places at dest, removes source | TestMovePutContentBlob, TestMoveWritterBlob, TestMoveOverwrite, TestMoveNonexistent, TestMoveInvalid |
|
| 9 | Move >5GB multipart (S3) | deferred | S3-specific; Step 16 |
| 10 | Move applies metadata atomically | TestMoveWithMetadata |
metadata verified via URLFor |
| 11 | Move error preserves source on metadata failure | deferred | needs fault injection; driver-specific (16-17) |
| 12 | URLFor valid pre-signed URL / ErrUnsupportedMethod |
TestURLFor |
skips on ErrUnsupportedMethod |
| 13 | Delete removes object | TestDelete, TestDeleteFileEqualFolderFileName, TestDeleteNonexistent |
|
| 14 | Reader(offset>0) range read | TestReaderWithOffset, TestConcurrentStreamReads |
|
| 15 | Cancel releases staging; later Writer(0) succeeds, Reader(0)→not-found |
TestWriterCancel |
|
| — | Lifecycle & large-object robustness | TestWriter{DoubleClose,DoubleCommit,CommitAfter*,CancelAfter*,WriteAfter*}, TestMaxUploadSize, TestZeroByteBlobUploadSession, TestConcurrentFileStreams |
Criteria 9/11 are inherently backend-specific (multipart >5 GB, metadata-failure fault injection) and land with the per-driver cases in Steps 16-17 (Adjust S3 / Adjust GCS), per the plan & spec §Testing. The suite stays generic (parameterized by a StorageDriver factory) and has no consumers yet — Steps 16/17 wire it to the concrete driver factories.
Acceptance
internal/storage/driver/testsuites/testsuites.gocompiles in isolation — no driver-package,docker/distribution, orlibtrustimports.- The promoted suite and both helper files lint clean with no
.golangci.yamlexclusion (_importedis the only exclusion left). internal/storage/_imported/driver/testsuites/no longer exists.- Full pre-commit hook chain passes (
go-test,golangci-lint,go-fmt,go-imports).
Plan: S06 Storage Layer — Step 15 · Spec: S06
Related to #160 (closed)