Skip to content

Cancel superseded PR test runs, run the suite daily, and cache the race build - #564

Merged
robzolkos merged 1 commit into
mainfrom
ci/faster-feedback
Oct 12, 2026
Merged

robzolkos merged 1 commit into
mainfrom
ci/faster-feedback

Conversation

@robzolkos

@robzolkos robzolkos commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Test runs on this repo take 2 to 3 minutes and nobody has tuned them. This makes three changes to test.yml that pay off, and explains why it leaves change-based job skipping out.

Changes

  • Cancel superseded PR runs. A new push to a PR cancels the Test run on its old head. Pushes to 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 with cancel-in-progress: false would still let a newer pending main run replace an older pending one, and that merge would never get its full run.
  • Daily full run on 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 change main goes red the next morning.
  • A race build cache of its own. setup-go keeps one build cache for every job, and whichever job finishes first saves it. That cache never holds -race objects, so Race Detection recompiled every dependency on every run. Its own cache path is keyed like setup-go's: go.mod carries the toolchain line and go.sum the dependency set.
  • Nix flake builds on schedule/dispatch. The paths filter now narrows only push and pull_request runs. 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 paths filter and no new job-level if on 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 to schedule/workflow_dispatch only. 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):

Job Duration
Wall time 113–189s
Race Detection (critical path) 109–185s, about 43s of it compiling before the first package result
Tests 94–123s
Nix flake builds 80–112s
Lint 50–80s

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/markdown ran its race tests in 84s (48s in the baseline run). Locally, a -race build 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 main has saved the race cache, Race Detection loses about 40s of compiling on every run. Its floor is now internal/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 in internal/markdown is so slow under the race detector. A fuzz seed corpus or a large table is the likely suspect, and cutting it under -race would take the critical path down further.

Not done, and why

  • Skipping jobs on docs-only changes. None of the last 60 merged PRs was docs-only, and 51 of them touched Go. The 9 others were 4 release-bump PRs (.claude-plugin/plugin.json + nix/package.nix) and 5 workflow, release or script PRs, which should run everything. Lint also reads *.md and docs/ (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-running changes job and if: ${{ !cancelled() && (github.event_name != 'pull_request' || needs.changes.result != 'success' || needs.changes.outputs.code == 'true') }} on each gated job. A job skipped by its own if reports as passing. A workflow-level paths filter would leave the required check pending forever. Nix flake builds and Installer (bash 3.2) already do this at step level.
  • Matrix trimming. Test has no matrix. The only matrix is CodeQL's two languages, and both are needed. The macOS installer job is already PR-only and path-gated.
  • Merging Tests and Race Detection into one job. That would rename a required check, and the old name would sit pending forever.

Checks

  • actionlint 1.7.12 (with shellcheck): clean. zizmor 1.30.0: no findings.
  • make check-release-lockstep: passes.
  • Sensitive Change Gate will flag .github/workflows/test.yml. It runs in shadow mode here (no mode: input), so that is an informational comment and no label is required.

…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.
@robzolkos
robzolkos requested a review from a team as a code owner October 12, 2026 17:01
Copilot AI balanced review requested due to automatic review settings October 12, 2026 17:01
@github-actions

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

View guided diff | Turn on auto-fix | Re-trigger cubic

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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 run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to 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.

@robzolkos
robzolkos merged commit 6346fce into main Oct 12, 2026
35 checks passed
@robzolkos
robzolkos deleted the ci/faster-feedback branch October 12, 2026 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants