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, " ")) +}