refactor: several improvements (#262)

- more tests
- file read in config
- reduce if depth
- reduce unnecessary requests

__tests__/assets/result

@@ -0,0 +1 @@
+hi there

__tests__/config.test.ts

@@ -1,18 +1,127 @@
-import * as process from 'process'
+beforeEach(() => {
-
+  process.env['GITHUB_REPOSITORY'] = 'marocchino/stick-pull-request-comment'
-test('test runs', () => {
-  process.env['INPUT_HEADER'] = ''
   process.env['INPUT_NUMBER'] = '123'
   process.env['INPUT_APPEND'] = 'false'
   process.env['INPUT_RECREATE'] = 'false'
   process.env['INPUT_DELETE'] = 'false'
   process.env['INPUT_GITHUB_TOKEN'] = 'some-token'
-  process.env['GITHUB_REPOSITORY'] = 'marocchino/stick-pull-request-comment'
+})
+
+afterEach(() => {
+  jest.resetModules()
+  delete process.env['GITHUB_REPOSITORY']
+  delete process.env['INPUT_REPO']
+  delete process.env['INPUT_HEADER']
+  delete process.env['INPUT_MESSAGE']
+  delete process.env['INPUT_NUMBER']
+  delete process.env['INPUT_APPEND']
+  delete process.env['INPUT_RECREATE']
+  delete process.env['INPUT_DELETE']
+  delete process.env['INPUT_GITHUB_TOKEN']
+  delete process.env['INPUT_PATH']
+})
+
+test('repo', () => {
+  process.env['INPUT_REPO'] = 'other'
   expect(require('../src/config')).toMatchObject({
     pullRequestNumber: expect.any(Number),
-    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    repo: {owner: 'marocchino', repo: 'other'},
-    message: '',
+    body: '',
-    path: '',
+    header: '',
+    append: false,
+    recreate: false,
+    deleteOldComment: false,
+    githubToken: 'some-token'
+  })
+})
+test('header', () => {
+  process.env['INPUT_HEADER'] = 'header'
+  expect(require('../src/config')).toMatchObject({
+    pullRequestNumber: expect.any(Number),
+    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    body: '',
+    header: 'header',
+    append: false,
+    recreate: false,
+    deleteOldComment: false,
+    githubToken: 'some-token'
+  })
+})
+test('append', () => {
+  process.env['INPUT_APPEND'] = 'true'
+  expect(require('../src/config')).toMatchObject({
+    pullRequestNumber: expect.any(Number),
+    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    body: '',
+    header: '',
+    append: true,
+    recreate: false,
+    deleteOldComment: false,
+    githubToken: 'some-token'
+  })
+})
+test('recreate', () => {
+  process.env['INPUT_RECREATE'] = 'true'
+  expect(require('../src/config')).toMatchObject({
+    pullRequestNumber: expect.any(Number),
+    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    body: '',
+    header: '',
+    append: false,
+    recreate: true,
+    deleteOldComment: false,
+    githubToken: 'some-token'
+  })
+})
+test('delete', () => {
+  process.env['INPUT_DELETE'] = 'true'
+  expect(require('../src/config')).toMatchObject({
+    pullRequestNumber: expect.any(Number),
+    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    body: '',
+    header: '',
+    append: false,
+    recreate: false,
+    deleteOldComment: true,
+    githubToken: 'some-token'
+  })
+})
+describe('path', () => {
+  test('when exists return content of a file', () => {
+    process.env['INPUT_PATH'] = './__tests__/assets/result'
+    expect(require('../src/config')).toMatchObject({
+      pullRequestNumber: expect.any(Number),
+      repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+      body: 'hi there\n',
+      header: '',
+      append: false,
+      recreate: false,
+      deleteOldComment: false,
+      githubToken: 'some-token'
+    })
+  })
+
+  test('when not exists return null string', () => {
+    process.env['INPUT_PATH'] = './__tests__/assets/not_exists'
+    expect(require('../src/config')).toMatchObject({
+      pullRequestNumber: expect.any(Number),
+      repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+      body: '',
+      header: '',
+      append: false,
+      recreate: false,
+      deleteOldComment: false,
+      githubToken: 'some-token'
+    })
+  })
+})
+
+test('message', () => {
+  process.env['INPUT_MESSAGE'] = 'hello there'
+  expect(require('../src/config')).toMatchObject({
+    pullRequestNumber: expect.any(Number),
+    repo: {owner: 'marocchino', repo: 'stick-pull-request-comment'},
+    body: 'hello there',
     header: '',
     append: false,
     recreate: false,

dist/index.js

@@ -105,25 +105,40 @@ var __importStar = (this && this.__importStar) || function (mod) {
 };
 var _a, _b;
 Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.githubToken = exports.deleteOldComment = exports.recreate = exports.append = exports.header = exports.path = exports.message = exports.repo = exports.pullRequestNumber = void 0;
+exports.body = exports.githubToken = exports.deleteOldComment = exports.recreate = exports.append = exports.header = exports.repo = exports.pullRequestNumber = void 0;
 const core = __importStar(__nccwpck_require__(186));
 const github_1 = __nccwpck_require__(438);
+const fs_1 = __nccwpck_require__(747);
 exports.pullRequestNumber = ((_b = (_a = github_1.context === null || github_1.context === void 0 ? void 0 : github_1.context.payload) === null || _a === void 0 ? void 0 : _a.pull_request) === null || _b === void 0 ? void 0 : _b.number) ||
     +core.getInput('number', { required: false });
 exports.repo = buildRepo();
-exports.message = core.getInput('message', { required: false });
-exports.path = core.getInput('path', { required: false });
 exports.header = core.getInput('header', { required: false });
 exports.append = core.getInput('append', { required: true }) === 'true';
 exports.recreate = core.getInput('recreate', { required: true }) === 'true';
 exports.deleteOldComment = core.getInput('delete', { required: true }) === 'true';
 exports.githubToken = core.getInput('GITHUB_TOKEN', { required: true });
+exports.body = buildBody();
 function buildRepo() {
     return {
         owner: github_1.context.repo.owner,
         repo: core.getInput('repo', { required: false }) || github_1.context.repo.repo
     };
 }
+function buildBody() {
+    const path = core.getInput('path', { required: false });
+    if (path) {
+        try {
+            return fs_1.readFileSync(path, 'utf-8');
+        }
+        catch (error) {
+            core.setFailed(error.message);
+            return '';
+        }
+    }
+    else {
+        return core.getInput('message', { required: false });
+    }
+}
 
 
 /***/ }),
@@ -166,7 +181,6 @@ const core = __importStar(__nccwpck_require__(186));
 const github = __importStar(__nccwpck_require__(438));
 const comment_1 = __nccwpck_require__(667);
 const config_1 = __nccwpck_require__(88);
-const fs_1 = __nccwpck_require__(747);
 function run() {
     return __awaiter(this, void 0, void 0, function* () {
         if (isNaN(config_1.pullRequestNumber) || config_1.pullRequestNumber < 1) {
@@ -174,37 +188,29 @@ function run() {
             return;
         }
         try {
-            const octokit = github.getOctokit(config_1.githubToken);
+            if (!config_1.deleteOldComment && !config_1.body) {
-            const previous = yield comment_1.findPreviousComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.header);
-            if (!config_1.deleteOldComment && !config_1.message && !config_1.path) {
                 throw new Error('Either message or path input is required');
             }
             if (config_1.deleteOldComment && config_1.recreate) {
                 throw new Error('delete and recreate cannot be both set to true');
             }
-            let body;
+            const octokit = github.getOctokit(config_1.githubToken);
-            if (config_1.path) {
+            const previous = yield comment_1.findPreviousComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.header);
-                body = fs_1.readFileSync(config_1.path, 'utf-8');
+            if (!previous) {
+                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.body, config_1.header);
+                return;
             }
-            else {
+            if (config_1.deleteOldComment) {
-                body = config_1.message;
+                yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
+                return;
             }
-            if (previous) {
+            const previousBody = config_1.append ? previous.body : undefined;
-                const previousBody = config_1.append ? previous.body : undefined;
+            if (config_1.recreate) {
-                if (config_1.deleteOldComment) {
+                yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
-                    yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
+                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.body, config_1.header, previousBody);
-                }
+                return;
-                else if (config_1.recreate) {
-                    yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
-                    yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, body, config_1.header, previousBody);
-                }
-                else {
-                    yield comment_1.updateComment(octokit, config_1.repo, previous.id, body, config_1.header, previousBody);
-                }
-            }
-            else {
-                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, body, config_1.header);
             }
+            yield comment_1.updateComment(octokit, config_1.repo, previous.id, config_1.body, config_1.header, previousBody);
         }
         catch (error) {
             core.setFailed(error.message);

lib/config.js

@@ -20,22 +20,37 @@ var __importStar = (this && this.__importStar) || function (mod) {
 };
 var _a, _b;
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.githubToken = exports.deleteOldComment = exports.recreate = exports.append = exports.header = exports.path = exports.message = exports.repo = exports.pullRequestNumber = void 0;
+exports.body = exports.githubToken = exports.deleteOldComment = exports.recreate = exports.append = exports.header = exports.repo = exports.pullRequestNumber = void 0;
 const core = __importStar(require("@actions/core"));
 const github_1 = require("@actions/github");
+const fs_1 = require("fs");
 exports.pullRequestNumber = ((_b = (_a = github_1.context === null || github_1.context === void 0 ? void 0 : github_1.context.payload) === null || _a === void 0 ? void 0 : _a.pull_request) === null || _b === void 0 ? void 0 : _b.number) ||
     +core.getInput('number', { required: false });
 exports.repo = buildRepo();
-exports.message = core.getInput('message', { required: false });
-exports.path = core.getInput('path', { required: false });
 exports.header = core.getInput('header', { required: false });
 exports.append = core.getInput('append', { required: true }) === 'true';
 exports.recreate = core.getInput('recreate', { required: true }) === 'true';
 exports.deleteOldComment = core.getInput('delete', { required: true }) === 'true';
 exports.githubToken = core.getInput('GITHUB_TOKEN', { required: true });
+exports.body = buildBody();
 function buildRepo() {
     return {
         owner: github_1.context.repo.owner,
         repo: core.getInput('repo', { required: false }) || github_1.context.repo.repo
     };
 }
+function buildBody() {
+    const path = core.getInput('path', { required: false });
+    if (path) {
+        try {
+            return fs_1.readFileSync(path, 'utf-8');
+        }
+        catch (error) {
+            core.setFailed(error.message);
+            return '';
+        }
+    }
+    else {
+        return core.getInput('message', { required: false });
+    }
+}

lib/main.js

@@ -32,7 +32,6 @@ const core = __importStar(require("@actions/core"));
 const github = __importStar(require("@actions/github"));
 const comment_1 = require("./comment");
 const config_1 = require("./config");
-const fs_1 = require("fs");
 function run() {
     return __awaiter(this, void 0, void 0, function* () {
         if (isNaN(config_1.pullRequestNumber) || config_1.pullRequestNumber < 1) {
@@ -40,37 +39,29 @@ function run() {
             return;
         }
         try {
-            const octokit = github.getOctokit(config_1.githubToken);
+            if (!config_1.deleteOldComment && !config_1.body) {
-            const previous = yield comment_1.findPreviousComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.header);
-            if (!config_1.deleteOldComment && !config_1.message && !config_1.path) {
                 throw new Error('Either message or path input is required');
             }
             if (config_1.deleteOldComment && config_1.recreate) {
                 throw new Error('delete and recreate cannot be both set to true');
             }
-            let body;
+            const octokit = github.getOctokit(config_1.githubToken);
-            if (config_1.path) {
+            const previous = yield comment_1.findPreviousComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.header);
-                body = fs_1.readFileSync(config_1.path, 'utf-8');
+            if (!previous) {
+                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.body, config_1.header);
+                return;
             }
-            else {
+            if (config_1.deleteOldComment) {
-                body = config_1.message;
+                yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
+                return;
             }
-            if (previous) {
+            const previousBody = config_1.append ? previous.body : undefined;
-                const previousBody = config_1.append ? previous.body : undefined;
+            if (config_1.recreate) {
-                if (config_1.deleteOldComment) {
+                yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
-                    yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
+                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, config_1.body, config_1.header, previousBody);
-                }
+                return;
-                else if (config_1.recreate) {
-                    yield comment_1.deleteComment(octokit, config_1.repo, previous.id);
-                    yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, body, config_1.header, previousBody);
-                }
-                else {
-                    yield comment_1.updateComment(octokit, config_1.repo, previous.id, body, config_1.header, previousBody);
-                }
-            }
-            else {
-                yield comment_1.createComment(octokit, config_1.repo, config_1.pullRequestNumber, body, config_1.header);
             }
+            yield comment_1.updateComment(octokit, config_1.repo, previous.id, config_1.body, config_1.header, previousBody);
         }
         catch (error) {
             core.setFailed(error.message);

src/config.ts

@@ -1,13 +1,12 @@
 import * as core from '@actions/core'
 import {context} from '@actions/github'
+import {readFileSync} from 'fs'
 
 export const pullRequestNumber =
   context?.payload?.pull_request?.number ||
   +core.getInput('number', {required: false})
 
 export const repo = buildRepo()
-export const message = core.getInput('message', {required: false})
-export const path = core.getInput('path', {required: false})
 export const header = core.getInput('header', {required: false})
 export const append = core.getInput('append', {required: true}) === 'true'
 export const recreate = core.getInput('recreate', {required: true}) === 'true'
@@ -15,9 +14,25 @@ export const deleteOldComment =
   core.getInput('delete', {required: true}) === 'true'
 export const githubToken = core.getInput('GITHUB_TOKEN', {required: true})
 
+export const body = buildBody()
+
 function buildRepo(): {repo: string; owner: string} {
   return {
     owner: context.repo.owner,
     repo: core.getInput('repo', {required: false}) || context.repo.repo
   }
 }
+
+function buildBody(): string {
+  const path = core.getInput('path', {required: false})
+  if (path) {
+    try {
+      return readFileSync(path, 'utf-8')
+    } catch (error) {
+      core.setFailed(error.message)
+      return ''
+    }
+  } else {
+    return core.getInput('message', {required: false})
+  }
+}

src/main.ts

@@ -9,15 +9,13 @@ import {
 import {
   pullRequestNumber,
   repo,
-  message,
+  body,
-  path,
   header,
   append,
   recreate,
   deleteOldComment,
   githubToken
 } from './config'
-import {readFileSync} from 'fs'
 
 async function run(): Promise<undefined> {
   if (isNaN(pullRequestNumber) || pullRequestNumber < 1) {
@@ -26,6 +24,14 @@ async function run(): Promise<undefined> {
   }
 
   try {
+    if (!deleteOldComment && !body) {
+      throw new Error('Either message or path input is required')
+    }
+
+    if (deleteOldComment && recreate) {
+      throw new Error('delete and recreate cannot be both set to true')
+    }
+
     const octokit = github.getOctokit(githubToken)
     const previous = await findPreviousComment(
       octokit,
@@ -34,49 +40,31 @@ async function run(): Promise<undefined> {
       header
     )
 
-    if (!deleteOldComment && !message && !path) {
+    if (!previous) {
-      throw new Error('Either message or path input is required')
-    }
-
-    if (deleteOldComment && recreate) {
-      throw new Error('delete and recreate cannot be both set to true')
-    }
-
-    let body
-
-    if (path) {
-      body = readFileSync(path, 'utf-8')
-    } else {
-      body = message
-    }
-
-    if (previous) {
-      const previousBody = append ? previous.body : undefined
-      if (deleteOldComment) {
-        await deleteComment(octokit, repo, previous.id)
-      } else if (recreate) {
-        await deleteComment(octokit, repo, previous.id)
-        await createComment(
-          octokit,
-          repo,
-          pullRequestNumber,
-          body,
-          header,
-          previousBody
-        )
-      } else {
-        await updateComment(
-          octokit,
-          repo,
-          previous.id,
-          body,
-          header,
-          previousBody
-        )
-      }
-    } else {
       await createComment(octokit, repo, pullRequestNumber, body, header)
+      return
     }
+
+    if (deleteOldComment) {
+      await deleteComment(octokit, repo, previous.id)
+      return
+    }
+
+    const previousBody = append ? previous.body : undefined
+    if (recreate) {
+      await deleteComment(octokit, repo, previous.id)
+      await createComment(
+        octokit,
+        repo,
+        pullRequestNumber,
+        body,
+        header,
+        previousBody
+      )
+      return
+    }
+
+    await updateComment(octokit, repo, previous.id, body, header, previousBody)
   } catch (error) {
     core.setFailed(error.message)
   }