From 40a3722cf4deed2c4b0f4c5161abc38d3e76a6ca Mon Sep 17 00:00:00 2001
From: Sara Tan <anarast@users.noreply.github.com>
Date: Wed, 27 Jan 2021 05:23:41 +0000
Subject: [PATCH] Continue updating PRs if a merge fails (#102)

* 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
---
 src/autoupdater.ts      | 10 ++++-
 test/autoupdate.test.ts | 93 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/src/autoupdater.ts b/src/autoupdater.ts
index 16983c44..f3bc5837 100644
--- a/src/autoupdater.ts
+++ b/src/autoupdater.ts
@@ -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;
   }
diff --git a/test/autoupdate.test.ts b/test/autoupdate.test.ts
index 898de3b3..430295cd 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,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 () => {
@@ -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);
+    }
+  });
 });