diff --git a/docs/reviews/CODE-REVIEW-650-r1.md b/docs/reviews/CODE-REVIEW-650-r1.md new file mode 100644 index 00000000..2a79330c --- /dev/null +++ b/docs/reviews/CODE-REVIEW-650-r1.md @@ -0,0 +1,150 @@ +# CODE-REVIEW-650-r1 + +Issue: #650 · заход r1 · блокирующих циклов израсходовано 0 из 4 +Материал: `43459bcbe0fdbee1da574ba5408ec29511da5eb0` (единственный коммит на +`origin/dev` `a55f5644`), рабочая копия уже на нём — фактическая проверка не +меняла ничего в дереве. + +## Скоуп + +Класс B (`scripts/mutation-guard-outcome.mjs`, `scripts/mutation-gate.mjs`, +`scripts/mutation-registry.mjs`, `test/mutation-guard-outcome.test.mjs`) + C +(`docs/TESTING.md`, `docs/testing-notes/*`). Файлов класса A нет. +`User-Visible: no` — правка внутреннего CI-инструмента (`mutation-gate`), не +поверхности карточки; changelog не тронут, что и ожидалось. + +Задача не привязана к строке Core user job из `docs/SCOPE.md` напрямую — это +инфраструктурный дефект таксономии мутационного гейта (линия #550/#558/#568), +такого же типа, что и уже принятые предшественники. Продуктовых пользователей +не касается; относится к «Keep the plan true» лишь косвенно — через +достоверность собственных тестов проекта. Отношу это к штатному +tech-debt/infra классу, отдельного вопроса по SCOPE не поднимаю. + +## Что делает дифф + +`node --test --test-name-pattern=X file` при отсутствии совпадений завершается +кодом 0 и печатает в TAP только строку самого файла — раньше это читалось как +здоровый чистый прогон и как `survived` на мутанте, хотя ни один ассерт не +выполнялся. Дифф: + +1. `shellWords`/`nodeTestSelection`/`executedTestNames`/`emptyTestSelection` — + парсят гард, находят фильтр `--test-name-pattern`, читают реальный TAP/spec + вывод и решают, было ли исполнено хоть одно именованное подтесто. +2. `classifyCommandResult` (фаза `oracle`, статус 0): если фильтр есть и + исполненных тестов ноль → `setup-failure` вместо `survived`. Существующая + инфраструктура #568 (`setupFailureOwner`, `attributeSetupFailure`) и её + печать `FAIL чистая подготовка` переиспользуются без изменений — код + заводится в них через тот же `MUTATION_OUTCOME.SETUP`. +3. `mutation-gate --check`: `staticTestSelectionProblems` статически проверяет, + что каждый `--test-name-pattern` совпадает хотя бы с одним литеральным + именем `test(`/`it(`/`describe(`/`suite(`/`t.test(` в указанных файлах + гварда; динамические имена (`` `${…}` ``) — `WARN`, не `FAIL`. +4. Реестр: три новых мутанта, каждый патчит одну из трёх новых функций. + +## Как проверялось + +Дешёвые гейты этого SHA уже зелёные на Validate +[36154735410](https://github.com/Matysh/houseplan-card/actions/runs/36155261126) +(ссылка в задаче ревью) — `tsc --noEmit`, `npm test`, `npm run build` со +сверкой бандла не перегонял. + +Дополнительно к этому лично прогнал и проверил построчно: + +| Гейт | Команда | Результат | +|---|---|---| +| Новый тестовый файл | `node --test test/mutation-guard-outcome.test.mjs` | 22/22 pass, включая все 6 новых `#650 …` | +| Реестр статически | `node scripts/mutation-gate.mjs --check` | 961 `ok`, 0 `FAIL`, 3 `WARN` (совпадает с заявленным в хендоффе) | +| Доки | `node scripts/check-docs.mjs --screenshots=warn` | `Documentation checks passed (7 files, 12 external links)` — новые якоря (`#пустое-совпадение---test-name-pattern-650`) резолвятся | +| Смоки | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «браузерный смок этим диффом не выбирается» — `src/**` не тронут, нечего выбирать | +| Реальный `node --test` | ad-hoc: временный файл с одним тестом, пустой/непустой `--test-name-pattern` | пустой фильтр → `SETUP`, совпадающий → `SURVIVED`, как заявлено (AC1) | +| Node-грамматика `--test-name-pattern` | ad-hoc с реальным `node --test` (форма `/pattern/flags`, регистр, несколько флагов как OR) | подтвердил, что `testNamePatternRegExp` и OR-логика в `staticTestSelectionProblems` соответствуют фактическому поведению установленного Node 22.23.2 | +| Покрытие реестра парсером | ad-hoc скрипт: у всех 439 вхождений `--test-name-pattern` в `MUTANT_DEFINITIONS` `nodeTestSelection` возвращает непустой результат | 0 несовпадений — статический чек не проходит мимо ни одного реального фильтра | + +### Мутанты — не «доказательство доверием», а лично воспроизведены (AC · чем краснеет) + +Для каждого из трёх новых мутантов вручную применил патч из `mutation-registry.mjs` +(строка `find` → `replace`), прогнал его собственный гард, убедился в красном +результате, откатил и подтвердил возврат к зелёному. + +| AC | Мутант | Патч | Прогон гарда на патче | Откат | +|---|---|---|---|---| +| AC1 (пустой фильтр → setup-failure, не survived) | `empty-test-selection-reads-as-survived` | `const empty = ...` → `const empty = null;` | `node --test --test-name-pattern="#650 (clean run\|mutant run\|the installed node)" test/mutation-guard-outcome.test.mjs` → **3 fail** (ожидали `SETUP`, получили `survived`) | подтверждено зелёным после возврата файла | +| AC3 (`--check` статически ловит несовпавший фильтр) | `static-test-selection-accepts-unmatched` | `if (names.some(...)) continue;` → `if (names.length \|\| !names.length) continue;` (всегда true) | `--test-name-pattern="#650 static check" test/mutation-guard-outcome.test.mjs` → **1 fail** | подтверждено | +| Экранирование кавычек (`shellWords`) | `shell-words-drops-regex-escape` | POSIX-условие экранирования внутри `"…"` ослаблено до «эскейпит любой следующий символ» | `--test-name-pattern="#650 (selection\|static check)" ...` → **2 fail** | подтверждено | + +Все три «умеют падать» — не формальная фраза, а лично прогнанный красный +вывод с конкретной сообщением ассерта. + +### Чего не проверял + +- Полный `npm test`/`tsc --noEmit`/`npm run build` с трёхсторонней сверкой + бандла — опираюсь на зелёный Validate этого SHA (ссылка в материале + ревью), сам не перегонял. +- Ночной полный прогон мутационного реестра (961 гард целиком с реальными + worktree) — не входит в объём ревью, это отдельный конвейер (#472). +- `golden:verify` — diff не меняет рендер, не запускал. +- `pytest tests_backend` — diff не трогает `custom_components/**/*.py`, не + запускал. +- `npm run invariants` — diff не трогает геометрию модели, не запускал. +- Поведение `shellWords` внутри одинарных кавычек с обратным слешем — + реестр такого не использует (только двойные кавычки), код формально + корректен (эскейпы внутри `'...'` не обрабатываются, как и положено в + POSIX), но отдельного юнита на этот путь нет; не блокирую, т.к. не + задействовано ни одним реальным гардом сейчас (проверил построчным + сопоставлением реестра). +- Windows-путь исполнения гвардов (`shell: true` на `cmd`) — автор сам + называет это в «чего не проверял»; согласен, что вне объёма: реестр и + раннер сейчас работают в POSIX-окружении CI. + +## Находки + +Нет ни одной находки High или Medium. Просмотрел код на предмет: +false negative статического чека (совпадение всех 439 реальных вхождений +`--test-name-pattern` в реестре — подтверждено), false positive на +`.skip()`-тестах, покрытых фильтром (это осознанно то же самое «ассерт не +исполнялся» — корректная классификация, не баг), порядок/OR-семантику +нескольких `--test-name-pattern` (подтверждена реальным `node --test`), +интеграцию с существующей атрибуцией #568 (переиспользуется без изменений, +путь проверен чтением `mutation-execution.mjs:202-224` и +`mutation-gate.mjs:254-269`). + +## Что проверено и корректно + +- AC1–AC4 задачи выполнены и доказаны либо автотестом с личным + воспроизведением красного (мутанты), либо прогоном на реальном + установленном Node (тест «the installed node really reports…» — + `test/mutation-guard-outcome.test.mjs`), либо чтением интеграции с + неизменной инфраструктурой #568 (проверено чтением, не исполнением, но + сам путь исполняется существующими тестами `#550`/`#568`, не тронутыми + диффом). +- Грамматика `--test-name-pattern` (`/pattern/flags` и OR нескольких + флагов) реализована верно — сверено с фактическим Node 22.23.2, а не + только с документацией. +- `docs/TESTING.md` — 800 строк, потолок #634 не нарушен; новые якоря в + `testing-notes/infrastructure.md` и `README.md` резолвятся + (`check-docs.mjs` зелёный). +- Трейлеры `Issue: #650` и `User-Visible: no` на месте; правка не + видимого пользователю поведения — changelog корректно не тронут. +- Реестр не переписывает существующие 349 гардов (соответствует «Не + скоуп» issue), добавляет только 3 новых мутанта под новый код. + +## Вердикт + +Зелёный. AC доказаны, защитные мутанты лично воспроизведены как красные, +дешёвые и относящиеся к диффу гейты (check, check-docs, smoke-select, +целевой тестовый файл) прогнаны и подтверждены; полный набор дорогих гейтов +опирается на уже зелёный Validate этого SHA. + +--- + + + +## Материал раунда + +- Ветка: `issue/650-empty-test-pattern`, коммит `43459bcbe0fd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `68bd13e9493d2b30523ce0eda7baa8580b08abf9` + ``` + git log --all --format='%H %T' | grep 68bd13e9493d + ``` +- Тело issue: `b9a1627c2a5c963fba9fc27814286268fdadde47cd8cfdb936d46236b99c9ef8` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 6db020c1..7201c519 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1061, issue: 374. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1062, issue: 375. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #650 | [CODE-REVIEW-650-r1.md](CODE-REVIEW-650-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` | | #649 | [SPEC-REVIEW-649-r2.md](SPEC-REVIEW-649-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #648 | [SPEC-REVIEW-648-r1.md](SPEC-REVIEW-648-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | «Сценарий» не называет персону и; AC10 называет provenance-gate | `docs/SCOPE.md` `scripts/validate-commit-provenance.mjs` `validate.yml` |