From 0c9cf9503ea55b97d31c3cf58a55f29db9741928 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 11:52:36 +0300 Subject: [PATCH] =?UTF-8?q?test:=20=D0=BF=D1=80=D0=B8=D1=91=D0=BC=D0=BA?= =?UTF-8?q?=D0=B0=20=D0=BF=D0=B5=D1=80=D0=B5=D0=BF=D0=B8=D1=81=D1=8B=D0=B2?= =?UTF-8?q?=D0=B0=D0=B5=D1=82=20=D1=82=D0=BE=D0=BB=D1=8C=D0=BA=D0=BE=20?= =?UTF-8?q?=D0=BE=D0=B1=D1=8A=D1=8F=D0=B2=D0=BB=D0=B5=D0=BD=D0=BD=D1=8B?= =?UTF-8?q?=D0=B5=20=D1=8D=D1=82=D0=B0=D0=BB=D0=BE=D0=BD=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `passed` означает «в пределах порога», а не «байт в байт»: comparePng считает diffRatio, и статус ставится по нему. А приёмка копировала кандидата поверх КАЖДОГО эталона матрицы, поэтому подпороговый дрейф уезжал в контракт молча — и накапливался: каждая приёмка подтягивала эталон к последней среде, порог не пересекался никогда, а эталон уходил. Так 1e341c60 заменил 22 картинки, объявив четыре. Проект уже сталкивался с этим: ad3f9981 восстанавливал девять уехавших эталонов руками. Такую работу обязан делать инструмент. Теперь копируются только сцены из --expect-change и --expect-new; остальные сохраняют и файл, и свой хеш из прежнего индекса. Индекс по-прежнему перезаписывается на полный набор — сирота или пропавшая запись делают манифест недействительным целиком. Решение вынесено в чистую функцию goldenAcceptancePlan: оно одно, и ошибка в нём дорога. Отсутствие прежнего хеша у необъявленной сцены — ошибка, а не повод взять кандидата: без эталона бывает только новая сцена, а она обязана быть названа в --expect-new. Логика вернулась в demo/golden/accept.mjs, где ей и место: после #344 эти файлы исключены из корпуса отпечатка, так что правка больше не требует пересборки и пересъёмки. scripts/golden-accept.mjs остался проходным вызовом ради документированной команды. Проверено сквозным прогоном на синтетическом кандидате: у двух сцен байты другие, объявлена одна — на диске изменились ровно два файла, эталон и индекс, а хеш второй сцены остался прежним. Два мутанта убиты руками: «брать кандидата вместо прежнего хеша» и «заменять всё». Issue: #351 User-Visible: no --- demo/golden/README.md | 6 +++- demo/golden/accept.mjs | 61 ++++++++++++++++++++++++++------ scripts/golden-accept.mjs | 65 ++++++----------------------------- scripts/golden-acceptance.mjs | 43 +++++++++++++++++++++++ test/golden-policy.test.mjs | 59 ++++++++++++++++++++++++++++++- 5 files changed, 166 insertions(+), 68 deletions(-) diff --git a/demo/golden/README.md b/demo/golden/README.md index 3a22c82c..f9612c7f 100644 --- a/demo/golden/README.md +++ b/demo/golden/README.md @@ -34,7 +34,11 @@ radial spokes visible instead of hiding them under a translucent room fill. baseline moved"; `--expect-new=` means "I have looked at this new frame". Anything that differs, or arrives without a baseline, and is not named refuses the whole acceptance before a single file is copied; naming a scenario - under the wrong flag refuses it too. The first flag is what makes a local + under the wrong flag refuses it too. Only the named scenarios are written: + everything else keeps its reviewed bytes and its manifest hash, because + `passed` means "within threshold", not "byte-identical", and copying every + candidate let sub-threshold drift ratchet the baselines to the newest + environment unseen (#351). The first flag is what makes a local capture admissible (see below) and blocks the one-command "accept everything so CI turns green"; the second stops an empty or clipped frame from becoming the contract unseen (#350). diff --git a/demo/golden/accept.mjs b/demo/golden/accept.mjs index 9753c8f5..ac6ff651 100644 --- a/demo/golden/accept.mjs +++ b/demo/golden/accept.mjs @@ -6,11 +6,21 @@ import { fileURLToPath } from 'node:url'; import { sourceFingerprint } from '../../scripts/source-fingerprint.mjs'; import { GOLDEN_MATRIX_VERSION, GOLDEN_SCENARIOS } from './matrix.mjs'; import { GOLDEN_BASELINE_MANIFEST } from './policy.mjs'; +import { + goldenAcceptancePlan, goldenAcceptanceRefusal, goldenSilentDeclarations, +} from '../../scripts/golden-acceptance.mjs'; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); const reviewed = process.argv.includes('--reviewed'); const fromArg = process.argv.find((arg) => arg.startsWith('--from=')); const from = resolve(fromArg ? fromArg.slice('--from='.length) : resolve(ROOT, 'artifacts/golden')); +const list = (name) => { + const found = process.argv.find((arg) => arg.startsWith(`--${name}=`)); + return (found ? found.slice(name.length + 3) : '') + .split(',').map((id) => id.trim()).filter(Boolean); +}; +const declared = list('expect-change'); +const declaredNew = list('expect-new'); if (!reviewed) throw new Error('refusing to replace baselines without explicit --reviewed'); const reportPath = resolve(from, 'golden-report.json'); @@ -24,11 +34,29 @@ if (typeof report.chromium !== 'string' || !report.chromium) throw new Error('candidate report does not identify its Chromium build'); if (!Array.isArray(report.results)) throw new Error('candidate report has no scenario results'); +const refusal = goldenAcceptanceRefusal(report.results, declared, declaredNew); +if (refusal) throw new Error(refusal); + const byId = new Map(report.results.map((result) => [result.id, result])); const baselineRoot = resolve(ROOT, 'demo/golden/baselines'); mkdirSync(baselineRoot, { recursive: true }); -const hashes = {}; -const candidates = []; +/** + * Прежний индекс: источник хешей для сцен, которые остаются как были (#351). + * + * `passed` не значит «байт в байт» — он значит «в пределах порога». Прежняя + * версия копировала кандидата поверх КАЖДОГО эталона, поэтому подпороговый + * дрейф уезжал в контракт молча, и накапливался: каждая приёмка подтягивала + * эталон к последней среде, порог не пересекался никогда, а эталон уходил. + * Так `1e341c60` заменил 22 картинки, объявив четыре. Владелец делал эту работу + * руками (`ad3f9981`: «nine unrelated baselines … were restored to their + * reviewed versions»); теперь её делает инструмент. + */ +const manifestPath = resolve(baselineRoot, GOLDEN_BASELINE_MANIFEST); +const previous = existsSync(manifestPath) + ? JSON.parse(readFileSync(manifestPath, 'utf8')).scenarios || {} + : {}; +// Кандидат проверяется целиком, до всякого решения о замене: сломанный отчёт +// не имеет права оставить каталог эталонов половинным. for (const scenario of GOLDEN_SCENARIOS) { const result = byId.get(scenario.id); const candidate = resolve(from, 'actual', `${scenario.id}.png`); @@ -36,16 +64,20 @@ for (const scenario of GOLDEN_SCENARIOS) { throw new Error(`review candidate has an invalid run status: ${scenario.id} (${result?.status || 'missing'})`); if (!result?.actualSha256 || !existsSync(candidate)) throw new Error(`review candidate missing: ${scenario.id}`); - const bytes = readFileSync(candidate); - const digest = createHash('sha256').update(bytes).digest('hex'); + const digest = createHash('sha256').update(readFileSync(candidate)).digest('hex'); if (digest !== result.actualSha256) throw new Error(`candidate changed after capture: ${scenario.id}`); - candidates.push({ scenario, candidate }); - hashes[scenario.id] = digest; } -// Validate the complete set first: a broken report must never leave a half- -// updated baseline directory behind. -for (const { scenario, candidate } of candidates) - copyFileSync(candidate, resolve(baselineRoot, `${scenario.id}.png`)); +const plan = goldenAcceptancePlan({ + scenarioIds: GOLDEN_SCENARIOS.map((scenario) => scenario.id), + results: report.results, + previousHashes: previous, + declared, + declaredNew, +}); +const hashes = plan.hashes; +for (const id of plan.replace) { + copyFileSync(resolve(from, 'actual', `${id}.png`), resolve(baselineRoot, `${id}.png`)); +} writeFileSync(resolve(baselineRoot, GOLDEN_BASELINE_MANIFEST), `${JSON.stringify({ schema: 1, matrixVersion: GOLDEN_MATRIX_VERSION, @@ -54,4 +86,11 @@ writeFileSync(resolve(baselineRoot, GOLDEN_BASELINE_MANIFEST), `${JSON.stringify chromium: report.chromium, scenarios: hashes, }, null, 2)}\n`, 'utf8'); -console.log(`Accepted ${GOLDEN_SCENARIOS.length} reviewed golden baselines.`); +const silent = goldenSilentDeclarations(report.results, declared); +if (silent.length) { + console.log(`Объявлены как изменённые, но совпали с эталоном: ${silent.join(', ')}.`); +} +console.log(`Заменено эталонов: ${plan.replace.length}` + + `${plan.replace.length ? ` (${[...plan.replace].sort().join(', ')})` : ''}.`); +console.log(`Сохранено без изменений: ${plan.keep.length}.`); +console.log(`Индекс перезаписан на ${GOLDEN_SCENARIOS.length} сцен.`); diff --git a/scripts/golden-accept.mjs b/scripts/golden-accept.mjs index 39a8ecc8..f774dd39 100644 --- a/scripts/golden-accept.mjs +++ b/scripts/golden-accept.mjs @@ -1,76 +1,31 @@ #!/usr/bin/env node /** - * Приёмка эталонов с объявлением намерения (#334). + * Проходной вызов `demo/golden/accept.mjs` (#334, #350, #351). * * node scripts/golden-accept.mjs --reviewed --expect-change= * node scripts/golden-accept.mjs --reviewed --expect-new= - * node scripts/golden-accept.mjs --reviewed --expect-change= --from=<распакованный артефакт> + * + * Обёртка появилась потому, что до #344 любой `.mjs` из `demo/golden` входил в + * корпус отпечатка, и правка инструмента приёмки объявляла устаревшими бандл и + * оба манифеста. После #344 это ограничение снято, правило живёт в самом + * `accept.mjs`, а этот файл остался ради документированной команды и передаёт + * аргументы как есть. * * Два флага утверждают разное: `--expect-change` — «я знаю, почему старый кадр - * изменился», `--expect-new` — «я посмотрел на новый кадр». Путаница между ними - * останавливает приёмку (#350). - * - * Обёртка над `demo/golden/accept.mjs`, а не правка его самого: любой `.mjs` из - * `demo/golden` входит в `sourceFingerprint`, поэтому его правка объявляет - * устаревшими бандл и оба манифеста — см. `scripts/golden-acceptance.mjs`. - * - * Проверка идёт ДО делегирования: `accept.mjs` копирует картинки целым набором, - * и запрет обязан сработать раньше, чем каталог эталонов будет тронут. + * изменился», `--expect-new` — «я посмотрел на новый кадр». Необъявленная сцена + * не переписывается вовсе: её эталон и хеш остаются прежними (#351). */ import { spawnSync } from 'node:child_process'; -import { existsSync, readFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { goldenAcceptanceRefusal, goldenSilentDeclarations } from './golden-acceptance.mjs'; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); const argv = process.argv.slice(2); -const value = (name) => { - const found = argv.find((item) => item.startsWith(`--${name}=`)); - return found ? found.slice(name.length + 3) : ''; -}; - if (!argv.includes('--reviewed')) { console.error('приёмка требует явного --reviewed'); process.exit(2); } -const from = resolve(value('from') || resolve(ROOT, 'artifacts/golden')); -const list = (name) => value(name).split(',').map((id) => id.trim()).filter(Boolean); -const declared = list('expect-change'); -const declaredNew = list('expect-new'); - -const reportPath = resolve(from, 'golden-report.json'); -if (!existsSync(reportPath)) { - console.error(`отчёт кандидатов не найден: ${reportPath}`); - process.exit(2); -} -const report = JSON.parse(readFileSync(reportPath, 'utf8')); - -const refusal = goldenAcceptanceRefusal(report.results, declared, declaredNew); -if (refusal) { - console.error(refusal); - process.exit(1); -} - -const silent = goldenSilentDeclarations(report.results, declared); -if (silent.length) { - console.log(`Объявлены как изменённые, но совпали с эталоном: ${silent.join(', ')}.`); -} -// Новые эталоны печатаются отдельной строкой, а не в общем списке: раньше они -// растворялись среди изменившихся, и три кадра каталога устройств уехали в -// контракт незамеченными (#350). -const fresh = (report.results || []).filter((result) => result.status === 'missing-baseline') - .map((result) => result.id).sort(); -const changed = (report.results || []).filter((result) => result.status === 'different') - .map((result) => result.id).sort(); -console.log(changed.length - ? `Будут заменены эталоны: ${changed.join(', ')}.` - : 'Ни один существующий эталон не изменился.'); -if (fresh.length) console.log(`СТАНУТ КОНТРАКТОМ ВПЕРВЫЕ: ${fresh.join(', ')}.`); -if (!changed.length && !fresh.length) console.log('Будет перезаписан только манифест.'); -console.log(`Съёмка: chromium ${report.chromium || '?'}, матрица ${report.matrixVersion}.`); - const accept = spawnSync(process.execPath, [ - resolve(ROOT, 'demo/golden/accept.mjs'), '--reviewed', `--from=${from}`, + resolve(ROOT, 'demo/golden/accept.mjs'), ...argv, ], { cwd: ROOT, stdio: 'inherit' }); process.exit(accept.status ?? 1); diff --git a/scripts/golden-acceptance.mjs b/scripts/golden-acceptance.mjs index e27a2193..e41c5107 100644 --- a/scripts/golden-acceptance.mjs +++ b/scripts/golden-acceptance.mjs @@ -98,3 +98,46 @@ export const goldenAcceptanceRefusal = (results, declared = [], declaredNew = [] export const goldenSilentDeclarations = (results, declared = []) => declared .filter((id) => results.find((result) => result.id === id)?.status === 'passed') .sort(); + +/** + * План замены: что переписать, что оставить как было (#351). + * + * Чистая функция, потому что решение здесь одно и ошибка в нём дорога: + * `passed` означает «в пределах порога», а не «байт в байт». Прежняя приёмка + * копировала кандидата поверх КАЖДОГО эталона, поэтому подпороговый дрейф уезжал + * в контракт молча и накапливался: каждая приёмка подтягивала эталон к последней + * среде, порог не пересекался никогда, а эталон уходил. Так `1e341c60` заменил + * 22 картинки, объявив четыре. + * + * Необъявленная сцена сохраняет и файл, и свой хеш из прежнего индекса. Хеша нет + * только у сцены без эталона, а такая обязана быть названа в `--expect-new` — + * поэтому его отсутствие здесь ошибка, а не повод взять кандидата. + */ +export const goldenAcceptancePlan = ({ + scenarioIds, results, previousHashes = {}, declared = [], declaredNew = [], +}) => { + const accepted = new Set([...declared, ...declaredNew].filter(Boolean)); + const byId = new Map((results || []).map((result) => [result.id, result])); + const replace = []; + const keep = []; + const hashes = {}; + for (const id of scenarioIds) { + if (accepted.has(id)) { + const digest = byId.get(id)?.actualSha256; + if (typeof digest !== 'string' || !digest) { + throw new Error(`объявленная сцена без хеша кандидата: ${id}`); + } + replace.push(id); + hashes[id] = digest; + continue; + } + const existing = previousHashes[id]; + if (typeof existing !== 'string' || !existing) { + throw new Error(`необъявленная сцена без прежнего эталона: ${id};` + + ' назовите её в --expect-new'); + } + hashes[id] = existing; + keep.push(id); + } + return { replace, keep, hashes }; +}; diff --git a/test/golden-policy.test.mjs b/test/golden-policy.test.mjs index d11abec7..bb96bc54 100644 --- a/test/golden-policy.test.mjs +++ b/test/golden-policy.test.mjs @@ -7,7 +7,7 @@ import { goldenScenarioSetsMatch, } from '../demo/golden/policy.mjs'; import { - goldenAcceptanceRefusal, goldenSilentDeclarations, + goldenAcceptancePlan, goldenAcceptanceRefusal, goldenSilentDeclarations, } from '../scripts/golden-acceptance.mjs'; test('golden metadata cannot be mistaken for a Home Assistant integration manifest', () => { @@ -119,3 +119,60 @@ test('объявленная, но совпавшая сцена называе assert.deepEqual(goldenSilentDeclarations(list, ['a', 'b']), ['a']); assert.deepEqual(goldenSilentDeclarations(list, ['b']), []); }); + +// #351. `passed` означает «в пределах порога», а не «байт в байт». Прежняя +// приёмка копировала кандидата поверх каждого эталона, поэтому подпороговый +// дрейф уезжал в контракт молча и накапливался. Так 1e341c60 заменил 22 +// картинки, объявив четыре. + +test('необъявленная сцена сохраняет свой эталон и свой хеш (#351)', () => { + const plan = goldenAcceptancePlan({ + scenarioIds: ['declared', 'drifted', 'fresh'], + results: [ + { id: 'declared', status: 'different', actualSha256: 'new-declared' }, + // Байты кандидата другие, но расхождение подпороговое — и именно поэтому + // сцена не имеет права попасть в эталоны без объявления. + { id: 'drifted', status: 'passed', actualSha256: 'new-drifted' }, + { id: 'fresh', status: 'missing-baseline', actualSha256: 'new-fresh' }, + ], + previousHashes: { declared: 'old-declared', drifted: 'old-drifted' }, + declared: ['declared'], + declaredNew: ['fresh'], + }); + assert.deepEqual(plan.replace.sort(), ['declared', 'fresh']); + assert.deepEqual(plan.keep, ['drifted']); + assert.deepEqual(plan.hashes, { + declared: 'new-declared', + drifted: 'old-drifted', + fresh: 'new-fresh', + }); +}); + +test('индекс перезаписывается на полный набор сцен, а не на заменённые (#351)', () => { + // Полнота манифеста — инвариант goldenScenarioSetsMatch: сирота в индексе или + // пропавшая запись делают весь манифест недействительным. + const plan = goldenAcceptancePlan({ + scenarioIds: ['a', 'b', 'c'], + results: [{ id: 'a', status: 'different', actualSha256: 'na' }], + previousHashes: { a: 'oa', b: 'ob', c: 'oc' }, + declared: ['a'], + }); + assert.deepEqual(Object.keys(plan.hashes).sort(), ['a', 'b', 'c']); +}); + +test('необъявленная сцена без прежнего эталона — ошибка, а не тихий кандидат (#351)', () => { + assert.throws(() => goldenAcceptancePlan({ + scenarioIds: ['orphan'], + results: [{ id: 'orphan', status: 'missing-baseline', actualSha256: 'x' }], + previousHashes: {}, + }), /без прежнего эталона: orphan/); +}); + +test('объявленная сцена без хеша кандидата — ошибка (#351)', () => { + assert.throws(() => goldenAcceptancePlan({ + scenarioIds: ['a'], + results: [{ id: 'a', status: 'different' }], + previousHashes: { a: 'oa' }, + declared: ['a'], + }), /без хеша кандидата: a/); +});