ci: split go-test into fast PR gate + main-push race+coverage gate#57
ci: split go-test into fast PR gate + main-push race+coverage gate#57
Conversation
The single 'Go Tests' job ran 'go test -race -coverprofile ./...' on
every PR and push, which took ~6min on GitHub-hosted runners. -race
adds 5-10x to test runtime; coverage adds another 10-20%. Most of
that cost was paid on PR builds, including the weekly stream of
dependabot patch bumps that touch zero Go code.
Split the job:
- go-test (Go Tests): fast 'go test -count=1 ./...' on every event.
No -race, no coverage. Cuts the PR feedback loop to ~45-60s.
- go-test-race (Go Tests (race + coverage)): full race + coverage
pass, gated to push events on main only. Ensures every merge
commit is race-clean and the Codecov dashboard tracks the
deployable history.
Pre-push hooks (.githooks/pre-push) already mirror the race run
locally, so committers still get the race-detector signal before a
push reaches the remote — the CI split moves the redundant PR-side
race run to a single per-merge run instead.
bufbuild/buf-action defaults to posting a 'lint passed' comment on the PR. The default GITHUB_TOKEN scoped via top-level 'permissions: contents: read' does not allow issue-comment writes, so even a clean lint reports 'Resource not accessible by integration' and turns the check red. GitHub already surfaces buf lint/breaking errors as inline annotations, so the comment is redundant — drop it instead of widening token scope.
There was a problem hiding this comment.
Pull request overview
Splits the Go test CI workload into a fast PR gate and a slower race+coverage gate intended to run only for main pushes, aiming to reduce PR feedback time.
Changes:
- Adds a new fast
go-testjob that runsgo test -count=1 ./...on PRs/pushes. - Adds a
go-test-racejob (race detector + coverage + Codecov) gated topushevents onmain.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Slow gate: race detector + coverage. Runs only on push to main so | ||
| # the merge commit is always race-clean, and Codecov tracks coverage | ||
| # at the same cadence as the deployable history. Skipped on PRs and | ||
| # pushes to feature branches. | ||
| go-test-race: | ||
| name: Go Tests (race + coverage) | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
The comment and the job-level if imply the workflow runs on pushes to feature branches, but the workflow trigger is on.push.branches: [main], so feature-branch pushes never start this workflow. Either update the comment/intent to reflect that this only runs on PRs + pushes to main, or broaden on.push.branches (and keep the if) if you truly want branch pushes to run go-test while skipping go-test-race.
| go-test: | ||
| name: Go Tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: actions/setup-go@v6 | ||
| with: | ||
| go-version: "1.26" | ||
| - run: go test -count=1 ./... |
There was a problem hiding this comment.
On pushes to main, this will run both go-test and go-test-race even though go-test-race already executes the full test suite. If the goal is to reduce CI minutes/queue time on main pushes, consider skipping go-test when github.ref == 'refs/heads/main' (or making go-test PR-only) so the test suite only runs once per merge.
| # Slow gate: race detector + coverage. Runs only on push to main so | ||
| # the merge commit is always race-clean, and Codecov tracks coverage | ||
| # at the same cadence as the deployable history. Skipped on PRs and | ||
| # pushes to feature branches. | ||
| go-test-race: | ||
| name: Go Tests (race + coverage) | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
Running go-test-race only on push to main won’t actually gate merges under typical branch protection, since required status checks are evaluated on the PR (pull_request) run. If the intent is a true “race-clean before merge” guarantee, consider running this on merge_group (merge queue) and/or on PRs when Go code changes (path filter), otherwise races will be detected only after the merge has landed on main.
Three valid points from Copilot's review: 1. The 'skipped on PRs and pushes to feature branches' comment was misleading — push events on feature branches do not start the workflow at all (on.push.branches: [main] in this repo). Reword so the skip set matches what actually happens. 2. On main pushes both go-test and go-test-race ran, even though race+coverage already executes the full suite. Make go-test pull_request-only so the merge commit only runs the slower job (with coverage) and we stop paying for a duplicate ~45s run. 3. go-test-race gated to push-on-main was a post-merge alert, not a true merge gate — branch protection evaluates required checks on the PR run. Add a detect-changes job that flips a Go-touched output flag, and trigger go-test-race on PRs whose changes hit **/*.go / go.mod / go.sum / the workflow file. Pushes to main keep running it for coverage upload and post-merge sanity. PRs that do not touch Go (dependabot npm bumps, doc tweaks) skip the slow job entirely.
SonarCloud flagged the dorny/paths-filter@v3 reference as a security hotspot — third-party action tags are mutable and can be rewritten by the upstream maintainer to point at a malicious commit. Pin to the exact commit SHA that v3 resolves to today, with a comment recording the tag for future readers. The other actions in this workflow (actions/checkout, setup-go, upload-artifact, codecov, bufbuild/buf-action, sqlc-dev/setup-sqlc, aquasecurity/trivy-action, gitleaks/gitleaks-action, golangci/golangci-lint-action) sit below the new-code analysis baseline and were not flagged. They carry the same risk and are good follow-up work, but pinning them is out of scope for this PR.
|
Summary
-race + coverageruns only on push to main (race-clean merge guarantee)Why
Every PR (including the weekly dependabot patch bumps) paid ~6min for
go test -race -coverprofile ./.... Most PRs touch zero Go code yet pay the full bill. Split into two jobs:go-test(Go Tests): fastgo test -count=1 ./...on every PR/push. ~45-60s.go-test-race(Go Tests (race + coverage)): full race + coverage pass, gated to push events onmainonly. Codecov upload moves to this job.Test plan