test(ci): run the tests hidden behind the mage build tag - #385
Conversation
`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.
666f9c2 to
2d9cbb0
Compare
The discriminating arm is doneSame tree, same injected defect, two commands. I replaced Old command — New command — The injection was diffed against a backup before each run and reverted with That closes the gap I flagged in the description. All three controls are now measured:
RebasedRebased onto Worth noting what #378 walked into: its third schema-version pin lives in |
|
Correcting a number in the description now that the job has run on a real runner. I wrote that the tagged suite "takes two minutes" against the root module job's 3 m 19 s. That two minutes was measured on my laptop and I did not say so. On the GitHub runner, in run 34971111435:
So the two are effectively the same length, not a third of it. The conclusion is unchanged — the job runs in parallel, so it adds nothing to the critical path, and |
1 participant
Closes #384.
What was not running
go test ./...never compiles anything behind//go:build mage, so the tagged tests executed in no workflow at all. Counted ond0cb24c:mage_validate_targets_test.go.)mage_bench.go,mage_gate.go,mage_cluster.go,mage_validate.go,mage_publish.go,mage_tier.go,mage_sampler.go,mage_diff.go,mage_bench_window.go,mage_helpers.go,magefile.go— the bench driver, the release gate, the cluster orchestration, publish and the validate targets.Lint compiles the magefiles, and the comment there explains why that gap was worth closing for compilation. The test half was never closed.
One concrete thing this was hiding:
mage_bench_sutenv_test.go:294pinsschema_versionto5.13and has never run. To be exact rather than dramatic —report/schema_test.gopins the same constant and does run, so the untagged suite is not blind to a bare constant change. What it is blind to is everything the tagged suite asserts about the shape the mage bench driver emits around that constant.The change
A separate
mage-taggedjob runninggo test -count=1 -race -tags mage -timeout=10m ./..., added toci-ok'sneeds../..., not a package list. A tagged test dropped into a third package is then covered the moment it lands, instead of waiting for someone to widen an enumeration. That is the same failure mode that produced this issue.root modulejob is the long pole at 3 m 19 s (run 34961609126), and the tagged suite is 2 m 00 s.ci-ok'sneeds, because a job nobody fans in is a job branch protection cannot require — the tagged suite could otherwise go red without failing the one required check.Guard, and its controls
workflow_mage_tests_run_test.gois untagged on purpose, so it runs in therootjob rather than in the job it guards.magefromci-ok'sneeds./...to. ./cmd/runnerTestMageTaggedTestsStillExistover an empty setEvery injection was diffed against a backup before the test ran, and reverted with
cp.What this PR does not yet claim
The discriminating measurement — that the new job catches a defect the old command cannot — is half done. I removed the
strings.TrimSpacefromresolveBenchCellsinmage_tier.goand confirmed the old commandgo test -count=1 -race .passes against it (blind). The-tags magearm did not complete: the host ran out of process slots mid-run, so it is absent, not a pass. I will post that arm's output here before this merges rather than let an unrun arm read as a green one.