From 3e98ac4dde6d9b78c17d55219d4d868e06d548b1 Mon Sep 17 00:00:00 2001 From: Matysh Date: Tue, 6 Oct 2026 16:44:30 +0300 Subject: [PATCH] fix(test): include late page errors in smoke teardown verdict Count delivered page errors after awaited browser close. Exercise the public harness lifecycle with deterministic negative subprocess cases and retain the unchanged Chromium probes for the real transport. Move the premature-snapshot mutation witness to the deterministic Node suite. Issue: #776 User-Visible: no --- demo/guard/README.md | 18 ++ demo/serve.mjs | 19 +- docs/STATUS.md | 1 + docs/testing-notes/mutation-browser-guards.md | 10 +- scripts/mutation-registry.mjs | 13 +- test/smoke-exception-guard.test.mjs | 35 +--- test/smoke-harness-lifecycle.test.mjs | 169 ++++++++++++++++++ 7 files changed, 217 insertions(+), 48 deletions(-) create mode 100644 test/smoke-harness-lifecycle.test.mjs diff --git a/demo/guard/README.md b/demo/guard/README.md index 87f81a66..495bbb62 100644 --- a/demo/guard/README.md +++ b/demo/guard/README.md @@ -17,6 +17,24 @@ Запускает их `verify-guard.mjs`; он же вызывается из job «Смоки в браузере» и служит guard'ом мутантов в `scripts/mutation-gate.mjs`. +## Граница вердикта (#776) + +`finish()` сначала делает round-trip по открытым страницам, затем дожидается +`browser.close()` и только после этого считает доставленные `pageerror`. +Round-trip не опустошает все очереди браузера: хвостовое отклонение промиса +может прийти во время закрытия. Снимок счётчика до `close()` давал `EXC`, затем +ложный `OK` и exit 0. Теперь любое событие, доставленное до итогового вердикта, +входит в единую диагностику и exit 1; произвольные будущие таймеры не ожидаются. +`reportPageErrors()` сохраняет отдельный контракт: round-trip без закрытия +браузера, итог по событиям, доставленным к возврату этого вызова. + +`node --test test/smoke-harness-lifecycle.test.mjs` детерминированно исполняет +публичные `watchPage`/`finish`/`reportPageErrors` с управляемой доставкой событий, +в том числе внутри асинхронного закрытия. Он не требует Chromium и проверяет +реальный код выхода дочернего процесса и отсутствие ложного `OK`. Исходные +браузерные пробы остаются без задержек и подавления ошибок: они проверяют +настоящий канал Playwright, а не подменяют его Node-свидетелем. + Одна проба живёт не здесь: `--guard-probe` у `demo/benchmark_backdrop_decode.mjs` (#430). Benchmark нельзя переселить в этот каталог — его запускают руками при рекалибровке порогов, — поэтому `verify-guard.mjs` умеет запускать файл выше diff --git a/demo/serve.mjs b/demo/serve.mjs index 67f7c625..97de3c0f 100644 --- a/demo/serve.mjs +++ b/demo/serve.mjs @@ -37,13 +37,12 @@ const _livePages = new Set(); * уносит недоставленное событие. В логе это видно дословно: `EXC` печатается * ПОСЛЕ результата и ДО `OK`. * - * Круговой запрос к странице вытесняет ранее поставленные макрозадачи, поэтому - * всё, что страница успела произвести до этого момента, к нам уже дошло. - * - * Честная граница: исключение, возникшее ПОСЛЕ этого round-trip'а — например, в - * обработчике `beforeunload` при закрытии браузера, — не учитывается. Ловить его - * значит ждать неизвестно чего неизвестно сколько; контракт формулируется как - * «всё, что произошло до вызова вердикта». + * Круговой запрос даёт странице возможность доставить накопленные события, + * но не является барьером для всех browser task queues: например, хвостовое + * отклонение промиса может прийти уже во время browser.close() (#776). + * finish() поэтому читает счётчик только после завершения закрытия. Граница — + * все pageerror, доставленные до вердикта; произвольных будущих таймеров не ждём. + * reportPageErrors() браузер не закрывает и судит события после своего запроса. */ async function roundTripLivePages() { for (const page of _livePages) { @@ -111,11 +110,11 @@ export async function reportPageErrors() { /** Print the result, report failures, close the browser, set the exit code. */ export async function finish(browser, out) { if (out !== undefined) console.log(JSON.stringify(out, null, 1)); - // Порядок обязателен: сначала дать странице доставить события, потом читать - // счётчик (#404). Обратный порядок и был дефектом. + // #404/#776: дать доставить события и дождаться закрытия, затем снять итог. + // Между round-trip и close тоже приходят pageerror: ранний снимок давал EXC + OK. await roundTripLivePages(); - if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`); await browser?.close?.(); + if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`); // #629: доказательство «перевод без потери утверждений» — отсортированный // список имён проверок; без переменной вывод прежний. if (process.env.HP_SMOKE_CHECKS === '1') { diff --git a/docs/STATUS.md b/docs/STATUS.md index 1c2737c9..ad33f118 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -29,6 +29,7 @@ Everything computable from the tree and git; regenerate, never edit by hand |---|---| | Current local cycle | **v1.80.0-beta.4 candidate** — #802 remote Zigbee destination names and collision-aware tooltips passed owner-authorized local review and are merged into `dev`. WSL unit/build, targeted browser checks and full golden are green; full exact-SHA Validate and artifact verification precede publication. `main` remains on stable v1.79.0. | | Branches | `main` carries stable releases only; pre-release tags point at `dev`. Work lands on `dev`, which is equal to or ahead of `main`, never behind. | +| Smoke exception guard | #776 makes `finish()` aggregate delivered page errors after awaited browser teardown, so a late rejection cannot print `EXC` followed by a false `OK`. Deterministic Node lifecycle tests complement the unchanged Chromium guard probes. | | Device battery | #792 adds a default-on, installation-wide own-device charge indicator, independent of static face modes. LED strips are excluded; multiple sources use only the first. Standard HA MDI icons in the designer's layout, frozen-frame diagnostics and Zigbee-caption priority share the same implementation across device surfaces. | | Zigbee routes | #798 replaces inferred neighbour trees with integration-reported end-parent and active coordinator next-hop evidence. Unknown/conflicting routes are not guessed; stale/partial snapshots remain labelled. Solid arrows have a separate 0–255 palette; ordinary device LQI colours are unchanged. | | Zigbee caption layout | #802 adds remote destination names and keeps the matching pointer tooltip clear of diagnostic text; only an unplaceable tooltip yields in a small card. Focus/touch/actions and provider transport remain unchanged. | diff --git a/docs/testing-notes/mutation-browser-guards.md b/docs/testing-notes/mutation-browser-guards.md index f08202c1..9064f576 100644 --- a/docs/testing-notes/mutation-browser-guards.md +++ b/docs/testing-notes/mutation-browser-guards.md @@ -32,12 +32,12 @@ that transition events alone prove disposal. | Category | Count | Why a browser is still required | | --- | ---: | --- | | Performance threshold | 4 | The witness measures real browser wall-time or frame work; a pure assertion cannot prove the budget. | -| Browser harness integrity | 4 | The mutation breaks page-error, round-trip or page-registration observation in the browser harness itself. | +| Browser harness integrity | 3 | The mutation breaks page-error, round-trip or page-registration observation in the browser harness itself. | | Paint, cascade and layer composition | 42 | The invariant depends on computed CSS, SVG paint, clipping, stacking or pixels produced by Chromium. | | Pointer geometry and trusted interaction | 50 | The invariant depends on hit testing, pointer capture, touch/keyboard dispatch or live DOM geometry. | | Responsive DOM layout | 39 | The invariant depends on measured element boxes, responsive breakpoints, native/HA dialog shells or focusable target size. | | Custom-element and HA browser lifecycle | 104 | The invariant crosses Lit/custom-element lifecycle, browser storage/events, lazy loading or a complete HA-card state transition. | -| **Total** | **243 / 200** | Above the guideline `mutation-gate --check` warns rather than fails (#699); each guard above it is held by its own reason in this inventory and its `because`. | +| **Total** | **242 / 200** | Above the guideline `mutation-gate --check` warns rather than fails (#699); each guard above it is held by its own reason in this inventory and its `because`. | ## Measured effect @@ -86,9 +86,13 @@ The mutation breaks page-error, round-trip or page-registration observation in t - `benchmark-page-verdict-unwatched` - `report-page-errors-skips-round-trip` -- `smoke-guard-blind-to-tail` - `smoke-guard-forgets-to-register-pages` +`smoke-guard-blind-to-tail` moved to `test/smoke-harness-lifecycle.test.mjs` +in #776: controlled delivery during awaited close deterministically rejects a +premature counter snapshot. The unchanged Chromium probes still check the +real Playwright page-error channel. + ### Paint, cascade and layer composition - `battery-icon-collapses-glyph` diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index 2581942b..c16eb0ed 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -1814,14 +1814,15 @@ const MUTANT_DEFINITIONS = [ }, { id: 'smoke-guard-blind-to-tail', - guard: 'node demo/guard/verify-guard.mjs', - because: 'the uncaught-exception guard must read its counter AFTER the page delivered ' - + 'its events; reading it first is the defect of #404 and looks identical to a ' - + 'working guard in everything but the outcome', + guard: 'node --test test/smoke-harness-lifecycle.test.mjs', + because: 'the public harness must include page errors delivered during awaited teardown ' + + '(#404/#776); controlled event delivery proves the premature snapshot without a browser race', patches: [{ file: 'demo/serve.mjs', - find: ' await roundTripLivePages();\n if (_pageErrors) _failures.push(', - replace: ' if (_pageErrors) _failures.push(', + find: ' await roundTripLivePages();\n await browser?.close?.();\n' + + ' if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`);', + replace: ' if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`);\n' + + ' await roundTripLivePages();\n await browser?.close?.();', }], }, { diff --git a/test/smoke-exception-guard.test.mjs b/test/smoke-exception-guard.test.mjs index 3c806b53..52d4202e 100644 --- a/test/smoke-exception-guard.test.mjs +++ b/test/smoke-exception-guard.test.mjs @@ -7,34 +7,13 @@ import { fileURLToPath } from 'node:url'; // срабатывал ни разу: счётчик читался синхронно, а Playwright доставляет // pageerror асинхронно. Поведение доказывается запуском проб — // demo/guard/verify-guard.mjs в job со браузером. Здесь закреплено то, что -// запуском не проверить: порядок операций в исходнике, место проб и наличие -// вызова там, где он должен быть. +// запуском этих проб не проверить: место проб и наличие вызова в CI. +// Порядок доставки/закрытия/вердикта проверяется исполнением публичного API в +// smoke-harness-lifecycle.test.mjs (#776), а не текстом реализации. const ROOT = fileURLToPath(new URL('../', import.meta.url)); const read = (rel) => readFileSync(new URL(rel, `file://${ROOT}`), 'utf8'); -test('счётчик читается ПОСЛЕ доставки событий, а не до (#404)', () => { - const serve = read('demo/serve.mjs'); - const flush = serve.indexOf('await roundTripLivePages();\n if (_pageErrors)'); - assert.ok(flush > 0, - 'round-trip обязан стоять непосредственно перед чтением счётчика: обратный' - + ' порядок и был дефектом #404'); - // Вторая половина правки: опрашивать нечего, если страницы не регистрируются. - assert.match(serve, /const _livePages = new Set\(\)/); - assert.match(serve, /export function watchPage\(page\)/); - assert.match(serve, /_livePages\.add\(page\)/); - assert.match(serve, /page\.on\('close', \(\) => _livePages\.delete\(page\)\)/, - 'закрытая страница обязана уходить из реестра'); - // Round-trip к закрытой странице бросает — и не имеет права ронять вердикт. - assert.match(serve, /try \{ await page\.evaluate\(\(\) => 0\); \} catch/); -}); - -test('оба читателя счётчика ждут доставки (#404, #407)', () => { - const serve = read('demo/serve.mjs'); - assert.match(serve, /export async function reportPageErrors\(\)\s*\{\s*await roundTripLivePages\(\);/, - 'вердикт для смоков со своей развязкой обязан ждать так же, как finish()'); -}); - test('пробы гарда лежат вне маски смоков и вне корпуса отпечатка (#404)', () => { const guard = readdirSync(new URL('demo/guard/', `file://${ROOT}`)); assert.ok(guard.includes('verify-guard.mjs')); @@ -70,12 +49,10 @@ test('пробы вызываются в job с браузером и служа assert.match(smoke, /if: matrix\.shard == 1/, 'один раз, а не в каждом шарде'); const mutants = read('scripts/mutation-registry.mjs'); - // Правка #404 состоит из двух половин, и мутант на одну оставил бы другую - // недоказанной. Список ведётся руками, и это осознанно: счётчик обязан - // совпадать с ним, поэтому новый мутант на этих пробах нельзя добавить, не - // назвав его здесь (в #430 так добавился четвёртый — гард page-benchmark). + // Только browser guards: premature snapshot с #776 доказывается без гонки + // в smoke-harness-lifecycle.test.mjs. Настоящий канал Playwright остаётся + // предметом verify-guard и остальных browser guards. const guarded = [ - 'smoke-guard-blind-to-tail', 'smoke-guard-forgets-to-register-pages', 'report-page-errors-skips-round-trip', 'benchmark-page-verdict-unwatched', diff --git a/test/smoke-harness-lifecycle.test.mjs b/test/smoke-harness-lifecycle.test.mjs new file mode 100644 index 00000000..a0fc541e --- /dev/null +++ b/test/smoke-harness-lifecycle.test.mjs @@ -0,0 +1,169 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import test from 'node:test'; + +// #776: execute the public harness API in an isolated process. Fake pages own +// delivery order, not the verdict: no Chromium, timer races or source matching. +const ROOT = fileURLToPath(new URL('../', import.meta.url)); +const SERVE_URL = new URL('../demo/serve.mjs', import.meta.url).href; +const PRELUDE = ` + import assert from 'node:assert/strict'; + import { EventEmitter } from 'node:events'; + import { check, finish, reportPageErrors, watchPage } from ${JSON.stringify(SERVE_URL)}; + const makePage = (evaluate = async () => 0) => { + const page = new EventEmitter(); + page.evaluate = evaluate; + return watchPage(page); + }; +`; + +function runHarness(body, expectedExit) { + const complete = 'HARNESS_SCENARIO_COMPLETED'; + const child = spawnSync(process.execPath, ['--input-type=module', '--eval', + PRELUDE + body + `\nconsole.log(${JSON.stringify(complete)});`], { + cwd: ROOT, encoding: 'utf8', timeout: 15_000, + }); + const output = `${child.stdout || ''}${child.stderr || ''}`; + assert.equal(child.error, undefined, `harness subprocess failed to run: ${child.error}\n${output}`); + assert.equal(child.signal, null, `harness subprocess was interrupted: ${child.signal}\n${output}`); + // An assertion inside a negative case also exits 1: require normal completion + // so it cannot impersonate the guard's expected nonzero exit. + assert.match(child.stdout, new RegExp(`^${complete}\\r?$`, 'm'), output); + assert.equal(child.status, expectedExit, output); + return output.replace(new RegExp(`^${complete}\\r?\\n`, 'm'), ''); +} + +function hasExceptionVerdict(output, count) { + assert.match(output, new RegExp(`^(?: - |FAILED: )${count} uncaught exception\\(s\\) inside the card\\r?$`, 'm')); + assert.doesNotMatch(output, /^OK\r?$/m, 'an exception verdict must not also claim success'); +} + +test('#776 finish includes a pageerror delivered during awaited browser close', () => { + const output = runHarness(` + const page = makePage(); + const browser = { close: async () => { + await Promise.resolve(); + page.emit('pageerror', new Error('guard-tail-rejection')); + page.emit('close'); + } }; + await finish(browser, { probe: 'async close delivery' }); + `, 1); + assert.match(output, /EXC Error: guard-tail-rejection/); + hasExceptionVerdict(output, 1); +}); + +test('#776 finish counts every watched page across round-trip and close exactly once', () => { + const calls = ['drain:first', 'drain:second', 'close']; + const output = runHarness(` + const calls = []; + const first = makePage(async () => { + calls.push('drain:first'); + first.emit('pageerror', new Error('first-page-drain')); + }); + const second = makePage(async () => { + calls.push('drain:second'); + second.emit('pageerror', new Error('second-page-drain')); + }); + await finish({ close: async () => { + calls.push('close'); + await Promise.resolve(); + second.emit('pageerror', new Error('second-page-close')); + first.emit('close'); + second.emit('close'); + } }); + assert.deepEqual(calls, ${JSON.stringify(calls)}); + `, 1); + hasExceptionVerdict(output, 3); + assert.equal((output.match(/uncaught exception\(s\) inside the card/g) || []).length, 1); +}); + +test('#776 clean finish drains registered pages before closing and keeps exit zero', () => { + const output = runHarness(` + const calls = []; + const page = makePage(async () => { calls.push('drain'); }); + await finish({ close: async () => { + await Promise.resolve(); + calls.push('close'); + page.emit('close'); + } }, { probe: 'clean' }); + assert.deepEqual(calls, ['drain', 'close']); + `, 0); + assert.match(output, /^OK\r?$/m); + assert.doesNotMatch(output, /FAILED|EXC/); +}); + +test('#776 closed pages leave the registry and finish accepts no browser', () => { + const output = runHarness(` + let reads = 0; + const page = makePage(async () => { reads++; }); + page.emit('close'); + await finish(undefined, { probe: 'already closed' }); + assert.equal(reads, 0); + `, 0); + assert.match(output, /^OK\r?$/m); + assert.doesNotMatch(output, /FAILED|EXC/); +}); + +test('#776 a closing-page round-trip rejection does not fabricate a card exception', () => { + const output = runHarness(` + let reads = 0; + const page = makePage(async () => { + reads++; + page.emit('close'); + throw new Error('page closed while round-trip was pending'); + }); + await finish(); + assert.equal(reads, 1); + `, 0); + assert.match(output, /^OK\r?$/m); + assert.doesNotMatch(output, /FAILED|EXC/); +}); + +test('#776 finish without a browser still reports errors delivered by its round-trip', () => { + const output = runHarness(` + const page = makePage(async () => { + await Promise.resolve(); + page.emit('pageerror', new Error('round-trip-without-browser')); + }); + await finish(); + `, 1); + hasExceptionVerdict(output, 1); +}); + +test('#776 finish preserves ordinary assertion failures through clean async close', () => { + const output = runHarness(` + const page = makePage(); + check('retained-assertion', false); + await finish({ close: async () => { + await Promise.resolve(); + page.emit('close'); + } }); + `, 1); + assert.match(output, /retained-assertion: expected true, got false/); + assert.doesNotMatch(output, /uncaught exception|^OK\r?$/m); +}); + +test('#776 reportPageErrors independently awaits delivery and returns the failure verdict', () => { + const output = runHarness(` + const page = makePage(async () => { + await Promise.resolve(); + page.emit('pageerror', new Error('reporter-drain')); + }); + assert.equal(await reportPageErrors(), true); + page.emit('close'); + `, 1); + hasExceptionVerdict(output, 1); +}); + +test('#776 reportPageErrors independently keeps a clean closed page quiet', () => { + const output = runHarness(` + let reads = 0; + const page = makePage(async () => { reads++; }); + assert.equal(await reportPageErrors(), false); + page.emit('close'); + assert.equal(await reportPageErrors(), false); + assert.equal(reads, 1); + `, 0); + assert.equal(output, ''); +});