From 2d9cbb02f6624291e659771f76748e11d2e0d98f Mon Sep 17 00:00:00 2001 From: Albert Bausili Date: Tue, 15 Sep 2026 14:31:35 +0200 Subject: [PATCH] test(ci): run the tests hidden behind the mage build tag (#384) `go test ./...` never compiled them, so fifty-two tests across nine files executed in no workflow. What they cover is not peripheral: the eleven //go:build mage source files are the bench driver, the release gate, the cluster orchestration, publish and the validate targets, and two of them pin the report schema version -- the constant a rebase has already auto-merged to the wrong value once, silently, because both sides wrote the same literal. Lint compiles the magefiles, and its own comment explains why that gap was worth closing for compilation. The test half was never closed. The job runs over ./... rather than the two packages that carry mage files today, so a tagged test dropped into a third package is covered the moment it lands instead of waiting for someone to widen a list. It is a separate job, so it costs nothing on the critical path: the root module job is the long pole at three minutes and nineteen seconds, and the tagged suite takes two minutes. ci-ok gains the job in its needs list. A job nobody fans in is a job branch protection cannot require, and the tagged suite could then go red without failing the one required check. The guard test is untagged on purpose, so it runs in the root job rather than in the job it is guarding. --- .github/workflows/test.yml | 25 ++++++++- workflow_mage_tests_run_test.go | 99 +++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 workflow_mage_tests_run_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8ebd328a..1ea47409 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -85,6 +85,29 @@ jobs: (cd "$d" && go build ./... && go vet ./...) done + # The mage build tag hides an entire layer from `go test ./...`: eleven + # source files (the bench driver, the release gate, the cluster + # orchestration, publish, the validate targets) and the 50-odd tests + # written against them. Lint only *compiles* the magefiles, so the + # tests never executed anywhere -- including the ones that pin the + # report schema version, which is exactly the constant a rebase has + # already auto-merged wrong once. Runs over ./... rather than the two + # packages that carry mage files today, so a mage-tagged test added to + # any other package is covered the moment it lands. + mage: + name: mage-tagged + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: "1.27.0" + cache: true + - name: go test -tags mage + run: go test -count=1 -race -tags mage -timeout=10m ./... + # Fan-in over every job above so branch protection can require ONE # stable check name ("ci-ok") instead of enumerating the adapter # matrix — adding or removing an adapter then never silently drops a @@ -94,7 +117,7 @@ jobs: # listed in needs must stay unconditional on push + pull_request. ci-ok: name: ci-ok - needs: [root, adapters, refapps] + needs: [root, adapters, refapps, mage] if: always() runs-on: ubuntu-latest steps: diff --git a/workflow_mage_tests_run_test.go b/workflow_mage_tests_run_test.go new file mode 100644 index 00000000..d7b0f52c --- /dev/null +++ b/workflow_mage_tests_run_test.go @@ -0,0 +1,99 @@ +package main + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// The mage build tag is a coverage hole that has re-opened twice, and it +// hides more than a helper or two: eleven source files behind //go:build +// mage carry the bench driver, the release gate, the cluster orchestration, +// publish and the validate targets, plus the tests written against them. +// Lint compiles the magefiles but never runs their tests, so for a long +// while `go test ./...` executed none of them -- including the ones that +// pin the report schema version, the constant a rebase has already +// auto-merged to the wrong value once. +// +// Two things have to hold. The Test workflow must run the tagged suite, +// and ci-ok must depend on that job: a job nobody fans in is a job branch +// protection cannot require, which is how the schedule guard describes its +// own failure mode too. Asserting on ./... rather than on a package list is +// deliberate -- a mage-tagged test dropped into a third package must be +// covered by the command that is already there, not by someone +// remembering to widen it. +func TestMageTaggedTestsRunInCI(t *testing.T) { + b, err := os.ReadFile(filepath.Join(".github", "workflows", "test.yml")) + if err != nil { + t.Fatalf("read test.yml: %v", err) + } + src := string(b) + + // The command itself: `go test` ... `-tags mage` ... `./...`, on one line. + runsTagged := regexp.MustCompile(`(?m)^\s*run:\s*go test\b.*\s-tags mage\b.*\s\./\.\.\.\s*$`) + if !runsTagged.MatchString(src) { + t.Errorf("test.yml runs no `go test -tags mage ... ./...` step: every test " + + "behind //go:build mage (the bench driver, the release gate, the cluster " + + "orchestration, the schema-version pins) executes in no workflow") + } + + // And the job carrying it has to be fanned into ci-ok, or branch + // protection cannot require it and a red tagged suite merges anyway. + needs := regexp.MustCompile(`(?m)^\s*needs:\s*\[([^\]]*)\]`) + m := needs.FindStringSubmatch(src) + if m == nil { + t.Fatalf("test.yml has no ci-ok needs list to check") + } + found := false + for _, dep := range strings.Split(m[1], ",") { + if strings.TrimSpace(dep) == "mage" { + found = true + } + } + if !found { + t.Errorf("ci-ok needs=[%s] does not include the mage job: the tagged suite "+ + "could go red without failing the one required check", strings.TrimSpace(m[1])) + } +} + +// A guard for a suite that does not exist is a guard that passes for the +// wrong reason. If the last mage-tagged test is ever deleted, this says so +// rather than letting TestMageTaggedTestsRunInCI keep reporting success +// over an empty set. +func TestMageTaggedTestsStillExist(t *testing.T) { + tag := regexp.MustCompile(`(?m)^//go:build mage\b`) + var files []string + err := filepath.WalkDir(".", func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + // Agent worktrees under .claude hold full copies of the repo. + if name := d.Name(); name == ".git" || name == ".claude" || name == "results" { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, "_test.go") { + return nil + } + b, rerr := os.ReadFile(path) + if rerr != nil { + return rerr + } + if tag.Match(b) { + files = append(files, path) + } + return nil + }) + if err != nil { + t.Fatalf("walk: %v", err) + } + if len(files) == 0 { + t.Error("no //go:build mage test files found: TestMageTaggedTestsRunInCI is " + + "now guarding an empty suite") + } + t.Logf("%d mage-tagged test file(s): %s", len(files), strings.Join(files, " ")) +}