Choose the branch before selecting a model. Coordinator work and deterministic commands proceed within invocation authority. Apply this method in the current role. It does not dispatch a worker. For a worker dispatch, use shared/references/model-preview.md and its one approved route snapshot. Nested skills and runners consume it without another selection or worker.
Judge or write tests through the framework in Unit Testing: Principles, Practices, and Patterns (Vladimir Khorikov). This is not the red-green loop (apply tdd for cadence) and not a bug hunt (that belongs in a code-review pass). It answers one question: is this test worth its maintenance cost, and does it test the right thing the right way?
The goal of a test suite is sustainable project growth — letting you add features and refactor without fear. A small number of highly valuable tests beats a large number of mediocre ones. Test code is a liability too. Set a high bar; delete tests that don't clear it.
Examples below are TypeScript + Jest. Apply the lens, then report findings concretely: cite file:line, name the rule, propose the fix. If a test is already good, say so plainly.
The core rubric: the four pillars
Every test is scored 0–1 on four attributes; value = the four multiplied. A zero on any pillar makes the test worthless — even if it's perfect on the other three.
- Protection against regressions — how much code (yours + libraries) the test exercises, weighted by complexity and domain significance. Trivial code → low protection.
- Resistance to refactoring — how few false positives (failures when behavior is intact) it produces. Driven entirely by coupling to implementation details. Non-negotiable.
- Fast feedback — how quickly it runs.
- Maintainability — how hard it is to read and to run (setup, out-of-process deps).
Key tension: the first three are mutually exclusive — you can't maximize all three. Since resistance-to-refactoring must stay high, you trade protection ⇄ fast feedback. The three classic failure modes, each maxing two pillars and zeroing a third:
| Test type | Sacrifices | Why it's low-value |
|---|---|---|
| End-to-end only | Fast feedback | Great coverage + resilient, but too slow to be the whole suite |
Trivial (expect(user.name).toBe('John') on a plain getter) |
Protection | Nothing to catch |
| Brittle (asserts SQL string / call order / private structure) | Resistance to refactoring | Breaks on every refactor → ignored → real bugs slip through |
Brittle tests are the dangerous ones. They train the team to ignore failures, then a real regression rides along with the noise into production.
Lens 1 — Observable behavior, not implementation details
The single most important rule. False positives come from coupling a test to how the code works instead of what it produces. Verify the end result meaningful to a domain expert / end user — disregard the steps taken to get there.
Red flags (all couple to implementation):
- Asserting which internal methods were called, in what order, or how many times (on a stub).
- Asserting a generated SQL string, the list/types of internal collaborators, or private state.
- A test that mirrors the production algorithm step-for-step ("leaking domain knowledge").
- A test you must edit every time you rename a private method or reshuffle internals.
A good test tells a story about the problem domain. If it fails, the story and the code disagree — and that's the only failure worth your attention.
Lens 2 — Pick the right style (ranked)
| Style | Verifies | Quality | Prefer when |
|---|---|---|---|
| Output-based | return value of a pure function | Best — naturally resists refactoring | always, if the code allows |
| State-based | object/system state after the act | OK — can tie to leaking state | mutation is the actual outcome |
| Communication-based | calls to collaborators (mocks) | Most brittle | only at the system edge, for unmanaged deps |
Push code toward output-based by separating decisions from actions: a functional core (pure business logic, easy to output-test) wrapped in a mutable shell (thin, does I/O). This is hexagonal architecture taken to its extreme. See references/decision-trees.md.
Lens 3 — Mocks: stub vs mock, managed vs unmanaged
The rules agents most often get wrong. Full flowchart in references/decision-trees.md.
- Stub = emulates an incoming interaction (input data the SUT reads). Never assert calls to a stub — that's overspecification and the #1 source of fragile tests.
- Mock = emulates an outcoming interaction (a side effect the SUT causes). Asserting it is fine only when that side effect is itself the observable outcome.
- Managed dependency (you fully control it; only your app touches it — e.g. your DB): interactions are implementation details → use the real thing in integration tests.
- Unmanaged dependency (externally observable — SMTP, message bus, third-party API): interactions are observable behavior / a contract → mock it.
- Mock only at the system edge, and only types you own — wrap third-party SDKs in your own adapter and mock the adapter, not the library.
Lens 4 — What to test at all (the code quadrant)
Classify the code under test on two axes — complexity/domain significance × number of collaborators:
| Few collaborators | Many collaborators | |
|---|---|---|
| High complexity/significance | Domain model / algorithms → unit-test hard (best ROI) | Overcomplicated → refactor: split into the two below |
| Low | Trivial → don't test | Controllers → a few integration tests |
The more important or complex the code, the fewer collaborators it should have.
The overcomplicated quadrant (e.g. a fat controller doing real logic and I/O) is the trap. Split it with the Humble Object pattern: extract the logic into a pure algorithm/domain class (unit-test it output-based), leaving a thin humble wrapper that just glues to dependencies (cover lightly via integration tests). 100% coverage is not the goal — it's possible to have high coverage and worthless tests (and vice versa); significant value per test is. Better no test than a bad test.
Anti-pattern checklist (flag these)
- Testing private methods → a private method doing complex work is a missing abstraction; extract a class.
- Exposing private state just to assert on it → test observable behavior instead.
- Leaking domain knowledge → test hardcodes the algorithm's expected intermediate math.
- Code pollution → test-only switches/flags in production code.
- Mocking concrete classes to keep half their behavior → SRP violation; split the class.
- Time as ambient context (
Date.now()reached directly) → inject time as an explicit dependency. - Asserting interactions with stubs, or chasing a coverage number as the target.
Mechanics (when writing tests)
- AAA — Arrange / Act / Assert, one of each; if you need multiple act sections it's probably an integration test. No
ifstatements in a test (split it). Keep the act to one line for a unit of behavior. - Name the test as a domain statement of behavior —
delivery_with_a_past_date_is_invalid, notIsValid_PastDate_ReturnsFalse. No method names in the title; a non-programmer should read it. - Parameterize similar cases (
it.each) — but keep distinct behaviors as separate tests. - Reuse fixtures via factory functions, not shared mutable state in
beforeEach(high coupling between tests is an anti-pattern).
Workflow
- Whose test is this? New code → guide style choice (Lens 2/4) before writing. Existing test under review → score it.
- Score the four pillars. Name the weakest. If any is ≈0, recommend rewrite or deletion.
- Apply the lenses that fit: observable-behavior (always), mock rules (if it has doubles), quadrant (if deciding what/whether to test).
- Run the anti-pattern checklist.
- Report concretely per finding:
file:line→ which rule → the fix. Skip lenses that don't apply rather than padding.
For the detailed decision trees (style transition, mock flowchart, Humble Object refactor, integration-test rules — path selection, logging, pyramid — and the full anti-pattern catalog with before/after code), read references/decision-trees.md.
Failure sequences and fixture boundaries
For stateful or asynchronous changes, derive a small case table from the actual
state machine before the first review. Include meaningful event orders and retained
invariants, not just each handler once. Carry a prior defect case into a related
slice when the same causal path exists. Read advanced-react's async reference
when editor cancellation, replacement, scope changes, or late responses apply.
A passing fixture proves only the boundary it exercises. Check production adapters with representative stored values, including absent, legacy, and malformed values when the schema permits them. Use actual transactions for lock and rollback claims. Do not let a fixture create a stronger schema or cleaner data contract than production. A default or fallback for bad data must follow the product contract.
Separate model tests, DOM interaction tests, partial application browser checks, and full application checks in the acceptance report. A DOM test or screenshot cannot establish deployed authorization. Test counts cannot establish completeness.