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 `TestMain`s (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](https://github.com/golang/go/issues/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.