diff options
Diffstat (limited to 'claude-rules')
| -rw-r--r-- | claude-rules/commits.md | 23 | ||||
| -rw-r--r-- | claude-rules/testing.md | 381 | ||||
| -rw-r--r-- | claude-rules/todo-format.md | 3 |
3 files changed, 42 insertions, 365 deletions
diff --git a/claude-rules/commits.md b/claude-rules/commits.md index e3f75a8..51c452a 100644 --- a/claude-rules/commits.md +++ b/claude-rules/commits.md @@ -91,6 +91,29 @@ Edge case: when one of these files *is* the change (a commit in the rulesets rep **Tooling-path enumeration is the same leak.** Citing a rule as authority isn't the only way the tooling layer leaks into history. A commit whose *content* must name these paths — a `.gitignore` adding `.claude/`, `CLAUDE.md`, `.ai/` — has unavoidable, correct file content, but its *message prose* must not enumerate them ("chore: ignore .claude tooling, CLAUDE.md, and session files"). On a public or shared-remote repo that enumeration exposes the tooling layer's structure in the log just as a citation would. Name the category instead: "chore: extend gitignore for local tooling and build artifacts". The same holds for any incidental mention, not only `.gitignore` commits. Two exemptions: a commit whose change *is* one of these files (the edge case above), and private single-user repos with no shared remote, where the history is the project and there's no third party to leak to. +## Write in the first person + +Everything I author in or about this repo is first person: code comments, +commit messages, PR descriptions, PR review comments, and any note that lands +in the repo or its history. When I made a choice, say so as a choice — "I swept +the copies rather than repairing them, because a drifted copy outranked the +global rule" beats "the copies are swept rather than repaired." + +Third-person constructions like "This change introduces X" or "This PR restores +Y" read as press-release self-narration. The commit is the change, so it does +not need announcing. + +**The one carve-out: code is the actor when describing behavior.** A comment +saying *what the code does* stays third person, because the subject genuinely +is the code and not me — "the sweep only fires when the global rule exists", +"the guard rejects a malformed payload". First person is for the decision +behind it, third person for the behavior itself. Both often belong in the same +comment: what it does, then why I chose it. + +This is the rule the publish flow already applied to commit bodies. It lives +here because code comments get written constantly and the publish skill is not +loaded then. + ## The publish flow lives in the `publish` skill Everything about *how* a commit, PR, or review comment gets written, reviewed, diff --git a/claude-rules/testing.md b/claude-rules/testing.md index 81bd391..dd15282 100644 --- a/claude-rules/testing.md +++ b/claude-rules/testing.md @@ -16,377 +16,30 @@ TDD is the default workflow for all code, including demos and prototypes. **Writ Do not skip TDD for demo code. Demos build muscle memory — the habit carries into production. -### Understand Before You Test -Before writing tests, invest time in understanding the code: +## Test Categories — required for all code -1. **Explore the codebase** — Read the module under test, its callers, and its dependencies. Understand the data flow end to end. -2. **Identify the root cause** — If fixing a bug, trace the problem to its origin. Don't test (or fix) surface symptoms when the real issue is deeper in the call chain. -3. **Reason through edge cases** — Consider boundary conditions, error states, concurrent access, and interactions with adjacent modules. Your tests should cover what could actually go wrong, not just the obvious happy path. +Every unit under test needs all three, not just the happy path: -### Adding Tests to Existing Untested Code +1. **Normal** — standard inputs, common workflows, typical volumes. +2. **Boundary** — zero, one, max, empty vs null, single-element collections, unicode, very long input, timezone and date edges. +3. **Error** — invalid input, type mismatches, network failure, missing parameters, permission denied, resource exhaustion, malformed data. -When working in a codebase without tests: +The negative and boundary cases are the ones that find bugs. A unit with only +Normal coverage is not tested, it is demonstrated. -1. Write a **characterization test** that captures current behavior before making changes -2. Use the characterization test as a safety net while refactoring -3. Then follow normal TDD for the new change +## The rest of the standard lives in the `testing-standards` skill -A characterization test asserts what the code *actually does* right now, not -what it *should* do. Write it by running the code against a fixed input, -reading the exact value or effect it currently produces, and asserting that -value — Feathers' recipe is to assert something you know is wrong, run it, and -paste the real value out of the failure. You don't need to know the correct -answer to write one; you record the observed one. That's what makes it -mechanical enough to bring a large untested surface under test without -re-deriving each unit's spec. +Characterization tests for untested code, the per-category detail, combinatorial +and property-based and mutation testing, organization and the pyramid, +integration-test rules, naming, the test-quality rules (independence, +determinism, mocking boundaries, signs of overmocking), the +refactor-when-tests-are-hard principle, coverage targets, the spike exception, +and the anti-pattern list are all in the `testing-standards` skill. Load it when +writing tests. -**Characterize with the same Normal/Boundary/Error set as any unit** (the three -categories below), not one happy-path capture per function. On a characterization -test the negative and boundary cases are the ones that find bugs: untested legacy -code is weakest exactly at the empty input, the malformed value, the missing -upstream, and pinning what it *currently* does there writes the wrong behavior -down in black and white, where it becomes a bug you can see and decide on. When a -pinned case turns out to be a bug rather than behavior worth preserving, that one -test graduates from "record current" to "assert correct" and you fix the code. -The happy-path case is the regression net; the negative and boundary cases are -the audit. - -Bugs that live *inside* a unit are caught by this three-category set; bugs in how -units compose — ordering, shared state handed between them — are invisible to any -per-unit test and need a functional/integration test over the composed path (see -Integration Tests below and the pyramid). - -## Test Categories (Required for All Code) - -Every unit under test requires coverage across three categories: - -### 1. Normal Cases (Happy Path) -- Standard inputs and expected use cases -- Common workflows and default configurations -- Typical data volumes - -### 2. Boundary Cases -- Minimum/maximum values (0, 1, -1, MAX_INT) -- Empty vs null vs undefined (language-appropriate) -- Single-element collections -- Unicode and internationalization (emoji, RTL text, combining characters) -- Very long strings, deeply nested structures -- Timezone boundaries (midnight, DST transitions) -- Date edge cases (leap years, month boundaries) - -### 3. Error Cases -- Invalid inputs and type mismatches -- Network failures and timeouts -- Missing required parameters -- Permission denied scenarios -- Resource exhaustion -- Malformed data - -## Combinatorial Coverage - -For functions with 3+ parameters that each take multiple values (feature-flag -combinations, config matrices, permission/role interactions, multi-field -form validation, API parameter spaces), the exhaustive test count explodes -(M^N) while 3-5 ad-hoc cases miss pair interactions. Use **pairwise / -combinatorial testing** — generate a minimal matrix that hits every 2-way -combination of parameter values. Empirically catches 60-90% of combinatorial -bugs with 80-99% fewer tests. - -Invoke `/pairwise-tests` on the offending function; continue using `/add-tests` -and the Normal/Boundary/Error discipline for the rest. The two approaches -complement: pairwise covers parameter *interactions*; category discipline -covers each parameter's individual edge space. - -Skip pairwise when: the function has 1-2 parameters (just write the cases), -the context requires *provably* exhaustive coverage (regulated systems — document -in an ADR), or the testing target is non-parametric (single happy path, -performance regression, a specific error). - -## Escalation Beyond Category and Pairwise - -The Normal/Boundary/Error categories and the pairwise matrix are the default -discipline. Two further techniques escalate beyond them — reach for them when -the default leaves a gap, not on every unit. - -### Property-Based Testing - -When an invariant holds across a broad input domain — round-trips -(`decode(encode(x)) == x`), idempotence (`f(f(x)) == f(x)`), ordering -invariants (output is always sorted), or any "output always satisfies X" — -generate inputs and assert the property instead of enumerating cases. The -generator explores corners you wouldn't think to write by hand, and a -failing case shrinks to a minimal reproducer. Use the standard tool for the -language (Hypothesis for Python, fast-check for JS, proptest for Rust). -State the property as the test name and let the framework supply the inputs. - -Reach for this when the behavior is a law over a domain rather than a fixed -set of examples. Keep category-discipline cases for the specific edges that -must always hold; the property test covers the space between them. - -### Mutation Testing - -When line coverage is high but you suspect the assertions are thin — tests -that execute the code without checking its output, or that pass with a -function body replaced by a stub — use mutation testing to measure whether -the suite actually kills injected faults. The tool flips conditionals, swaps -operators, and deletes statements, then reruns the suite; a surviving mutant -is a fault the tests didn't catch. Use mutmut or cosmic-ray for Python, -Stryker for JS. High line coverage with a low mutation score means weak -assertions, not a tested codebase. - -Reach for this on critical logic where coverage looks reassuring but you -want evidence the tests would fail on a regression. It's a diagnostic, not a -gate on every change — mutation runs are slow. - -## Test Organization - -Typical layout: - -``` -tests/ - unit/ # One test file per source file - integration/ # Multi-component workflows - e2e/ # Full system tests -``` - -Per-language files may adjust this (e.g. Elisp collates ERT tests into -`tests/test-<module>*.el` without subdirectories). - -### Testing Pyramid - -Rough proportions for most projects: -- Unit tests: 70-80% (fast, isolated, granular) -- Integration tests: 15-25% (component interactions, real dependencies) -- E2E tests: 5-10% (full system, slowest) - -Don't duplicate coverage: if unit tests fully exercise a function's logic, -integration tests should focus on *how* components interact — not repeat the -function's case coverage. - -## Integration Tests - -Integration tests exercise multiple components together. Two rules: - -**The docstring names every component integrated** and marks which are real vs -mocked. Integration failures are harder to pinpoint than unit failures; -enumerating the participants up front tells you where to start looking. - -Example: - -``` -def test_integration_refund_during_sync_updates_ledger_atomically(): - """Refund processed mid-sync updates order and ledger in one transaction. - - Components integrated: - - OrderService.refund (entry point) - - PaymentGateway.reverse (MOCKED — returns success) - - Ledger.credit (real) - - db.transaction (real) - - Validates: - - Refund rolls back if ledger write fails - - Both tables updated or neither - """ -``` - -**Write an integration test when** multiple components must work together, -state crosses function boundaries, or edge cases combine. **Don't** when -single-function behavior suffices, or when mocking would erase the interaction -you meant to test. - -## Naming Convention - -- Unit: `test_<module>_<function>_<scenario>_<expected>` -- Integration: `test_integration_<workflow>_<scenario>_<outcome>` - -Examples: -- `test_cart_apply_discount_expired_coupon_raises_error` -- `test_integration_order_sync_network_timeout_retries_three_times` - -Languages that prefer camelCase, kebab-case, or other conventions keep the -structure but use their idiom. Consistency within a project matters more than -the specific case choice. - -## Test Quality - -### Independence -- No shared mutable state between tests -- Each test runs successfully in isolation -- Explicit setup and teardown - -### Determinism -- Never hardcode dates or times — generate them relative to `now()` -- No reliance on test execution order -- No flaky network calls in unit tests -- Time/clock-mocking helpers must avoid two recurring failure modes: - - *Infinite recursion.* The helper must not call the primitive it's - replacing. If the mock for `now()` calls `now()`, the test stack - overflows. Compute the mock value from a fixed source (a captured - instant, an injected fake clock). - - *Scope-shadowing without reach.* A mock that only exists inside - the test function won't affect production code that reads the - symbol through its canonical path. Replace the symbol at its - definition site (monkey-patch the module attribute in Python, - redefine the global in Lisp, swap the package-level binding in - Go, replace the named export in JavaScript) — or inject a fake - via dependency-inversion. Don't lean on scope-shadowing - primitives (Lisp `let`, Python local rebind, JS shadowed `let`) - that fence the mock to the test's lexical scope; production code - won't see them and the test passes against the real clock. - -### Performance -- Unit tests: <100ms each -- Integration tests: <1s each -- E2E tests: <10s each -- Mark slow tests with appropriate decorators/tags - -### Mocking Boundaries -Mock external dependencies at the system boundary: -- Network calls (HTTP, gRPC, WebSocket) -- File I/O and cloud storage -- Time and dates -- Third-party service clients - -Never mock: -- The code under test -- Internal domain logic -- Framework behavior (ORM queries, middleware, hooks, buffer primitives) - -### Signs of Overmocking - -Ask yourself: - -- Would this test still pass if I replaced the function body with `raise NotImplementedError` (or equivalent)? If yes, the mocks are doing the work — you're testing mocks, not code. -- Is the mock more complex than the function being tested? Smell. -- Am I mocking internal string / parsing / decoding helpers? Those aren't boundaries — they're the work. -- Does the test break when I refactor without changing behavior? Good tests survive refactors; overmocked ones couple to implementation. - -When tests demand heavy internal mocking, the fix isn't better mocks — it's -restructuring the code (see *If Tests Are Hard to Write* below). - -### Testing Code That Uses Frameworks - -When a function mostly delegates to framework or library code, test *your* -integration logic: -- ✓ "I call the library with the right arguments in the right context" -- ✓ "I handle its return value correctly" -- ✗ "The library works in 50 scenarios" — trust it; it has its own tests - -For polyglot behavior (e.g., comment handling across C/Java/Go/JS), test 2-3 -representative modes thoroughly plus a minimal smoke test in the others. -Exhaustive permutations are diminishing returns. - -### Test Real Code, Not Copies - -Never inline or copy production code into test files. Always `require`/`import` -the module under test. Copied code passes even when production breaks — the -bug hides behind the duplicate. - -Mock dependencies at their boundary; exercise the real function body. - -### Error Behavior, Not Error Text - -Test that errors occur with the right type; don't assert exact wording: -- ✓ Right exception type (`pytest.raises(ValueError)`, `(should-error ... :type 'user-error)`) -- ✓ Regex on values the message *must* contain (e.g., the offending filename) -- ✗ `assert str(e) == "File 'foo' not found"` — breaks when prose changes even though behavior is unchanged - -Production code should emit clear, contextual errors. Tests verify the -behavior (raised, caught, returned nil) and values that must appear — not the -prose. - -## If Tests Are Hard to Write, Refactor the Code - -If a test needs extensive mocking of internal helpers, elaborate fixture -scaffolding, or mocks that recreate the function's own logic, the production -code needs restructuring — not the test. - -Signals: -- Deep nesting (callbacks inside callbacks) -- Long functions doing multiple things ("fetch AND parse AND decode AND save") -- Tests that mock internal string / parsing / I/O helpers -- Tests that break on refactors with no behavior change - -Fix: extract focused helpers (one responsibility each), test each in isolation -with real inputs, compose them in a thin outer function. Several small unit -tests plus one composition test beats one monster test behind a wall of mocks. - -When the untestable function is legacy code you're hardening, this extraction -**is** the hardening — not a detour around it. A function whose boundary or -error case can't be exercised without mocking the world (a shell function that -calls `tmux`/`git` directly, a handler that reaches straight into I/O) can't be -characterized, so you can't refactor it safely and you can't pin its edge -behavior. Extracting the pure decision logic into a helper that takes plain -inputs and returns a plain result makes that logic characterizable with the full -Normal/Boundary/Error set; the I/O calls become a thin wrapper you cover once -with a single composition test. "It needs too much mocking to test" is therefore -never a reason to skip the boundary and error cases — it's the signal to reshape -the function so those cases are writable. - -## Coverage Targets - -- Business logic and domain services: **90%+** -- API endpoints and views: **80%+** -- UI components: **70%+** -- Utilities and helpers: **90%+** -- Overall project minimum: **80%+** - -New code must not decrease coverage. PRs that lower coverage require justification. - -## TDD Discipline - -TDD is non-negotiable. These are the rationalizations agents use to skip it — don't fall for them: - -| Excuse | Why It's Wrong | -|--------|----------------| -| "This is too simple to need a test" | Simple code breaks too. The test takes 30 seconds. Write it. | -| "I'll add tests after the implementation" | You won't, and even if you do, they'll test what you wrote rather than what was needed. Test-after validates implementation, not behavior. | -| "Let me just get it working first" | That's not TDD. If you can't write a failing test, you don't understand the requirement yet. | -| "This is just a refactor" | Refactors without tests are guesses. Write a characterization test first, then refactor while it stays green. | -| "I'm only changing one line" | One-line changes cause production outages. Write a test that covers the line you're changing. | -| "The existing code has no tests" | Start with a characterization test. Don't make the problem worse. | -| "This is demo/prototype code" | Demos build habits. Untested demo code becomes untested production code. | -| "I need to spike first" | Spikes are fine — under the protocol below. Throw the spike away, then write the first failing test before productionizing. | - -If you catch yourself thinking any of these, stop and write the test. - -### The Spike Exception (Disciplined) - -TDD stays the default. The one sanctioned way to write code before a test is -a spike — exploratory code that answers "is this approach even viable?" when -you can't yet write a meaningful failing test because the shape of the -solution is unknown. A spike is disciplined only when all three hold: - -1. **Timebox it.** Set a limit before starting (an hour, an afternoon) and - stop when it's up. An open-ended spike is just untested implementation - wearing a different name. -2. **Do not commit spike code.** The spike is a learning artifact, not a - deliverable. It never enters the branch history. Keep it in a scratch - file or a throwaway worktree. -3. **Throw the spike away, then start with a failing test.** Once the spike - has answered the viability question, delete it. Write the first failing - test against the now-understood behavior, then productionize under normal - Red/Green/Refactor. The production code is written test-first even though - the exploration wasn't — you don't promote the spike into production by - bolting tests on after. - -The spike buys understanding, not code. If you find yourself keeping the -spike because rewriting it feels wasteful, the timebox was too long or the -problem was tractable enough to TDD from the start. - -## Anti-Patterns (Do Not Do) - -- Hardcoded dates or timestamps (they rot) -- Testing implementation details instead of behavior -- Mocking the thing you're testing -- Mocking internal helpers (string ops, parsing, decoding) — those are the work -- Inlining production code into test files — always `require` / `import` the real module -- Asserting exact error-message text instead of type + key values -- Shared mutable state between tests -- Non-deterministic tests (random without seed, network in unit tests) -- Testing framework behavior instead of your code -- Ignoring or skipping failing tests without a tracking issue +What stays here is what has to be true before any code is written, which is when +no skill has been summoned yet: test first, and cover all three categories. ## Content scope diff --git a/claude-rules/todo-format.md b/claude-rules/todo-format.md index 58570b1..8038b98 100644 --- a/claude-rules/todo-format.md +++ b/claude-rules/todo-format.md @@ -139,7 +139,8 @@ measurable one: against the list, so "covered" is checkable where "found everything" isn't. 2. **Net the behavior.** Bring the surface under characterization tests - (Normal/Boundary/Error per unit — see `testing.md`) before changing + (Normal/Boundary/Error per unit — see `testing.md`, and the + `testing-standards` skill for the characterization recipe) before changing anything. This is the objective floor: writing a characterization test is mechanical (record what the code does, not what it should), so it scales across the surface, and it doubles as the safety net that makes any |