From d293143475a75aeea2c4f3a23a574a7a1730b714 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Wed, 7 Jun 2017 15:42:20 +0200
Subject: [PATCH] Fix github api base branch update (#279)
* Update base commit SHA after merge
* drop unnecessary getcommittree
---
lib/api/github.js | 3 +-
test/api/__snapshots__/github.spec.js.snap | 76 ----------------------
test/api/github.spec.js | 60 +++++------------
3 files changed, 19 insertions(+), 120 deletions(-)
diff --git a/lib/api/github.js b/lib/api/github.js
index ed1a9efba1..71b50165ff 100644
--- a/lib/api/github.js
+++ b/lib/api/github.js
@@ -148,7 +148,6 @@ async function initRepo(repoName, token, endpoint) {
}
logger.debug(`${repoName} default branch = ${config.defaultBranch}`);
config.baseCommitSHA = await getBranchCommit(config.defaultBranch);
- config.baseTreeSHA = await getCommitTree(config.baseCommitSHA);
} catch (err) /* istanbul ignore next */ {
logger.error(`GitHub init error: ${JSON.stringify(err)}`);
throw err;
@@ -410,6 +409,8 @@ async function mergePr(pr) {
}
}
}
+ // Update base branch SHA
+ config.baseCommitSHA = await getBranchCommit(config.defaultBranch);
// Delete branch
await deleteBranch(pr.head.ref);
}
diff --git a/test/api/__snapshots__/github.spec.js.snap b/test/api/__snapshots__/github.spec.js.snap
index 31795af9e9..949713d859 100644
--- a/test/api/__snapshots__/github.spec.js.snap
+++ b/test/api/__snapshots__/github.spec.js.snap
@@ -54,9 +54,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -71,9 +68,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -88,9 +82,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -105,9 +96,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -122,9 +110,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -139,9 +124,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -156,9 +138,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/thebranchname",
],
@@ -173,9 +152,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/master",
],
@@ -249,9 +225,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/git/refs/heads/master",
],
@@ -351,9 +324,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"search/code?q=repo:some/repo+filename:package.json",
],
@@ -376,9 +346,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/pulls?head=theowner:master&state=all",
],
@@ -402,9 +369,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/pulls?head=theowner:master&state=all",
],
@@ -419,9 +383,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/pulls?head=theowner:master&state=all",
],
@@ -446,9 +407,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/pulls?state=open&base=master&head=theowner:somebranch",
],
@@ -463,9 +421,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/pulls?state=open&base=master&head=theowner:somebranch",
],
@@ -497,9 +452,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/contents/package.json?ref=master",
],
@@ -514,9 +466,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/contents/package.json?ref=master",
],
@@ -531,9 +480,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/contents/package.json?ref=master",
],
@@ -548,9 +494,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/contents/package.json?ref=master",
],
@@ -565,9 +508,6 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
Array [
"repos/some/repo/contents/package.json?ref=master",
],
@@ -737,16 +677,12 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
]
`;
exports[`api/github initRepo should initialise the config for the repo - 0 2`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "rebase",
"owner": "theowner",
@@ -762,16 +698,12 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
]
`;
exports[`api/github initRepo should initialise the config for the repo - 1 2`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "rebase",
"owner": "theowner",
@@ -787,16 +719,12 @@ Array [
Array [
"repos/some/repo/git/refs/heads/master",
],
- Array [
- "repos/some/repo/git/commits/1234",
- ],
]
`;
exports[`api/github initRepo should initialise the config for the repo - 2 2`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "rebase",
"owner": "theowner",
@@ -807,7 +735,6 @@ Object {
exports[`api/github initRepo should merge 1`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "merge",
"owner": "theowner",
@@ -818,7 +745,6 @@ Object {
exports[`api/github initRepo should not guess at merge 1`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"owner": "theowner",
"repoName": "some/repo",
@@ -828,7 +754,6 @@ Object {
exports[`api/github initRepo should rebase 1`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "rebase",
"owner": "theowner",
@@ -839,7 +764,6 @@ Object {
exports[`api/github initRepo should squash 1`] = `
Object {
"baseCommitSHA": "1234",
- "baseTreeSHA": "5678",
"defaultBranch": "master",
"mergeMethod": "squash",
"owner": "theowner",
diff --git a/test/api/github.spec.js b/test/api/github.spec.js
index dd02013fc6..60467fb33d 100644
--- a/test/api/github.spec.js
+++ b/test/api/github.spec.js
@@ -150,14 +150,6 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
- ghGot.mockImplementationOnce(() => ({
- body: {
- tree: {
- sha: '5678',
- },
- },
- }));
return github.initRepo(...args);
}
@@ -216,14 +208,6 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
- ghGot.mockImplementationOnce(() => ({
- body: {
- tree: {
- sha: '5678',
- },
- },
- }));
return github.initRepo(...args);
}
const config = await squashInitRepo('some/repo', 'token');
@@ -251,14 +235,6 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
- ghGot.mockImplementationOnce(() => ({
- body: {
- tree: {
- sha: '5678',
- },
- },
- }));
return github.initRepo(...args);
}
const config = await mergeInitRepo('some/repo', 'token');
@@ -286,14 +262,6 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
- ghGot.mockImplementationOnce(() => ({
- body: {
- tree: {
- sha: '5678',
- },
- },
- }));
return github.initRepo(...args);
}
const config = await mergeInitRepo('some/repo', 'token');
@@ -318,14 +286,6 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
- ghGot.mockImplementationOnce(() => ({
- body: {
- tree: {
- sha: '5678',
- },
- },
- }));
return github.initRepo(...args);
}
const config = await mergeInitRepo('some/repo', 'token');
@@ -682,6 +642,14 @@ describe('api/github', () => {
describe('mergePr(prNo)', () => {
it('should merge the PR', async () => {
await initRepo('some/repo', 'token');
+ // getBranchCommit
+ ghGot.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1235',
+ },
+ },
+ }));
const pr = {
number: 1234,
head: {
@@ -691,6 +659,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(1);
expect(ghGot.delete.mock.calls).toHaveLength(1);
+ expect(ghGot.mock.calls).toHaveLength(3);
});
});
describe('mergePr(prNo)', () => {
@@ -708,6 +677,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(1);
expect(ghGot.delete.mock.calls).toHaveLength(0);
+ expect(ghGot.mock.calls).toHaveLength(2);
});
});
describe('mergePr(prNo) - autodetection', () => {
@@ -730,11 +700,11 @@ describe('api/github', () => {
},
},
}));
- // getCommitTree
+ // getBranchCommit
ghGot.mockImplementationOnce(() => ({
body: {
- tree: {
- sha: '5678',
+ object: {
+ sha: '1235',
},
},
}));
@@ -753,6 +723,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(1);
expect(ghGot.delete.mock.calls).toHaveLength(1);
+ expect(ghGot.mock.calls).toHaveLength(3);
});
it('should try squash after rebase', async () => {
const pr = {
@@ -767,6 +738,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(2);
expect(ghGot.delete.mock.calls).toHaveLength(1);
+ expect(ghGot.mock.calls).toHaveLength(3);
});
it('should try merge after squash', async () => {
const pr = {
@@ -784,6 +756,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(3);
expect(ghGot.delete.mock.calls).toHaveLength(1);
+ expect(ghGot.mock.calls).toHaveLength(3);
});
it('should give up', async () => {
const pr = {
@@ -804,6 +777,7 @@ describe('api/github', () => {
await github.mergePr(pr);
expect(ghGot.put.mock.calls).toHaveLength(3);
expect(ghGot.delete.mock.calls).toHaveLength(0);
+ expect(ghGot.mock.calls).toHaveLength(2);
});
});
describe('getFile(filePatch, branchName)', () => {
--
GitLab