ci: stop scheduling the test-durations refresh - #9511
Conversation
Balancing pytest-split's shards by recorded runtime is not worth what the
recording costs. Measured 2026-09-08 against a full 89,281-test file:
* by recorded runtime, every shard lands at 18.1 min;
* by test count -- what pytest-split falls back to with no file -- the
shards are 18.7 / 15.1 / 19.9 / 18.7, slowest 19.9 min.
So ~1.8 min on the critical path, against a 40-minute shard cap that already
has 20 minutes of headroom. The price was an 11.7 MB JSON file in which every
value changes on each refresh, so each weekly run rewrote all 89k lines -- git
history that cannot be reclaimed -- plus a full unsharded suite run and a PR
that nobody could merge automatically anyway (the org disallows Actions
opening PRs, so the job pushed its branch and exited green with nothing to
show).
Trimming the file instead does not work, which is worth recording so it is not
retried: pytest-split charges any test missing from the file
`sum(kept)/len(kept)`, an average over what SURVIVED the trim, not over what
was removed. Keeping only tests >= 50 ms leaves 9.5% of entries and inflates
that average 23x (15.7 ms real -> 362.1 ms charged), so the 80,794 dropped
tests contribute 487 minutes of fake weight, the slow tests' real signal
drowns, and the split degrades to count-based regardless.
The trigger becomes workflow_dispatch only rather than being deleted, so the
measurement stays one click away if the suite grows enough to change the
arithmetic. The refresh job, its checkout config and its PR-opening step are
untouched, which also keeps the three tests that assert on this workflow
(test_ci_failure_annotations, test_coverage_omit_contract,
test_workflow_checkout_credentials) passing unchanged.
Slow tests remain visible without any of this: setup.cfg's addopts already
carry --durations=5, so every pytest run -- including each CI shard, with
coverage on and under real runner contention -- prints its slowest tests.
Design Review (Fable 5) — ✅ PASSDesign-level review of Verified: Design-Verdict: PASS Removes a measured-zero-value weekly cost while keeping the measurement one click away; reversible, evidence-backed, and correctly scoped away from [DESIGN-REVIEWED] 5be0a05 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of The diff matches the description exactly: two hunks in one file — the cron trigger removed (workflow_dispatch kept) and the header comment rewritten to record the measurement. Every in-repo claim checks out: First-Principles-Verdict: PASS Only the author's local shard timings (18.1 vs 19.9 min) are unverifiable here — but the removal stands without them: What this change shipsIntent: stop paying weekly CI cost for a test-balancing file that never landed and buys ~2 minutes. This is a FIX (a subtraction). Inventory (2 items)
[FIRST-PRINCIPLES-REVIEWED] 5be0a05 |
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: |
Problem / Motivation
Update Test Durationsruns the full unsharded suite every Monday to recordper-test runtime into
.test_durations, sopytest-splitcan balance ci.yml'sfour shards by recorded time instead of by test count.
Every value in that file changes on each refresh, so each run rewrote the whole
thing: the last one was +89,283 / -0 across a single 11.7 MB file. And it
could never land on its own — this org disallows GitHub Actions from opening
pull requests, so the job pushed its branch, downgraded the refusal to a
::notice::, and exited green with nothing to show. Both PRs it ever produced(#7727, #8506) were closed unmerged, so
.test_durationshas never existed onmainand the balancing this job exists to enable has never actually been ineffect.
Why it matters
The cost is permanent: a weekly full-file rewrite of ~89k lines is git history
that cannot be reclaimed later, on the order of 90 MB/yr, plus a full unsharded
suite run and a PR that needs a human every week.
What it buys was never measured. So I measured it.
What changed (motivation → approach → change)
Measured on 2026-09-08 against the full 89,281-test recording from the branch
that last job produced:
The file buys ~1.8 min on the critical path, against a shard cap of 40
minutes that already has 20 minutes of headroom. That does not justify the
recording cost, so the schedule goes.
Shrinking the file instead does not work, and this is recorded here so it is
not retried.
pytest_split/algorithms.pycharges any test missing from the filesum(durations.values()) / len(durations)— an average over what survived thetrim, not over what was removed:
Keeping only tests
>= 50 msleaves 9.5% of entries and inflates that average23x (15.7 ms real → 362.1 ms charged). The 80,794 dropped tests then
contribute 487 minutes of fake weight, pytest-split believes the suite takes
538.8 min rather than 72.4, the slow tests' real signal drowns, and the split
degrades to count-based anyway — while still carrying 8,487 entries.
The trigger becomes
workflow_dispatchonly rather than deleting the workflow,so the measurement stays one click away if the suite ever grows enough to change
this arithmetic. The
refreshjob, its checkout config and its PR-opening stepare all untouched.
ci.ymlis deliberately not modified:pytest-splitalready falls back toan even count split when the file is absent, which is the state
mainhas beenin all along, so
--splits/--groupkeep working with no change. Four openPRs currently touch
ci.yml; staying out of that file keeps this diffconflict-free.
Slow tests remain visible without any of this —
setup.cfg'saddoptsalreadycarry
--durations=5, so every pytest run, including each CI shard withcoverage on and under real runner contention, prints its slowest tests.
Tests
No new tests. Three existing suites assert on this workflow and all keep
passing unchanged, which is why the job was left intact rather than deleted:
test/test_ci_failure_annotations.py— assertstest-durations.yml:refreshremoves the python problem matchertest/test_coverage_omit_contract.py— scans every workflow, not just ci.ymltest/test_workflow_checkout_credentials.py— asserts this workflow'spersist-credentialspostureManual verification
yaml.safe_loadon the edited file reports triggers['workflow_dispatch']— the schedule is gone and nothing else in theon:block changed..github/workflows/test-durations.yml(checked with--limit 500; the four that overlap touch onlyci.yml).Related Issues
no linked issue: CI cost cleanup found while investigating why this job's PRs
were never landing.
Pattern harvest
Rule candidate: review-prompt
Pattern: a scheduled job that swallows a permission refusal into a
::notice::and exits 0 reports success forever while landing nothing — and the value of
what it produces goes unmeasured precisely because nobody sees it fail.
Checklist