# Testing Conventions

> Use when adding or changing tests, the e2e harness, test process isolation, or CI test sharding.

- Skill: `mvanhorn/testing-conventions` (Agent Skill)
- Install (CLI): `npx skillmds@latest add mvanhorn/testing-conventions`
- Raw SKILL.md: https://api.skillmd.com/api/skills/mvanhorn/testing-conventions/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: mvanhorn (https://skillmd.com/u/mvanhorn)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/mvanhorn/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`).
- 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 split into a git-heavy shard and a core remainder 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. 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. 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.