Review Testing

What the source does

• FilesystemNone observedNo reads or writes in the source
• NetworkNone observedNo outbound calls in the source
• ShellNone observedNo commands run in the source
• DependenciesPinnedNothing floats
• SecretsNone foundNo keys in source

Five audit layers

Gates applied: no behavioural pass.

Audit 0.4.1 · source sha babb6bf12fac · 18 Oct 2026Report an issue or request a re-scan

• Publisherposit-dev GitHub organizationOwner of the repository this skill is read from
• Source folderposit-dev/skills/posit-dev/review-testingThe exact folder we audited
• LicenceMITAs the repository states it
• Commit pinned for this verdictbabb6bf12facA new audit runs when the source changes
• Repository statusNot archivedLast push 2 days ago
• Sandboxed runNot runSkills are read, not executed; layer L4 is skipped

Its description: Review test code for quality, design, and completeness after implementing a feature or fixing a bug. Use when the user asks to "review my tests", "check my test quality", "are these tests good enough", "review testing",…

---
name: review-testing
description: Review test code for quality, design, and completeness after implementing a feature or fixing a bug. Use when the user asks to "review my tests", "check my test quality", "are these tests good enough", "review testing", or after completing a feature implementation that includes tests. Also use when tests feel brittle, flaky, or superficial. Cross-references production code to find coverage gaps.
metadata:
  author: Garrick Aden-Buie (@gadenbuie)
  version: "0.1"
license: MIT
---

You are reviewing test code written alongside a feature implementation or bug fix. Ensure the tests are well-designed, thorough, and maintainable — not just that they pass. Tests that merely mirror implementation details create false confidence and become a maintenance burden during refactoring.

## Review Scope

Identify what to review:

1. **Find changed test files** on the current branch (relative to the base branch).
2. **Find the production code** those tests cover — trace imports, function calls, and file naming conventions to map tests to their targets.
3. **Find related existing tests** for the same modules or functions that weren't changed — these may need updates or reveal gaps.

Read test files first, before production code. If you can infer the feature's requirements and edge cases from the tests alone, that's a sign the tests are well-written. If you need to read the implementation to understand what the tests are doing, that's a finding worth reporting.

## What Makes a Test Valuable

Weigh each test against four qualities:

**Regression protection** — Does this test actually catch bugs? A test that exercises trivial code or skips complex branches protects against nothing. Check: does the test touch business-critical logic, or only verify the happy path of a simple getter?

**Refactoring resilience** — Will this test break when someone restructures code without changing behavior? Tests coupled to internal method names, call sequences, or private state punish every cleanup with false failures, eroding trust in the suite.

**Fast feedback** — Unit tests should run in milliseconds. If a test hits the filesystem, network, or database unnecessarily, that's a design issue. But don't confuse speed with value — an integration test verifying a real database query is better than a fast unit test that mocks everything and verifies nothing.

**Maintainability** — Can someone unfamiliar with this code read the test and understand what it verifies and why? Tests with sprawling setup, cryptic names, or deeply nested mocking fail this check.

## Review Areas

### 1. Assertion Completeness

The most common weakness in generated tests: asserting only the obvious output and missing the full "blast radius" of a state change.

When a test triggers an action, ask what *else* changed. If a test adds an item to a cart, does it only check the item count? Or does it also verify the price calculation, subtotal update, and that other items are unaffected?

**Flag when:**
- A test asserts a return value but ignores side effects
- A test checks that an operation succeeded but not that it produced the right result
- A test verifies the happy path but skips boundary conditions (empty inputs, nulls, maximum values, off-by-one)
- Error-path tests only check that an error was thrown, not the error message, type, or cleanup behavior

### 2. Test Structure

Each test should have exactly one Arrange-Act-Assert cycle. If a test acts and asserts multiple times in sequence, it's testing multiple behaviors and should be split.

**Flag when:**
- A test has multiple act phases (testing a workflow, not a behavior)
- The arrange phase is so large the actual behavior being tested is buried
- Assertions appear inside setup helpers or utility functions (hides what's being verified)
- Test logic contains conditionals or loops — tests should be straight-line code

### 3. Fixture and State Management

How test data is created determines whether the suite is maintainable at scale. **Inline setup** (data created in the test body) is fine for simple tests but painful when constructor signatures change across many tests. **Implicit setup** (shared `beforeEach`/`setUp` blocks) eliminates duplication but obscures what each test actually depends on. **Delegated setup** (factory functions or builders called explicitly) keeps tests readable while centralizing construction logic.

**Flag when:**
- A shared setup block creates objects most tests don't use
- Tests depend on external files, database state, or globals defined elsewhere ("Mystery Guest" — everything a test needs should be visible in its body or one function call away)
- Tests assume resources exist without creating them (files, database records, environment variables)
- Tests mutate shared state without cleanup, causing order-dependent failures

### 4. Mocking Boundaries

Mocks are essential for isolation, but overuse turns tests into mirrors of the implementation.

**The key principle:** mock at architectural boundaries, not at every function call. Use stubs (canned data) for query-type dependencies and mocks (interaction verification) only for commands with side effects like sending emails or writing to external systems.

Respect the codebase's existing mocking convention. Flag inconsistency within the project, not deviation from a universal rule.

**Flag when:**
- A test mocks types it doesn't own (third-party libraries, framework internals) — when the library updates, the mock passes against outdated assumptions. Prefer: thin adapters, real implementations in integration tests, or official testing utilities (MemoryRouter, test databases, in-memory caches).
- Mock density is high — 4+ mock configurations suggests too many responsibilities in the production code
- A test verifies call counts or argument sequences on internal methods (couples the test to *how* the code works, not *what* it does)
- Stubs are being verified (checking a stub was called tests imp

Excerpt, first 6,000 characters of SKILL.md, verbatim from the audited commit. It is text from someone else’s repository, quoted so you can judge it before your agent reads it.

git clone https://github.com/posit-dev/skills
git -C skills checkout babb6bf12fac

Works in

1. 2026-10-18
   ✓SAFE B 89 / 100

   first audit · source babb6bf12fac

Questions about Review Testing

• github.com/posit-dev/skills/tree/main/posit-dev/review-testing — the audited source at commit babb6bf12fac and every finding aboveObserved · 18 Oct 2026

Spotted something wrong? Report a correction · Methodology