From 4b2adf4cb2bb217f6105b0cf36a5d52e80e702aa Mon Sep 17 00:00:00 2001 From: Sara Tan Date: Tue, 5 Oct 2021 16:50:15 -0700 Subject: [PATCH 1/2] Continue if comparing commits returns an error --- src/autoupdater.ts | 33 +++++++++++++++++++++------------ test/autoupdate.test.ts | 15 +++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/autoupdater.ts b/src/autoupdater.ts index 0a1bd85c..9c484b81 100644 --- a/src/autoupdater.ts +++ b/src/autoupdater.ts @@ -172,6 +172,7 @@ export class AutoUpdater { } async update(sourceEventOwner: string, pull: PullRequest): Promise { + console.log('Updating...............'); const { ref } = pull.head; ghCore.info(`Evaluating pull request #${pull.number}...`); @@ -227,6 +228,7 @@ export class AutoUpdater { } async prNeedsUpdate(pull: PullRequest): Promise { + console.log('Checking if PR needs update'); if (pull.merged === true) { ghCore.warning('Skipping pull request, already merged.'); return false; @@ -244,18 +246,25 @@ export class AutoUpdater { return false; } - const { data: comparison } = await this.octokit.rest.repos.compareCommits({ - owner: pull.head.repo.owner.login, - repo: pull.head.repo.name, - // This base->head, head->base logic is intentional, we want - // to see what would happen if we merged the base into head not - // vice-versa. - base: pull.head.label, - head: pull.base.label, - }); - - if (comparison.behind_by === 0) { - ghCore.info('Skipping pull request, up-to-date with base branch.'); + try { + const { data: comparison } = + await this.octokit.rest.repos.compareCommitsWithBasehead({ + owner: pull.head.repo.owner.login, + repo: pull.head.repo.name, + // This base->head, head->base logic is intentional, we want + // to see what would happen if we merged the base into head not + // vice-versa. This parameter expects the format {base}...{head}. + basehead: `${pull.head.label}...${pull.base.label}`, + }); + + if (comparison.behind_by === 0) { + ghCore.info('Skipping pull request, up-to-date with base branch.'); + return false; + } + } catch (e: unknown) { + if (e instanceof Error) { + `Caught error trying to evaluate PR: ${e.message}`; + } return false; } diff --git a/test/autoupdate.test.ts b/test/autoupdate.test.ts index bc68072b..1259b962 100644 --- a/test/autoupdate.test.ts +++ b/test/autoupdate.test.ts @@ -1038,6 +1038,21 @@ describe('test `merge`', () => { } }); + test('handles errors from compareCommitsWithBasehead', async () => { + (config.retryCount as jest.Mock).mockReturnValue(0); + const updater = new AutoUpdater(config, emptyEvent); + + const scope = nock('https://api.github.com:443') + .get(`/repos/${owner}/${repo}/compare/${head}...${base}`) + .reply(404, { + message: 'Not Found', + }); + + const needsUpdate = await updater.update(owner, validPull); + expect(needsUpdate).toEqual(false); + expect(scope.isDone()).toEqual(true); + }); + test('handles fork authorisation errors', async () => { (config.retryCount as jest.Mock).mockReturnValue(0); const updater = new AutoUpdater(config, emptyEvent); From 5c560ecd678a348e6b6a87b40fba7b6935483d4f Mon Sep 17 00:00:00 2001 From: Sara Tan Date: Tue, 5 Oct 2021 17:01:45 -0700 Subject: [PATCH 2/2] Remove console logs --- src/autoupdater.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/autoupdater.ts b/src/autoupdater.ts index 9c484b81..4ee6c7a0 100644 --- a/src/autoupdater.ts +++ b/src/autoupdater.ts @@ -172,7 +172,6 @@ export class AutoUpdater { } async update(sourceEventOwner: string, pull: PullRequest): Promise { - console.log('Updating...............'); const { ref } = pull.head; ghCore.info(`Evaluating pull request #${pull.number}...`); @@ -228,7 +227,6 @@ export class AutoUpdater { } async prNeedsUpdate(pull: PullRequest): Promise { - console.log('Checking if PR needs update'); if (pull.merged === true) { ghCore.warning('Skipping pull request, already merged.'); return false; @@ -263,7 +261,9 @@ export class AutoUpdater { } } catch (e: unknown) { if (e instanceof Error) { - `Caught error trying to evaluate PR: ${e.message}`; + ghCore.error( + `Caught error trying to compare base with head: ${e.message}`, + ); } return false; }