Przeglądaj źródła

feat(skills): absorb falsifiability discipline into writing-good-tests

Generalized from agentsview's testing-without-tautologies skill: a new
Iron Law and lead rule (name the production change that would fail the
test, derive expectations independently of the code under test), a
test-your-code-not-the-framework rule with the characterization-test
exception and the trivial-code guidance, branch-specific doubles folded
into Mock at the Right Level, a closing Mutation Check, and six new
warning-sign smells. Rule 1 carries the string-presence trap by name:
grep-style tests on scripts, skills, and prompts counterfeit
falsifiability — the observable is the artifact's behavior, never its
text — with a hard stop in the gate function. Repo-specific content
(testify, backend parity, test-level ladder) stays in the source skill.
Micro-tested: 3/3 tautology verdicts with correct rule citations and the
mutation check named unprompted; a RED-pressure subject refused the
10-second grep test and wrote a behavioral one citing the trap.
Jesse Vincent 2 miesięcy temu
rodzic
commit
6a2d0c211f

+ 1 - 0
skills/test-driven-development/SKILL.md

@@ -204,6 +204,7 @@ Next failing test for next feature.
 | **Shows intent** | Demonstrates desired API | Obscures what code should do |
 
 When writing or changing any test, read [writing-good-tests.md](writing-good-tests.md) for the rules that keep tests honest:
+- Name the production change that would make the test fail — before writing it
 - Assert on real behavior, never on mock behavior
 - Keep test-only code in test utilities, out of production classes
 - Understand a dependency's side effects before mocking it

+ 144 - 13
skills/test-driven-development/writing-good-tests.md

@@ -8,22 +8,86 @@ adding cleanup/helper methods for tests.
 Good tests verify real behavior. Mocks exist to isolate the code under
 test — they are never the thing being tested.
 
-**Core principle:** Test what the code does, not what the mocks do.
+**Core principle:** Test what the code does, not what the mocks do — and
+make every test able to fail.
 
 Strict TDD produces every rule below naturally: a test written first and
-watched failing against real code only earns a mock when the real
-dependency proves slow or external. A test asserting on a mock means TDD
-was skipped somewhere.
+watched failing against real code has already proven it can fail, and
+only earns a mock when the real dependency proves slow or external. A
+test asserting on a mock means TDD was skipped somewhere.
 
 ## The Iron Laws
 
 ```
-1. Assert on real behavior, never on mock behavior
-2. Production classes carry production methods only
-3. Understand a dependency's side effects before mocking it
+1. Every test can fail — name the production change that would fail it
+2. Assert on real behavior, never on mock behavior
+3. Production classes carry production methods only
+4. Understand a dependency's side effects before mocking it
 ```
 
-## Rule 1: Assert on Real Behavior
+## Rule 1: Write Tests That Can Fail
+
+Before writing or changing a test, name the production change that would
+make it fail. If you cannot, redesign the test around an observable
+behavior — a test that cannot fail protects nothing.
+
+Derive expected values independently of the code under test: literals,
+hand-checked fixtures, small worked examples, or invariant assertions.
+Keep test logic simple enough to review by inspection — table-driven
+tests with literal `want` values are the preferred shape.
+
+```typescript
+// ✅ GOOD: literal, hand-derived expectation
+test('builds tag query', () => {
+  expect(buildSearchQuery({ tag: 'urgent' })).toBe('tag:"urgent"');
+});
+```
+
+```typescript
+// ❌ The violation: expectation computed by the logic under test
+test('builds tag query', () => {
+  const expected = buildSearchQuery({ tag: 'urgent' });  // same builder!
+  expect(buildSearchQuery({ tag: 'urgent' })).toBe(expected);  // always true
+});
+
+// ❌ Subtler: the expectation reuses the same helper the code calls
+test('formats timestamp', () => {
+  expect(render(entry)).toContain(formatTime(entry.ts));  // mirrors implementation
+});
+```
+
+A mirror assertion re-derives the answer with the answer's own machinery:
+it passes no matter what that machinery does.
+
+**The string-presence trap.** For a script, skill, prompt, or config, a
+test that asserts the source contains an exact line counterfeits this
+rule: it can fail (delete the line), so it passes the letter of
+falsifiability while asserting only that the source is the source. It
+breaks on every legitimate rewording and survives every real regression.
+The observable for a script is what it does — run it against controlled
+inputs and assert outputs, side effects, or exit codes. The observable
+for a document that instructs an agent is the consuming agent's behavior
+— pressure-test it. Text containment is never the observable.
+
+### Gate Function
+
+```
+BEFORE writing the test body:
+  Ask: "What production change should make this test fail?"
+
+  IF you cannot name one:
+    STOP - Redesign the test around an observable behavior
+
+  IF the only answer is "the source text changed":
+    STOP - Run the artifact and assert its effects instead
+
+  Ask: "Is the expected value derived independently of the code under test?"
+
+  IF it reuses the code's own logic or helpers:
+    STOP - Replace it with a literal or hand-checked fixture
+```
+
+## Rule 2: Assert on Real Behavior
 
 ```typescript
 // ✅ GOOD: Test the real component
@@ -60,7 +124,7 @@ BEFORE asserting on any mock element:
   Test real behavior instead
 ```
 
-## Rule 2: Keep Test Cleanup in Test Utilities
+## Rule 3: Keep Test Cleanup in Test Utilities
 
 ```typescript
 // ✅ GOOD: Test utilities own test cleanup
@@ -110,12 +174,18 @@ BEFORE adding any method to a production class:
     STOP - Wrong class for this method
 ```
 
-## Rule 3: Mock at the Right Level
+## Rule 4: Mock at the Right Level
 
 Learn what the real method does — every side effect — before replacing
 it. Mock the slow or external operation and preserve the behavior your
 test depends on.
 
+Make doubles specific to their contract: when arguments, call counts, or
+ordering matter, assert them — a fake that accepts anything verifies
+nothing. And give each branch its own double: success, error, and
+malformed paths each get their own fixture or spy, so the wrong branch
+cannot satisfy the expectation.
+
 ```typescript
 // ✅ GOOD: Mock the slow part, preserve behavior the test needs
 test('detects duplicate server', () => {
@@ -165,7 +235,7 @@ BEFORE mocking any method:
     - Mocking before tracing the dependency chain
 ```
 
-## Rule 4: Mirror Real Data Completely
+## Rule 5: Mirror Real Data Completely
 
 Mock the COMPLETE data structure as it exists in reality, not just the
 fields your immediate test uses.
@@ -209,13 +279,50 @@ BEFORE creating mock responses:
   If uncertain: include all documented fields
 ```
 
-## Rule 5: Tests Ship With the Implementation
+## Rule 6: Test Your Code, Not the Framework
+
+Test the contract your code makes at its boundaries — the route you
+register, the query you emit, the payload shape you produce, the value
+handoff between layers. Dependencies' documented mechanics are their
+maintainers' tests to write.
+
+```typescript
+// ✅ GOOD: your contract at the boundary
+test('GET /sessions/:id returns 404 for unknown id', async () => {
+  const res = await request(app).get('/sessions/nope');
+  expect(res.status).toBe(404);
+  expect(res.body.error).toBe('session not found');
+});
+```
+
+```typescript
+// ❌ The violation: re-proving the router works as documented
+test('router calls handler for matching route', () => {
+  const handler = vi.fn();
+  router.get('/x', handler);
+  router.handle(makeRequest('/x'));
+  expect(handler).toHaveBeenCalled();
+});
+```
+
+When upstream behavior genuinely surprised you (a quoting rule, an event
+ordering), write one narrow characterization test around your integration
+point and name the assumption in the test name or a comment.
+
+The same boundary applies inside your own code: test behavior, not that
+the implementation is written the way it is currently written. Plain
+constructor assignment, getters, trivial forwarding, and data-only
+structs 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.
+
+## Rule 7: Tests Ship With the Implementation
 
 Testing is part of implementation. The TDD cycle — failing test, minimal
 implementation, refactor — is what "complete" means; "implementation
 complete, ready for testing" describes an unfinished task.
 
-## Rule 6: Prefer Real Components Over Complex Mocks
+## Rule 8: Prefer Real Components Over Complex Mocks
 
 Integration tests with real components are often simpler than elaborate
 mocks. Reach for one when you see:
@@ -227,15 +334,33 @@ mocks. Reach for one when you see:
 
 **your human partner's question:** "Do we need to be using a mock here?"
 
+## 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 (row not written, event not emitted)
+- Empty or default return
+- Missing validation for zero, empty, nil, unauthorized, or malformed input
+
+A mutation no test can catch marks the behavior as unprotected — or the
+test as tautological.
+
 ## Quick Reference
 
 | When you... | Do |
 |-------------|-----|
+| Write any test | Name the production change that would make it fail |
+| Build an expected value | Derive it independently — literal or hand-checked fixture |
 | Want to assert on a mocked element | Test the real component, or unmock it |
 | Need cleanup that only tests use | Put it in test utilities |
 | Are about to mock a method | Learn its side effects first; mock the slow/external level |
 | Build a mock response | Mirror the real structure completely |
+| Reach for a dependency test | Test your boundary contract, not their documented mechanics |
 | Finish an implementation | Tests already exist (TDD) — or it is unfinished |
+| Finish a test file | Run the mutation check |
 | Watch mock setup balloon | Switch to an integration test with real components |
 
 ## Warning Signs
@@ -246,3 +371,9 @@ mocks. Reach for one when you see:
 - The test fails when you remove the mock
 - You can't explain why the mock is needed
 - Mocking "just to be safe"
+- Setup and assertion share the same object, guaranteeing equality
+- The test can fail only through a panic, crash, or missing selector
+- The test would still matter if only the framework remained
+- Expected values are hidden behind loops, builders, or helpers
+- The test greps source text instead of observing behavior
+- The test asserts that a removed function, file, or symbol stays removed