/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

1. Identify the changed test surface: git diff --stat <base>...HEAD -- '*_test.go' 'go/test/**' 'go/cmd/testlatency/**'.
2. Read each changed test file and score it against the checklist below.
3. 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.