diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index a985acd2..9fd7e9d0 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -1,8 +1,9 @@ # Pre-merge gates for the OpenWatch Go tree (repo root). # # Runs: make vet, make lint (staticcheck + gosec + ...), make vuln -# (govulncheck), make test-race (data races + full integration suite -# against a Postgres service container), and `specter sync` (spec +# (govulncheck), a single `go test -race -json` run (data races + full +# integration suite against a Postgres service container, AND the JSON +# specter ingests — one pass, not two), and `specter sync` (spec # validation + strict AC coverage). # # Spec: specs/release/ci-gates.spec.yaml. @@ -97,15 +98,40 @@ jobs: cache: true cache-dependency-path: go.sum + # Cache the from-source golangci-lint binary so we don't rebuild it + # (~1-3 min) on every run. Keyed on the lint version + Go toolchain: + # the binary MUST match the runner's Go (the prebuilt v1.64.8 was + # built with Go 1.24 and refuses Go 1.25+ configs), so a toolchain + # bump invalidates the cache and triggers a fresh build. + - name: Cache golangci-lint binary + if: steps.paths.outputs.go == 'true' + id: cache-golangci-bin + uses: actions/cache@v4 + with: + path: ~/go/bin/golangci-lint + key: golangci-bin-v1.64.8-go1.26.4-${{ runner.os }} + - name: Set up golangci-lint + if: steps.paths.outputs.go == 'true' && steps.cache-golangci-bin.outputs.cache-hit != 'true' + # Build from source against the runner's Go toolchain (see cache + # key rationale above). Only runs on a cache miss. + run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 + + # Cache golangci-lint's own analysis cache so `make lint` reuses + # prior results and only re-lints changed packages. restore-keys + # falls back to the newest partial cache when no exact hit. + - name: Cache golangci-lint analysis cache if: steps.paths.outputs.go == 'true' - # Build from source against the runner's Go toolchain. The - # prebuilt v1.64.8 binary was compiled with Go 1.24, which - # refuses to load configs targeting Go 1.25. Building from - # source gets us a binary matched to the runner's toolchain. - run: | - go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 - golangci-lint --version + uses: actions/cache@v4 + with: + path: ~/.cache/golangci-lint + key: golangci-cache-${{ runner.os }}-${{ hashFiles('**/*.go') }} + restore-keys: | + golangci-cache-${{ runner.os }}- + + - name: golangci-lint version + if: steps.paths.outputs.go == 'true' + run: golangci-lint --version - name: Set up specter if: steps.paths.outputs.go == 'true' @@ -151,10 +177,6 @@ jobs: if: steps.paths.outputs.go == 'true' run: make vuln - - name: make test-race - if: steps.paths.outputs.go == 'true' - run: make test-race - # Annotation hygiene: cross-reference every test's @spec / @ac # annotation against the spec registry and fail on a dangling spec # ref, unknown AC id, malformed AC id, or unreachable annotation @@ -174,14 +196,19 @@ jobs: exit 1 fi - # Spec coverage gate. Split into discrete steps so a failure - # attributes to the actual command (go test vs. ingest vs. sync) - # instead of getting hidden behind a single bundled step. - - name: go test (json) for specter ingest + # Single full-suite run that BOTH detects data races and emits the + # JSON specter ingests — replacing the previous two end-to-end passes + # (`make test-race` + a separate non-race `go test -json`), which ran + # the whole serial DB-bound suite twice. Coverage is unaffected: the + # only `//go:build !race` test file holds constants (with a `race` + # twin), so every test still compiles+runs under -race, and the perf + # budgets are non-gating (internal/perftest.Budgetf). Timeout raised + # to 600s because race instrumentation slows the hot DB packages. + - name: go test (race + json) for specter ingest if: steps.paths.outputs.go == 'true' run: | set +e - go test -json -timeout 360s -p 1 ./... > /tmp/go-test.json + go test -race -json -timeout 600s -p 1 ./... > /tmp/go-test.json EXIT=$? if [ "$EXIT" -ne 0 ]; then echo "::group::Failed tests" diff --git a/packaging/tests/ci_gates_test.go b/packaging/tests/ci_gates_test.go index b8834e5c..24bb9dae 100644 --- a/packaging/tests/ci_gates_test.go +++ b/packaging/tests/ci_gates_test.go @@ -176,8 +176,11 @@ func TestCIGates_WorkflowHasPostgresService(t *testing.T) { } // @ac AC-09 -// AC-09: workflow runs each gate as its own step (vet, lint, vuln, -// test-race, specter sync). +// AC-09: workflow runs each gate as its own step (vet, lint, vuln, the +// race+JSON test run, specter sync). Race detection and the specter-ingest +// JSON are produced by a single `go test -race -json` step (replacing the +// former separate `make test-race` + non-race json passes), so the gate is +// the presence of `-race` AND `-json` on the test run, not `make test-race`. func TestCIGates_WorkflowRunsAllGates(t *testing.T) { t.Run("release-ci-gates/AC-09", func(t *testing.T) { wf := readAppFile(t, ".github/workflows/go-ci.yml") @@ -185,7 +188,6 @@ func TestCIGates_WorkflowRunsAllGates(t *testing.T) { "make vet", "make lint", "make vuln", - "make test-race", "specter sync", } for _, g := range gates { @@ -193,6 +195,11 @@ func TestCIGates_WorkflowRunsAllGates(t *testing.T) { t.Errorf("workflow missing step that runs %q", g) } } + // The single race+coverage run must still detect data races AND + // emit JSON for specter ingest. + if !strings.Contains(wf, "go test -race") || !strings.Contains(wf, "-json") { + t.Error("workflow missing the race+JSON test run (`go test -race ... -json`) — race detection must still gate") + } }) } diff --git a/specs/release/ci-gates.spec.yaml b/specs/release/ci-gates.spec.yaml index 1c7f8660..2bc84974 100644 --- a/specs/release/ci-gates.spec.yaml +++ b/specs/release/ci-gates.spec.yaml @@ -87,7 +87,7 @@ spec: description: make help lists every gate target (vet, lint, vuln, test-race, check) with one-line descriptions so contributors discover them without reading the Makefile. priority: high - id: AC-07 - description: .github/workflows/go-ci.yml exists, triggers on every PR/push to main without a paths filter, references the Go source paths (cmd/, internal/, ...) in a path-detection step, and gates the heavy gate steps (Go setup, lint, vuln, test-race, specter) on that step so non-Go PRs short-circuit to success while still reporting the required "Quality + security gates" status check. + description: .github/workflows/go-ci.yml exists, triggers on every PR/push to main without a paths filter, references the Go source paths (cmd/, internal/, ...) in a path-detection step, and gates the heavy gate steps (Go setup, lint, vuln, the race+coverage test run, specter) on that step so non-Go PRs short-circuit to success while still reporting the required "Quality + security gates" status check. priority: critical references_constraints: [C-06] - id: AC-08 @@ -95,7 +95,7 @@ spec: priority: critical references_constraints: [C-06] - id: AC-09 - description: The workflow executes (in order) make vet, make lint, make vuln, make test-race, and specter sync — each as its own step so a failure pinpoints which gate broke. + description: The workflow executes (in order) make vet, make lint, make vuln, a single race-detector + JSON test run (`go test -race -json`, which both detects data races and feeds specter ingest — replacing the former separate `make test-race` + non-race json passes), and specter sync — each as its own step so a failure pinpoints which gate broke. priority: critical references_constraints: [C-06] - id: AC-10