acceptance: shard acceptance tests across 2-8 parallel CI jobs#5435
Draft
denik wants to merge 14 commits into
Draft
acceptance: shard acceptance tests across 2-8 parallel CI jobs#5435denik wants to merge 14 commits into
denik wants to merge 14 commits into
Conversation
Adds SHARD_INDEX / SHARD_TOTAL env-var support to getTests() so each CI job runs only its share of the 854 acceptance tests (~214 each). The sorted test list ensures the split is deterministic and stable. The CI matrix gains a shard_index dimension [0,1,2,3], turning 6 test jobs into 24 (and 2 → 8 in the merge queue). The test-result aggregator and testmask gating are unaffected — GitHub Actions waits for all matrix combinations automatically. Co-authored-by: Isaac
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
The sharded acceptance matrix re-ran the full unit suite on all 24 jobs (4 shards x 3 OS x 2 deployment), and all of them tried to save the Go cache under the same key on main — only the first writer wins, so the rest wasted time. Split into: - test-unit: one job per OS (no deployment/shard dimension), runs `task test-unit`, and is the sole writer of the shared "test" cache. - test (acc): runs `task test-acc` only, restores the "test" cache (save-cache=false so the many shard/deployment instances don't collide on the key). setup-build-environment gains a save-cache input (default true, so the test-exp-* / test-pipelines jobs with unique keys keep saving as before) that gates the on-main cache save. test-result now also waits on test-unit. Co-authored-by: Isaac
TestInprocessMode calls testAccept with a specific singleTest
("selftest/basic"). The shard filter lived in getTests(), so it ran
before singleTest selection and could strip the requested test out of
the shard, failing with "did not match any tests" on every shard that
didn't own selftest/basic.
Move the shard filter into a shardTests helper applied in testAccept
only when singleTest == "", leaving named-test selection unsharded.
Co-authored-by: Isaac
Collaborator
|
Commit: 6a010b7
22 interesting tests: 15 SKIP, 7 KNOWN
Top 29 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
A static cross-product matrix forces one shard count for every (os, engine). Windows is the long pole (TASK_CONCURRENCY=1 serializes within a job) while the direct engine is fast, so a uniform count over- or under-shards most combinations. Generate the acceptance shard matrix in testmask as an explicit include-list and consume it via fromJSON. Shard counts: windows/terraform: 8 windows/direct: 8 linux/terraform: 4 linux/direct: 2 macos/terraform: 4 macos/direct: 2 merge_group still runs Linux only (6 jobs). PR/push runs 28 acc jobs. Co-authored-by: Isaac
The gotestsum JSON upload is debug-only timing telemetry, yet a transient GitHub artifact-service error during finalization failed an otherwise- passing windows test-acc shard. Mark both upload steps continue-on-error so infra hiccups on a debug artifact never block the merge. Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Data collected across 5 CI runs at ×1/×2/×4/×8: linux/terraform ×4 (~6m vs 13m unsharded) linux/direct ×2 (~5m vs 6m unsharded) ×4 saves only 30s more macos/terraform ×4 (~9m vs 17m unsharded) macos/direct ×2 (~7.5m vs 9m unsharded) ×4 saves only 8s more windows/terraform ×8 (~15m vs 35m unsharded) keeps improving to ×8 windows/direct ×4 (~14m vs 23m unsharded) ×8 saves only 12s (overhead-bound) Total acc jobs: 22 (PR/push), 6 (merge queue, linux only). Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
denik
changed the title
Contributor
Author
|
Claude analysis: Additional analysis of 5 commits added in a row: |
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.
Summary
SHARD_INDEX/SHARD_TOTALenv-var support togetTests()inacceptance/acceptance_test.go: each CI job runs only its slice of the 854 acceptance tests (~214 each). The list is already sorted alphabetically, so the modulo split is deterministic and stable across runs.shard_index: [0, 1, 2, 3]to the CI matrix inpush.yml, passingSHARD_INDEXandSHARD_TOTAL: 4to the test step.test-resultaggregator andtestmaskgating require no changes — GitHub Actions waits for all matrix combinations of a job automatically.Local runs are unaffected: without
SHARD_TOTALset (or withSHARD_TOTAL=1),getTests()returns the full list as before.Test plan
shard 0..shard 3) and each runs ~214 teststest-resultwaits for all 24 (or 8 in merge queue) jobs before passingThis pull request was AI-assisted by Isaac.