AI QA Review

<objective> QA-focused code review that detects test smells, analyzes testability of application code, and identifies coverage gaps. A test that asserts toBe(true) and a 95%-coverage suite that only feeds happy-path input both look green and both hide bugs — this skill names the smell, cites the line, and converts "looks fine" into a mutation-score gate.

Before starting: Check for .agents/qa-project-context.md in the project root. It contains test framework conventions, naming patterns, and project-specific quality standards that calibrate review feedback. </objective>

Quick Route

Three distinct entry paths. Pick the row, then jump to the named section.

All three share the same smell vocabulary (the six buckets below) and the same Verification commands.

Discovery Questions

First, read .agents/qa-project-context.md if present and skip any question it already answers.

1. Review scope: Reviewing test code for quality, application code for testability, or both? Each triggers a different Quick Route path.
2. Framework conventions: What test framework (Jest, Vitest, Playwright, pytest)? Conventions differ — describe/it nesting, fixture usage, assertion style — and the Verification commands are per-framework.
3. PR review or batch audit? A PR review runs and scores only the changed files. A batch audit scans the entire suite for systemic patterns.
4. Existing quality standards: Does the team have documented test conventions? Check for .eslintrc test rules, CONTRIBUTING.md test guidelines, or a test style guide.
5. Known pain points: Recurring flaky tests, slow suites, unclear failures? These prioritize which smells to focus on first.

Core Principles

1. Test code is production code. Apply the same quality standards: readability, maintainability, single responsibility. Test code that is hard to read is hard to trust.

1. Review what is asserted, not just what is executed. Coverage proves a line ran; it says nothing about whether a wrong value would be caught. A 95%-coverage suite of toBeTruthy assertions catches almost nothing. Mutation score (see Verification) measures the thing coverage can't.

1. Testability review prevents test debt. Reviewing application code for testability catches design problems before they force awkward test workarounds. If code is hard to test, it is usually hard to maintain.

1. Codify patterns, not just knowledge. Turn recurring review feedback into lint rules, custom ESLint plugins, or shared fixtures. Reviews that repeat the same feedback indicate missing automation.

1. Smells are symptoms, not verdicts. A test smell indicates a potential problem; context decides whether it is actually harmful. A long test for a complex workflow may be appropriate. A mock-heavy test for a boundary may be correct.

1. Actionable feedback only. Every review comment must include what is wrong, why it matters, and how to fix it. "This test is bad" is not actionable. "This test uses sleep-based waiting which causes flakiness — replace with an explicit wait condition" is.

Test Smell Buckets

Six dimensions. Each smell is categorized by the dimension it affects and links to a specific review action. Full SMELL/FIX code for every catalogued smell lives in references/smell-examples.md — keep the inline pointers prominent; the before/after code is the load-bearing part.

Readability Smells

Problems that make tests hard to understand at a glance.

Obscure Setup

What it looks like: 30+ lines of object construction with irrelevant fields drowning the test intent. The reader cannot tell which fields matter for the assertion.

Fix: Extract to factories. Only test-relevant data should appear in the test body: buildOrder({ items: [buildItem({ weight: 2.5, quantity: 2 })] }) instead of constructing full user/product/order objects inline.

Review action: Request factory extraction.

Mystery Guest

What it looks like: loadFixture('report.json') — the test depends on external data the reader cannot see. They must open another file to understand the assertion.

Fix: Inline the test-relevant data or use descriptively named fixtures. The reader should understand the test without opening other files.

Review action: Request inline data or descriptive fixture names.

Duplicate Assertions

What it looks like: Multiple tests assert the same behavior with varying specificity (toBe('Alice'), toHaveProperty('name'), toBeDefined()). Three tests, one behavior.

Review action: Request consolidation. Keep the most specific assertion. Redundant tests increase maintenance cost without increasing confidence.

Reliability Smells

Problems that cause tests to fail intermittently or in unexpected environments.

Sleep-Based Waiting

What it looks like: setTimeout, sleep(), waitForTimeout() used for synchronization. See references/smell-examples.md for the SMELL/FIX pair (replace waitForTimeout with an explicit toBeVisible wait).

Review action: Reject. Sleep-based waiting is never acceptable. Require explicit wait conditions.

Order Dependency

What it looks like: Tests pass when run together but fail in isolation or different order. See references/smell-examples.md for the SMELL/FIX pair (each test creating its own preconditions).

Review action: Request data isolation. Each test must create its own preconditions.

External Service Coupling

What it looks like: Tests call real external APIs (payment gateways, email providers, third-party services). See references/smell-examples.md for the SMELL/FIX pair (mocking the service boundary).

Review action: Request mock or fake at the service boundary. External calls belong in integration/contract tests, not unit tests.

Diagnostic Smells

Problems that make test failures hard to understand and debug.

Weak Assertion Messages

What it looks like: Assertion fails with no context about what was expected or why. See references/smell-examples.md for the SMELL/FIX pair (replacing toBe(true) with specific assertions like expect(result.errors).toEqual([]) that surface the offending value).

Review action: Request stronger assertions with diagnostic value. The failure message should explain the problem without reading the test source.

Multiple Failure Causes Per Test

What it looks like: A single test covers multiple independent behaviors. When it fails, you do not know which behavior broke. See references/smell-examples.md for the SMELL/FIX pair (splitting a lifecycle test into one-behavior-per-test).

Review action: Request test splitting. Each test should have one reason to fail.

Design Smells

Problems in test architecture that increase maintenance cost.

Conditional Test Logic

What it looks like: if/else, switch, ternaries, or for loops inside test bodies. Branching logic in a test is itself untested — you cannot tell which cases actually ran. See references/smell-examples.md for the SMELL/FIX pair (converting a branching loop into it.each).

Review action: Request parameterized tests (it.each / test.each). Conditional logic in tests hides which cases are actually verified.

Giant Fixtures