Test Review

---
name: test-review
description: Ревью только что написанных или изменённых автотестов на соответствие best practices TypeScript + Playwright (по официальной документации) и конвенциям вашего проекта. Используй по /test-review либо после написания/правки любого теста (UI E2E, API, UI+API, моки, visual, mobile) или Page Object/фикстуры/констант — до коммита. Выдаёт приоритизированный список замечаний с severity, привязкой к строкам и готовыми фиксами.
allowed-tools:
  - Read
  - Grep
  - Glob
  - Bash
  - Edit
  - AskUserQuestion
---

# Ревью автотестов (TypeScript + Playwright)

Проверь только что написанный или изменённый тест-код на соответствие best practices и выдай приоритизированный список замечаний с фиксами. Источник правил — **официальная документация Playwright** ([best-practices](https://playwright.dev/docs/best-practices), [locators](https://playwright.dev/docs/locators), [test-assertions](https://playwright.dev/docs/test-assertions)) и [TypeScript](https://playwright.dev/docs/test-typescript) + конвенции конкретного проекта.

> **Источник правды по проекту** — его корневой `CLAUDE.md` (если есть) и стиль соседнего кода. Этот скилл — **фаза проверки**: он дополняет проектные правила, не заменяет их. Развёрнутые `❌ до → ✅ после` и ссылки на источники по каждому правилу — в [`references/rules-catalog.md`](references/rules-catalog.md).

---

## Когда применять и режим работы

- **Режим по умолчанию — диагностика.** Прочитать код, прогнать статический анализ, выдать отчёт. Файлы **не править**, пока пользователь явно не попросит «применяй / чини». Тогда — **итеративно**, по одному изменению с прогоном между ними (не big-bang переписывание рабочего теста).
- **Scope — только новое/изменённое, не весь suite.** По умолчанию — незакоммиченные изменения (`git status` + `git diff`). Если пользователь указал файл/папку — ревьюй их.
- **Любой тип теста:** UI E2E, API, UI+API, visual regression, mobile, моки, а также Page Object, фикстуры, константы.
- Не уходи в автономные действия за пределами ревью (воспроизведение через браузер, прогон всего suite, правки) без подтверждения — задача по умолчанию «прочитать и оценить».

## Доказательная дисциплина (без галлюцинаций)

- Каждое замечание — по **реально прочитанной строке** (`file:line`) или по **наблюдённому выводу** typecheck / lint / прогона. Не выдумывай нарушения «по аналогии» и не ссылайся на строки, которых не видел.
- Правило проверяется инструментом (tsc, ESLint, прогон) → **сначала запусти инструмент, потом репорти его вывод**, а не «вероятно есть».
- Не уверен, что это дефект, а не осознанное решение проекта → помечай **«под вопросом»**, не утверждай. Сверяйся с `CLAUDE.md` проекта и соседним кодом: часть «анти-паттернов» может быть намеренной (легаси-хелперы, нестандартная разметка, осознанные исключения из правил). Имена файлов/эндпоинтов/селекторов из памяти и прошлого контекста — фон, перепроверяй на живом коде.
- Не нашёл нарушений в категории — так и пиши «чисто», не придумывай замечание ради объёма.

## Процесс

1. **Scope.** Определи файлы под ревью: `git status --short` + `git diff --name-only` (учитывай untracked), либо переданные пути. Для каждого spec найди связанные Page Object'ы, константы, фикстуры.
2. **Контекст.** `Read` изменённых файлов + связанных POM/констант/фикстур. `Read` соседнего spec в той же папке как эталон стиля. Сверь правила директории/сьюта (smoke / regress / api и т.п.) по `CLAUDE.md` проекта, если он есть.
3. **Статический анализ (обязательно — дёшево и доказательно):**
   - Typecheck: `tsc --noEmit` (или typecheck-скрипт проекта из `package.json`). Любая ошибка типов в новом коде = 🔴 Blocker.
   - ESLint: найди конфиг проекта и **прочитай, какие правила реально включены** (особенно из `eslint-plugin-playwright`) — не предполагай по памяти. Вывод линта — источник правды.
   - **Что реально ловит линт — проверка двухуровневая.** (1) Плагина eslint-plugin-playwright нет вообще → floating promise, ручные ассерты и `networkidle` невидимы; предложи подключить recommended. (2) Recommended подключён → `missing-playwright-await`, `prefer-web-first-assertions`, `no-networkidle` уже error и ловятся, но `no-wait-for-timeout`, `no-force-option` и `expect-expect` там только **warn** (сверено по v2.10.5) — без `--max-warnings 0` эти предупреждения не валят CI. Предложи поднять их до error. `@typescript-eslint/no-floating-promises` требует type-aware линтинга — включён мало у кого. Что осталось вне линта — проверяй вручную (A/C/H).
   - При необходимости — проверка форматирования (Prettier), если настроена в проекте.
4. **Чеклист.** Пройди категории A–J ниже + K (правила вашего проекта). На каждое нарушение — severity + `file:line` + фикс. Глубже по правилу — [`references/rules-catalog.md`](references/rules-catalog.md).
5. **Верификация стабильности** (только если пользователь просит убедиться, что тест рабочий, и есть sandbox): прогон **только этого теста** в нативном параллелизме проекта (НЕ `--workers=1`):
   ```bash
   npx playwright test <file> --grep "<id>" --project="<projectName>" --retries=0 --repeat-each=5
   ```
   Pass-на-ретрае или плавающий результат = флак = 🟠 Major, чинить причину (гонки/гидратация/ожидания), а не прятать за retries.
6. **Отчёт** — в формате из раздела «Формат отчёта».

## Severity

| Метка | Значение | Типичные примеры |
|---|---|---|
| 🔴 **Blocker** | Тест сломан, недетерминирован или маскирует баг. Не мержить. | Ошибка typecheck; пропущенный `await` (floating promise); `waitForTimeout`/in-page `setTimeout`-пауза; pass только на ретрае; `{ force: true }` / `dispatchEvent` / прямой `setter` в обход реального UI; тест без ассертов; `test.only`; условный `expect`, который может не выполниться. |
| 🟠 **Major** | Хрупкость или флак при смене контента/окружения; нарушение ключевого правила проекта. | CSS/XPath-цепочки вместо role/label; мгновенный `count()`/`isVisible()`/`allTextContents()` как gate; точные цены/тексты/даты вместо regex; нарушение конвенции проекта (импорт базового `@playwright/test` там, где проект требует кастомную фикстуру; пропущена обязательная проектная проверка — напр. монитор сетевых ошибок); `waitForLoadState('networkidle')`; тест зависит от состояния другого. |
| 🟡 **Minor** | Стиль/читаемость/поддерживаемость; на стабильность не влияет. | Нет `test.step` по бизнес-шагам; инлайн-комментарии вместо самодокументирования; `.nth()` где годится `.filter()`; рабочий, но не приоритетный локатор; неиспользуемый импорт/константа. |
| ⚪ **Nit** | Косметика. | Именование, порядок импортов, форматирование (если не ловит Prettier). |

---

## Чеклист ревью

Каждый пункт — что искать; в скобках — severity нарушения. Развёрнутые примеры и пруфы — в каталоге.

### A. Детерминизм, ожидания, асинхронность
- [ ] Нет `page.waitForTimeout(ms)` и нет `setTimeout`/`sleep` внутри `page.evaluate` (грепни оба). Ждать состояние, не время. (🔴)
- [ ] Нет «висящих» промисов: каждый `expect`, `test.step`, действие (`click/fill/goto`), `waitFor*` — под `await`/`return`/`void`. Пропущенный `await` = молчаливый флак. (🔴)
- [ ] Сеть — паттерн **promise → действие → await**: `const p = page.waitForResponse(...); await click(); await p`. Объявление после действия = гонка. (🔴)
- [ ] Нет `waitForLoadState('networkidle')`. Навигация — `goto(url, { waitUntil: 'domcontentloaded' })`, без дублирующего `waitForLoadState` следом. (🟠)
- [ ] Производные/составные проверки (несколько связанных условий, замер коллекции сразу после появления) — в `await expect(async () => {...}).toPass({ timeout })`, а не цепочка `await`-ов. (🟠)
- [ ] Учтена SSR-гидратация: SSR-фреймворки (Nuxt/Next и др.) могут перемонтировать контент после гидратации → мгновенный `count()`/`allTextContents()` сразу после появления ловит окно пустоты. Замер через `toPass`. (🟠)

### B. Локаторы
- [ ] Приоритет: `getByRole({ name })` → `getByText` → `getByLabel` → `getByPlaceholder` → `getByAltText` → `getByTitle` → `getByTestId` → CSS (крайний случай) → XPath (почти никогда). (🟠 при CSS/XPath без причины)
- [ ] Нет хрупких CSS-цепочек по структуре DOM (`div > div > span`, `.episode-actions-later`). Ломаются при ребрендинге. (🟠)
- [ ] Strict mode: локатор резолвится в один элемент; уточнение через `{ name }` / `.filter({ hasText })` / `.filter({ has })`, а не `.nth()`. `.nth()` — только с обоснованием. (🟡)
- [ ] Плавающие элементы (дропдауны, тосты, модалки, портальный контент, iframe) ищутся **глобально от `page`**, не от секции. (🟠)
- [ ] Длинный `getByText('целое предложение')` не используется как якорь — хрупко к правкам копирайта; брать стабильный фрагмент/role. (🟡)

### C. Ассерты
- [ ] Только **web-first** (авто-ретрай): `toBeVisible/toHaveText/toHaveCount/toHaveValue/toBeChecked/toHaveAttribute/toHaveURL`. Нет `expect(await loc.isVisible()).toBe(true)` и `expect(await loc.count()).toBe(n)` — не ретраятся. (🔴/🟠)
- [ ] Каждый тест что-то проверяет (нет теста, который только кликает без `expect`). (🔴)
- [ ] Блок независимых проверок одной секции — через `expect.soft`, чтобы собрать все падения разом. (🟡)
- [ ] Нет ассертов на точные цены/числа/даты/динамический контент — regex или диапазон. (🟠)
- [ ] Известный незакрытый баг — `test.fail()` (с единственным баг-ассертом), не `test.fixme()`; рабочее поведение — отдельным обычным тестом. (🟡)

### D. Изоляция и независимость
- [ ] Тесты независимы: состояние НЕ передаётся между тестами. `let x` на уровне `describe`, переинициализируемый в `beforeEach`, — распространённый валидный паттерн; нарушение — когда тест читает результат другого. Прогон в одиночку и в любом порядке должен проходить. (🔴 если ломает изоляцию)
- [ ] `describe.configure({ mode: 'serial' })` — только при реальной зависимости, не «на всякий случай». (🟠)
- [ ] Setup/teardown — в `beforeEach`/фикстурах, без копипасты; созданные сущности (API) удаляются. (🟠)
- [ ] Тест не зависит от внешних сайтов и third-party виджетов — тестируем только то, что контролируем; внешнее — мок/проверка факта запроса. (🟠)

### E. TypeScript и линт
- [ ] Typecheck зелёный для нового кода (`tsc --noEmit`, strict). (🔴)
- [ ] `any` в **POM / fixtures / utils** — нежелателен, типизируй (`Locator`/`Page`/`Route`/`APIResponse`). Сверь с конфигом проекта: `any` может быть осознанно разрешён в спеках (напр. для мок-данных) — тогда там не флагай. `@ts-ignore` — только с причиной/тикетом. (🟠 для POM/utils)
- [ ] Поля POM — `readonly Locator`; фикстуры типизированы (`base.extend<{...}>`); тело ответа API типизируй явно, если на него опираются ассерты. (🟡)
- [ ] **Пропущенный `await` линт часто НЕ ловит** (проверь конфиг: есть ли `no-floating-promises` / `missing-playwright-await`; `valid-expect` покрывает лишь часть) → перечитай глазами, см. A2. (🔴)
- [ ] Сверь модульную систему (ESM vs CJS) и стиль импортов (относительные vs алиасы) с фактическим кодом проекта — следуй существующему стилю, не навязывай свой. Неиспользуемые импорты/переменные/константы/методы POM убрать. (🟡)

### F. Сеть и моки
- [ ] Моки (`page.route`) — только для edge cases (5xx, пустой ответ, таймаут, офлайн). Позитивный happy-path — против реального API. (🟠)
- [ ] Проверка контракта, где это суть теста: `waitForResponse` (статус + тело) / `waitForRequest` + `postDataJSON()`. Для форм — инспекция payload на `[object Object]`, пустые/несериализованные поля, а не «кнопка активна». (🟠)
- [ ] Роуты ставятся **до** триггерящего действия; область — тест/фикстура, не глобально на suite. (🟠)
- [ ] Внешний хост, который может не отвечать (напр. внешний личный кабинет, платёжный шлюз): переход не проверяем «вглубь» — оракул это инициированный навигационный запрос, а не загрузка цели. Гасить запрос по ситуации: `route.abort()` годится, только если тест на этом заканчивается; если тесту дальше жить на странице (повторный `goto`, следующие шаги) — `abort()` навигации уводит Chromium в `chrome-error://` и ломает следующую навигацию, а `204` Chromium всё равно трактует как переход. Рабочий вариант — `route.fulfill` html-заглушкой (`200 text/html`) + повторный `goto`. (🟠)
- [ ] Свои `route` снимаются точечно — `page.unroute(matcher, handler)`. `unrouteAll()` сносит и роуты, поставленные фикстурами проекта (заглушки зависших third-party хостов и т.п.) — после него другие тесты/шаги флачат. (🟠)

### G. Структура, читаемость, гигиена
- [ ] Логические шаги обёрнуты в `test.step('Императив', …)` (видно в Allure/HTML/trace). `return` — снаружи коллбэка. Не дробить на каждое действие. (🟡)
- [ ] **Нет инлайн-комментариев** в тестах — самодокументирование (осмысленные имена, semantic-локаторы, шаги). Контекст — в описании/аннотации репортера (напр. `allure.description`), если проект их использует. (🟡)
- [ ] Параметризация однотипных кейсов через `for...of` **снаружи** `test.describe`, а не копии теста. (🟡)
- [ ] Нет `test.only`, закомментированных тестов, временных файлов/черновиков, отладочных `console.log`/`page.pause()`. (🔴 для `test.only`/`page.pause`, иначе 🟡)
- [ ] Имена тестов/шагов осмысленны; формат ID/тегов (`@allure.id:N`, ключ ТК и т.п.) — как у соседних тестов в файле. (⚪)

### H. Маскировка багов и флак
- [ ] Нет синтетических обходов реального UX: `{ force: true }`, `dispatchEvent`, прямой React/Vue-setter, ручной скролл вместо авто-actionability — если только это не оправдано контролируемым input'ом (напр. кастомные `display:none` инпуты — проверь в браузере). Фикс должен ловить регрессию, если фича сломается, а не прятать её. (🔴)
- [ ] `retries`/`mode: serial`/увеличенный timeout не используются как «лекарство» от флака. Карантин допустим только временно, со ссылкой на тикет. (🟠)
- [ ] `try/catch` не глушит падения действий/ассертов (auto-waiting встроен; `.catch()` прячет баг). (🟠)
- [ ] Пред-релизный тест (написан до выката фичи) **падает честно**, не спрятан за `skip`/флагом. (🟠)
- [ ] Упавший тест **продиагностирован до фикса**: баг продукта или плановое изменение? Доказательства, не догадка — *когда* сломалось (история прогонов; группа тестов, покрасневшая в одну дату, — релиз, а не дрейф контента), тот же элемент *сравнён между окружениями* (есть на стенде, пропал на проде → регрессия прода; нет на обоих → выкачено осознанно), *внутренняя асимметрия* (виден на мобильном, но не на десктопе; есть в DOM, но скрыт CSS; только одно плечо A/B). Вердикт «баг» → сообщить и оформить ассерт как `test.fail` с тикетом, а не подгонять его под текущий DOM. (🔴)
- [ ] Фикс не **ослабляет оракул** ради зелёного: `toBeVisible()` → `toBeAttached()` (скрытый CSS элемент начинает проходить), адресный ассерт заменён щедрым счётчиком, проверка удалена со ссылкой «покрыто в другом месте» без чтения того спека. Погасить сигнал — не значит починить. (🔴)

### I. Спецслучаи по типу теста
- [ ] **API:** проверяется статус И тело; идентификаторы запросов — свежий `randomUUID()` из встроенного `crypto` на каждый запрос (не тащи пакет `uuid`, если его нет в проекте); учтён rate limit; cleanup созданного. (🟠)
- [ ] **iframe:** `frameLocator`; контент ищется внутри фрейма. **Новый таб:** `context.waitForEvent('page')`. **Download:** `waitForEvent('download')` + проверка имени. **Upload:** `setInputFiles`. **Время:** `page.clock`. **Геолокация/права:** `grantPermissions`/`setGeolocation`. (🟠 при ручных обходах)
- [ ] **visual:** `toHaveScreenshot` с `animations:'disabled'` и `mask` на динамику; эталоны — на платформе CI (macOS-эталон против Linux-CI = гарантированный diff). Только если тест-кейс требует эталон. (🟠)

### J. Соответствие намерению (оракул реально проверяет заявленное)
- [ ] Тест проверяет то, что обещает имя/описание, а не суррогат. «Валидация формы» → инспекция реального payload, не только «кнопка активна». «Загрузка ещё» → реальная догрузка и сверка, не только клик. (🟠)
- [ ] Привязка к контенту структурная (наличие, непустота, `count > 0`, regex), чтобы тест пережил смену копирайта/цен — особенно для регрессов после фикса. (🟠)
- [ ] **Пороги соразмерны наблюдённым фактам.** Счётчик с кратным запасом (`>= 8` при 15 на странице, `>= 20 ссылок` при 40 в футере) переживёт потерю половины страницы — это оракул-плацебо. Где есть адресный якорь (собственный класс блока, ссылки в дочерний раздел, колонки футера) — ассертить его, а не общий счётчик по странице; порог ставить от замеренных значений на каждом окружении. Для каждого порога назвать дефект, который он ещё ловит, — и тот, который уже нет. (🟠)
- [ ] Оракул адекватен ограничению окружения: где UI не различает 404/5xx (одна заглушка на оба) — проверка сетевая, не «увидел текст ошибки». (🟠)
- [ ] **Оракул, выведенный из первого экземпляра коллекции, проверен на всех.** Раскладка/структура блока №1 не обязана совпадать с №3 (первая мозаика — колонка, третья — «1 + 2»; у первой карточки есть CTA-кнопка, у соседней нет). Обобщение по `first()` даёт либо ложное падение на корректной вёрстке, либо тест, зависящий от порядка контента; проверять инвариант, общий для всех экземпляров, а не свойство первого. (🟠)

### K. Правила вашего проекта (шаблон — заполните под свой репозиторий)
> У зрелого тест-репозитория всегда есть конвенции, которые не проверит ни один универсальный чеклист. Зафиксируйте их здесь или в `CLAUDE.md` проекта — тогда ревью будет ловить их нарушения. Типовые категории с примерами:

- [ ] **Кастомные фикстуры:** где импортировать `test` из кастомной фикстуры (`./fixtures/custom-test`) вместо `@playwright/test`, и какие обязательные проверки она даёт (напр. монитор сетевых ошибок, вызываемый в конце теста / в `afterEach`). (🟠)
- [ ] **Паттерны директорий:** чем отличаются правила smoke / regress / api сьютов — композиция vs фикстуры для POM, репортер-аннотации, testMatch/testIgnore, куда добавлять новые тесты. (🟠)
- [ ] **Окружения:** тест и POM проверены на всех целевых стендах, не только на одном (DOM на тест-стенде может отличаться от прода); известные особенности стендов зафиксированы списком. (🟠)
- [ ] **Skipped-гигиена:** тест не добавляет постоянных skipped в штатные прогоны; окружение-специфичное исключается конфигом (`testIgnore`/`testMatch`), runtime `test.skip` — только для динамических условий (фича-флаг, известный баг с тикетом). Итог прогона: passed = ок, failed = проблема, skipped = требует объяснения. (🟡)
- [ ] **Зависимости/конфиг:** не бампать версию `@playwright/test` и не добавлять зависимости без сверки с CI (Docker-образ, lock-файл); не трогать `playwright.config.ts` без необходимости. (🔴 если затронуто без запроса)

---

## Формат отчёта

```
## Ревью: <файлы / scope>

**Статический анализ:** typecheck ✅/❌ · lint ✅/❌ · (прогон: N/N pass, --repeat-each=5)

### 🔴 Blocker (N)
1. `path/to/spec.ts:42` — <что не так>.
   Почему: <ссылка на правило/категорию>.
   Фикс:
   ```ts
   // ❌ было / ✅ стало
   ```

### 🟠 Major (N)
…

### 🟡 Minor (N)
…

### ⚪ Nit (N)
…

### ✅ Что хорошо
- <что соответствует best practice — кратко>

### Вердикт
<Готов к коммиту / К доработке: список Blocker+Major> · <команда для прогона с правильным --project>
```

Правила:
- Сортировка строго по severity (Blocker → Nit). Внутри — по файлу/строке.
- Каждый Blocker/Major — с конкретным фиксом (сниппет `❌ было → ✅ стало`).
- Чисто в категории — пиши «чисто», не выдумывай.
- В конце — однострочный вердикт и команда запуска с верным `--project`.
- Если просили применить фиксы — делай **итеративно** (одно изменение → typecheck/прогон → следующее), не переписывай рабочий тест целиком.