ci: parallelise the test suite and stop paying for superseded runs - #676
Merged
Conversation
padak
added a commit
that referenced
this pull request
Aug 23, 2026
`cancel-in-progress: false` does not do what the previous comment claimed. It protects a RUNNING run. GitHub keeps at most one running plus one PENDING run per concurrency group and, per the workflow-syntax docs, "any existing `pending` job or workflow in the same concurrency group will be canceled and the new queued job or workflow will take its place". With every main push sharing one ref-keyed group, three merges landing inside a single run duration therefore cancel the middle commit's queued run -- losing exactly the post-merge signal the comment promised was never dropped. Not a theoretical window: of the last 120 main pushes, 6 sat in a 3-merges-within-5-minutes span, one burst landing five merges about 15 seconds apart. Keying pushes on `github.sha` gives every main commit its own group, so it can never be the pending run that gets displaced. PRs stay keyed on the ref, which is what makes a new push supersede the previous run. Raised by Devin review on #676.
A PR used to cost ~39 billed GitHub Actions minutes and ~9.5 minutes of wall clock. Measured over 13 days the repo produced 300 CI runs (219 on PRs, 81 on main), so roughly 27,000 billed minutes a month. Step-level timing showed the whole bill was one thing: pytest running single-threaded, three times per PR (516s + 402s + 477s). Setup was ~20s a job and irrelevant next to that. Changes, in order of what they save: * pytest-xdist + `-n auto` on all three test jobs. The ~6k tests are in-process CliRunner invocations with every HTTP call mocked and no shared mutable state, so they parallelise cleanly: 152s -> 37s on 4 workers locally, verified stable over repeated full runs and over the port-binding and cross-process-lock suites specifically. The Windows full-suite step was 477s of that job's 568s and bills at 2x, making it the single most expensive step in the pipeline. * `concurrency` with PR-only `cancel-in-progress`. 29 of the 219 PR runs (13%) were still executing when the next push to the same branch landed, and one iterating branch accumulated 23 runs. Main pushes are deliberately never cancelled -- that run verifies an already-shipped commit. * Coverage on main pushes only. It costs ~50% extra wall clock (152s -> 230s) and enforces no threshold, so it never could have blocked a PR. * 3.13 on main pushes only; PRs run 3.12, the requires-python floor. * Drop three Windows steps that were strict subsets of the full suite running right after them. Their rationale (#529, #528, #427) is folded into the full-suite comment. * Cache uv's resolved wheels on uv.lock. `make test` / `test-unit` / `test-cov` get `-n auto` too (152s -> 29s locally). `-v` goes with it: four workers interleaving 6k lines is unreadable, and a failure prints its own node id. Projected: 39 -> ~15 billed minutes per run, 9.5 -> ~4 minutes wall clock, ~16,600 billed minutes a month saved.
The first parallel CI run surfaced exactly one failure across 6,035 Windows tests: test_callback_arriving_just_before_timeout_succeeds fired its callback at 0.05s and gave wait() a 0.3s deadline, so it tolerated a 250ms scheduling stall. On an idle machine that is plenty; with four pytest workers on a Windows runner it is not, and the test then reported a callback-handling bug that does not exist. Not a parallelism-safety problem -- a wall-clock assumption that only ever held because the box happened to be idle. Both directions widened. The success case now gives wait() 5.0s, which costs nothing: it returns when the callback lands, not when the deadline expires. The timeout case pushes its too-late callback out to 5.0s and keeps the 0.1s deadline, so it still completes in ~0.1s -- the pending callback fires from a daemon timer against a closed server afterwards, which `_get` already swallows by design. No coverage is lost: that the deadline is honoured is asserted by test_timeout_raises_pkce_callback_timeout and by the timeout case itself. Both tests drop "just" from their names, which described the tight margin rather than the behaviour. Verified with 10 consecutive parallel runs of the file.
`cancel-in-progress: false` does not do what the previous comment claimed. It protects a RUNNING run. GitHub keeps at most one running plus one PENDING run per concurrency group and, per the workflow-syntax docs, "any existing `pending` job or workflow in the same concurrency group will be canceled and the new queued job or workflow will take its place". With every main push sharing one ref-keyed group, three merges landing inside a single run duration therefore cancel the middle commit's queued run -- losing exactly the post-merge signal the comment promised was never dropped. Not a theoretical window: of the last 120 main pushes, 6 sat in a 3-merges-within-5-minutes span, one burst landing five merges about 15 seconds apart. Keying pushes on `github.sha` gives every main commit its own group, so it can never be the pending run that gets displaced. PRs stay keyed on the ref, which is what makes a new push supersede the previous run. Raised by Devin review on #676.
padak
force-pushed
the
claude/ci-tests-optimization-costs-8272b3
branch
from
August 23, 2026 23:12
c5fc551 to
7aa89cc
Compare
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
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.
What this is
CI cost and latency work. No product code changes —
src/is untouched.The measurement
A PR run today (measured on run
32669264298, typical):checktest (3.12)test (3.13)build-windowsVolume over the 13 days to 2026-08-23: 300 runs (219 PR + 81 main push) → ~692/month → ~27,000 billed minutes/month.
Step-level timing says the entire bill is one thing — pytest running single-threaded, three times per PR:
Setup is noise next to 1,395s of serial pytest.
Changes
1.
pytest-xdist+-n autoon all three test jobsThe ~6k tests are in-process
CliRunnerinvocations with every HTTP call mocked and no shared mutable state, so they parallelise cleanly. Measured locally:-n 4(matches a 4-vCPU GitHub runner)-n 4+ coverage-n auto(11 cores)Verified stable: three consecutive full runs (
6042 passedevery time) plus three repeated runs of the suites most likely to break under parallelism —test_auth_pkce.py(binds loopback ports),test_token_provider.pyandtest_job_idempotency_store.py(cross-process locks). No port or lock collisions.Coverage still combines correctly across workers — totals are identical to a sequential run (
31296 / 5405 / 9186 / 1121, 80 %).The Windows full-suite step was 477s of that job's 568s and bills at 2×, so it was the most expensive single step in the pipeline. If it ever turns flaky on Windows specifically, the comment in place says to swap
-n autofor-p no:xdistrather than disable the suite — Windows-only file-locking and atomic-replace semantics are exactly what it is there to catch.2.
concurrencywith PR-onlycancel-in-progressci.ymlandfrontend.ymlhad no concurrency group, so pushing again to an open PR left the previous run to finish and bill in full. Measured: 29 of 219 PR runs (13 %) were still executing when the next push landed; branchms/dmd-1833alone accumulated 23 runs (~875 billed minutes).Main pushes are deliberately never cancelled — that run is the post-merge verification of an already-shipped commit, and cancelling it because the next merge arrived would drop the signal for the commit in between.
3. Coverage on main pushes only
It costs ~50 % extra wall clock and enforces no
--cov-fail-under, so it could never have blocked a PR. Locally it stays onemake test-covaway.4. 3.13 on main pushes only
PRs run 3.12 (the
requires-pythonfloor and what the wheel is built on); main runs the full matrix. A 3.13-only regression is caught on main instead of on the PR. Accepted trade — reversible with a one-word change to the matrix expression, which the comment says.5. Three redundant Windows steps removed
-k export(#529),test_update_runner.py(#528) andtest_job_idempotency_store.py(#427) each ran as a named step immediately before the full suite that contains them — duplicated Windows minutes at 2×. Their rationale is folded into the full-suite comment; a failure now names the test node id instead of the step, which is the same information.6. uv dependency cache
enable-cachekeyed onuv.lock, on all four jobs.7.
make test/test-unit/test-covget-n auto152s → 29s locally.
make checkend-to-end went from ~3.5 min to 53 s.-vis dropped with it — four workers interleaving 6k lines is unreadable, and a failing test prints its own node id.make test-filestays sequential and verbose for single-file debugging.Projected result
Considered and rejected: path filtering for docs-only PRs
A first pass suggested ~19 % of PRs were "docs/plugin-only" and could skip the Python jobs. Checking which repo paths the suite actually reads from the live tree killed it:
plugins/**— read bytest_agent_prompt.py(prompt byte budget),test_skill_frontmatter.py,test_mcp_migration_recipe.py,test_license_consistency.py;test_migrate_cicd.pyandtest_generate_promotion_pipeline.pyimport scripts from under itREADME.md,LICENSE,build/package/chocolatey/*.nuspec—test_license_consistency.pyCLAUDE.md—test_check_command_sync.py::test_live_command_tree_has_no_driftruns the gate against the live treescripts/**— manyOnce those are excluded, the genuinely safe skip set is
docs/**and.claude/**, and zero of the last 58 commits on main qualify. Real drift risk, no measured benefit — left out.Verification
integration-marked tests):6042 passedand6045 passedmake checkgreen end-to-endServe endpoint reference freshness checkstep from docs(serve): generate the endpoint reference from the app, gate it in CI (#656) #671 is intact in thecheckjobNot verified here
-n autoon the Windows runner. I have no Windows machine, so the 477s → ~150s projection for that step is inference from the Linux measurement. First run on this PR will show it — if it is flaky rather than fast, the fix is the one-line fallback documented in the comment.