fix(ci): give the coverage matrix arm its own job timeout - #7517
fix(ci): give the coverage matrix arm its own job timeout#7517SebastianYuSun wants to merge 1 commit into
Conversation
backend-test runs four shards twice, on 3.10 with --no-cov and on 3.12 with --cov, and timeout-minutes is a job level key shared by all eight matrix cells. Coverage roughly doubles wall time, so one cap sized for the cheap arm cancels the expensive one at the wall while its sibling finishes under half the budget. Measured from completed_at minus started_at on three heads: head 3.10 (--no-cov) 3.12 (+cov) d94950d 7.68 to 13.87 min 27.33 to 30.25 CANCELLED 760124a 10.00 to 13.60 min 22.22 to 30.27 CANCELLED 92c3b5f 12.02 to 13.83 min 18.32 to 27.13 min It is not confined to pull requests. Of seven consecutive pushes to main sampled on 2026-09-01, three had a 3.12 shard cancelled at the wall and the rest finished at 28.3 to 28.9 minutes, which is 94 to 96 percent of the cap. A cancelled shard makes Coverage Gate fail closed, so the red lands on diffs that cannot account for a 28 minute shard. The same file already accepts the principle. backend-test-windows carries 40 minutes because those runners run the same shards measurably slower, and its slowest observed shard used 18.03 of the 40. The coverage arm is closer to its budget than Windows is to its, and still shares 30. 45 comes from the measurement rather than from taste: the slowest uncancelled coverage shard was 28.93 minutes, which is 64 percent of 45. The cancelled shards are censored at the wall, so their real length is unknown, and that is why the split itself is the other half of this problem. Without a committed .test_durations file pytest-split falls back to an even split by test count, so which shard carries the heavy tests is arbitrary. Landing that file is a maintainer action and is not part of this change. Adds test/test_backend_test_timeout_contract.py, which resolves the job cap per matrix arm and asserts the two arms are not capped identically. Two of its three assertions fail on the unfixed workflow.
|
@bolichen97 @iamwhatever this one touches Context for why it is worth the round trip: The split half of the problem is not in this PR. |
Closing — no longer applicable: #7780 merged and #7552 already raised the capVerified relationship: #7780 merged as 5119816 and its content is on origin/main 1a765b8: pyproject.toml requires-python = ">=3.12", dep_sync.python_floor_breach wired at dep_sync.py:955 (the reinstall branch), and backend-test's matrix collapsed to python-version: ["3.12"]. #7517's entire production change is one line that no longer applies: I isolated its ci.yml hunk and ran Carry this over firstThis closure is about redundancy, and these items are the exception: they are not on From test/test_backend_test_timeout_contract.py (#7517) carry over exactly one assertion, rewritten for a single-arm matrix: the constant WINDOWS_PRECEDENT_MINUTES = 40 plus the body of test_the_coverage_arm_gets_at_least_the_windows_arms_budget, re-expressed as a scalar floor -- yaml-load .github/workflows/ci.yml and assert jobs['backend-test']['timeout-minutes'] >= 40, the budget backend-test-windows already carries. Nothing in test/ pins that value today and it regressed once already (closed issue #7527), so this is the one real gap #7517 leaves behind. Drop _TERNARY, _coverage_arm(), _cap_for() and test_the_coverage_arm_is_not_capped_like_the_cheap_arm entirely: they require the two-arm python-version axis and the --cov/--no-cov ternary that #7780 removed, and all three tests currently raise AssertionError against main. Do NOT carry the 40 -> 45 value change -- it is unmeasured against 40 and second-guesses landed 7827b03. Housekeeping, not code: close issue #7516 as resolved by 7827b03 + 5119816 (its rationale comment is already in ci.yml on main) rather than leaving it open as a reason to revive the branch. Current state#7780 has MERGED (5119816; requires-python >=3.12, dep_sync.python_floor_breach on the reinstall branch, and python-version: ["3.12"] all verified on main). By itself it only PARTIALLY covers #7517 -- it never touches timeout-minutes (zero +/- timeout lines in its whole 29-file patch), so it does not deliver #7517's outcome. What it does is delete the two-arm python-version: ["3.10", "3.12"] axis and the --cov/--no-cov ternary, which is the sole premise of #7517's expression-valued cap and the precondition every one of its three tests reads, making them permanently red. #7517's actual outcome was delivered by a different landed commit, 7827b03 (#7527/#7552), which set timeout-minutes: 40 under a comment naming #7517's exact failure mode. Between the two landed changes the remainder is: a scalar guard test on backend-test's cap (which exists nowhere on main) and an unevidenced 40 -> 45 nudge. From a repository-wide duplicate/overlap audit of every pull request open against |
Problem / Motivation
backend-testruns the same four shards twice, on 3.10 with--no-covand on 3.12 with--cov, andtimeout-minutesis a job level key shared by all eight matrix cells. Coverage roughly doubles wall time, so the 30 minute cap that the cheap arm uses less than half of is the cap the coverage arm hits. A shard cancelled at the wall makesCoverage Gatefail closed withbackend-test=cancelled -- failing closed.Measured as
completed_at - started_atfrom/commits/<sha>/check-runs:--no-cov)+cov)d94950d39760124a7592c3b5fffWhy it matters
It is red on
main, not only on pull requests. Seven consecutive pushes tomainsampled on 2026-09-01:dd9e002bfbb01943c1fd38eff0815b062558da28c697f7eea2bea178cc85a83Three of seven lost a shard at the wall. The runs that survived finished at 94 to 96% of the cap, so ordinary variance is enough to flip the rest. Every red costs a maintainer a re-run and teaches contributors to read a real
Coverage Gatefailure as noise.What changed (motivation → approach → change)
Symptom: a lone 3.12 shard reports
cancelledwhile every sibling passes, andCoverage Gatefails closed on it.Root cause:
timeout-minutesis one scalar for a matrix whose two arms do measurably different amounts of work. Coverage is enabled on 3.12 only, and tracing costs about 2x wall time here, so a cap sized for the--no-covarm is 46% occupied on that arm and 101% occupied on the other.Change: resolve the cap per arm.
Two things I checked rather than assumed, because guessing at CI semantics is the same class of error as the defect:
jobs.<job_id>.timeout-minutesaccepts thematrixcontext. GitHub's own context availability table listsgithub, needs, strategy, matrix, vars, inputsfor that key. This is the first expression valuedtimeout-minutesin this repository, so it is worth a maintainer's eye on the shape even though the syntax is documented.backend-test-windowsalready carries 40 minutes for its slowest observed shard of 18.03, so this leaves the coverage arm proportionally tighter than the Windows arm, not looser.Deliberately not in this PR:
.test_durationsfile pytest-split falls back to an even split by test count, so which shard carries the heavy tests is arbitrary and a raised cap does not make it deterministic. That file exists onchore/update-test-durations(ab0eb7c71) and has never been opened as a PR. Landing it is a maintainer action and Backend Tests (Windows) (2): xdist worker crash then 40-minute job cancellation; count-based shard split makes it hit arbitrary PRs #4227 already routed it as a CI policy decision, so it is asked for in Backend Tests: the coverage matrix arm shares the no-cov arm 30-minute cap, so a 3.12 shard is cancelled at the wall and Coverage Gate fails closed #7516 rather than done here.Coverage Gateitself,test-durations.yml, or the 3.10 arm's cap.Tests
test/test_backend_test_timeout_contract.py, modelled on the existingtest/test_xdist_worker_restart_contract.pyworkflow contract test.test_ci_still_has_a_coverage_conditional_backend_matrixis the anti-vacuity guard. Every other assertion reads the job, matrix key and coverage-bearing value discovered here, so a job rename or a move of coverage into its own job fails loudly instead of turning the file into a no-op.test_the_coverage_arm_is_not_capped_like_the_cheap_armresolvestimeout-minutesfor each matrix arm and asserts the coverage arm gets strictly more. A plain scalar resolves to itself for every arm, which is exactly the unfixed state, so this fails on unfixedci.yml.test_the_coverage_arm_gets_at_least_the_windows_arms_budgetpins the floor at the Windows arm's 40, since the coverage arm's measured maximum already exceeds the Windows arm's.It is deliberately one directional. It does not pin the cheap arm's number, does not forbid raising both, and does not care which expression shape encodes the split, so a future change that gives the coverage arm even more room stays green.
Load bearing, verified by reverting the workflow with
git show HEAD~1:.github/workflows/ci.yml:Manual verification
N/A for the runtime behaviour, since this changes a workflow and not code that a test can execute. What was run locally on the pushed head:
.github/workflows/ci.yml, 24 files, 1899 passedpython3 scripts/check_black_formatting.py,isort --check-only src/kiro_crew test conftest.py xdist_budget.py,flake8 src/kiro_crew test conftest.py xdist_budget.py,mypy src/kiro_crew/, all clean./scripts/scrub-lint.sh --no-history, all checks passedScreenshots / video
N/A, no user-visible UI change. This edits
.github/workflows/ci.ymland adds a test that parses it, so there is no rendered surface to show.Related Issues
Fixes #7516
Two independent PRs are sitting red on this today, #7414 and #7376, and neither diff can account for a 28 minute shard. Prior art on the split half is #4227, closed on its fail fast half by #5043 and #5267 with the durations question left open.
Pattern harvest
Rule candidate: review-prompt
Pattern: a job level resource budget shared by matrix arms that do measurably different amounts of work.
timeout-minutessits abovestrategy.matrix, so a cap written when every cell was equivalent silently becomes wrong the moment one arm gains work that the others do not have. Worth asking on any PR that adds a per arm conditional flag to a matrix step: does the cheap arm's budget still fit the arm you just made expensive?Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement