Skill Tracer
Skills › Superpowers › test-driven-development › writing-good-tests.md

writing-good-tests.md

198 lines · skills/test-driven-development/writing-good-tests.md at 2ce8aa1

Open on GitHub

Most marks come from matching words in the file, so treat them as pointers.

1# Writing Good Tests
2
3**Load this reference when:** writing or changing tests, adding mocks, or
4adding cleanup/helper methods for tests.
5

Writing Good Tests

Load this reference when: writing or changing tests, adding mocks, or adding cleanup/helper methods for tests.

Raw lines 1 to 5
6## Overview
7
8A test exists to catch a specific break. Two principles govern everything
9here:
11```
121. Every test names the break it catches
132. Every test exercises the real thing
14```
16Strict TDD produces both naturally: a test written first and watched
17failing against real code has already proven it can fail, and only earns
18a mock when the real dependency proves slow or external.

Overview

A test exists to catch a specific break. Two principles govern everything here:

1. Every test names the break it catches
2. Every test exercises the real thing

Strict TDD produces both naturally: a test written first and watched failing against real code has already proven it can fail, and only earns a mock when the real dependency proves slow or external.

Raw lines 6 to 19
20## Principle 1: Name the Break
22Before writing the test body, answer: **what production change should
23make this test fail — and is that change a bug or a decision?** A test
24earns its place by catching a wrong branch, missing side effect, wrong
25argument, boundary case, or broken contract.
27**Derive expectations independently.** Use literals and hand-checked
28fixtures; table-driven tests with literal `want` values are the preferred
29shape. An expectation computed by the code under test — or its helpers —
30passes no matter what that code does:
32```typescript
33// ❌ Mirror assertion: the same builder computes both sides — always true
34const expected = buildSearchQuery({ tag: 'urgent' });
35expect(buildSearchQuery({ tag: 'urgent' })).toBe(expected);
37// ✅ Hand-derived literal
38expect(buildSearchQuery({ tag: 'urgent' })).toBe('tag:"urgent"');
39```
41**No change detectors.** If only intentional decisions can fail a test —
42a constant's value, exact message wording, private structure — it fires
43on redesign and sleeps through bugs. Test the behavior that depends on
44the decision: not `expect(MAX_RETRIES).toBe(5)` but "a failing call is
45retried 5 times and the 6th attempt never happens."
47**Behavior, not text.** Asserting that a script, skill, or config
48contains an exact line proves only that the source is the source. Run
49scripts against controlled inputs and assert outputs, side effects, or
50exit codes. Documents that instruct agents are tested by the consuming
51agent's behavior (superpowers:writing-skills); prose for humans earns no
52test at all.
54**Your code, not the framework.** Test the contract your code makes at
55its boundaries — the route you register, the query you emit, the payload
56you produce. Upstream mechanics are their maintainers' tests to write
57(the classic: asserting your router invokes a registered handler — that
58is the framework's test, not yours). When upstream behavior genuinely
59surprised you, write one narrow characterization test naming the
60assumption. The same boundary applies inside your code: constructors,
61getters, constants, and trivial forwarding earn tests only when they
62validate, normalize, default, derive, enforce, or cause side effects —
63otherwise assert the first consumer-visible result that depends on them.

Principle 1: Name the Break

Before writing the test body, answer: what production change should make this test fail — and is that change a bug or a decision? A test earns its place by catching a wrong branch, missing side effect, wrong argument, boundary case, or broken contract.

Derive expectations independently. Use literals and hand-checked fixtures; table-driven tests with literal want values are the preferred shape. An expectation computed by the code under test — or its helpers — passes no matter what that code does:

// ❌ Mirror assertion: the same builder computes both sides — always true
const expected = buildSearchQuery({ tag: 'urgent' });
expect(buildSearchQuery({ tag: 'urgent' })).toBe(expected);

// ✅ Hand-derived literal
expect(buildSearchQuery({ tag: 'urgent' })).toBe('tag:"urgent"');

No change detectors. If only intentional decisions can fail a test — a constant's value, exact message wording, private structure — it fires on redesign and sleeps through bugs. Test the behavior that depends on the decision: not expect(MAX_RETRIES).toBe(5) but "a failing call is retried 5 times and the 6th attempt never happens."

Behavior, not text. Asserting that a script, skill, or config contains an exact line proves only that the source is the source. Run scripts against controlled inputs and assert outputs, side effects, or exit codes. Documents that instruct agents are tested by the consuming agent's behavior (superpowers:writing-skills); prose for humans earns no test at all.

Your code, not the framework. Test the contract your code makes at its boundaries — the route you register, the query you emit, the payload you produce. Upstream mechanics are their maintainers' tests to write (the classic: asserting your router invokes a registered handler — that is the framework's test, not yours). When upstream behavior genuinely surprised you, write one narrow characterization test naming the assumption. The same boundary applies inside your code: constructors, getters, constants, and trivial forwarding earn tests only when they validate, normalize, default, derive, enforce, or cause side effects — otherwise assert the first consumer-visible result that depends on them.

Raw lines 20 to 64
Names a skill writing-skills :51
65### Gate Function
67```
68BEFORE writing the test body:
69 Name the production change that would make this test fail.
71 Cannot name one → redesign around an observable behavior
72 "The source text changed" → run the artifact and assert its effects
73 Only intentional decisions → change detector; test the behavior
74 that depends on the decision
76 Confirm the expected value is derived without the code under test.
77 IF it reuses the code's logic or helpers:
78 Replace it with a literal or hand-checked fixture
79```

Gate Function

BEFORE writing the test body:
  Name the production change that would make this test fail.

  Cannot name one            → redesign around an observable behavior
  "The source text changed"  → run the artifact and assert its effects
  Only intentional decisions → change detector; test the behavior
                               that depends on the decision

  Confirm the expected value is derived without the code under test.
  IF it reuses the code's logic or helpers:
    Replace it with a literal or hand-checked fixture
Raw lines 65 to 80
81## Principle 2: Exercise the Real Thing
83**The mock earns no assertions.** A mock assertion passes when the mock
84is present and fails when it is absent — it says nothing about the
85component. Assert the real component's behavior; if the mock is what you
86are checking, unmock it or delete the assertion.
88```typescript
89// ✅ Real behavior
90expect(screen.getByRole('navigation')).toBeInTheDocument();
92// ❌ Mock existence
93expect(screen.getByTestId('sidebar-mock')).toBeInTheDocument();
94```
96**your human partner's correction:** "Are we testing the behavior of a
97mock?"
99**Mock at the right level.** Learn every side effect of the real method
100before replacing it; mock the slow or external operation and keep what
101the test depends on real. When unsure, run the test against the real
102implementation first and observe what actually needs to happen.
104```typescript
105// ❌ The mock swallows the config write that duplicate detection reads
106vi.mock('ToolCatalog', () => ({
107 discoverAndCacheTools: vi.fn().mockResolvedValue(undefined)
108}));
110// ✅ Mock only the slow server startup; the config write stays real
111vi.mock('MCPServerManager');
112```
114**Make doubles specific.** When arguments, call counts, or ordering are
115part of the contract, assert them — a fake that accepts anything verifies
116nothing. Give each branch (success, error, malformed) its own fixture or
117spy, so the wrong branch cannot satisfy the expectation.
119**Mirror real data completely.** Mock the complete structure as it exists
120in reality — all documented fields — not just the ones your test reads.
121Partial mocks fail silently when downstream code reads an omitted field:
122the test passes while integration breaks.
124**Production classes carry production methods only.** Cleanup that only
125tests need lives in test utilities, never as a `destroy()` on the
126production class. Ask: is this method called only from tests? Does this
127class own this resource's lifecycle? Wrong answers → test utility.
129**Prefer real components over complex mocks.** When mock setup outgrows
130the test logic, mocks miss methods the real components have, or tests
131break when the mock changes, switch to an integration test with real
132components. **your human partner's question:** "Do we need to be using a
133mock here?"

Principle 2: Exercise the Real Thing

The mock earns no assertions. A mock assertion passes when the mock is present and fails when it is absent — it says nothing about the component. Assert the real component's behavior; if the mock is what you are checking, unmock it or delete the assertion.

// ✅ Real behavior
expect(screen.getByRole('navigation')).toBeInTheDocument();

// ❌ Mock existence
expect(screen.getByTestId('sidebar-mock')).toBeInTheDocument();

your human partner's correction: "Are we testing the behavior of a mock?"

Mock at the right level. Learn every side effect of the real method before replacing it; mock the slow or external operation and keep what the test depends on real. When unsure, run the test against the real implementation first and observe what actually needs to happen.

// ❌ The mock swallows the config write that duplicate detection reads
vi.mock('ToolCatalog', () => ({
  discoverAndCacheTools: vi.fn().mockResolvedValue(undefined)
}));

// ✅ Mock only the slow server startup; the config write stays real
vi.mock('MCPServerManager');

Make doubles specific. When arguments, call counts, or ordering are part of the contract, assert them — a fake that accepts anything verifies nothing. Give each branch (success, error, malformed) its own fixture or spy, so the wrong branch cannot satisfy the expectation.

Mirror real data completely. Mock the complete structure as it exists in reality — all documented fields — not just the ones your test reads. Partial mocks fail silently when downstream code reads an omitted field: the test passes while integration breaks.

Production classes carry production methods only. Cleanup that only tests need lives in test utilities, never as a destroy() on the production class. Ask: is this method called only from tests? Does this class own this resource's lifecycle? Wrong answers → test utility.

Prefer real components over complex mocks. When mock setup outgrows the test logic, mocks miss methods the real components have, or tests break when the mock changes, switch to an integration test with real components. your human partner's question: "Do we need to be using a mock here?"

Raw lines 81 to 134
135### Gate Function
137```
138BEFORE adding a mock or test helper:
139 List the real method's side effects; keep the ones the test
140 depends on real — mock the slow/external level below them.
142 Mock responses mirror the complete real structure.
144 A method only tests call lives in test utilities, not production.
146 About to assert on the mock itself?
147 Unmock it or delete the assertion.
148```

Gate Function

BEFORE adding a mock or test helper:
  List the real method's side effects; keep the ones the test
  depends on real — mock the slow/external level below them.

  Mock responses mirror the complete real structure.

  A method only tests call lives in test utilities, not production.

  About to assert on the mock itself?
    Unmock it or delete the assertion.
Raw lines 135 to 149
150## Tests Ship With the Implementation
152The TDD cycle — failing test, minimal implementation, refactor — is what
153"complete" means. Ship the tests the behavior needs and only those:
154trivial code and human prose earn none, and a test written to satisfy
155process costs maintenance forever.

Tests Ship With the Implementation

The TDD cycle — failing test, minimal implementation, refactor — is what "complete" means. Ship the tests the behavior needs and only those: trivial code and human prose earn none, and a test written to satisfy process costs maintenance forever.

Raw lines 150 to 156
157## The Mutation Check
159Before finishing, mentally mutate the production code; at least one test
160should fail for each realistic mutation:
162- Wrong constant or argument
163- Wrong branch handler
164- Missing state change or side effect
165- Empty or default return
166- Missing validation for zero, empty, nil, unauthorized, or malformed input
168A mutation nothing catches marks the behavior as unprotected — or the
169test as tautological.

The Mutation Check

Before finishing, mentally mutate the production code; at least one test should fail for each realistic mutation:

  • Wrong constant or argument
  • Wrong branch handler
  • Missing state change or side effect
  • Empty or default return
  • Missing validation for zero, empty, nil, unauthorized, or malformed input

A mutation nothing catches marks the behavior as unprotected — or the test as tautological.

Raw lines 157 to 170
171## Quick Reference
173| When you... | Do |
174|-------------|-----|
175| Write any test | Name the break it catches — a bug, not a decision |
176| Build an expected value | Derive it by hand; never with the code under test |
177| Test a script or document | Run it / pressure-test its consumer; never grep its text |
178| Reach for a dependency test | Test your boundary contract, not their documented mechanics |
179| Want to assert on a mocked element | Test the real component, or unmock it |
180| Are about to mock a method | Learn its side effects; mock the slow/external level |
181| Build a mock response | Mirror the real structure completely |
182| Need cleanup only tests use | Put it in test utilities |
183| Watch mock setup balloon | Switch to an integration test with real components |
184| Finish a test file | Run the mutation check |

Quick Reference

When you...Do
Write any testName the break it catches — a bug, not a decision
Build an expected valueDerive it by hand; never with the code under test
Test a script or documentRun it / pressure-test its consumer; never grep its text
Reach for a dependency testTest your boundary contract, not their documented mechanics
Want to assert on a mocked elementTest the real component, or unmock it
Are about to mock a methodLearn its side effects; mock the slow/external level
Build a mock responseMirror the real structure completely
Need cleanup only tests usePut it in test utilities
Watch mock setup balloonSwitch to an integration test with real components
Finish a test fileRun the mutation check
Raw lines 171 to 185
186## Warning Signs
188- Setup and assertion share the same object, guaranteeing equality
189- The test can fail only through a panic, crash, or missing selector
190- The test fails on every intentional change, never on accidental breakage
191- Expected values are hidden behind loops, builders, or helpers
192- The test greps source text, or asserts a removed symbol stays removed
193- The test would still matter if only the framework remained
194- The test exists for coverage, checking no side effect or outcome
195- An assertion checks a `*-mock` test ID, or fails if you remove the mock
196- A method is called only from test files
197- Mock setup is more than half the test, or you can't explain why the mock is needed
198- Mocking "just to be safe"

Warning Signs

  • Setup and assertion share the same object, guaranteeing equality
  • The test can fail only through a panic, crash, or missing selector
  • The test fails on every intentional change, never on accidental breakage
  • Expected values are hidden behind loops, builders, or helpers
  • The test greps source text, or asserts a removed symbol stays removed
  • The test would still matter if only the framework remained
  • The test exists for coverage, checking no side effect or outcome
  • An assertion checks a *-mock test ID, or fails if you remove the mock
  • A method is called only from test files
  • Mock setup is more than half the test, or you can't explain why the mock is needed
  • Mocking "just to be safe"
Raw lines 186 to 198