Repository navigation
Cancel superseded PR test runs, run the suite daily, and cache the race build - #564
Conversation
…ce build - concurrency: a new push to a PR cancels the Test run on its old head. Pushes to main, the schedule and manual dispatches get a group of their own, so none of them is ever queued behind or replaced by another run. - schedule: a daily full run on main, so a new govulncheck advisory against a module or Go release we already ship turns main red the next morning rather than at the next push or release dry run. - Race Detection: its own build cache. setup-go's shared cache never holds race-instrumented objects, so the job recompiled every dependency with -race on each run (~40s of its ~100s). - Nix flake builds: the paths filter narrows only push and PR runs; the schedule and a dispatch build the flake in full. Required check names and their conditions on PRs are unchanged.
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
There was a problem hiding this comment.
No issues found across 1 file
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
There was a problem hiding this comment.
🟢 Approved
The workflow changes correctly implement the stated CI behavior without disrupting required checks.
0 open findings
What changed in this PR
Improves CI efficiency and coverage without changing required PR checks.
Changes:
- Cancels superseded PR runs and adds daily scheduled testing.
- Adds a dedicated race-build cache.
- Ensures scheduled/manual runs perform full Nix builds.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
.github/workflows/test.yml |
Adds concurrency, scheduling, race caching, and event-aware Nix filtering. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test runs on this repo take 2 to 3 minutes and nobody has tuned them. This makes three changes to
test.ymlthat pay off, and explains why it leaves change-based job skipping out.Changes
main, the schedule and manual dispatches each get their own group (the run id), so none of them is ever queued behind or replaced by another run. A shared group withcancel-in-progress: falsewould still let a newer pendingmainrun replace an older pending one, and that merge would never get its full run.main(23 6 * * *). govulncheck reads a database that changes without any commit here. Today a new advisory against a module or Go release we already ship is only caught at the next push or the next release dry run. With this changemaingoes red the next morning.-raceobjects, so Race Detection recompiled every dependency on every run. Its own cache path is keyed like setup-go's:go.modcarries the toolchain line andgo.sumthe dependency set.pushandpull_requestruns. The schedule and a dispatch have no change set to filter by, so they skip the filter step and build the flake in full.Required checks still report
This PR adds no workflow-level
pathsfilter and no new job-levelifon any required check, so every required check reports on every PR exactly as before. That covers Tests, Lint, GitHub Actions audit, Security, Race Detection, CLI Surface Check, Secret scanning, Bats e2e, Installer (bash 3.2) and Nix flake builds. The only conditions that changed are step-level, inside Nix flake builds, and they apply toschedule/workflow_dispatchonly. A cancelled run sits on a head that is no longer the PR's head, and the new head gets its own complete run.Timings
Before, measured over the last 8 successful Test runs (4 PR, 4
main):After, measured on this PR. The first run had no race cache to restore: the race step took 160s compiling everything, then saved the cache in 4s. A re-run of Race Detection on the same head (attempt 2) restored that cache in 4s. Compiling before the first package result dropped from about 43s to 2s. The step still took 90s, because
internal/markdownran its race tests in 84s (48s in the baseline run). Locally, a-racebuild of./...takes 40.5s from a cache warm only with plain objects (what CI has today) and 4.1s from a race-warm one.What that means. Once
mainhas saved the race cache, Race Detection loses about 40s of compiling on every run. Its floor is nowinternal/markdown's race test time, 48–84s, against 7.5s without-race. Expected wall time is roughly 100–150s, down from 113–189s (estimated). Cancelling superseded runs saves about 10 runner-minutes for each push made while a run is in flight, and doesn't change the latest run's wall time. A follow-up worth having is to find what ininternal/markdownis so slow under the race detector. A fuzz seed corpus or a large table is the likely suspect, and cutting it under-racewould take the critical path down further.Not done, and why
.claude-plugin/plugin.json+nix/package.nix) and 5 workflow, release or script PRs, which should run everything. Lint also reads*.mdanddocs/(check-release-lockstep), so it could never be skipped. The payoff is about 4 PRs in 60. The cost is a fail-open condition on five required jobs. If we want it later, the safe shape is an always-runningchangesjob andif: ${{ !cancelled() && (github.event_name != 'pull_request' || needs.changes.result != 'success' || needs.changes.outputs.code == 'true') }}on each gated job. A job skipped by its ownifreports as passing. A workflow-levelpathsfilter would leave the required check pending forever. Nix flake builds and Installer (bash 3.2) already do this at step level.Checks
actionlint1.7.12 (with shellcheck): clean.zizmor1.30.0: no findings.make check-release-lockstep: passes..github/workflows/test.yml. It runs in shadow mode here (nomode:input), so that is an informational comment and no label is required.