/test-review

name: test-review
description: Review of just-written or modified autotests against TypeScript + Playwright best practices (per official documentation) and your project's conventions. Use via /test-review or after writing/editing any test (UI E2E, API, UI+API, mocks, visual, mobile) or a Page Object/fixture/constants — before commit. Produces a prioritized list of findings with severity, line references, and ready-made fixes.
allowed-tools:
  - Read
  - Grep
  - Glob
  - Bash
  - Edit
  - AskUserQuestion

Autotest review (TypeScript + Playwright)

Check just-written or modified test code against best practices and produce a prioritized list of findings with fixes. Rule source — the **official Playwright documentation** ([best-practices](https://playwright.dev/docs/best-practices), [locators](https://playwright.dev/docs/locators), [test-assertions](https://playwright.dev/docs/test-assertions)) and [TypeScript](https://playwright.dev/docs/test-typescript) + the specific project's conventions.

> **Source of truth for the project** — its root `CLAUDE.md` (if present) and the style of neighboring code. This skill is the **review phase**: it complements project rules, it does not replace them. Expanded `❌ before → ✅ after` examples and source links for each rule — in [`references/rules-catalog.md`](references/rules-catalog.md).

---

When to apply and operating mode

• **Default mode — diagnostics.** Read the code, run static analysis, produce a report. Do **not** edit files until the user explicitly asks to "apply / fix". Then — **iteratively**, one change at a time with a run in between (no big-bang rewrite of a working test).
• **Scope — only new/changed code, not the whole suite.** Default — uncommitted changes (`git status` + `git diff`). If the user specified a file/directory — review those.
• **Any test type:** UI E2E, API, UI+API, visual regression, mobile, mocks, plus Page Objects, fixtures, constants.
• Do not drift into autonomous actions beyond the review (browser reproduction, running the whole suite, edits) without confirmation — the default task is "read and assess".

Evidence discipline (no hallucinations)

• Every finding — from an **actually read line** (`file:line`) or from **observed output** of typecheck / lint / a run. Do not invent violations "by analogy" and do not reference lines you have not seen.
• A rule is checkable by a tool (tsc, ESLint, a run) → **run the tool first, then report its output**, not "probably present".
• Not sure it is a defect rather than a deliberate project decision → mark it **"questionable"**, do not assert. Cross-check against the project's `CLAUDE.md` and neighboring code: some "anti-patterns" may be intentional (legacy helpers, non-standard markup, deliberate rule exceptions). File names/endpoints/selectors from memory and past context are background — re-verify against live code.
• Found no violations in a category — say so: "clean", do not invent a finding for volume.

Process

1. **Scope.** Determine the files under review: `git status --short` + `git diff --name-only` (include untracked), or the paths provided. For each spec, find the related Page Objects, constants, fixtures. 2. **Context.** `Read` the changed files + related POM/constants/fixtures. `Read` a neighboring spec in the same directory as a style reference. Cross-check the directory/suite rules (smoke / regress / api etc.) against the project's `CLAUDE.md`, if present. 3. **Static analysis (mandatory — cheap and evidence-based):**

• Typecheck: `tsc --noEmit` (or the project's typecheck script from `package.json`). Any type error in new code = 🔴 Blocker.
• ESLint: find the project config and **read which rules are actually enabled** (especially from `eslint-plugin-playwright`) — do not assume from memory. Lint output is the source of truth.
• **What lint actually catches — a two-level check.** (1) No eslint-plugin-playwright at all → floating promises, manual asserts and `networkidle` are invisible; suggest enabling recommended. (2) Recommended is on → `missing-playwright-await`, `prefer-web-first-assertions`, `no-networkidle` are already errors and get caught, but `no-wait-for-timeout`, `no-force-option` and `expect-expect` are only **warn** there (verified against v2.10.5) — without `--max-warnings 0` these warnings never fail CI. Suggest raising them to error. `@typescript-eslint/no-floating-promises` needs type-aware linting — rarely enabled. Whatever lint still misses — verify manually (A/C/H).
• If needed — a formatting check (Prettier), if configured in the project.

4. **Checklist.** Go through categories A–J below + K (your project's rules). For each violation — severity + `file:line` + fix. Deeper per rule — [`references/rules-catalog.md`](references/rules-catalog.md). 5. **Stability verification** (only if the user asks to confirm the test works and a sandbox is available): run **only this test** in the project's native parallelism (NOT `--workers=1`):

   npx playwright test <file> --grep "<id>" --project="<projectName>" --retries=0 --repeat-each=5

Pass-on-retry or a floating result = flake = 🟠 Major; fix the cause (races/hydration/waits), do not hide it behind retries. 6. **Report** — in the format from the "Report format" section.

Severity

| Label | Meaning | Typical examples | |---|---|---| | 🔴 **Blocker** | Test is broken, non-deterministic, or masks a bug. Do not merge. | Typecheck error; missing `await` (floating promise); `waitForTimeout`/in-page `setTimeout` pause; pass only on retry; `{ force: true }` / `dispatchEvent` / direct `setter` bypassing the real UI; test without asserts; `test.only`; conditional `expect` that may never execute. | | 🟠 **Major** | Brittleness or flake on content/environment change; violation of a key project rule. | CSS/XPath chains instead of role/label; instant `count()`/`isVisible()`/`allTextContents()` as a gate; exact prices/texts/dates instead of regex; project conv