mirror of
https://github.com/mattpocock/skills.git
synced 2026-07-29 11:02:41 +07:00
Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
80e9dcc685 | ||
|
|
e81f97660a | ||
|
|
43ea0884b0 | ||
|
|
a116824938 |
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"mattpocock-skills": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Reshape the `tdd` skill into reference-only. The red → green → refactor loop is anchored by leading words the model already holds, so the step-by-step Workflow was largely restating the loop and duplicating the horizontal-slicing anti-pattern. Dropped the Workflow and per-cycle checklist; folded their one durable idea — vertical slices / tracer bullets — into the Anti-patterns section and a short Rules-of-the-loop list. Introduced **seam** as the leading word for where tests go, collapsing the old Philosophy "public interfaces" prose and the Planning "confirm interface / behaviors" handshake into one rule: test only at pre-agreed seams, confirmed with the user before any test is written.
|
||||||
|
|
||||||
|
Also dropped the refactor stage — TDD is now red → green, not red → green → refactor. Refactoring belongs to the review stage, not the implementation loop, so the refactor rule and `refactoring.md` were removed (its home is the `review` skill).
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"mattpocock-skills": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Add the **tautological test** anti-pattern to the `tdd` skill. Tests whose assertion is recomputed the way the code computes it pass by construction and give zero confidence — distinct from the implementation-coupling anti-pattern already covered. Added as a peer at the same three sites: a Philosophy principle (expected values must come from an independent source of truth), a per-cycle checklist gate, and a BAD/GOOD example pair in `tests.md`.
|
||||||
@@ -5,104 +5,32 @@ description: Test-driven development. Use when the user wants to build features
|
|||||||
|
|
||||||
# Test-Driven Development
|
# Test-Driven Development
|
||||||
|
|
||||||
## Philosophy
|
TDD is the red → green loop. This skill is the reference that makes that loop produce tests worth keeping: what a good test is, where tests go, the anti-patterns, and the rules of the loop. Every section applies on every cycle — consult them before and during the loop, not after.
|
||||||
|
|
||||||
**Core principle**: Tests should verify behavior through public interfaces, not implementation details. Code can change entirely; tests shouldn't.
|
When exploring the codebase, read `CONTEXT.md` (if it exists) so test names and interface vocabulary match the project's domain language, and respect ADRs in the area you're touching.
|
||||||
|
|
||||||
**Good tests** are integration-style: they exercise real code paths through public APIs. They describe _what_ the system does, not _how_ it does it. A good test reads like a specification - "user can checkout with valid cart" tells you exactly what capability exists. These tests survive refactors because they don't care about internal structure.
|
## What a good test is
|
||||||
|
|
||||||
**Bad tests** are coupled to implementation. They mock internal collaborators, test private methods, or verify through external means (like querying a database directly instead of using the interface). The warning sign: your test breaks when you refactor, but behavior hasn't changed. If you rename an internal function and tests fail, those tests were testing implementation, not behavior.
|
Tests verify behavior through public interfaces, not implementation details. Code can change entirely; tests shouldn't. A good test reads like a specification — "user can checkout with valid cart" tells you exactly what capability exists — and survives refactors because it doesn't care about internal structure.
|
||||||
|
|
||||||
See [tests.md](tests.md) for examples and [mocking.md](mocking.md) for mocking guidelines.
|
See [tests.md](tests.md) for examples and [mocking.md](mocking.md) for mocking guidelines.
|
||||||
|
|
||||||
## Anti-Pattern: Horizontal Slices
|
## Seams — where tests go
|
||||||
|
|
||||||
**DO NOT write all tests first, then all implementation.** This is "horizontal slicing" - treating RED as "write all tests" and GREEN as "write all code."
|
A **seam** is the public boundary you test at: the interface where you observe behavior without reaching inside. Tests live at seams, never against internals.
|
||||||
|
|
||||||
This produces **crap tests**:
|
**Test only at pre-agreed seams.** Before writing any test, write down the seams under test and confirm them with the user. No test is written at an unconfirmed seam. You can't test everything — agreeing the seams up front is how testing effort lands on the critical paths and complex logic instead of every edge case.
|
||||||
|
|
||||||
- Tests written in bulk test _imagined_ behavior, not _actual_ behavior
|
Ask: "What's the public interface, and which seams should we test?"
|
||||||
- You end up testing the _shape_ of things (data structures, function signatures) rather than user-facing behavior
|
|
||||||
- Tests become insensitive to real changes - they pass when behavior breaks, fail when behavior is fine
|
|
||||||
- You outrun your headlights, committing to test structure before understanding the implementation
|
|
||||||
|
|
||||||
**Correct approach**: Vertical slices via tracer bullets. One test → one implementation → repeat. Each test responds to what you learned from the previous cycle. Because you just wrote the code, you know exactly what behavior matters and how to verify it.
|
## Anti-patterns
|
||||||
|
|
||||||
```
|
- **Implementation-coupled** — mocks internal collaborators, tests private methods, or verifies through a side channel (querying the database instead of using the interface). The tell: the test breaks when you refactor but behavior hasn't changed.
|
||||||
WRONG (horizontal):
|
- **Tautological** — the assertion recomputes the expected value the way the code does (`expect(add(a, b)).toBe(a + b)`, a snapshot derived by hand the same way, a constant asserted equal to itself), so it passes by construction and can never disagree with the code. Expected values must come from an independent source of truth — a known-good literal, a worked example, the spec.
|
||||||
RED: test1, test2, test3, test4, test5
|
- **Horizontal slicing** — writing all tests first, then all implementation. Bulk tests verify _imagined_ behavior: you test the _shape_ of things rather than user-facing behavior, the tests go insensitive to real changes, and you commit to test structure before understanding the implementation. Work in **vertical slices** instead — one test → one implementation → repeat, each test a **tracer bullet** that responds to what the last cycle taught you.
|
||||||
GREEN: impl1, impl2, impl3, impl4, impl5
|
|
||||||
|
|
||||||
RIGHT (vertical):
|
## Rules of the loop
|
||||||
RED→GREEN: test1→impl1
|
|
||||||
RED→GREEN: test2→impl2
|
|
||||||
RED→GREEN: test3→impl3
|
|
||||||
...
|
|
||||||
```
|
|
||||||
|
|
||||||
## Workflow
|
- **Red before green.** Write the failing test first, then only enough code to pass it. Don't anticipate future tests or add speculative features.
|
||||||
|
- **One slice at a time.** One seam, one test, one minimal implementation per cycle.
|
||||||
### 1. Planning
|
- **Refactoring is not part of the loop.** It belongs to the review stage (see the `review` skill), not the red → green implementation cycle.
|
||||||
|
|
||||||
When exploring the codebase, read `CONTEXT.md` (if it exists) so that test names and interface vocabulary match the project's domain language, and respect ADRs in the area you're touching.
|
|
||||||
|
|
||||||
Before writing any code:
|
|
||||||
|
|
||||||
- [ ] Confirm with user what interface changes are needed
|
|
||||||
- [ ] Confirm with user which behaviors to test (prioritize)
|
|
||||||
- [ ] Identify opportunities for deep modules (small interface, deep implementation) — run the `/codebase-design` skill for the vocabulary and the testability checks
|
|
||||||
- [ ] List the behaviors to test (not implementation steps)
|
|
||||||
- [ ] Get user approval on the plan
|
|
||||||
|
|
||||||
Ask: "What should the public interface look like? Which behaviors are most important to test?"
|
|
||||||
|
|
||||||
**You can't test everything.** Confirm with the user exactly which behaviors matter most. Focus testing effort on critical paths and complex logic, not every possible edge case.
|
|
||||||
|
|
||||||
### 2. Tracer Bullet
|
|
||||||
|
|
||||||
Write ONE test that confirms ONE thing about the system:
|
|
||||||
|
|
||||||
```
|
|
||||||
RED: Write test for first behavior → test fails
|
|
||||||
GREEN: Write minimal code to pass → test passes
|
|
||||||
```
|
|
||||||
|
|
||||||
This is your tracer bullet - proves the path works end-to-end.
|
|
||||||
|
|
||||||
### 3. Incremental Loop
|
|
||||||
|
|
||||||
For each remaining behavior:
|
|
||||||
|
|
||||||
```
|
|
||||||
RED: Write next test → fails
|
|
||||||
GREEN: Minimal code to pass → passes
|
|
||||||
```
|
|
||||||
|
|
||||||
Rules:
|
|
||||||
|
|
||||||
- One test at a time
|
|
||||||
- Only enough code to pass current test
|
|
||||||
- Don't anticipate future tests
|
|
||||||
- Keep tests focused on observable behavior
|
|
||||||
|
|
||||||
### 4. Refactor
|
|
||||||
|
|
||||||
After all tests pass, look for [refactor candidates](refactoring.md):
|
|
||||||
|
|
||||||
- [ ] Extract duplication
|
|
||||||
- [ ] Deepen modules (move complexity behind simple interfaces)
|
|
||||||
- [ ] Apply SOLID principles where natural
|
|
||||||
- [ ] Consider what new code reveals about existing code
|
|
||||||
- [ ] Run tests after each refactor step
|
|
||||||
|
|
||||||
**Never refactor while RED.** Get to GREEN first.
|
|
||||||
|
|
||||||
## Checklist Per Cycle
|
|
||||||
|
|
||||||
```
|
|
||||||
[ ] Test describes behavior, not implementation
|
|
||||||
[ ] Test uses public interface only
|
|
||||||
[ ] Test would survive internal refactor
|
|
||||||
[ ] Code is minimal for this test
|
|
||||||
[ ] No speculative features added
|
|
||||||
```
|
|
||||||
|
|||||||
@@ -1,10 +0,0 @@
|
|||||||
# Refactor Candidates
|
|
||||||
|
|
||||||
After TDD cycle, look for:
|
|
||||||
|
|
||||||
- **Duplication** → Extract function/class
|
|
||||||
- **Long methods** → Break into private helpers (keep tests on public interface)
|
|
||||||
- **Shallow modules** → Combine or deepen
|
|
||||||
- **Feature envy** → Move logic to where data lives
|
|
||||||
- **Primitive obsession** → Introduce value objects
|
|
||||||
- **Existing code** the new code reveals as problematic
|
|
||||||
@@ -59,3 +59,19 @@ test("createUser makes user retrievable", async () => {
|
|||||||
expect(retrieved.name).toBe("Alice");
|
expect(retrieved.name).toBe("Alice");
|
||||||
});
|
});
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Tautological tests**: Expected value restates the implementation, so the test passes by construction.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// BAD: Expected value is recomputed the way the code computes it
|
||||||
|
test("calculateTotal sums line items", () => {
|
||||||
|
const items = [{ price: 10 }, { price: 5 }];
|
||||||
|
const expected = items.reduce((sum, i) => sum + i.price, 0);
|
||||||
|
expect(calculateTotal(items)).toBe(expected);
|
||||||
|
});
|
||||||
|
|
||||||
|
// GOOD: Expected value is an independent, known literal
|
||||||
|
test("calculateTotal sums line items", () => {
|
||||||
|
expect(calculateTotal([{ price: 10 }, { price: 5 }])).toBe(15);
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|||||||
@@ -14,12 +14,14 @@ Assets created during tickets should be linked to from the map, not duplicated w
|
|||||||
|
|
||||||
### Structure
|
### Structure
|
||||||
|
|
||||||
Numbered entries ("tickets"), each its own section keyed by its number:
|
Entries ("tickets"), each its own section keyed by a short dash-case slug that
|
||||||
|
reads as a mini-title (e.g. `relational-db`, `auth-strategy`, `cache-layer`) —
|
||||||
|
terse enough to stay token-efficient, and unique within the map.
|
||||||
|
|
||||||
```markdown
|
```markdown
|
||||||
## #1: Relational Or Non-Relational Database?
|
## relational-db: Relational Or Non-Relational Database?
|
||||||
|
|
||||||
Blocked by: #<ticket-number>, #<ticket-number>
|
Blocked by: <slug>, <slug>
|
||||||
Status: open | in-progress | resolved
|
Status: open | in-progress | resolved
|
||||||
Type: Research | Prototype | Grilling
|
Type: Research | Prototype | Grilling
|
||||||
|
|
||||||
@@ -32,7 +34,11 @@ Type: Research | Prototype | Grilling
|
|||||||
<answer-here>
|
<answer-here>
|
||||||
```
|
```
|
||||||
|
|
||||||
A ticket is **unblocked** when every ticket in its `Blocked by` list is `resolved`. A session **claims** its ticket by setting `Status: in-progress` and saving the map before any work, so concurrent sessions skip it.
|
The slug is the canonical id, used in every `Blocked by` edge and prose
|
||||||
|
reference; the title after the colon is optional. A ticket
|
||||||
|
is **unblocked** when every ticket in its `Blocked by` list is `resolved`. A
|
||||||
|
session **claims** its ticket by setting `Status: in-progress` and saving the map
|
||||||
|
before any work, so concurrent sessions skip it.
|
||||||
|
|
||||||
Each ticket must be sized to one 100K token agent session.
|
Each ticket must be sized to one 100K token agent session.
|
||||||
|
|
||||||
@@ -62,10 +68,10 @@ User invokes with a loose idea.
|
|||||||
|
|
||||||
### Work through the map
|
### Work through the map
|
||||||
|
|
||||||
User invokes with a path to an existing map. A ticket number is **optional** — without one, you pick the next decision, not the user.
|
User invokes with a path to an existing map. A ticket slug is **optional** — without one, you pick the next decision, not the user.
|
||||||
|
|
||||||
1. Load the **whole map** as context.
|
1. Load the **whole map** as context.
|
||||||
2. Choose the ticket. If the user named one, use it. Otherwise pick the lowest-numbered `open` ticket that is [unblocked](#structure). [Claim it](#structure): set `Status: in-progress` and save before any work.
|
2. Choose the ticket. If the user named one, use it. Otherwise pick the first `open` ticket in document order that is [unblocked](#structure). [Claim it](#structure): set `Status: in-progress` and save before any work.
|
||||||
3. Resolve it, invoking skills as needed. If in doubt, use `/grilling` and `/domain-modeling`.
|
3. Resolve it, invoking skills as needed. If in doubt, use `/grilling` and `/domain-modeling`.
|
||||||
4. Record the answer in the ticket's body and set `Status: resolved`.
|
4. Record the answer in the ticket's body and set `Status: resolved`.
|
||||||
5. Add newly-discovered tickets with correct `Blocked by` edges. If the decisions made invalidate other parts of the map, update or delete those nodes.
|
5. Add newly-discovered tickets with correct `Blocked by` edges. If the decisions made invalidate other parts of the map, update or delete those nodes.
|
||||||
@@ -79,7 +85,7 @@ End every session by clearing the context and opening one or more fresh sessions
|
|||||||
|
|
||||||
**Open tickets remain.** List the currently-unblocked tickets, then give two copy-paste options: a bare command for one session (you pick the next ticket), and one pinned command per unblocked ticket for running them in parallel. Paste one line per fresh window — opening one, some, or all of them.
|
**Open tickets remain.** List the currently-unblocked tickets, then give two copy-paste options: a bare command for one session (you pick the next ticket), and one pinned command per unblocked ticket for running them in parallel. Paste one line per fresh window — opening one, some, or all of them.
|
||||||
|
|
||||||
> **Next steps** — 3 tickets unblocked: #4, #5, #6.
|
> **Next steps** — 3 tickets unblocked: `auth-strategy`, `cache-layer`, `rate-limits`.
|
||||||
> Clear the context, then open fresh sessions.
|
> Clear the context, then open fresh sessions.
|
||||||
>
|
>
|
||||||
> **One session** — resolves the next unblocked ticket:
|
> **One session** — resolves the next unblocked ticket:
|
||||||
@@ -89,9 +95,9 @@ End every session by clearing the context and opening one or more fresh sessions
|
|||||||
>
|
>
|
||||||
> **Parallel** — paste one line per window, up to all 3:
|
> **Parallel** — paste one line per window, up to all 3:
|
||||||
> ```
|
> ```
|
||||||
> Invoke /decision-mapping with the map at <path>, ticket #4.
|
> Invoke /decision-mapping with the map at <path>, ticket auth-strategy.
|
||||||
> Invoke /decision-mapping with the map at <path>, ticket #5.
|
> Invoke /decision-mapping with the map at <path>, ticket cache-layer.
|
||||||
> Invoke /decision-mapping with the map at <path>, ticket #6.
|
> Invoke /decision-mapping with the map at <path>, ticket rate-limits.
|
||||||
> ```
|
> ```
|
||||||
|
|
||||||
**No open tickets remain.** The fog is pushed back far enough that the path to the finish line is clear — the map is done. (The initial grilling may also surface no fog at all, in which case there was never a map to build.) Recommend implementing directly, or using `/to-prd` to schedule a multi-session implementation.
|
**No open tickets remain.** The fog is pushed back far enough that the path to the finish line is clear — the map is done. (The initial grilling may also surface no fog at all, in which case there was never a map to build.) Recommend implementing directly, or using `/to-prd` to schedule a multi-session implementation.
|
||||||
|
|||||||
Reference in New Issue
Block a user