diff --git a/.github/workflows/commit-queue.yml b/.github/workflows/commit-queue.yml index f9db4b0be..3203b1bfa 100644 --- a/.github/workflows/commit-queue.yml +++ b/.github/workflows/commit-queue.yml @@ -64,6 +64,10 @@ jobs: # anything is merged. Applying a label needs only triage. LAND_ACTOR: ${{ github.event.sender.login }} NUMBER: ${{ github.event.pull_request.number }} + # What this job reports as, which is the `name:` above. A refusal + # exits non-zero and leaves a failed check behind, so without this + # the first one would be cited by every attempt after it. + LAND_CHECK_NAME: Land run: node build/tasks/land-pull-request.mts "${NUMBER}" # The label is a request, not a state: once the queue has answered it, diff --git a/build/shared/landing.mts b/build/shared/landing.mts index dcb3dd5cc..d32032f45 100644 --- a/build/shared/landing.mts +++ b/build/shared/landing.mts @@ -107,25 +107,30 @@ export type CommitStatus = { context: string; state: string }; * running counts against it: a label applied while a check was in flight says * nothing about how that check turned out. Neutral and skipped do not count * against it, since neither is a complaint. - * The queue is itself a check, so its own run is left out. Waiting for it - * would be waiting for a job that cannot finish until it stops waiting. - * @param {CheckRun[]} runs The check runs reported on the commit. + * + * The queue is itself a check, and none of its runs count -- neither the one + * doing the asking, which cannot finish until it stops waiting, nor any run + * before it. A refusal exits non-zero and so leaves a failed check of its own + * behind, and counting that would let the first refusal decide every later + * one: label a pull request while a check is in flight, and the queue would + * go on citing its own complaint for as long as the branch sat at that commit. + * @param {CheckRun[]} all The check runs reported on the commit. * @param {CommitStatus[]} statuses The commit statuses reported on it. * @param {string} ownRunId The workflow run doing the asking, if it is one. + * @param {string} ownCheckName What the queue reports as, if it knows. * @returns {string} The reason, or an empty string if there is none. */ export function checksVerdict( all: CheckRun[], statuses: CommitStatus[], - ownRunId = '' + ownRunId = '', + ownCheckName = '' ) { - const runs = - ownRunId === '' - ? all - : all.filter( - (run) => - !(run.detailsUrl ?? '').includes(`/actions/runs/${ownRunId}/`) - ); + const isOwn = (run: CheckRun) => + (ownRunId !== '' && + (run.detailsUrl ?? '').includes(`/actions/runs/${ownRunId}/`)) || + (ownCheckName !== '' && run.name === ownCheckName); + const runs = all.filter((run) => !isOwn(run)); const pending = [ ...runs.filter((run) => run.status !== 'completed').map((run) => run.name), ...statuses diff --git a/build/shared/landing.test.mts b/build/shared/landing.test.mts index 0e3fcf5b1..dbed9cd3c 100644 --- a/build/shared/landing.test.mts +++ b/build/shared/landing.test.mts @@ -273,4 +273,39 @@ describe('checksVerdict and its own run', () => { match(checksVerdict([other], [], '999'), /have not finished: Lint/); }); + + test('does not cite the refusal it left behind last time', () => { + // A refusal exits non-zero, so an attempt made while a check was still + // running leaves a failed check of its own on the commit. Counting it + // would mean the first refusal decided every later one, and the pull + // request could never be landed from that commit again. + const earlier = { + name: 'Land', + status: 'completed', + conclusion: 'failure', + detailsUrl: 'https://github.com/o/r/actions/runs/998/job/1', + }; + + deepStrictEqual( + checksVerdict( + [earlier, { name: 'Lint', status: 'completed', conclusion: 'success' }], + [], + '999', + 'Land' + ), + '' + ); + }); + + test('still reports a failure that is not the queue', () => { + match( + checksVerdict( + [{ name: 'Lint', status: 'completed', conclusion: 'failure' }], + [], + '999', + 'Land' + ), + /did not pass: Lint/ + ); + }); }); diff --git a/build/tasks/land-pull-request.mts b/build/tasks/land-pull-request.mts index 5e2f8819e..92f3479ce 100644 --- a/build/tasks/land-pull-request.mts +++ b/build/tasks/land-pull-request.mts @@ -157,7 +157,8 @@ const checksRefuse = (sha: string) => '[.statuses[] | {context, state}]' ) ), - process.env.GITHUB_RUN_ID ?? '' + process.env.GITHUB_RUN_ID ?? '', + process.env.LAND_CHECK_NAME ?? '' ); /**