diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 5e41eb8b..b45c8e76 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -26,6 +26,33 @@ const WAIT_TIME_SINGLE_APPROVAL = 24 * 7; const GITHUB_SUCCESS_CONCLUSIONS = ['SUCCESS', 'NEUTRAL', 'SKIPPED']; const GITHUB_ACTIONS_APP = 'github-actions'; +// A commit keeps every check suite ever attached to it, so a branch that is +// force-pushed away from a SHA and back again accumulates one suite per run. +// Runs of the same workflow supersede one another, but runs triggered by +// different events do not, so both make up the key. +function selectLatestCheckSuites(nodes) { + const latest = new Map(); + const unkeyed = []; + + for (const suite of nodes) { + const { workflowRun } = suite; + if (!workflowRun) { + // A suite without run metadata cannot be compared against another, so + // it is kept as is rather than dropped. + unkeyed.push(suite); + continue; + } + + const key = `${workflowRun.workflow.id}:${workflowRun.event}`; + const current = latest.get(key); + if (!current || workflowRun.runNumber > current.workflowRun.runNumber) { + latest.set(key, suite); + } + } + + return [...unkeyed, ...latest.values()]; +} + const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to approve\.$/; const FAST_TRACK_MIN_APPROVALS = 2; const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork'; @@ -460,7 +487,8 @@ export default class PRChecker { const pendingJobs = []; // GitHub new Check API - for (const { status, conclusion, checkRuns } of checkSuites.nodes) { + for (const { status, conclusion, checkRuns } of + selectLatestCheckSuites(checkSuites.nodes)) { if (status !== 'COMPLETED') { pendingJobs.push({ status, conclusion }); continue; diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 10400af4..4c2631a8 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -37,6 +37,13 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { nodes { conclusion, status, + workflowRun { + event + runNumber + workflow { + id + } + } checkRuns(first: 40) { nodes { name diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index a64c4855..0183f458 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -2117,6 +2117,133 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); + it('should ignore a check suite superseded by a later run', async() => { + const cli = new TestCLI(); + + const supersededSuite = [{ + commit: { + committedDate: '2017-10-26T12:10:20Z', + oid: '9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d', + messageHeadline: 'doc: add api description README', + author: { login: 'foo' }, + checkSuites: { + nodes: [ + { + status: 'COMPLETED', + conclusion: 'CANCELLED', + workflowRun: { + event: 'pull_request', + runNumber: 1, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'CANCELLED', + detailsUrl: 'https://github.com/nodejs/node/runs/1' + }] + } + }, + { + status: 'COMPLETED', + conclusion: 'SUCCESS', + workflowRun: { + event: 'pull_request', + runNumber: 2, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'SUCCESS', + detailsUrl: 'https://github.com/nodejs/node/runs/2' + }] + } + } + ] + } + } + }]; + + const expectedLogs = { + ok: [['Last GitHub CI successful']] + }; + + const data = Object.assign({}, baseData, { commits: supersededSuite }); + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should not let a run supersede one from a different event', async() => { + const cli = new TestCLI(); + + const differentEvents = [{ + commit: { + committedDate: '2017-10-26T12:10:20Z', + oid: '9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d', + messageHeadline: 'doc: add api description README', + author: { login: 'foo' }, + checkSuites: { + nodes: [ + { + status: 'COMPLETED', + conclusion: 'FAILURE', + workflowRun: { + event: 'push', + runNumber: 1, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'FAILURE', + detailsUrl: 'https://github.com/nodejs/node/runs/1' + }] + } + }, + { + status: 'COMPLETED', + conclusion: 'SUCCESS', + workflowRun: { + event: 'pull_request', + runNumber: 2, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'SUCCESS', + detailsUrl: 'https://github.com/nodejs/node/runs/2' + }] + } + } + ] + } + } + }]; + + const expectedLogs = { + error: [ + ['1 GitHub CI job(s) failed:'], + [' - test-linux: FAILURE (https://github.com/nodejs/node/runs/1)'] + ] + }; + + const data = Object.assign({}, baseData, { commits: differentEvents }); + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + it('should handle empty checkRuns array', async() => { const cli = new TestCLI();