From 0aed8cf683d108c4ee1f400c45eaf7beb09452aa Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 26 Nov 2023 11:31:34 -0700 Subject: [PATCH] fix: backfill files by commit for rebased PRs Fixes #2140 --- __snapshots__/github.js | 17 +++++ src/github.ts | 24 ++++++- test/fixtures/commits-since-rebase.json | 88 +++++++++++++++++++++++++ test/github.ts | 33 ++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/commits-since-rebase.json diff --git a/__snapshots__/github.js b/__snapshots__/github.js index 9524e2afd..34ef9d7e5 100644 --- a/__snapshots__/github.js +++ b/__snapshots__/github.js @@ -2,6 +2,23 @@ exports['GitHub commentOnIssue can create a comment 1'] = { "body": "This is a comment" } +exports['GitHub commitsSince backfills commit files for pull requests rebased and merged 1'] = [ + { + "sha": "b29149f890e6f76ee31ed128585744d4c598924c", + "message": "feat: feature-branch-rebase-merge commit 1", + "files": [ + "abc" + ] + }, + { + "sha": "27d7d7232e2e312d1380e906984f0823f5decf61", + "message": "feat: feature-branch-rebase-merge commit 2", + "files": [ + "def" + ] + } +] + exports['GitHub commitsSince backfills commit files for pull requests with lots of files 1'] = [ { "sha": "e6daec403626c9987c7af0d97b34f324cd84320a", diff --git a/src/github.ts b/src/github.ts index db3297753..c3c3d5518 100644 --- a/src/github.ts +++ b/src/github.ts @@ -469,6 +469,18 @@ export class GitHub { } const history = response.repository.ref.target.history; const commits = (history.nodes || []) as GraphQLCommit[]; + // Count the number of pull requests associated with each merge commit. This is + // used in the next step to make sure we only find pull requests with a + // merge commit that contain 1 merged commit. + const mergeCommitCount: Record = {}; + for (const commit of commits) { + for (const pr of commit.associatedPullRequests.nodes) { + if (pr.mergeCommit?.oid) { + mergeCommitCount[pr.mergeCommit.oid] ??= 0; + mergeCommitCount[pr.mergeCommit.oid]++; + } + } + } const commitData: Commit[] = []; for (const graphCommit of commits) { const commit: Commit = { @@ -476,7 +488,17 @@ export class GitHub { message: graphCommit.message, }; const pullRequest = graphCommit.associatedPullRequests.nodes.find(pr => { - return pr.mergeCommit && pr.mergeCommit.oid === graphCommit.sha; + return ( + // Only match the pull request with a merge commit if there is a + // single merged commit in the PR. This means merge commits and squash + // merges will be matched, but rebase merged PRs will only be matched + // if they contain a single commit. This is so PRs that are rebased + // and merged will have files backfilled from each commit instead of + // the whole PR. + pr.mergeCommit && + pr.mergeCommit.oid === graphCommit.sha && + mergeCommitCount[pr.mergeCommit.oid] === 1 + ); }); if (pullRequest) { const files = (pullRequest.files?.nodes || []).map(node => node.path); diff --git a/test/fixtures/commits-since-rebase.json b/test/fixtures/commits-since-rebase.json new file mode 100644 index 000000000..aa1c9091c --- /dev/null +++ b/test/fixtures/commits-since-rebase.json @@ -0,0 +1,88 @@ +{ + "repository": { + "ref": { + "target": { + "history": { + "nodes": [ + { + "associatedPullRequests": { + "nodes": [ + { + "number": 7, + "title": "feat: feature that will be rebase merged", + "baseRefName": "main", + "headRefName": "feature-branch-rebase-merge", + "labels": { + "nodes": [] + }, + "body": "", + "mergeCommit": { + "oid": "b29149f890e6f76ee31ed128585744d4c598924c" + }, + "files": { + "nodes": [] + } + } + ] + }, + "sha": "b29149f890e6f76ee31ed128585744d4c598924c", + "message": "feat: feature-branch-rebase-merge commit 1" + }, + { + "associatedPullRequests": { + "nodes": [ + { + "number": 7, + "title": "feat: feature that will be rebase merged", + "baseRefName": "main", + "headRefName": "feature-branch-rebase-merge", + "labels": { + "nodes": [] + }, + "body": "", + "mergeCommit": { + "oid": "b29149f890e6f76ee31ed128585744d4c598924c" + }, + "files": { + "nodes": [] + } + } + ] + }, + "sha": "27d7d7232e2e312d1380e906984f0823f5decf61", + "message": "feat: feature-branch-rebase-merge commit 2" + }, + { + "associatedPullRequests": { + "nodes": [ + { + "number": 6, + "title": "feat: other pr", + "baseRefName": "main", + "headRefName": "feature-branch-other", + "labels": { + "nodes": [] + }, + "body": "", + "mergeCommit": { + "oid": "2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5" + }, + "files": { + "nodes": [] + } + } + ] + }, + "sha": "2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5", + "message": "fix: feature-branch-other" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "e6daec403626c9987c7af0d97b34f324cd84320a 12" + } + } + } + } + } +} diff --git a/test/github.ts b/test/github.ts index 35e6b1925..613baae89 100644 --- a/test/github.ts +++ b/test/github.ts @@ -546,6 +546,39 @@ describe('GitHub', () => { snapshot(commitsSinceSha); req.done(); }); + + it('backfills commit files for pull requests rebased and merged', async () => { + const graphql = JSON.parse( + readFileSync(resolve(fixturesPath, 'commits-since-rebase.json'), 'utf8') + ); + req + .post('/graphql') + .reply(200, { + data: graphql, + }) + .get( + '/repos/fake/fake/commits/b29149f890e6f76ee31ed128585744d4c598924c' + ) + .reply(200, {files: [{filename: 'abc'}]}) + .get( + '/repos/fake/fake/commits/27d7d7232e2e312d1380e906984f0823f5decf61' + ) + .reply(200, {files: [{filename: 'def'}]}); + const targetBranch = 'main'; + const commitsSinceSha = await github.commitsSince( + targetBranch, + commit => { + // this commit is the 3rd most recent + return commit.sha === '2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5'; + }, + {backfillFiles: true} + ); + expect(commitsSinceSha.length).to.eql(2); + expect(commitsSinceSha[0].files).to.eql(['abc']); + expect(commitsSinceSha[1].files).to.eql(['def']); + snapshot(commitsSinceSha); + req.done(); + }); }); describe('mergeCommitIterator', () => {