Skip to content

Commit

Permalink
Continue updating PRs if a merge fails (#102)
Browse files Browse the repository at this point in the history
* Return false from updated() if merge fails
* Add test to confirm that autoupdate continues on error

Adds a test that throws a 403 half-way through a list of PRs that need
updating to confirm that autoupdate continues to the next PR in line.

Co-authored-by: Chin Godawita <1452384+chinthakagodawita@users.noreply.github.com>

Fixes #100
  • Loading branch information
anarast authored Jan 27, 2021
1 parent e467e67 commit 40a3722
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 11 deletions.
10 changes: 9 additions & 1 deletion src/autoupdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,15 @@ export class AutoUpdater {
mergeOpts.commit_message = mergeMsg;
}

await this.merge(mergeOpts);
try {
await this.merge(mergeOpts);
} catch (e) {
ghCore.error(
`Caught error running merge, skipping and continuing with remaining PRs`,
);
ghCore.setFailed(e);
return false;
}

return true;
}
Expand Down
93 changes: 83 additions & 10 deletions test/autoupdate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ const owner = 'chinthakagodawita';
const repo = 'not-a-real-repo';
const base = 'master';
const head = 'develop';
const branch = 'not-a-real-branch';
const dummyEvent = {
ref: `refs/heads/${branch}`,
repository: {
owner: {
name: owner,
},
name: repo,
},
};
const validPull = {
number: 1,
merged: false,
Expand Down Expand Up @@ -316,16 +326,6 @@ describe('test `prNeedsUpdate`', () => {
});

describe('test `handlePush`', () => {
const branch = 'not-a-real-branch';
const dummyEvent = {
ref: `refs/heads/${branch}`,
repository: {
owner: {
name: owner,
},
name: repo,
},
};
const cloneEvent = () => JSON.parse(JSON.stringify(dummyEvent));

test('push event on a non-branch', async () => {
Expand Down Expand Up @@ -604,4 +604,77 @@ describe('test `merge`', () => {

expect(scope.isDone()).toEqual(true);
});

test('continue if merging throws an error', async () => {
(config.mergeMsg as jest.Mock).mockReturnValue(null);
const updater = new AutoUpdater(config, dummyEvent);

const pullsMock = [];
const expectedPulls = 5;
for (let i = 0; i < expectedPulls; i++) {
pullsMock.push({
id: i,
number: i,
base: {
ref: base,
label: base,
},
head: {
label: head,
ref: head,
repo: {
name: repo,
owner: {
login: owner,
},
},
},
});
}

const needsUpdateSpy = jest
.spyOn(updater, 'prNeedsUpdate')
.mockResolvedValue(true);

const pullsScope = nock('https://api.github.com:443')
.get(
`/repos/${owner}/${repo}/pulls?base=${branch}&state=open&sort=updated&direction=desc`,
)
.reply(200, pullsMock);

const mergeScopes = [];
for (let i = 0; i < expectedPulls; i++) {
let httpStatus = 200;
let response: Record<string, unknown> = {
data: {
sha: 'dummy-sha',
},
};

// Throw an error halfway through the PR list to confirm that autoupdate
// continues to the next PR.
if (i === 3) {
httpStatus = 403;
response = {
message: 'Resource not accessible by integration',
};
}

mergeScopes.push(
nock('https://api.github.com:443')
.post(`/repos/${owner}/${repo}/merges`)
.reply(httpStatus, response),
);
}

const updated = await updater.handlePush();

// Only 4 PRs should have been updated, not 5.
expect(updated).toBe(expectedPulls - 1);
expect(needsUpdateSpy).toHaveBeenCalledTimes(expectedPulls);
expect(pullsScope.isDone()).toBe(true);
for (const scope of mergeScopes) {
expect(scope.isDone()).toBe(true);
}
});
});

0 comments on commit 40a3722

Please sign in to comment.