Follow-up from the test-hardening review of PRs #210–#215. Several tests passed while proving nothing; the fixes taught two mechanical rules worth enforcing repo-wide.
1. Keep noUnusedLocals / noUnusedParameters on the test tsconfig
tsconfig.tests.json already sets these (added in #213). The reason to keep it explicit: a dead assertion — a value parsed from output into a local that is then never asserted on — becomes a compile error instead of a silently green test. Do not relax these flags for test files.
2. Field-scoped negative assertions (convention)
A negative assertion over a whole serialized blob is a coincidence waiting to happen. expect(raw).not.toContain('42') also matches a timestamp, an id, or an unrelated field — it can pass for the wrong reason and fail flakily. Real case in #215: not.toContain('42') collided with a timestamp.
Rule: a negative assertion must name the field it denies. Parse the output and assert on the specific key (obj.route, obj.err.type), not on the raw string.
3. Mutation check as the bar for "does this test catch the bug"
Standard from the same review: run every new regression test once against the un-fixed code. If it stays green there, it proves nothing (a fallback path, a duplicate-key dedupe, or an env-driven later failure can keep it green for the wrong reason — all three happened across #214/#215). Make this an explicit step in the test-writing checklist.
Follow-up from the test-hardening review of PRs #210–#215. Several tests passed while proving nothing; the fixes taught two mechanical rules worth enforcing repo-wide.
1. Keep
noUnusedLocals/noUnusedParameterson the test tsconfigtsconfig.tests.jsonalready sets these (added in #213). The reason to keep it explicit: a dead assertion — a value parsed from output into a local that is then never asserted on — becomes a compile error instead of a silently green test. Do not relax these flags for test files.noUnusedLocals+noUnusedParametersare set on the test tsconfig and cannot be overridden per-file2. Field-scoped negative assertions (convention)
A negative assertion over a whole serialized blob is a coincidence waiting to happen.
expect(raw).not.toContain('42')also matches a timestamp, an id, or an unrelated field — it can pass for the wrong reason and fail flakily. Real case in #215:not.toContain('42')collided with a timestamp.Rule: a negative assertion must name the field it denies. Parse the output and assert on the specific key (
obj.route,obj.err.type), not on the raw string.not.toContain(/not.toMatch(over raw serialized output; scope each to a field3. Mutation check as the bar for "does this test catch the bug"
Standard from the same review: run every new regression test once against the un-fixed code. If it stays green there, it proves nothing (a fallback path, a duplicate-key dedupe, or an env-driven later failure can keep it green for the wrong reason — all three happened across #214/#215). Make this an explicit step in the test-writing checklist.