testing-conventions

Testing Conventions

• Prefer e2e tests for behavior that crosses a process or I/O boundary (CLI flags, config loading, git operations, agent spawning, daemon coordination, stdout/stderr, recorded fixtures); unit-test pure helpers where speed and failure localization matter. Prefer creating real git repos in temp dirs over heavy mocking.
• The e2e suite is behind the e2e build tag; make e2e runs scripts/e2e.sh, which sweeps ./internal/e2e/... and ./internal/pipeline/steps/..., so keep new step-local e2e tests behind the tag too.
• Temporary e2e daemons (NM_TEST_START_DAEMON=1 / harness) are owned by internal/e2edaemon: exact inventory, concurrency cap (NM_E2E_DAEMON_MAX, default 2), bounded argv checks, and reapers in harness Cleanup, package TestMain, and scripts/e2e.sh EXIT/INT/TERM. A SIGKILL of the wrapper shell does not run its trap; next-run inventory recovery covers that. External sleep-loop keepalives are out of scope. Never point inventory reaping at the shared ~/.no-mistakes service. Regressions: internal/e2edaemon/*_test.go.
• Packages whose tests shell out to git unset GIT_CONFIG_COUNT in TestMain so ambient GIT_CONFIG_* injection from agent harnesses cannot leak in; a test exercising injected config re-sets it with t.Setenv (see internal/git, internal/gate, internal/daemon, internal/pipeline/steps, internal/pipeline/steps/citest).
• Those TestMains (plus internal/evidence, internal/branchsync, internal/gatecontext) also set GIT_CONFIG_NOSYSTEM=1: some distributions ship safe.bareRepository=explicit in the system git config, which refuses running git inside the bare fixture repositories. Production code passes --git-dir for bare repositories and must keep working without this.
• Every package whose tests run git also points GIT_CONFIG_GLOBAL at a nonexistent path in a temp directory (write no file), so fixture commits never follow the developer's commit.gpgsign. A HOME override alone does not isolate it, because git also reads $XDG_CONFIG_HOME/git/config. A test that needs a global config writes its own file and re-sets GIT_CONFIG_GLOBAL with t.Setenv (see TestCIStep_CommitAndPush_GitCommandsUseStandardCredentialEnv).
• Packages whose tests can start a daemon or touch ambient state (cmd/no-mistakes, internal/cli, internal/update) use a package-wide TestMain that points NM_HOME and HOME at fresh temp dirs and disables telemetry/update-check env vars, so a full test run never touches a real ~/.no-mistakes. Follow the same pattern in new such packages.
• paths.New() refuses the default ~/.no-mistakes root under go test; tests that touch app state must set NM_HOME to a temp dir, and only the production-default path test may opt in with NO_MISTAKES_ALLOW_DEFAULT_ROOT_IN_TESTS=1.
• Isolate filesystem and environment state with t.TempDir() and t.Setenv().
• The Windows CI leg is process-spawn bound, not compute bound: git-backed packages cost roughly 10x their Linux time (internal/git 5.7s -> 53s, internal/branchsync 31s -> 415s). The Windows matrix is three shards so each job's wall stays inside timeout-minutes: 40 and a hang still surfaces as go test -timeout (15m) rather than an evidence-free job cancel: windows-steps runs ./internal/pipeline/steps/... alone (including steps/citest), windows-git runs the remaining git-heavy packages (internal/git, internal/branchsync, internal/gate, internal/evidence, internal/daemon, internal/eval), and windows-core is the go list remainder filtered by NM_CI_WINDOWS_GIT_EXCLUDE (the union of the other two shards). Combining steps with the other git-heavy packages made windows-git a ~21 min floor; the split is the lockstep pin in TestCIWorkflow_WindowsHangSurfacesAsGoTimeoutNotJobCancellation. Keep long git-heavy packages off the serial critical path (internal/branchsync runs t.Parallel() for exactly that reason) and keep the Defender scan-exclusion step in ci.yml, whose comment owns the rationale. GitHub-hosted Windows runners are 4-core; do not assume larger machines. Regressions: TestCIWorkflow_WindowsTestsRunWithScanExclusions, TestCIWorkflow_WindowsHangSurfacesAsGoTimeoutNotJobCancellation.
• Go applies an implicit GOOS constraint from a filename suffix, so a test file named *_windows_test.go (or _linux, _darwin) silently compiles only on that platform. Name platform-agnostic tests about Windows something else.
• On macOS with Go 1.26.0-1.26.4, a git-heavy package under -race intermittently loses a git child before it runs git: darwin syscall.rawSyscall was race-instrumented, and forkAndExecInChild calls it in the forked child before execve (golang/go#79804, fixed in Go 1.26.5). It surfaces as git <cmd>: signal: segmentation fault; exit status 66 with empty stderr beside a bare ThreadSanitizer: CHECK failed: tsan_rtl.cpp:94 "((part)) == ((part1))" line; exec: WaitDelay expired before I/O complete from a git that exited 0 (a wedged sibling child still holds its inherited pipe); or goroutines parked in syscall.forkExec -> readlen until the package timeout. Code that folds a git error into a verdict then fails a test (branchsync's blocked_assumptions_changed with relation unknown). None of it is a repo bug: check go version and upgrade the toolchain rather than adding retries, waits, or longer timeouts. ~/Library/Logs/DiagnosticReports/*.ips records the crash as procName: <pkg>.test, parentProc: <pkg>.test, asi: "crashed on child side of fork pre-exec"; the CI legs are Linux and Windows. The same bug explains a stray <pkg>.test -test.timeout=... process at high CPU that ignores its own deadline: it is a pre-exec child spinning in __tsan::TraceSwitchPartImpl that inherited the parent's name, argv, and cwd, so no test-side timeout applies to it. internal/procreap reaps those by cwd.

Pipeline step tests (CI latency)

• Fake gh/glab/git on PATH must be the tiny helper at internal/pipeline/fakecli, compiled once per test process without -race (stepstest.Init / LinkFakeCLI). Tests that need the underlying Git binary use internal/testgit.RealGit, which resolves fixed absolute locations without consulting PATH. Do not re-exec the race-instrumented test binary as those names: that was ~0.8-1.1s per spawn and pushed internal/pipeline/steps into the 10-minute package timeout.
• CI-monitor tests live in internal/pipeline/steps/citest so no child of internal/pipeline/steps sits near that cap. Both packages run under go test ./...; do not move them behind the e2e tag.