Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions .github/workflows/ready.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.'
Expand Down
18 changes: 18 additions & 0 deletions tests/ready.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down