Skip to content

Commit

Permalink
fix: backfill files by commit for rebased PRs
Browse files Browse the repository at this point in the history
Fixes #2140
  • Loading branch information
lukekarrys committed Nov 26, 2023
1 parent cae5ac0 commit 0aed8cf
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 1 deletion.
17 changes: 17 additions & 0 deletions __snapshots__/github.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -469,14 +469,36 @@ 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<string, number> = {};
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 = {
sha: graphCommit.sha,
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);
Expand Down
88 changes: 88 additions & 0 deletions test/fixtures/commits-since-rebase.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
33 changes: 33 additions & 0 deletions test/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 0aed8cf

Please sign in to comment.