Review EXISTING test code for quality, smells, and testability issues. Detects test smells across six dimensions — readability, reliability, diagnostic value, design, AI-generated, and coverage — analyzes testability of application code, and backs the qualitative smells with mutation testing. Use wh

<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. Situation Path Jump to PR with changed test files Run the changed files, score them, check the diff against the PR checklist Verification → PR Review Checklist Whole suite needs a health pass Quantify, sample, find the 3 5 systemic smells, propose lint/mutation gates Batch Audit Process Application code, "why is this hard to test?" Flag DI / side effect / pure function / interface problems with before/after Testability Analysis 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. 2. 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. 3. 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. 4. 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. 5. 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. 6. 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 What it looks like: A beforeEach or fixture that sets up 20+ objects for every test, even though each test uses 2 3 of them. See references/smell examples.md for the SMELL/FIX pair (replacing a monolithic beforeEach with per test inline setup). Review action: Request inline setup. Move shared setup to factories, not monolithic beforeEach blocks. Over Mocking What it looks like: Every collaborator is mocked, including simple value objects and pure functions. See references/smell examples.md for the SMELL/FIX pair (dropping a mock of the very function under test). Review action: Request removal of unnecessary mocks. Mock boundaries, not internals. AI Generated Test Smells When the test code came from a coding agent (Claude Code, Codex, Cursor, Copilot), the smell taxonomy is the same — but a few signature failures recur often enough to deserve their own pass. Smell Detection Hallucinated locator Run the test against a real page once. If the locator never matches, the LLM invented a data testid that doesn't exist. Fabricated import Static check every imported symbol — does the file or package actually export it? LLMs invent plausible APIs ( @testing library/something that doesnt exist ). Generic test data example.com , test@test.com , Lorem ipsum , John Doe — boilerplate the agent generated because it had no project specific factory. Replace with the project's data factory. Closed AI loop Both implementation and tests authored by the same agent in the same session. The tests just describe what the agent produced; they don't constrain it. Pair the agent's tests with at least one human authored boundary test, or use TDD (test first) per shift left testing . A low mutation score (see Verification ) is the objective tell. Project convention drift Page Object, fixture, naming, or assertion style different from the rest of the suite. AI generated code rarely matches local conventions out of the box. For first time test generation patterns and the Step 7 review checklist, cross link ai test generation . For AI system eval suites (the equivalent of ESLint for prompts), wire each tool's CLI runner — promptfoo eval , deepeval test run (Apache 2.0), Ragas ragas evaluate / experiments (Apache 2.0) — as quality gates parallel to your test runner. Promptfoo ownership note: Promptfoo was acquired by OpenAI (announced 9 Mar 2026). The core stays MIT licensed, open source, and model agnostic; red team capabilities are being folded into OpenAI Frontier. promptfoo eval is still the correct quality gate command — just expect the vendor to be OpenAI going forward. Coverage Smells Problems that leave gaps in what is verified. Happy Path Only What it looks like: Every test provides valid input and expects success. No error paths tested. See references/smell examples.md for the SMELL/FIX pair (adding zero, max, negative, and boundary cases to a discount calculator). Review action: Request missing scenarios. Use the BOUNDARY framework: Boundary values, Null/empty, Duplicates, Ordering, Range limits. Missing Boundary Cases What it looks like: Tests for "normal" values (5 items) but not for 0, 1, max, or max+1. Use it.each to cover boundaries explicitly: empty collection, single item, exact page size, one over, large set. Review action: Request boundary tests. Every numeric parameter, string length, and collection size has boundaries to test. Missing Error/Negative Cases What it looks like: No tests for what happens when things go wrong — network failures, invalid input, permission denied, concurrent modification. Review action: For each happy path test, ask: "What is the corresponding failure mode?" Request tests for the failure. Testability Analysis When reviewing application code, assess whether it is structured for testability. Each subsection has a hard to test vs. testable before/after in references/testability refactors.md . Dependency Injection Flag classes that instantiate dependencies directly ( new PostgresDatabase() , new StripeClient() inside methods). Suggest constructor injection so tests can substitute mocks/fakes. See references/testability refactors.md . Side Effect I

ai-qa-review