test(artifact): run internal/artifact tests in parallel - #1328
Merged
Conversation
internal/artifact is the slowest package in three of the four CI test
lanes at roughly 580s, and until now every one of its 249 top-level tests
ran one after another. The work is dominated by waiting on disk: each
test builds its own on-disk SQLite/docbank fixture under t.TempDir, and
CI fsync latency is far worse than local. One test,
TestExportToStoreFullDrainsMoreThanOneClaimPage, takes 248s in CI versus
1.5s here.
Every test was audited for shared state before being marked parallel:
package-level variables mutated by tests, environment mutation, working
directory changes, shared fixture paths, global registries, ports, and
time-dependent assertions. The package turned out to be well isolated —
no t.Setenv or os.Setenv anywhere, and every fixture already scoped to
its own temp directory. Five tests stay serial and are the only ones that
genuinely cannot run alongside others:
- TestOpenRepositoryRetainsAbsoluteCanonicalRoot calls t.Chdir, which
panics in a parallel test.
- TestExportSpoolConstructionJoinsCleanupFailures swaps the
package-level exportSpoolChmod and exportSpoolCleanup hooks.
- TestPreflightImportCheckpointVersionBoundsAllocations and
TestDecodeImportCheckpointRejectsCurrentExtraFieldBeforeItsValue use
testing.AllocsPerRun, which pins GOMAXPROCS process-wide and measures
global allocation counters.
- TestWireCodecAllocatedBytesStayBoundedAsArtifactsGrow runs
testing.Benchmark and asserts on AllocedBytesPerOp, derived from
process-global memory statistics that concurrent tests would inflate.
Go runs all serial top-level tests to completion before releasing the
parallel ones, so these five still get an uncontended process.
The win depends on how much concurrency the runner allows. At -parallel 4,
which is what a four-core CI runner defaults to, the package goes from
23.4s to 19.3s locally. At -parallel 32 on a 32-core workstation it
regresses to 27.2s: the fixtures contend on filesystem journal and fsync
rather than CPU, and kernel time grows from 9s to 217s. Peak resident set
grows from 620MB serial to 812MB at -parallel 4 and 982MB at -parallel
32, which is worth watching if the parallel set grows further.
Validation: go test -race passed three consecutive runs; timings above
are best-of-three on an otherwise idle machine.
Generated with Claude Code
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
roborev: Combined Review (
|
Most artifact tests now run in parallel, so the few intentional exceptions are easy to mistake for omissions. Record the process-global state each relies on to prevent future cleanup from making them unsafe or flaky.
roborev: Combined Review (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Runs 197 isolated
internal/artifacttop-level tests in parallel to shorten anI/O-bound CI package. Five tests that change process-wide state or measure
process-wide allocations remain serial, and subtests are unchanged.
The change only affects test scheduling, not assertions or fixtures.
Parallelism uses more memory and can become slower at very high concurrency
because the fixtures compete for filesystem writes.