From 5c8cb58e1ff89e9631072e405b26e16717f66889 Mon Sep 17 00:00:00 2001 From: Matysh Date: Mon, 31 Aug 2026 09:40:52 +0300 Subject: [PATCH] ci: prove the screenshot environment instead of trusting the place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Правило приёмки скриншотов было про место: снимать только в CI. Обоснование измерено — съёмка в другом окружении переписывает файлы без содержательных изменений, в #231 два из девяти на 7–8 байт, набор с беты все девять. Но держалось правило на комментарии, а не на механизме: кандидат проверялся на самосогласованность и принимался целиком, ни разу не сравниваясь с тем, что лежит в репозитории. Цена видна на #390: правка типов, которая физически не может сдвинуть пиксель, потребовала прогона workflow, а затем правки одиннадцати полей манифеста руками. Теперь правило про доказательство, и оно то же, что у golden с #334: среда доказана, если каждый кадр, который менять не собирались, совпал с закоммиченным байт-в-байт. Расхождение растеризации спрятать нельзя — оно задевает все кадры с текстом сразу. Снимать можно где угодно, включая WSL; принять получится только оттуда, где кадры воспроизводятся, и перестанет получаться в тот день, когда обновятся шрифты. Остальное следует из того же правила: намерение объявляется --expect-change, необъявленное расхождение останавливает приёмку, объявленное без расхождения — тоже (ложная декларация обесценивает список), заменяются ровно объявленные файлы, а тотальная перерисовка требует --no-witnesses --reason, и причина уезжает в манифест. Частый случай закрылся сам: ничего не объявлено, все кадры совпали — принимается один манифест, руками ничего писать не надо. Проверено шестью сквозными прогонами на подделанном артефакте, не только юнитами: идентичный кандидат, необъявленное расхождение, объявленное, молчаливая декларация, тотальная перерисовка без причины и с ней. Issue: #401 User-Visible: no --- .github/workflows/docs-screenshots.yml | 6 ++ scripts/docs-accept.mjs | 85 +++++++++++++--- scripts/docs-acceptance.mjs | 128 +++++++++++++++++++++++++ test/docs-accept.test.mjs | 18 ++++ test/docs-acceptance.test.mjs | 108 +++++++++++++++++++++ 5 files changed, 334 insertions(+), 11 deletions(-) create mode 100644 scripts/docs-acceptance.mjs create mode 100644 test/docs-acceptance.test.mjs diff --git a/.github/workflows/docs-screenshots.yml b/.github/workflows/docs-screenshots.yml index 0efef313..6316646a 100644 --- a/.github/workflows/docs-screenshots.yml +++ b/.github/workflows/docs-screenshots.yml @@ -9,6 +9,12 @@ # локально через `npm run docs:accept -- --reviewed --from=<распакованный>`. # Та же конструкция, что у golden-эталонов, и по той же причине: картинки # попадают в репозиторий через явное решение, а не через бота. +# +# Снимать здесь больше не обязанность, а удобство (#401). Приёмка проверяет не +# место съёмки, а её воспроизводимость: каждый кадр, не объявленный изменённым, +# должен совпасть с закоммиченным байт-в-байт. Эта джоба потому и удобна, что +# среда у неё та же, в которой снят закоммиченный набор, — но принять получится +# из любой, где кадры воспроизводятся, и не получится ни из одной, где нет. name: Скриншоты документации on: diff --git a/scripts/docs-accept.mjs b/scripts/docs-accept.mjs index abde8d47..31c7f780 100644 --- a/scripts/docs-accept.mjs +++ b/scripts/docs-accept.mjs @@ -1,16 +1,24 @@ #!/usr/bin/env node /** - * Приёмка скриншотов документации, снятых в CI (#246). + * Приёмка скриншотов документации (#246, правило среды переписано в #401). * * npm run docs:accept -- --reviewed --from=artifacts/docs + * npm run docs:accept -- --reviewed --from=… --expect-change=device-editor * - * Зачем приёмка вообще. Съёмка на машине исполнителя даёт байтово разный PNG - * при одинаковом содержимом кадра: сглаживание и хинтинг зависят от окружения. - * Измерено на истории — пересъёмка в #231 изменила два файла из девяти на 7–8 - * байт, а набор, приехавший с бетой, все девять целиком. Поэтому картинки - * рождаются в одном месте (`.github/workflows/docs-screenshots.yml`), а сюда - * приезжают артефактом. Та же конструкция, что у golden-эталонов, и по той же - * причине. + * Съёмка в другом окружении даёт байтово разный PNG при том же содержимом + * кадра: сглаживание и хинтинг зависят от шрифтового стека, а не только от + * браузера. Измерено — пересъёмка в #231 изменила два файла из девяти на 7–8 + * байт, а набор, приехавший с бетой, все девять целиком. + * + * Раньше отсюда следовало правило про МЕСТО: снимать только в CI. Оно держалось + * на этом комментарии, а не на механизме, и стоило прогона workflow даже там, + * где пиксель измениться не мог (#390). + * + * Теперь правило про ДОКАЗАТЕЛЬСТВО, и оно то же, что у golden с #334: среда + * доказана, если каждый кадр, который менять не собирались, совпал с + * закоммиченным байт-в-байт. Разбор правила и его границ — в + * scripts/docs-acceptance.mjs. Снимать можно где угодно; принять получится + * только оттуда, где кадры воспроизводятся. * * Что здесь НЕ делается: коммит. Файлы заменяются, коммит делает человек — * приёмка не должна быть способом протащить картинки мимо чужих глаз. @@ -21,6 +29,7 @@ import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { DOC_SCREENSHOT_VERSION, DOC_SCREENSHOTS } from '../demo/docs/screenshots.mjs'; +import { docsAcceptancePlan } from './docs-acceptance.mjs'; import { visualFingerprint } from './source-fingerprint.mjs'; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); @@ -82,6 +91,11 @@ export function verifyDocsCandidate({ return { manifest, files }; } +const list = (argv, name) => argv + .filter((arg) => arg.startsWith(`--${name}=`)) + .map((arg) => arg.slice(name.length + 3)) + .filter(Boolean); + function main(argv) { if (!argv.includes('--reviewed')) { console.error('отказ: замена скриншотов без явного --reviewed'); @@ -96,13 +110,62 @@ function main(argv) { } const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); const plan = verifyDocsCandidate({ root: ROOT, from, manifest }); - for (const file of plan.files) copyFileSync(file.from, file.to); + + // Хеши закоммиченного считаются по файлам на диске, а не по закоммиченному + // манифесту: манифест — утверждение о файлах, а сравнивать надо сами файлы. + const ids = DOC_SCREENSHOTS.map((scenario) => scenario.id); + const committed = {}; + const candidate = {}; + for (const scenario of DOC_SCREENSHOTS) { + const entry = manifest.scenarios[scenario.id]; + candidate[scenario.id] = entry.imageSha256; + const onDisk = resolve(ROOT, 'docs/images', entry.file); + if (existsSync(onDisk)) committed[scenario.id] = sha256(readFileSync(onDisk)); + } + + const declared = list(argv, 'expect-change'); + const skipWitnesses = argv.includes('--no-witnesses'); + const skipReason = (list(argv, 'reason')[0] || '').trim(); + const decision = docsAcceptancePlan({ + ids, committed, candidate, declared, skipWitnesses, skipReason, + }); + if (decision.refusal) { + console.error(`отказ: ${decision.refusal}`); + return 1; + } + + const byId = new Map(plan.files.map((file, index) => [ids[index], file])); + for (const id of decision.replace) { + const file = byId.get(id); + copyFileSync(file.from, file.to); + } + const accepted = { + ...manifest, + acceptance: { + declared: [...decision.replace], + witnesses: decision.witnesses.length, + floor: decision.floor, + ...(skipWitnesses ? { witnessesSkippedBecause: skipReason } : {}), + }, + }; writeFileSync( resolve(ROOT, 'docs/images/screenshots.json'), - `${JSON.stringify(plan.manifest, null, 2)}\n`, + `${JSON.stringify(accepted, null, 2)}\n`, 'utf8', ); - console.log(`Принято ${plan.files.length} скриншотов, снятых ${plan.manifest.chromium}.`); + if (!decision.replace.length) { + console.log('Кадры не менялись: принят только манифест' + + ` (отпечаток исходников ${manifest.sourceFingerprint.slice(0, 8)}).`); + } else { + console.log(`Принято кадров: ${decision.replace.length}` + + ` (${decision.replace.join(', ')}), снято ${manifest.chromium}.`); + } + console.log(`Сохранено без изменений: ${decision.keep.length}.`); + if (skipWitnesses) { + console.log(`Свидетели пропущены осознанно: ${skipReason}`); + } else { + console.log(`Кадров-свидетелей среды: ${decision.witnesses.length} (порог ${decision.floor}).`); + } console.log('Коммит — за вами: приёмка ничего не коммитит.'); return 0; } diff --git a/scripts/docs-acceptance.mjs b/scripts/docs-acceptance.mjs new file mode 100644 index 00000000..47f777ec --- /dev/null +++ b/scripts/docs-acceptance.mjs @@ -0,0 +1,128 @@ +/** + * Правило допустимости съёмки скриншотов документации (issue #401). + * + * До этого правило было про МЕСТО: снимать только в CI, потому что растеризация + * шрифтов на другой машине отличается. Обоснование измерено — пересъёмка в #231 + * изменила два файла из девяти на 7–8 байт, а набор с беты все девять. Но + * держалось оно на комментарии в шапке `docs-accept.mjs`, а не на механизме: + * кандидат проверялся на самосогласованность и принимался целиком, без единого + * сравнения с тем, что лежит в репозитории. + * + * Цена такого правила видна на #390. Правка типов, которая физически не может + * сдвинуть пиксель, потребовала прогона workflow, а затем правки одиннадцати + * полей манифеста руками. Гейт, заставляющий писать хеши руками, работает + * против себя. + * + * Здесь правило заменено на проверяемое, и оно ровно то же, что у golden с + * #334: среда съёмки доказана, если КАЖДЫЙ кадр, который менять не собирались, + * совпал с закоммиченным БАЙТ-В-БАЙТ. Расхождение растеризации спрятать нельзя + * — оно задевает все кадры с текстом, а не только правленные. Поэтому автор + * объявляет намерение списком `--expect-change`, и всё, что разошлось помимо + * списка, приёмку запрещает: либо это незамеченная регрессия рендера, либо + * среда не та. + * + * То же правило ловит второе, независимо от среды: «принять всё, чтобы гейт + * позеленел». Так скриншот документации перестаёт показывать продукт — молча, + * одной командой, без единого названного намерения. + * + * Отличие от golden только в строгости, и оно в пользу скриншотов: сцен десять, + * а не сто сорок три, и сравнение точное, без порога. Поэтому «совпал» здесь + * значит буквально совпал, а не «в пределах допуска». + */ + +/** + * Порог свидетелей. Формула та же, что у golden (`goldenWitnessFloor`), и это + * намеренно: два набора картинок в одном репозитории не должны требовать от + * человека помнить два разных правила. + * + * Для десяти сцен порог равен одной. Этого достаточно, потому что основную + * работу делает не порог, а требование «все необъявленные совпали»: расхождение + * среды не бывает точечным. Порог закрывает единственную оставшуюся щель — + * попытку объявить изменёнными все кадры разом, когда свидетелей не остаётся + * вовсе и подтвердить среду становится нечем. + */ +export const docsWitnessFloor = (committedCount) => (committedCount > 0 + ? Math.min(10, Math.ceil(committedCount * 0.1)) + : 0); + +/** Объявленные имена, которых нет в наборе сценариев. */ +export const docsUnknownDeclarations = (ids, declared = []) => declared + .filter((id) => id && !ids.includes(id)) + .sort(); + +/** + * Объявленные кадры, которые в действительности не изменились. + * + * Это не придирка к аккуратности. Декларация — утверждение «я знаю, почему этот + * кадр другой»; если он не другой, утверждение ложное, и вместе с ним теряет + * смысл весь список. На практике так выглядит усталость: автор перечисляет + * «всё, что покраснело», захватывая заодно то, что не краснело. + */ +export const docsSilentDeclarations = ({ committed = {}, candidate = {}, declared = [] }) => + declared + .filter((id) => committed[id] && candidate[id] && committed[id] === candidate[id]) + .sort(); + +/** + * План приёмки и причина отказа. + * + * @param ids все сценарии набора, в стабильном порядке + * @param committed id → sha256 закоммиченного файла (отсутствует — значит нет файла) + * @param candidate id → sha256 файла в артефакте + * @param declared что автор объявил изменённым + * @returns {{ refusal: string|null, replace: string[], keep: string[], + * witnesses: string[], floor: number }} + */ +export function docsAcceptancePlan({ + ids = [], committed = {}, candidate = {}, declared = [], + skipWitnesses = false, skipReason = '', +}) { + const empty = { replace: [], keep: [...ids], witnesses: [], floor: 0 }; + if (skipWitnesses && (typeof skipReason !== 'string' || !skipReason.trim())) { + return { ...empty, refusal: '--no-witnesses требует --reason="…": причина обхода обязана' + + ' остаться в манифесте, а не только в истории shell' }; + } + + const unknown = docsUnknownDeclarations(ids, declared); + if (unknown.length) { + return { ...empty, refusal: `объявлены кадры, которых нет в наборе: ${unknown.join(', ')}` }; + } + + const silent = docsSilentDeclarations({ committed, candidate, declared }); + if (silent.length) { + return { ...empty, refusal: `объявлены изменёнными, но не изменились: ${silent.join(', ')}.` + + ' Декларация — утверждение «я знаю, почему этот кадр другой»; на неизменившемся' + + ' кадре оно ложное и обесценивает весь список' }; + } + + const announced = new Set(declared.filter(Boolean)); + const strayed = ids.filter((id) => !announced.has(id) && committed[id] !== candidate[id]); + if (strayed.length) { + return { ...empty, refusal: `разошлись, но не объявлены: ${strayed.join(', ')}.` + + ' Либо это незамеченное изменение продукта — тогда объявите его' + + ' --expect-change; либо съёмка велась в другой среде, и тогда её кадры' + + ' принимать нельзя: растеризация задевает все кадры с текстом сразу' }; + } + + const withCommitted = ids.filter((id) => committed[id]); + const floor = docsWitnessFloor(withCommitted.length); + const witnesses = withCommitted.filter((id) => !announced.has(id) + && committed[id] === candidate[id]); + if (!skipWitnesses && witnesses.length < floor) { + return { ...empty, refusal: 'кадров-свидетелей недостаточно:' + + ` ${witnesses.length} из необходимых ${floor}.` + + ' Свидетель — необъявленный кадр, совпавший с закоммиченным байт-в-байт;' + + ' только он доказывает, что среда съёмки та же. Если перерисовка' + + ' действительно тотальная и осознанная — --no-witnesses --reason="…"' + + ' оставит причину в манифесте' }; + } + + const replace = ids.filter((id) => announced.has(id)); + return { + refusal: null, + replace, + keep: ids.filter((id) => !announced.has(id)), + witnesses, + floor, + }; +} diff --git a/test/docs-accept.test.mjs b/test/docs-accept.test.mjs index 9526ca8f..07a50481 100644 --- a/test/docs-accept.test.mjs +++ b/test/docs-accept.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { createHash } from 'node:crypto'; +import { readFileSync } from 'node:fs'; import { verifyDocsCandidate } from '../scripts/docs-accept.mjs'; import { DOC_SCREENSHOT_VERSION, DOC_SCREENSHOTS } from '../demo/docs/screenshots.mjs'; @@ -128,3 +129,20 @@ test('разбор пути фикстуры не зависит от разде assert.equal(basename('C:/artifact/sub\\02-view-touch.png'), '02-view-touch.png'); assert.equal(basename('01-view-desktop.png'), '01-view-desktop.png'); }); + +test('правило среды в шапках совпадает с реализацией (#401)', () => { + // Предыдущее правило («снимать только в CI») жило исключительно в + // комментарии, и разошлось с реальностью в тот день, когда появилась цена. + // Этот тест держит текст и механизм вместе. + const script = readFileSync(new URL('../scripts/docs-accept.mjs', import.meta.url), 'utf8'); + const workflow = readFileSync( + new URL('../.github/workflows/docs-screenshots.yml', import.meta.url), 'utf8', + ); + for (const [name, text] of [['docs-accept.mjs', script], ['docs-screenshots.yml', workflow]]) { + assert.match(text, /#401/, `${name}: правило приёмки не сослано на решение`); + assert.match(text, /байт-в-байт/, `${name}: не назван признак доказанной среды`); + } + assert.match(script, /--expect-change/, 'декларация намерения обязана быть в описании'); + assert.equal(/снимать только в CI|только из артефакта CI/.test(workflow), false, + 'старое правило про место съёмки осталось в тексте'); +}); diff --git a/test/docs-acceptance.test.mjs b/test/docs-acceptance.test.mjs new file mode 100644 index 00000000..daa9fa5f --- /dev/null +++ b/test/docs-acceptance.test.mjs @@ -0,0 +1,108 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +import { + docsAcceptancePlan, docsSilentDeclarations, docsUnknownDeclarations, docsWitnessFloor, +} from '../scripts/docs-acceptance.mjs'; + +// #401. Правило приёмки скриншотов перестало быть про место съёмки и стало про +// доказательство: среда доказана, если каждый кадр, который менять не +// собирались, совпал с закоммиченным байт-в-байт. Здесь закреплён каждый отказ +// и каждый путь приёмки — иначе правило живёт только в комментарии, как жило +// предыдущее. + +const IDS = ['alpha', 'beta', 'gamma']; +const same = { alpha: 'a', beta: 'b', gamma: 'c' }; + +test('ничего не объявлено и всё совпало — принимается один манифест (#401)', () => { + // Частый случай: правка исходников, которая не может сдвинуть пиксель. + // Раньше он требовал прогона workflow и правки манифеста руками (#390). + const plan = docsAcceptancePlan({ ids: IDS, committed: same, candidate: { ...same } }); + assert.equal(plan.refusal, null); + assert.deepEqual(plan.replace, []); + assert.deepEqual(plan.keep, IDS); + assert.equal(plan.witnesses.length, 3); +}); + +test('объявленный кадр заменяется, остальные не трогаются (#401)', () => { + const plan = docsAcceptancePlan({ + ids: IDS, committed: same, candidate: { ...same, beta: 'иное' }, declared: ['beta'], + }); + assert.equal(plan.refusal, null); + assert.deepEqual(plan.replace, ['beta']); + // Половина принятого набора хуже непринятого: рядом окажется кадр от одного + // дерева и манифест от другого. + assert.deepEqual(plan.keep, ['alpha', 'gamma']); + assert.deepEqual(plan.witnesses, ['alpha', 'gamma']); +}); + +test('расхождение без декларации останавливает приёмку (#401)', () => { + const plan = docsAcceptancePlan({ ids: IDS, committed: same, candidate: { ...same, gamma: 'иное' } }); + assert.match(plan.refusal, /разошлись, но не объявлены: gamma/); + // Отказ обязан называть оба объяснения: автор не знает, какое из них его. + assert.match(plan.refusal, /изменение продукта/); + assert.match(plan.refusal, /другой среде/); + assert.deepEqual(plan.replace, []); +}); + +test('молчаливая декларация — тоже отказ (#401)', () => { + const plan = docsAcceptancePlan({ + ids: IDS, committed: same, candidate: { ...same, beta: 'иное' }, declared: ['beta', 'alpha'], + }); + assert.match(plan.refusal, /не изменились: alpha/); +}); + +test('объявленного имени нет в наборе — отказ до всякой проверки (#401)', () => { + const plan = docsAcceptancePlan({ + ids: IDS, committed: same, candidate: { ...same }, declared: ['опечатка'], + }); + assert.match(plan.refusal, /которых нет в наборе: опечатка/); +}); + +test('тотальная перерисовка требует явного обхода с причиной (#401)', () => { + const all = { alpha: 'x', beta: 'y', gamma: 'z' }; + const declared = [...IDS]; + const refused = docsAcceptancePlan({ ids: IDS, committed: same, candidate: all, declared }); + assert.match(refused.refusal, /свидетелей недостаточно: 0 из необходимых 1/); + + const noReason = docsAcceptancePlan({ + ids: IDS, committed: same, candidate: all, declared, skipWitnesses: true, + }); + assert.match(noReason.refusal, /требует --reason/); + + const bypassed = docsAcceptancePlan({ + ids: IDS, committed: same, candidate: all, declared, + skipWitnesses: true, skipReason: 'сменился шрифтовый стек', + }); + assert.equal(bypassed.refusal, null); + assert.deepEqual(bypassed.replace, IDS); +}); + +test('порог свидетелей считается так же, как в golden (#401)', () => { + // Два набора картинок в одном репозитории не должны требовать от человека + // помнить два разных правила. + assert.equal(docsWitnessFloor(0), 0); + assert.equal(docsWitnessFloor(10), 1); + assert.equal(docsWitnessFloor(143), 10); +}); + +test('вспомогательные проверки называют виновников по именам (#401)', () => { + assert.deepEqual(docsUnknownDeclarations(IDS, ['gamma', 'нет-такого']), ['нет-такого']); + assert.deepEqual( + docsSilentDeclarations({ committed: same, candidate: { ...same }, declared: ['beta', 'alpha'] }), + ['alpha', 'beta'], + ); +}); + +test('кадр без закоммиченной пары не может быть свидетелем (#401)', () => { + // Первичная съёмка ничего не доказывает про среду: сравнивать не с чем. + const plan = docsAcceptancePlan({ + ids: IDS, + committed: { alpha: 'a' }, + candidate: { alpha: 'a', beta: 'новое', gamma: 'новое' }, + declared: ['beta', 'gamma'], + }); + assert.equal(plan.refusal, null); + assert.deepEqual(plan.witnesses, ['alpha']); + assert.equal(plan.floor, 1); +});