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).
• 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().
• Pipeline-step tests put a tiny non-race helper (internal/pipeline/fakecli, built once by stepstest.Init) on PATH as gh/glab/git. Never re-link the race-instrumented test binary as those names.
• 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 a git-heavy package under -race intermittently reports git <cmd>: signal: segmentation fault. That is not a git or repo bug: ~/Library/Logs/DiagnosticReports/*.ips records the crash as procName: <pkg>.test, parentProc: <pkg>.test, asi: "crashed on child side of fork pre-exec" - the forked child dies before execve. Confirm there before chasing it in Go code; the CI legs are Linux and Windows. The same fork mechanic explains a stray <pkg>.test -test.timeout=... process at high CPU that appears to ignore its own deadline: a pre-exec child inherits the parent's name, argv, and cwd, so it is not a running test binary and no test-side timeout applies to it. internal/procreap reaps those by cwd.