chore(test): land .test_durations so pytest-split shards balance by runtime - #7727
chore(test): land .test_durations so pytest-split shards balance by runtime#7727iamwhatever wants to merge 1 commit into
Conversation
|
This file count is too crazy, cancel. |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Lands the exact artifact the documented sharding mechanism has been silently missing; scope, evidence, and the remaining delivery gap are all stated honestly. [DESIGN-REVIEWED] ab0eb7c |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe diff is exactly as the candidate file describes: a single new data file No findings. [OPUS-REVIEWED] ab0eb7c Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
|
Your diagnosis here was right and is still true on What the thread never got was the number behind "is the fallback actually costing anything", and "77,783 lines is too crazy" is hard to weigh without it. So I measured it. Your instinct to cancel looks correct. Controlled host, one machine, all four shards back to back, unpatched
Spread 1.21×; mean 334s; the critical-path shard sits 7.8% above the mean. Count-split is already close to balanced on this suite, so perfect duration balancing could recover about eight percent of one shard's wall time — for a file that rewrites on every test-timing change and conflicts with itself on any branch that adds tests. And CI's own numbers say the imbalance you would be fixing is mostly not imbalance. Four recently merged PRs,
Two things fall out. The slow shard is not stable — it is 3 on #8243 and 2 on #8317 — which is not what a systematically mis-split suite looks like. And the same four-way split ranges from 6–10 min on one run to 29–34 min on another, so runner variance is 3–5×, against a split imbalance of 1.21×. Durations recorded on one runner cannot predict another when the host is that much noisier than the thing being balanced; that is the deeper reason this mechanism underdelivers here, independent of file size. For what it is worth I arrived at this from the opposite direction and expected to argue the other way: a shard on one of my PRs (#8228) was cancelled after forty minutes of which twelve were silent, and I went looking for a mis-split as the cause. It was not that — the same shard runs in 5.5 minutes on a controlled host and the failure signature matches the memory-pressure class The one thing I would still change is a line, not a file: |
Problem
ci.ymlshards the backend suite with pytest-split and documents the intent atline 649:
That file has never existed on
main. So every backend shard since shardinglanded has been balanced by test count, not runtime — the fallback path, not the
designed one.
The
Update Test Durationsworkflow (.github/workflows/test-durations.yml) hasbeen running every Monday and doing its job correctly: it measures the full suite
and pushes this branch. It just cannot open the pull request, because this
repository has "Allow GitHub Actions to create and approve pull requests" turned
off (deliberately — the same switch also grants approving, and
main's merge gateis a required human review). The workflow already anticipates this and exits green
with a
::notice::carrying a compare link, which is why it went unnoticed: thescheduled run is green every week and the branch is pushed every week, but no PR
was ever opened and
gh pr list --head chore/update-test-durations --state allreturns empty.
What this changes
Adds
.test_durations— 77,781 recorded per-test durations, 60.5 minutes ofmeasured suite time, produced by the 2026-08-31 scheduled run. Nothing else.
pytest-split will now balance shards by recorded runtime, and brand-new tests are
auto-assigned the average duration so the split degrades gracefully between weekly
refreshes.
Honest scope note
This is a correctness fix for the sharding mechanism, not a large win on
today's numbers. Measured over the last five weeks, the four
Backend Tests (3.12)shards already land within 5% of each other (21.8 / 22.6 / 22.3 / 22.7 min at
week of 08-24), so count-based balancing happened to be close to time-based. The
value here is that the mechanism now works as designed and stays balanced as the
suite grows unevenly, rather than depending on that coincidence holding.
The actual driver of backend lane duration is separate and tracked elsewhere:
coverage instrumentation costs ~2x wall time on the 3.12 lane (measured 20.45s →
40.35s on a 1,517-test subset), which is why 3.12 takes 26.3 min and 3.10 takes
12.6 min on identical tests.
Tests
No code change; nothing to test. The file is consumed by pytest-split at shard
time, and the next CI run on this PR exercises it on all eight backend shards.
Manual verification
git ls-remote --heads origin refs/heads/chore/update-test-durations→ branchpresent at
ab0eb7c71, pushed 2026-08-31 07:52 UTC.gh pr list --head chore/update-test-durations --state all→ empty, confirmingno PR was ever opened for any weekly run.
git log --all -- .test_durations→ the single commit on this branch is the onlyone that has ever touched the file.
Follow-up
The workflow will keep pushing to this branch every Monday and still cannot open
the PR itself. Once this merges, the next refresh can push to the same branch
while a PR is open (the push path works; only create is blocked), but after this
PR merges and the branch is deleted the create path is reached again and blocked
again. Two options worth a maintainer decision: mint a GitHub App token for this
workflow, or accept a standing manual step. Same constraint affects
cleanup-screenshots,memory-benchmark, andadd-contributors.