| name | golang-test-review |
| description | Use when reviewing Go test code (new or changed *_test.go files, test harnesses, fixtures, or test tooling) for a Go-test-expert pass — behavior-over-surface, determinism, parallel-safety, build-tag correctness, and harness reuse. Complements the general code-reviewer with test-specific rigor. |
/evo:golang-test-review
A focused Go-test-engineering review. Run it over the changed *_test.go files
(and any test harness / fixtures / test tooling) after writing tests and
before merge. It does not replace code-reviewer — it adds the test-specific
lens that general review misses.
How to run
- Identify the changed test surface:
git diff --stat <base>...HEAD -- '*_test.go' 'go/test/**' 'go/cmd/testlatency/**'.
- Read each changed test file and score it against the checklist below.
- Report findings by severity (CRITICAL / HIGH / MEDIUM / LOW), each with
file:line, the rule violated, and a concrete fix. End with a verdict:
APPROVE (no CRITICAL/HIGH) / CHANGES-REQUESTED.
Checklist
1. Behavior over surface (the highest-signal rule)
- Does the test pin the invariant under change, or just re-walk lines for
coverage? A test that would still pass if the behavior broke is a no-op —
flag it CRITICAL. (AGENTS.md Rule 9.)
- Are assertions specific (exact value / error) rather than "not nil" / "len > 0"
where an exact check is possible?
- Negative + edge cases present, not only the happy path?
2. Determinism (non-negotiable)
- No dependence on ambient state: live repo,
$HOME, real clock, network,
.evolve/runs/. Tests build isolated state (t.TempDir() + git init, or the
shared fixtures.NewWorkspace).
- No real
time.Sleep for synchronization where an injected clock / channel /
poll-with-deadline would be deterministic. Real sleeps belong only in
build-tagged integration/e2e tiers.
- No ordering dependence between tests; no shared mutable package globals.
3. Parallel-safety
t.Parallel() is present where safe — BUT never combined with t.Setenv
(panics) or shared-mutable-state tests.- Subtests that capture a loop variable are Go-1.22-safe or copy the variable.
-race clean (the suite runs with -race).
4. Build-tag / cost-axis correctness (this repo's two-axis model)
- Slow tests (real subprocess: git/gh/tmux/bash; real timing) carry
//go:build integration or //go:build e2e and are NOT in the fast default.
- Self-containment: an untagged (fast) test must not reference a symbol
defined only in a tagged file — the default build would break. Shared helpers
the fast suite needs live in an untagged file.
- Tag sits at the top, followed by a blank line, before
package.
5. Harness reuse (no re-duplication)
- Uses
go/test/fixtures (Workspace builder, Fake{Storage,Ledger,Runner,Bridge},
FixedClock, assertion facade) instead of a new local temp-dir builder, fake,
clock, or must* helper. Reintroducing a local clone of a harness primitive
is a HIGH finding.
fixtures.FilePresent (pure bool) for skip preconditions — NOT
acsassert.FileExists (which logs an Errorf and shouldn't gate a skip).
6. Naming & structure
- Behavior-named (
TestShipGate_BlocksWhenRedCountNonZero), never cycle-pegged
(TestC102_*).
- AAA (Arrange / Act / Assert) visually distinct; table-driven where it reduces
repetition;
t.Run subtests named for the case.
t.Helper() on every helper so failures point at the caller.
7. Debuggability of failures
- Failure messages state got-vs-want and the value under test
(
got %q, want %q), not bare t.Fatal("failed").
- Skips explain why and what's missing (
t.Skip("git not on PATH")).
8. Hygiene
t.Cleanup / defer restore any mutated state; no leaked goroutines,
temp files (prefer t.TempDir), or open handles.- No
time.Now()/rand without a seam where the result is asserted.
Severity guide
- CRITICAL: tautological/no-op test, non-deterministic test,
-race violation,
build-tag self-containment break.
- HIGH: missing negative/edge coverage on changed behavior, re-duplicated
harness primitive,
t.Setenv+t.Parallel combo, real sleep in fast tier.
- MEDIUM: weak assertions, missing
t.Helper(), poor failure messages.
- LOW: naming, AAA structure, comment nits.
