agentleFS
shalombshalomb/agent-skills/skills/test-design-review/SKILL.md

test-design-review

Skill3 starsChanged 3 months agoMITView on GitHub

Critically review tests or diffs against Dave Farley's Properties of Good Tests and Gojko Adzic's Specification by Example principles. Evaluates test speed, determinism, atomicity, maintainability, necessity, and behavior-versus-implementation fidelity, flagging violations with concrete fixes. Triggers on: 'test review', 'review tests', 'review these tests', 'critique tests', 'test design review'.

2 other projects use a version of this.

Adopted by

Most recently taken up by

The file

We split this skill into the rules it gives an agent

6 rules. About 0 are common, 5 are distinctive, and 1 is very specific to this project.

  • ≈0

    Common · estimate

    Many files like this have a rule of this kind.

  • ≈5

    Distinctive · estimate

    This author's own choices. Worth a look.

  • 1

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Distinctive:

    Every unit and integration test must satisfy Farley's 6 core properties:

    line 21
  2. Distinctive:

    If specific files/lines are provided, review those.

    line 52
  3. Very specific:

    If a diff or branch is provided, extract all modified or added test files (git diff origin/main...HEAD -- '*test*' '*spec*').

    line 53
  4. Distinctive:

    Check for test pollution risks (global state, uncleaned fixtures, ordering dependencies).

    line 56
  5. Distinctive:

    Check if tests read as executable specifications.

    line 58
  6. Distinctive:

    Check signal-to-noise: flag incidental setup or redundant assertions.

    line 60

≈ “Common” is an estimate: at least 1 in 5 of the 350,495 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Test Design Review

Review tests (or a git diff containing tests) against an authoritative dual rubric: Dave Farley's Properties of Good Tests and Gojko Adzic's Specification by Example (BDD).


The Dual Rubric

1. Dave Farley's Properties of Good Tests

Every unit and integration test must satisfy Farley's 6 core properties:

PropertyStandardViolation Tells
FastUnit tests execute in milliseconds; test suite runs in seconds.Use of sleep(), artificial delays, network calls, slow disk I/O in unit tests.
MaintainableTests change only when requirements change, not when internal code refactors.Fragile mocks coupling to private methods; copy-pasted 50-line test setups.
RepeatableDeterministic: passes 1,000 times out of 1,000 runs in any order on any machine.Flakiness, reliance on system timezone/clock, random seeds without fixed seeding, database pollution across runs.
AtomicEach test is completely self-contained, tests one logical behavior, and fails for exactly one reason.Cascading failures (Test B fails only if Test A runs first), .first or .last queries dependent on DB ordering.
NecessaryEvery assertion validates a required domain capability or invariant.Tautological assertions (assert true), testing standard library or framework features, redundant is_successful checks before body checks.
UnderstandableServes as living documentation. Clear Arrange-Act-Assert structure and intention-revealing names.Vague test names (test_case_1, it_works), mixed levels of abstraction, obscure assertions.

2. Gojko Adzic's Specification by Example (BDD)

Tests are executable specifications. They must reflect business behavior, not mechanical wiring:

PrincipleStandardViolation Tells
Behavior over ImplementationSpecify what happens in response to a business event, not how functions communicate internally.Asserting method call counts (verify(service, times(3))), mocking internal collaborators excessively, inspecting private state.
High Signal-to-Noise RatioEvery variable, input, and assertion in the test directly influences the outcome.Incidental boilerplate, 20 unneeded mock returns, arbitrary dummy data obscuring the relevant parameter.
Concrete Domain ExamplesIllustrate edge cases and rules with realistic domain examples rather than abstract placeholders.Meaningless test values (foo, bar, asdf, test1), lack of domain edge-case verification.
Living DocumentationSpecification names describe scenarios and business consequences.Technical names (test_process_data_flag_false) instead of domain rules (when_account_is_suspended_reject_withdrawal).

Review Process

When reviewing tests:

  1. Locate Target Tests:
    • If specific files/lines are provided, review those.
    • If a diff or branch is provided, extract all modified or added test files (git diff origin/main...HEAD -- '*test*' '*spec*').
  2. Evaluate Farley Properties:
    • Audit test speed, isolation, determinism, and atomicity.
    • Check for test pollution risks (global state, uncleaned fixtures, ordering dependencies).
  3. Evaluate Adzic Specification Fidelity:
    • Check if tests read as executable specifications.
    • Identify tests coupled to implementation details rather than behavior.
    • Check signal-to-noise: flag incidental setup or redundant assertions.
  4. Produce Structured Review Report:
    • Output the Farley Scorecard & Adzic Scorecard.
    • For every flagged violation: provide file path, line numbers, offending code, explanation, and a drop-in replacement fix.

Review Report Output Template

# Test Design Review

## Summary & Recommendation
- **Target:** `<file or diff>`
- **Verdict:** [PASS / NEEDS REVISION / BLOCKING]
- **Key Takeaway:** <1-2 sentences on overall test quality>

## Farley Properties Scorecard
- **Fast:** [Pass / Warning / Fail] — <notes>
- **Maintainable:** [Pass / Warning / Fail] — <notes>
- **Repeatable:** [Pass / Warning / Fail] — <notes>
- **Atomic:** [Pass / Warning / Fail] — <notes>
- **Necessary:** [Pass / Warning / Fail] — <notes>
- **Understandable:** [Pass / Warning / Fail] — <notes>

## Adzic Specification Scorecard
- **Behavior over Implementation:** [Pass / Warning / Fail] — <notes>
- **Signal-to-Noise Ratio:** [Pass / Warning / Fail] — <notes>
- **Concrete Examples:** [Pass / Warning / Fail] — <notes>

## Detailed Findings & Fixes

### 1. [Violation Name] (`path/to/test.ext:L12-L25`)
- **Rubric Rule:** Farley (Atomic) / Adzic (Behavior over Implementation)
- **Problem:** <explanation of the defect>
- **Offending Code:**
```code
<bad code>
  • Recommended Fix:
<improved code>

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.