Merge pull request #270 from chinthakagodawita/merge-conflict-output

chinthakagodawita · Dec 12, 2021 · c203734 · c203734
2 parents 1be90ce + 248ca91
commit c203734
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 26 deletions.
diff --git a/README.md b/README.md
@@ -104,6 +104,36 @@ jobs:
           MERGE_CONFLICT_ACTION: "fail"
 ```
 
+## Outputs
+
+| Name         | Description                                                                 |
+|--------------|-----------------------------------------------------------------------------|
+| `conflicted` | `true` or `false` which indicates whether merge conflicts were found or not |
+
+Here's an example workflow file with the outputs above:
+
+```yaml
+name: autoupdate
+on:
+  push: {}
+jobs:
+  autoupdate:
+    name: autoupdate
+    runs-on: ubuntu-18.04
+    steps:
+      - uses: docker://chinthakagodawita/autoupdate-action:v1
+        id: autoupdate
+        env:
+          GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+          MERGE_CONFLICT_ACTION: "ignore"
+
+      - run: echo 'Merge conflicts found!'
+        if: ${{ steps.autoupdate.outputs.conflicted }}
+
+      - run: echo 'No merge conflicts'
+        if: ${{ !steps.autoupdate.outputs.conflicted }}
+```
+
 ## Examples
 
 See [chinthakagodawita/autoupdate-test/pulls](https://github.com/chinthakagodawita/autoupdate-test/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc) for a repository where autoupdate is enabled. This is currently configured to only run on PRs that have the `autoupdate` tag added to them.

diff --git a/src/Output.ts b/src/Output.ts
@@ -0,0 +1,4 @@
+/** Defines all outputs returned by this action */
+export enum Output {
+  Conflicted = 'conflicted',
+}
diff --git a/src/autoupdater.ts b/src/autoupdater.ts
@@ -9,17 +9,19 @@ import {
   WorkflowRunEvent,
 } from '@octokit/webhooks-definitions/schema';
 import { ConfigLoader } from './config-loader';
-import { Endpoints, RequestError } from '@octokit/types';
+import { Output } from './Output';
 
 type PullRequestResponse =
-  Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response'];
+  octokit.Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response'];
 type MergeParameters =
-  Endpoints['POST /repos/{owner}/{repo}/merges']['parameters'];
+  octokit.Endpoints['POST /repos/{owner}/{repo}/merges']['parameters'];
 
 type PullRequest =
   | PullRequestResponse['data']
   | PullRequestEvent['pull_request'];
 
+type SetOutputFn = typeof ghCore.setOutput;
+
 export class AutoUpdater {
   // See https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads
   eventData: WebhookEvent;
@@ -388,6 +390,8 @@ export class AutoUpdater {
     sourceEventOwner: string,
     prNumber: number,
     mergeOpts: MergeParameters,
+    // Allows for mocking in tests.
+    setOutputFn: SetOutputFn = ghCore.setOutput,
   ): Promise<boolean> {
     const sleep = (timeMs: number) => {
       return new Promise((resolve) => {
@@ -423,7 +427,11 @@ export class AutoUpdater {
     while (true) {
       try {
         ghCore.info('Attempting branch update...');
+
         await doMerge();
+
+        setOutputFn(Output.Conflicted, false);
+
         break;
       } catch (e: unknown) {
         if (e instanceof Error) {
@@ -433,7 +441,7 @@ export class AutoUpdater {
            */
           if (
             'status' in e &&
-            (e as RequestError).status === 403 &&
+            (e as octokit.RequestError).status === 403 &&
             sourceEventOwner !== mergeOpts.owner
           ) {
             const error = e as Error;
@@ -442,23 +450,23 @@ export class AutoUpdater {
               `Could not update pull request #${prNumber} due to an authorisation error. This is probably because this pull request is from a fork and the current token does not have write access to the forked repository. Error was: ${error.message}`,
             );
 
-            return false;
-          }
-
-          // Ignore conflicts if configured to do so.
-          if (
-            e.message === 'Merge conflict' &&
-            mergeConflictAction === 'ignore'
-          ) {
-            ghCore.info('Merge conflict detected, skipping update.');
+            setOutputFn(Output.Conflicted, false);
 
             return false;
           }
 
-          // Else, throw an error so we don't continue retrying.
           if (e.message === 'Merge conflict') {
-            ghCore.error('Merge conflict error trying to update branch');
-            throw e;
+            setOutputFn(Output.Conflicted, true);
+
+            if (mergeConflictAction === 'ignore') {
+              // Ignore conflicts if configured to do so.
+              ghCore.info('Merge conflict detected, skipping update.');
+              return false;
+            } else {
+              // Else, throw an error so we don't continue retrying.
+              ghCore.error('Merge conflict error trying to update branch');
+              throw e;
+            }
           }
 
           ghCore.error(`Caught error trying to update branch: ${e.message}`);
@@ -472,6 +480,8 @@ export class AutoUpdater {
           retries++;
           await sleep(retrySleep);
         } else {
+          setOutputFn(Output.Conflicted, false);
+
           throw e;
         }
       }

diff --git a/test/autoupdate.test.ts b/test/autoupdate.test.ts
@@ -8,6 +8,7 @@ import { createMock } from 'ts-auto-mock';
 import nock from 'nock';
 import config from '../src/config-loader';
 import { AutoUpdater } from '../src/autoupdater';
+import { Output } from '../src/Output';
 import { Endpoints } from '@octokit/types';
 import {
   PullRequestEvent,
@@ -1056,12 +1057,19 @@ describe('test `merge`', () => {
         })
         .reply(responseTest.code);
 
+      const setOutput = jest.fn();
+
       if (responseTest.success) {
-        await updater.merge(owner, 1, mergeOpts);
+        await updater.merge(owner, 1, mergeOpts, setOutput);
       } else {
-        await expect(updater.merge(owner, 1, mergeOpts)).rejects.toThrowError();
+        await expect(
+          updater.merge(owner, 1, mergeOpts, setOutput),
+        ).rejects.toThrowError();
       }
 
+      expect(setOutput).toHaveBeenCalledTimes(1);
+      expect(setOutput).toHaveBeenCalledWith(Output.Conflicted, false);
+
       expect(scope.isDone()).toEqual(true);
     });
   }
@@ -1105,7 +1113,12 @@ describe('test `merge`', () => {
         message: 'Must have admin rights to Repository.',
       });
 
-    await updater.merge('some-other-owner', 1, mergeOpts);
+    const setOutput = jest.fn();
+
+    await updater.merge('some-other-owner', 1, mergeOpts, setOutput);
+
+    expect(setOutput).toHaveBeenCalledTimes(1);
+    expect(setOutput).toHaveBeenCalledWith(Output.Conflicted, false);
 
     expect(scope.isDone()).toEqual(true);
   });
@@ -1124,9 +1137,14 @@ describe('test `merge`', () => {
         message: 'Must have admin rights to Repository.',
       });
 
-    await expect(updater.merge(owner, 1, mergeOpts)).rejects.toThrowError(
-      'Must have admin rights to Repository.',
-    );
+    const setOutput = jest.fn();
+
+    await expect(
+      updater.merge(owner, 1, mergeOpts, setOutput),
+    ).rejects.toThrowError('Must have admin rights to Repository.');
+
+    expect(setOutput).toHaveBeenCalledTimes(1);
+    expect(setOutput).toHaveBeenCalledWith(Output.Conflicted, false);
 
     expect(scope.isDone()).toEqual(true);
   });
@@ -1146,9 +1164,13 @@ describe('test `merge`', () => {
         message: 'Merge conflict',
       });
 
-    await updater.merge(owner, 1, mergeOpts);
+    const setOutput = jest.fn();
+    await updater.merge(owner, 1, mergeOpts, setOutput);
 
     expect(scope.isDone()).toEqual(true);
+
+    expect(setOutput).toHaveBeenCalledTimes(1);
+    expect(setOutput).toHaveBeenCalledWith(Output.Conflicted, true);
   });
 
   test('not ignoring merge conflicts', async () => {
@@ -1166,11 +1188,15 @@ describe('test `merge`', () => {
         message: 'Merge conflict',
       });
 
-    await expect(updater.merge(owner, 1, mergeOpts)).rejects.toThrowError(
-      'Merge conflict',
-    );
+    const setOutput = jest.fn();
+    await expect(
+      updater.merge(owner, 1, mergeOpts, setOutput),
+    ).rejects.toThrowError('Merge conflict');
 
     expect(scope.isDone()).toEqual(true);
+
+    expect(setOutput).toHaveBeenCalledTimes(1);
+    expect(setOutput).toHaveBeenCalledWith(Output.Conflicted, true);
   });
 
   test('continue if merging throws an error', async () => {