review-tests

Files (15)

• SKILL.md
• openai.yaml · agents/
• README.md · evals/
• evals.json · evals/
• CONTRACT.md · evals/fixtures/oracles/
• orders.py · evals/fixtures/oracles/
• suite.py · evals/fixtures/oracles/
• CONTRACT.md · evals/fixtures/reliability/
• suite.py · evals/fixtures/reliability/
• worker.py · evals/fixtures/reliability/
• CONTRACT.md · evals/fixtures/review-only/
• sender.py · evals/fixtures/review-only/
• suite.py · evals/fixtures/review-only/
• review-cases.md · references/
• sources.md · references/

SKILL.md (first lines)

# Review Tests

Make tests sensitive to broken behavior and resilient to behavior-preserving changes. For each finding, answer: **What real code runs, what independently establishes correctness, and what plausible regression would this test catch?** Passing tests and execution coverage alone cannot answer those questions.

Default to reviewing and correcting test defects. Honor requests for findings only, a particular path, or a limited test layer. Ordinary feature implementation belongs to its existing workflow; this skill audits the quality of the tests themselves.

## Establish scope and baseline

Read the target project's instructions, test commands, CI configuration, and relevant behavior contracts. Record the revision and existing staged, unstaged, and untracked work so repairs preserve unrelated changes.

Inventory the requested scope using both repository files and the runners' actual collection. Include test helpers, fixtures, doctests, browser/native suites, and standalone verification scripts where applicable. Identify disabled, skipped, uncollected, and environment-gated tests. Run the established baseline with disposable state and external services replaced or isolated according to project conventions.

For a whole-repository request, keep a review ledger covering every in-scope test file and each materially different test family. Track inventoried, reviewed, executed, and blocked work separately. A search for suspicious patterns is triage, not a review of every test. If a baseline cannot run, continue the source review and record the blocker.

**Done:** scope, collection, baseline outcome, and outstanding files are explicit. Preserve that ledger across context boundaries.

## Trace the behavior and the assertion

Read the tests together with the production paths they claim to validate. Follow fixtures, patches, imports, generated cases, workers, and subprocesses far enough to identify:

- The behavior and its source of truth: user requirement, documented contract, worked example, trusted fixture, or domain invariant.
- The actual production entrypoint and relevant decision, transformation, validation, persistence, or dispatch that executes.
- Which dependencies are replaced and whether a replacement supplies the very answer being asserted.
- The observed result or effect and a concrete defect that would change it.

Inspect every in-scope test, including parameterized branches. Group equivalent cases in the ledger without overlooking different setup or failure paths. Coverage and tracing can resolve uncertainty about execution; account for setup-only calls and code running in other threads or processes.

Read [review-cases.md](references/review-cases.md) when judging mocks, snapshots, concurrency, or whether a structural assertion protects a real contract.

Apply these review criteria:

| Risk | Evidence to seek | Repair direction |
| --- | --- | --- |
| Tautological oracle | Self-comparison, expected values generated by the same production helper, duplicated algorithm, or assertions of fake-owned constants | Independently chosen inputs and outcomes, trusted examples, or domain properties with contrasting cases |
| Production bypass | Mocked subject under test, local reimplementation, import-only checks, or mocks replacing all relevant policy | Invoke the real entrypoint; retain the behavior being tested and replace the necessary boundary |
| Structure coupling | Source-string searches, private helper choreography, reflection, incidental markup/CSS, giant snapshots, or exact incidental serialization | Assert observable behavior through a stable interface; parse structured output or use rendered UI |
| Weak failure checks | Broad exceptions, swallowed assertions, worker errors lost, existence/truthiness where the actual result matters | Assert the expected failure category and absence of forbidden effects; propagate asynchronous failures |
| Nondeterminism | Shared state, order dependence, ambient credentials/proxies, wall-clock assumptions, fixed ports, live third-party copy | Isolated state, controlled inputs, local peers, and explicit environment prerequisites |
| Timing coupling | Sleeps used to assume ordering, tiny performance deadlines, blocked teardown | Events or barriers, bounded condition waits and cleanup, controlled time where appropriate |

These are heuristics, not automatic violations. Public schemas, promised compatibility, wire formats, security settings, resource bounds, and idempotency can require exact checks. Verify which contract makes an assertion necessary. Interaction assertions are useful when the interaction is the behavior, such as preventing duplicate sends or verifying a required protocol exchange.

**Done:** each test family has a defensible behavior/oracle/code path, or a concrete finding. A clean pattern scan does not meet this criterion.

## Correct without weakening coverage

Work one finding at a time. Prefer the smallest reliable test boundary that catches the defect. Keep production logic real; use real lightweight implementations or temporary stores when practical, then maintained fakes or boundary mocks where isolation requires them. More end-to-end tests are not automatically better.

Replace mirrored algorithms with independent examples. Property and metamorphic tests need domain-justified invariants; a round trip alone can miss complementary bugs. Pair rejection cases with accepted neighboring cases and verify no forbidden side effect occurred.

For UI tests, exercise rendered behavior using accessible roles/names or intentional stable test IDs, with framework condition waits. For structured protocols, compare meaningful fields and preserve ordering only when it matters. Update snapshots only after confirming the intended contract independently.

For asynchronous tests, coordinate actual worker progress, deliver worker failures to the test thread, release blockers in cleanup, and bound teardown as well as completion waits. A fake clock should control expiration logic without bypassing the scheduler or renewal behavior under test. Real deadline tests still need real elapsed-time evidence with scheduling margin.

Keep required coverage when removing a test: identify the redundant behavioral test or add its replacement first. Preserve required gates and assertions; adding retries, broadening exceptions, or loosening expectations just to obtain green results is not a repair.

If a stronger test exposes a product bug, establish the expected behavior independently. Make a narrow production fix when the user's authorized task includes that fix; otherwise report the regression and the remaining production work. Resolve ambiguous requirements before changing the contract. Routine test choices within existing authorization do not need another approval.

**Done:** the repaired test observes the intended behavior and retains the original meaningful guarantees. Review the diff for accidental weakening.

## Prove regression sensitivity

For important rewrites and critical guards, run a targeted counterexample: reproduce the known bug or temporarily mutate a relevant production decision, effect, or output. Examples include bypassing a rejection, omitting a durable write, duplicating a send, dropping lease renewal, or rendering untrusted text as HTML. Choose mutations from plausible defects in the reviewed behavior.

Use an isolated copy or a narrow reversible patch that preserves existing work. Establish green, introduce one defect, observe the intended assertion or contract failure, restore, and establish green again. Verify restoration even after failure. Syntax/import/setup errors and unrelated crashes do not prove a useful test. Report timeouts separately and explain whether they demonstrate the specific deadline or liveness contract.

Inspect surviving mutants: they may expose a gap, equivalent behavior, or an irrelevant implementation change. Strengthen meaningful coverage instead of chasing a universal mutation score. A bounded sample demonstrates sensitivity to those defects, not every possible regression. When mutation execution is infeasible, label the proposed counterexample as unexecuted.

Run affected tests and required project checks after repairs, then the relevant broader suite. Investigate ordering or timing concerns with deliberate reordered/repeated runs; passing a few repeats does not prove absence of flakiness. Exercise optional acceptance gates only with their prerequisites and authorization intact.

**Done:** validation commands, counts, failures, skips, mutation outcomes, and restoration are recorded. Explain any incomplete verification.

## Summarize every finding

Give the user a self-contained report containing:

- Scope reviewed versus requested, with unfinished or blocked areas named.
- Every issue found: file/test, concrete consequence, correction or reason it remains open. Group repeated instances and give counts.
- Production bugs exposed and any source corrections, distinguished from test corrections.
- Exact validation evidence and environment limits, separating executed checks from static review and proposed experiments.

Include meaningful checks deliberately retained when a tempting simplification would remove a guarantee. State whether review shared the implementation context. Commit or publish only under the user's existing authorization, isolating task-owned changes from unrelated work.

The research behind these choices is in [sources.md](references/sources.md). Consult it for rationale or source attribution; it is not an extra checklist to load on every run.

From corynaegle-ai/agent-skills, MIT.

More from corynaegle-ai/agent-skills