From a83d451849be7c970ece5462b53a6a3fea76d6a6 Mon Sep 17 00:00:00 2001 From: Chinthaka Godawita Date: Mon, 25 Jan 2021 11:51:51 +1100 Subject: [PATCH] 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. --- src/autoupdater.ts | 5 +- test/autoupdate.test.ts | 100 +++++++++++++++++++++++++++++++--------- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/src/autoupdater.ts b/src/autoupdater.ts index d95819ef..f3bc5837 100644 --- a/src/autoupdater.ts +++ b/src/autoupdater.ts @@ -125,7 +125,10 @@ export class AutoUpdater { try { await this.merge(mergeOpts); } catch (e) { - ghCore.info('Autoupdate will attempt to update any remaining PRs.'); + ghCore.error( + `Caught error running merge, skipping and continuing with remaining PRs`, + ); + ghCore.setFailed(e); return false; } diff --git a/test/autoupdate.test.ts b/test/autoupdate.test.ts index e5e2a96e..ad5bb22a 100644 --- a/test/autoupdate.test.ts +++ b/test/autoupdate.test.ts @@ -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, @@ -316,17 +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 () => { @@ -606,19 +605,76 @@ describe('test `merge`', () => { expect(scope.isDone()).toEqual(true); }); - test('autoupdate continues with valid PRs if merging throws an error', async () => { - const mergeMsg = 'dummy-merge-msg'; - (config.mergeMsg as jest.Mock).mockReturnValue(mergeMsg); - (config.dryRun as jest.Mock).mockReturnValue(false); + test('continue if merging throws an error', async () => { + (config.mergeMsg as jest.Mock).mockReturnValue(null); + const updater = new AutoUpdater(config, dummyEvent); - const updater = new AutoUpdater(config, {}); + 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, + }, + }, + }, + }); + } - jest.spyOn(updater, 'prNeedsUpdate').mockResolvedValue(true); - jest.spyOn(updater, 'merge').mockImplementation(() => { - throw new Error('Resource not accessible by integration'); - }); + const needsUpdateSpy = jest + .spyOn(updater, 'prNeedsUpdate') + .mockResolvedValue(true); - const updated = await updater.update(validPull); - expect(updated).toEqual(false); + 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 = { + 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 5 PRs should have been updated. + expect(updated).toBe(expectedPulls - 1); + expect(needsUpdateSpy).toHaveBeenCalledTimes(expectedPulls); + expect(pullsScope.isDone()).toBe(true); + for (const scope of mergeScopes) { + expect(scope.isDone()).toBe(true); + } }); });