diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index cef50a5..2ead6b8 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -100,7 +100,13 @@ jobs: await setReview('pending', 'Draft: publication is not authorized.'); return; } - if (requiredTests.length) await setReview('pending', 'Checking the current Test run and attempt.'); + // Every event reconciles every open PR, and GitHub refuses more than 1000 + // statuses per commit and context. Block an old success before checking + // only when the event concerns this PR, such as a Test rerun of its head. + const {payload} = context; + const ownEvent = payload.pull_request?.number === number + || payload.workflow_run?.head_sha === sha || payload.sha === sha; + if (requiredTests.length && ownEvent) await setReview('pending', 'Checking the current Test run and attempt.'); async function testsPassed() { if (!requiredTests.length) return true; // Read the current run and attempt on every event. Check runs can @@ -146,12 +152,23 @@ jobs: const {data} = await github.rest.repos.getCombinedStatusForRef({owner, repo, ref: sha, per_page: 100}); return names.every(name => data.statuses.find(status => status.context === name)?.state === 'success'); } - const approved = await testsPassed(); - const previews = await previewsPassed(); - const current = await getPull(); - if (current.state !== 'open' || current.base.ref !== pull.base.ref - || current.head.sha !== sha || current.draft !== pull.draft) return; - const reviewPassed = !pull.draft && approved && previews && await testsPassed(); + let previews, reviewPassed; + try { + const approved = await testsPassed(); + previews = await previewsPassed(); + const current = await getPull(); + if (current.state !== 'open' || current.base.ref !== pull.base.ref + || current.head.sha !== sha || current.draft !== pull.draft) { + // The change's own event reconciles it; an old success must not stay meanwhile. + if (review?.state === 'success') await setReview('pending', 'The PR changed while it was checked.'); + return; + } + reviewPassed = !pull.draft && approved && previews && await testsPassed(); + } catch (error) { + // A failed read must not leave a previous success in force. + await setReview('pending', 'Unable to verify the Ready requirements of this revision.'); + throw error; + } await setReview(reviewPassed ? 'success' : 'pending', reviewPassed ? 'Ready requirements passed for this revision.' : previews ? 'Waiting for required tests of this revision.' diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index 40b3767..bbd0446 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -311,6 +311,24 @@ test('the exact latest Test attempt supplies every required successful job', asy assert.equal(result.statuses.at(-1).state, 'success'); }); +test('other PRs\' events do not rewrite an unchanged successful Review', async () => { + // GitHub refuses more than 1000 statuses per commit and context. + const passed = {...trial, runs: [testRun], jobs: [testJob], + review: {state: 'success', description: 'Ready requirements passed for this revision.'}}; + for (const event of [{pull_request: {number: 2}}, {workflow_run: {name: 'Check', head_sha: 'other'}}, {sha: 'other'}]) { + const result = await scenario({...passed, event}); + assert.deepEqual(result.statuses, []); + assert.deepEqual(result.errors, []); + } + // The PR's own events still block the old success before checking. + const own = await scenario(passed); + assert.deepEqual(own.statuses.map(status => status.state), ['pending', 'success']); +}); +test('a PR changing while it is checked loses its old success', async () => { + const result = await scenario({changed: true, review: {state: 'success'}, event: {pull_request: {number: 2}}}); + assert.deepEqual(result.statuses.map(status => status.state), ['pending']); +}); + test('completion retries metadata lag and passes without requiring another event', async () => { let jobReads = 0; const result = await scenario({...trial,