diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 889d6cb4..6aa6b0ff 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -94,8 +94,11 @@ jobs: REPO: ${{ github.repository }} FALLBACK: ${{ github.event.before }} run: | + # `status=completed`, а не `success`: гейтам диапазона нужен факт + # «коммит судили», а не «вердикт был оправдательный». Упавший прогон + # коммит судил; отменённый — нет, его отсеивает judgedShas (#388). gh api -X GET "repos/$REPO/actions/workflows/validate.yml/runs" \ - -f branch=dev -f status=success -F per_page=100 \ + -f branch=dev -f status=completed -F per_page=100 \ > /tmp/validate-runs.json || echo '{}' > /tmp/validate-runs.json node scripts/classify-base.mjs --head="$HEAD_SHA" --mode=range \ --fallback="$FALLBACK" --runs=/tmp/validate-runs.json @@ -208,8 +211,10 @@ jobs: git fetch -q origin dev # Недоступность API — не отказ гейта: пустой ответ уводит базу в # сторону БОЛЬШЕГО объёма проверок, а не меньшего. + # Один запрос на оба режима: `completed` — надмножество `success`, + # а нужный предикат применяет скрипт (#387 — успех, #388 — судимость). gh api -X GET "repos/$REPO/actions/workflows/validate.yml/runs" \ - -f branch="$BRANCH" -f status=success -F per_page=100 \ + -f branch="$BRANCH" -f status=completed -F per_page=100 \ > /tmp/validate-runs.json || echo '{}' > /tmp/validate-runs.json if [ "$REF" = "refs/heads/dev" ]; then # На dev классифицировать нечего (всё true), но база диапазона diff --git a/scripts/classify-base.mjs b/scripts/classify-base.mjs index 9038fbe6..43449f0c 100644 --- a/scripts/classify-base.mjs +++ b/scripts/classify-base.mjs @@ -39,6 +39,29 @@ import { appendFileSync, readFileSync } from 'node:fs'; */ export const MAX_CANDIDATES = 300; +/** + * SHA прогонов, которые ДОШЛИ ДО КОНЦА — успешно или нет, но не отменённые. + * + * Разница с `greenShas` — суть issue #388, и я её сначала перепутал, чем уронил + * dev. Классификация (#387) спрашивает «доказано ли, что тяжёлые гейты на этом + * дереве прошли» — там нужен именно `success`. Гейты диапазона спрашивают + * другое: «судил ли этот коммит хоть кто-нибудь». Упавший прогон коммит СУДИЛ, + * просто вынес обвинительный вердикт, и переоткрывать его диапазоном не надо. + * + * Цена ошибки была наглядной: backend на dev красный несколько дней по своей + * причине, поэтому «зелёных» прогонов не было вовсе, база уезжала на десятки + * коммитов назад, и `no-new-any` начал предъявлять текущему пушу чужой долг. + */ +export function judgedShas(payload) { + const runs = payload && Array.isArray(payload.workflow_runs) ? payload.workflow_runs : []; + return new Set( + runs + .filter((run) => run && run.status === 'completed' && run.conclusion !== 'cancelled' + && typeof run.head_sha === 'string') + .map((run) => run.head_sha), + ); +} + /** * SHA прогонов, завершившихся успешно. Вход — тело ответа * `/actions/workflows/validate.yml/runs`; всё, что не массив прогонов, @@ -136,7 +159,7 @@ export function baseSummary(choice, { head, mergeBase }) { if (choice.reason === 'fallback') { return [ '### База диапазона (#388)', - `Ни у одного из ${choice.skipped} предков нет завершённого зелёного Validate.` + `Ни один из ${choice.skipped} предков не был судим завершённым Validate.` + ` Диапазон взят от \`${short(choice.base)}\` — головы предыдущего пуша,` + ' и это НЕ доказательство проверенности: прогон того пуша мог быть отменён.' + ' Коммиты в этом окне могли не пройти ни одного гейта.', @@ -184,10 +207,9 @@ function main(argv) { 'rev-list', `--max-count=${MAX_CANDIDATES}`, '--skip=1', span, ], { encoding: 'utf8' }).split('\n').map((line) => line.trim()).filter(Boolean); - const green = greenShas(payload); const choice = mode === 'range' - ? pickRangeBase({ candidates, green, fallback: arg(argv, 'fallback') }) - : pickBase({ candidates, green, mergeBase }); + ? pickRangeBase({ candidates, green: judgedShas(payload), fallback: arg(argv, 'fallback') }) + : pickBase({ candidates, green: greenShas(payload), mergeBase }); const summary = baseSummary(choice, { head, mergeBase }); process.stdout.write(`${summary.join('\n')}\n`); // Имя выхода задаётся явно: одна и та же job считает базу для двух разных diff --git a/test/classify-base.test.mjs b/test/classify-base.test.mjs index 8c2afc4a..df97eda6 100644 --- a/test/classify-base.test.mjs +++ b/test/classify-base.test.mjs @@ -7,7 +7,7 @@ import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { - MAX_CANDIDATES, baseSummary, greenShas, pickBase, pickRangeBase, + MAX_CANDIDATES, baseSummary, greenShas, judgedShas, pickBase, pickRangeBase, } from '../scripts/classify-base.mjs'; const SCRIPT = fileURLToPath(new URL('../scripts/classify-base.mjs', import.meta.url)); @@ -170,8 +170,8 @@ test('CLI режима range считает базу по истории и пи const out = join(dir, 'out.txt'); writeFileSync(runsFile, JSON.stringify({ workflow_runs: [ - { head_sha: green, conclusion: 'success' }, - { head_sha: cancelled, conclusion: 'cancelled' }, + { head_sha: green, status: 'completed', conclusion: 'success' }, + { head_sha: cancelled, status: 'completed', conclusion: 'cancelled' }, ], })); writeFileSync(out, ''); @@ -189,3 +189,36 @@ test('CLI режима range считает базу по истории и пи rmSync(dir, { recursive: true, force: true }); } }); + +test('судимость и успех — разные предикаты, и путать их дорого (#388)', () => { + const payload = { + workflow_runs: [ + { head_sha: 'зелёный', status: 'completed', conclusion: 'success' }, + // Прогон упал по своей причине — backend на dev был красным несколько + // дней. Коммит при этом СУДИЛИ: вердикт вынесен, автор его видел. + { head_sha: 'красный', status: 'completed', conclusion: 'failure' }, + // Отменён следующим пушем — вот этот коммит не судил никто. + { head_sha: 'отменён', status: 'completed', conclusion: 'cancelled' }, + { head_sha: 'идёт', status: 'in_progress', conclusion: null }, + ], + }; + assert.deepEqual([...greenShas(payload)], ['зелёный'], + 'классификации нужен доказанный успех тяжёлых гейтов (#387)'); + assert.deepEqual([...judgedShas(payload)].sort(), ['зелёный', 'красный'], + 'гейтам диапазона нужен факт суда, а не оправдательный вердикт (#388)'); +}); + +test('красный прогон не переоткрывает уже осуждённые коммиты (#388)', () => { + // Если бы база уезжала за каждый упавший прогон, гейт предъявлял бы текущему + // пушу чужой долг — ровно это и уронило dev 30 августа. + const judged = judgedShas({ + workflow_runs: [{ head_sha: 'предыдущий', status: 'completed', conclusion: 'failure' }], + }); + const choice = pickRangeBase({ + candidates: ['предыдущий', 'давний'], + green: judged, + fallback: 'before', + }); + assert.equal(choice.base, 'предыдущий'); + assert.equal(choice.skipped, 0); +}); diff --git a/test/validate-workflow.test.mjs b/test/validate-workflow.test.mjs index a340bbef..47b4958f 100644 --- a/test/validate-workflow.test.mjs +++ b/test/validate-workflow.test.mjs @@ -183,6 +183,11 @@ test('гейты диапазона судят от доказанного пр preflight.match(/BEFORE_SHA: \$\{\{ steps\.range\.outputs\.base \|\| github\.event\.before \}\}/g)?.length, 2, 'провенанс и процессный гейт читают доказанную базу'); assert.match(preflight, /--mode=range/); + // `completed`, а не `success`: нужен факт суда над коммитом, а не + // оправдательный вердикт. Запрос за success уводил базу на десятки коммитов + // назад, пока backend на dev был красным по своей причине (#388). + assert.match(preflight, /-f status=completed/); + assert.equal(/-f status=success/.test(preflight), false); assert.match(preflight, /actions: read/, 'чтение прогонов требует прав'); assert.match(preflight, /issues: read/, 'проверка 8 читает issue'); // Считать базу имеет смысл только на пуше в dev: на ветках диапазон и так