165 lines
9.4 KiB
Markdown
165 lines
9.4 KiB
Markdown
---
|
|
name: unit-testing
|
|
description: My unit-testing style. Use whenever writing, reviewing, or pruning unit tests in any project, before writing the first test function. Covers what deserves a unit test, what must not be tested, and the required mechanics (testify, table-driven, real dependencies first, mocks only as a last resort).
|
|
---
|
|
|
|
# Unit testing style
|
|
|
|
I want few, high-value tests, in two tiers.
|
|
|
|
1. **Unit tests.** Small, fast, and mainly covering serialization boundaries:
|
|
marshal/unmarshal, parse/encode, round-trips, and on-disk or wire-format
|
|
invariants.
|
|
2. **Integration tests.** The primary safety net. They exercise the project as
|
|
a whole against real systems (real backends, real servers, real databases)
|
|
and usually live in a separate suite behind build tags, run with
|
|
`make test-integration`.
|
|
|
|
More tests are not better. I have had redundant tests removed by the dozens
|
|
("Slim down the tests", "remove simple logic tests"). Write fewer tests than
|
|
feels natural, and never pad coverage.
|
|
|
|
## What deserves a unit test
|
|
|
|
- **Serialization and parsing.** JSON/YAML marshal and unmarshal round-trips,
|
|
config parsing, enum-to-string stability (a backup must stay readable even
|
|
if an iota constant gets reordered). For codecs, encode, decode, and
|
|
compare, with the invalid-input case paired in the same test.
|
|
- **External-format invariants.** Scan/rewrite behavior against fixtures
|
|
shaped exactly like what the external tool or system really emits,
|
|
especially the shapes an integration run can't easily seed.
|
|
- **Contract tests against an external system's declared interface.** Assert
|
|
the encoder's output against what the external system's spec or
|
|
introspection declares; the declared signature or schema string can itself
|
|
be the assertion. Include tolerance tests: truncated or old-version input
|
|
must not panic or misalign.
|
|
- **Facts about the outside world** the code depends on: a real tool's exact
|
|
output spelling, a protocol quirk, a version-capability boundary pinned to
|
|
the real deployment targets. These stay even when the test is four lines.
|
|
- **Real algorithmic logic** with a non-obvious result: coverage and overlap
|
|
relations, canonicalization that guards against churn, index math, scoring.
|
|
One representative test per axis, never an exhaustive fan-out.
|
|
- **API-boundary behavior** of a service. Drive the real handler with
|
|
`httptest` and assert observable effects (status codes, files written or
|
|
removed), not internals.
|
|
|
|
## What must NOT be unit tested
|
|
|
|
- **Simple logic.** Trivial predicates, single-field identity assertions
|
|
("Equal ignores field X"), tests that restate a small helper's own body.
|
|
These are useless. They don't test anything worthwhile.
|
|
- **Anything the integration suite already proves live.** Lifecycle
|
|
operations (add, remove, sync, backup, restore) that a named integration
|
|
subtest pushes through a real backend don't get duplicated in miniature
|
|
with fakes.
|
|
- **Pure reject-only guards**, meaning a lone `require.Error` on one
|
|
validation. The real system fails loudly anyway. A rejection test earns its
|
|
place only when it also pins a positive boundary case next to the
|
|
rejection.
|
|
- **Mock-based tests of internal collaborators.** Mock-manager style tests
|
|
get deleted. Mocks aren't banned outright, see "Real dependencies" below,
|
|
but mocking your own internals never qualifies.
|
|
|
|
For borderline cases, one question decides: does the test encode an external
|
|
fact? If it pins a real spelling, format, or quirk of a system outside this
|
|
codebase, keep it. If it only restates this codebase's own code, delete it. A
|
|
bug there fails loudly downstream.
|
|
|
|
These rules govern what gets committed, not how to work. A throwaway test to
|
|
confirm behavior while developing or debugging is fine, even for simple
|
|
logic. Write it, run it, learn from it, then delete it before the work is
|
|
done. It must not land in the suite.
|
|
|
|
## Mechanics
|
|
|
|
- Go tests use `testing` plus `github.com/stretchr/testify`, never
|
|
hand-rolled `if got != want`. Some older projects predate this, but every
|
|
migration has been toward testify. Split the roles: `require` for setup and
|
|
anything that invalidates the rest of the test, `assert` for the actual
|
|
claims, so one run reports every wrong field.
|
|
- Assertion messages are sentences that carry the reason, not labels:
|
|
`assert.Equal(t, 1, *calls, "a ready service must not be polled twice")`.
|
|
In tables, the message identifies the case: `"signature mismatch for %s"`.
|
|
- Go table-driven wherever more than one input shape exists: anonymous struct
|
|
slice with `name`, inputs, and `want` fields. Rows may carry a `why` column
|
|
so the table holds the rationale ("leading zeros do not change the value").
|
|
`t.Run(tc.name, ...)` is optional when the assert message already names the
|
|
case. Sub-test names outside tables read as full English claims:
|
|
`t.Run("metric zero falls back to default", ...)`.
|
|
- Every test gets a doc comment stating what invariant it locks and why,
|
|
ideally naming the real-world source: the distro or tool version, the
|
|
kernel flag, or the regression it pins. Spell out arithmetic inline so an
|
|
expected number isn't magic; the comment shows the terms that add up to it.
|
|
- Test helpers are small, local to the file, `t.Helper()`-marked, use
|
|
`t.Cleanup` to restore anything they change, and build realistic fixtures
|
|
laid out exactly as the real system writes them.
|
|
- Re-read from the source of truth before asserting. Re-construct the backend
|
|
from disk after a write, so the test exercises the written config rather
|
|
than in-memory state. Re-query the kernel rather than trusting a cached
|
|
handle.
|
|
|
|
## Real dependencies, never substitutes
|
|
|
|
- HTTP: `httptest` servers, or a real TLS listener with self-signed test
|
|
certs when the TLS path itself matters. Never live external services. Live
|
|
URLs get replaced with a local server plus fixture repos built with docker
|
|
dummy packages.
|
|
- Protocol peers: a real in-process server speaking the real wire protocol at
|
|
the boundary, fed fixture files authored in the protocol's native format,
|
|
or a fake that decodes real requests and streams captured responses. Drive
|
|
error paths by mutating the fake mid-test, walking happy, data-missing, and
|
|
unreachable phases with one golden file per phase.
|
|
- External commands: fake them as executable scripts on disk, selected via
|
|
config paths or a prepended `$PATH` entry. The exec boundary stays real.
|
|
- Databases: the real database engine in a container (testcontainers), never
|
|
a lighter engine standing in for the one production runs.
|
|
- Filesystem: real files in `t.TempDir()`, byte-for-byte like production.
|
|
Functions that walk absolute paths take a root-prefix parameter so a
|
|
tempdir can stand in for `/`.
|
|
- System and network APIs: a network namespace (with `runtime.LockOSThread()`,
|
|
dummy links, and `t.Skip` when not root) lets tests drive the real kernel
|
|
safely. A fresh namespace per test is the isolation.
|
|
- OS backends beyond that belong to the integration suite, which may be a
|
|
cross-compiled test binary shipped to a target host. Destructive
|
|
integration tests operate only on throwaway PID-named objects, never a
|
|
system default or live resource whose loss could cut the SSH session.
|
|
Cleanup uses `context.Background()` rather than the
|
|
possibly-cancelled test context, and pre-deletes idempotently.
|
|
|
|
Fixtures: prefer real captured output, checked into `testdata/` with
|
|
provenance noted (which host, which tool version). Assert on specific parsed
|
|
fields with a comment per quirk, rather than golden-diffing everything.
|
|
Synthetic samples are a fallback for conditions no real capture shows, and
|
|
get labeled synthetic. Whole-output golden files (metrics exposition,
|
|
rendered configs) are compared with a diff that prints the golden path on
|
|
mismatch. When two tools report the same underlying state, cross-check
|
|
their fixtures against each other.
|
|
|
|
Mocks are okay only when there is no safe way to exercise the real thing,
|
|
when no isolation mechanism (namespace, container, tempdir, httptest,
|
|
in-process protocol server) makes the real dependency safe or practical. The
|
|
order is real thing, then real thing isolated, then mock as last resort.
|
|
Never mock to dodge the setup cost of an isolation mechanism that exists.
|
|
When a test double is warranted, hand-write it at the external boundary: a
|
|
call-recording fake of the external interface, a plain function field, a
|
|
closure with a call counter. No gomock, no mockery, no testify-mock, and no
|
|
interfaces introduced solely for testing. Small documented seams in
|
|
production code are fine instead: a sentinel test value, a package-level
|
|
poll-interval knob swapped and restored via `t.Cleanup`, an override
|
|
timestamp field for deterministic output.
|
|
|
|
## Workflow
|
|
|
|
- Wire testing through the Makefile (`make test`, `make test-integration`)
|
|
and document that in the README. Verify tooling (schema and config
|
|
validation) gets its own command and runs under `make test`.
|
|
- Integration suites sit behind build tags (`//go:build <proj>_integration`)
|
|
so plain `go test ./...` stays dependency-free. Capability differences are
|
|
handled with `t.Skip` in both directions: an old server skips the new
|
|
feature, a new server skips the legacy guard.
|
|
- After adding or removing tests, `gofmt`, `go vet` (including other-GOOS
|
|
vets for platform-gated files), and the make target must pass. Confirm
|
|
removed tests left no dead helpers or unused imports.
|
|
- When asked to add tests, propose the small set that fits the rules above.
|
|
When touching an existing suite, look for tests that violate these rules
|
|
and offer to prune them.
|