ci: bound the demo and pre-commit jobs - #1234
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is minimal, valid YAML, and cleanly achieves the stated goal without altering job logic.
Pull request overview
This PR makes the CI time budget more predictable by explicitly bounding the demo and pre-commit GitHub Actions jobs, which previously inherited GitHub’s 360-minute default timeout and could block required status checks for hours.
Changes:
- Add
timeout-minutes: 10to thepre-commitjob. - Add
timeout-minutes: 10to thedemojob.
File summaries
| File | Description |
|---|---|
.github/workflows/run-tests.yaml |
Adds explicit 10-minute timeouts to pre-commit and demo, bringing all required checks under explicit caps (with tests already at 30 minutes). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1234 +/- ##
==========================================
- Coverage 62.34% 62.23% -0.11%
==========================================
Files 40 40
Lines 3930 3930
==========================================
- Hits 2450 2446 -4
- Misses 1480 1484 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Only the tests job sets timeout-minutes, so demo and pre-commit inherit GitHub's 360-minute default. Both are required status checks, so the merge queue waits on them, and its check_response_timeout_minutes cannot be derived from anything while two of the nine contexts it blocks on are bounded only by a six-hour default. A hung browser in demo would burn that before anything noticed. Ten minutes against observed runtimes of roughly two and a half minutes each. With this the bound is real: every required check is capped at 30 minutes, so the queue's 60-minute budget is that maximum plus an equal allowance for the runner wait, which sits inside the queue's window and inside no job timeout.
112a008 to
c01d9d3
Compare
Stacked on #1233 — same concern, making CI's time budget coherent rather than arbitrary.
Only
testssetstimeout-minutes.demoandpre-commitinherit GitHub's default of 360 minutes, and both are required status checks, so the merge queue blocks on them.That has two consequences:
demodrives a real browser; if it wedges, nothing stops it for the rest of the working day.check_response_timeout_minutesshould be the maximum time any required check can take, plus an allowance for runner wait. With two of the nine required contexts bounded only by a six-hour default, "the maximum" is six hours, so any queue timeout is a guess.Ten minutes, against observed runtimes of roughly 2m17s (
demo) and 2m27s (pre-commit) — generous by a factor of four.Why this makes the queue timeout principled
The two clocks measure different things and start at different moments:
timeout-minutesbounds execution; the merge queue bounds time to hear back, which additionally contains the runner wait — unbounded, and inside no job's timeout. Withgrouping_strategy: ALLGREENthe batch waits on the slowest of all nine required contexts at once.After this change every required check is capped at 30 minutes, so the queue's 60-minute setting is exactly that maximum plus an equal allowance for contention, instead of a number picked to be larger than the worst run anyone happened to observe.
Context: merge queue entries were timing out against the previous 20-minute setting while every underlying run succeeded — observed merge-queue durations of 27, 33, 39 and 42 minutes. That setting has been raised to 60 separately, in the ruleset.
actionlintpasses.