Merge pull request #5 from github/improvements

Improvements

Author: GrantBirki

README.md

@@ -83,7 +83,7 @@ test('skips the process of completing the run', async () => {
     return skipped[name]
   })
   expect(await post()).toBeUndefined()
-  expect(infoMock).toHaveBeenCalledWith('skip_completing set, exiting')
+  expect(infoMock).toHaveBeenCalledWith('⏩ skip_completing set, exiting')
 })
 
 test('throws an error', async () => {

__tests__/functions/post.test.js

@@ -1,10 +1,12 @@
 import {prechecks} from '../../src/functions/prechecks'
+import {COLORS} from '../../src/functions/colors'
 import * as isAllowed from '../../src/functions/allowlist'
 import * as validPermissions from '../../src/functions/valid-permissions'
 import * as core from '@actions/core'
 
 // Globals for testing
 const infoMock = jest.spyOn(core, 'info')
+const debugMock = jest.spyOn(core, 'debug')
 const defaultContextType = 'pull_request'
 
 var context
@@ -97,7 +99,7 @@ test('runs prechecks and finds that the IssueOps command is valid', async () =>
       octokit // octokit
     )
   ).toStrictEqual({
-    message: '✔️ PR is approved and all CI checks passed - OK',
+    message: '✅ PR is approved and all CI checks passed',
     ref: 'test-ref',
     status: true,
     sha: 'abc123'
@@ -117,7 +119,7 @@ test('runs prechecks and finds that the IssueOps command is valid and exits earl
       octokit // octokit
     )
   ).toStrictEqual({
-    message: '✔️ operation requested on an issue - OK',
+    message: `✅ operation requested on an ${COLORS.highlight}issue`,
     ref: null,
     status: true,
     sha: null
@@ -144,17 +146,19 @@ test('runs prechecks and finds that the IssueOps command is valid without define
       octokit
     )
   ).toStrictEqual({
-    message:
-      '✔️ CI checks have not been defined but the PR has been approved - OK',
+    message: '✅ CI checks have not been defined but the PR has been approved',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
   })
-  expect(infoMock).toHaveBeenCalledWith(
-    "Could not retrieve PR commit status: TypeError: Cannot read properties of undefined (reading 'nodes') - Handled: OK"
+  expect(debugMock).toHaveBeenCalledWith(
+    "could not retrieve PR commit status: TypeError: Cannot read properties of undefined (reading 'nodes') - Handled: OK"
   )
-  expect(infoMock).toHaveBeenCalledWith(
-    'Skipping commit status check and proceeding...'
+  expect(debugMock).toHaveBeenCalledWith(
+    'this repo may not have any CI checks defined'
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    'skipping commit status check and proceeding...'
   )
 })
 
@@ -198,20 +202,23 @@ test('runs prechecks and finds that reviews and CI checks have not been defined'
     )
   ).toStrictEqual({
     message:
-      '⚠️ CI checks have not been defined and required reviewers have not been defined... proceeding - OK',
+      '🎛️ CI checks have not been defined and required reviewers have not been defined',
     status: true,
 
     ref: 'test-ref',
     sha: 'abc123'
   })
-  expect(infoMock).toHaveBeenCalledWith(
-    "Could not retrieve PR commit status: TypeError: Cannot read properties of undefined (reading 'nodes') - Handled: OK"
+  expect(debugMock).toHaveBeenCalledWith(
+    "could not retrieve PR commit status: TypeError: Cannot read properties of undefined (reading 'nodes') - Handled: OK"
   )
-  expect(infoMock).toHaveBeenCalledWith(
-    'Skipping commit status check and proceeding...'
+  expect(debugMock).toHaveBeenCalledWith(
+    'this repo may not have any CI checks defined'
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    'skipping commit status check and proceeding...'
   )
   expect(infoMock).toHaveBeenCalledWith(
-    '⚠️ CI checks have not been defined and required reviewers have not been defined... proceeding - OK'
+    '🎛️ CI checks have not been defined and required reviewers have not been defined'
   )
 })
 
@@ -249,57 +256,16 @@ test('runs prechecks and finds CI checks pass but reviews are not defined', asyn
       octokit
     )
   ).toStrictEqual({
-    message:
-      '⚠️ CI checks have been defined but required reviewers have not been defined... proceeding - OK',
+    message: '✅ CI checks are passing and reviews are not defined',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
   })
   expect(infoMock).toHaveBeenLastCalledWith(
-    '⚠️ CI checks have been defined but required reviewers have not been defined... proceeding - OK'
+    '✅ CI checks are passing and reviews are not defined'
   )
 })
 
-test('runs prechecks and finds CI is passing and the PR has not been reviewed', async () => {
-  octokit.graphql = jest.fn().mockReturnValue({
-    repository: {
-      pullRequest: {
-        reviewDecision: 'REVIEW_REQUIRED',
-        commits: {
-          nodes: [
-            {
-              commit: {
-                checkSuites: {
-                  totalCount: 1
-                },
-                statusCheckRollup: {
-                  state: 'SUCCESS'
-                }
-              }
-            }
-          ]
-        }
-      }
-    }
-  })
-
-  expect(
-    await prechecks(
-      '123',
-      true, // allow forks
-      false, // skip_ci
-      false, // skip_reviews
-      false, // allow_drafts
-      defaultContextType, // contextType
-      context,
-      octokit
-    )
-  ).toStrictEqual({
-    message: '⚠️ CI checks are passing but the PR has not been reviewed',
-    status: false
-  })
-})
-
 test('runs prechecks and finds that the IssueOps command is valid for a branch operation and is from a forked repository', async () => {
   octokit.graphql = jest.fn().mockReturnValue({
     repository: {
@@ -347,7 +313,7 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch o
       octokit
     )
   ).toStrictEqual({
-    message: '✔️ PR is approved and all CI checks passed - OK',
+    message: '✅ PR is approved and all CI checks passed',
     status: true,
 
     ref: 'abcde12345',
@@ -669,7 +635,8 @@ test('runs prechecks and finds CI is passing but the PR is missing an approval',
       octokit
     )
   ).toStrictEqual({
-    message: '⚠️ CI checks are passing but the PR has not been reviewed',
+    message:
+      '### ⚠️ Cannot proceed with operation\n\n> CI checks are passing but the PR has not been reviewed',
     status: false
   })
 })
@@ -789,7 +756,7 @@ test('runs prechecks and finds the skip_ci is set and reviews are not required',
     )
   ).toStrictEqual({
     message:
-      '⚠️ CI requirements have been disabled for this operation and required reviewers have not been defined... proceeding - OK',
+      '✅ CI requirements have been disabled for this operation and reviews are not required',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
@@ -900,7 +867,7 @@ test('runs prechecks and finds the PR is a DRAFT PR and drafts are allowed', asy
       octokit
     )
   ).toStrictEqual({
-    message: '✔️ PR is approved and all CI checks passed - OK',
+    message: '✅ PR is approved and all CI checks passed',
     ref: 'test-ref',
     status: true,
     sha: 'abc123'
@@ -919,49 +886,6 @@ test('runs prechecks and fails with a non 200 permissionRes.status', async () =>
   })
 })
 
-test('runs prechecks and finds that no CI checks exist and reviews are not defined', async () => {
-  octokit.graphql = jest.fn().mockReturnValue({
-    repository: {
-      pullRequest: {
-        reviewDecision: null,
-        commits: {
-          nodes: [
-            {
-              commit: {
-                checkSuites: {
-                  totalCount: 0
-                },
-                statusCheckRollup: null
-              }
-            }
-          ]
-        }
-      }
-    }
-  })
-  expect(
-    await prechecks(
-      '123',
-      true,
-      false,
-      false,
-      false,
-      defaultContextType, // contextType
-      context,
-      octokit
-    )
-  ).toStrictEqual({
-    message:
-      '⚠️ CI checks have not been defined and required reviewers have not been defined... proceeding - OK',
-    status: true,
-    ref: 'test-ref',
-    sha: 'abc123'
-  })
-  expect(infoMock).toHaveBeenLastCalledWith(
-    '⚠️ CI checks have not been defined and required reviewers have not been defined... proceeding - OK'
-  )
-})
-
 test('runs prechecks and finds that no CI checks exist but reviews are defined', async () => {
   octokit.graphql = jest.fn().mockReturnValue({
     repository: {
@@ -994,8 +918,7 @@ test('runs prechecks and finds that no CI checks exist but reviews are defined',
       octokit
     )
   ).toStrictEqual({
-    message:
-      '✔️ CI checks have not been defined but the PR has been approved - OK',
+    message: '✅ CI checks have not been defined but the PR has been approved',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
@@ -1036,7 +959,7 @@ test('runs prechecks and finds that skip_ci is set and the PR has been approved'
     )
   ).toStrictEqual({
     message:
-      '✔️ CI requirements have been disabled for this operation and the PR has been approved - OK',
+      '✅ CI requirements have been disabled for this operation and the PR has been approved',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
@@ -1100,14 +1023,14 @@ test('runs prechecks and finds that skip_ci is set and no reviews are defined',
       octokit
     )
   ).toStrictEqual({
-    message: '✔️ CI and PR reviewers are not required for this operation - OK',
+    message: '✅ CI and PR reviewers are not required for this operation',
     ref: 'test-ref',
     status: true,
     sha: 'abc123'
   })
 
   expect(infoMock).toHaveBeenCalledWith(
-    '✔️ CI and PR reviewers are not required for this operation - OK'
+    '✅ CI and PR reviewers are not required for this operation'
   )
 })
 
@@ -1146,14 +1069,14 @@ test('runs prechecks and finds that skip_ci is set and skip_reviews is set', asy
       octokit
     )
   ).toStrictEqual({
-    message: '✔️ CI and PR reviewers are not required for this operation - OK',
+    message: '✅ CI and PR reviewers are not required for this operation',
     ref: 'test-ref',
     status: true,
     sha: 'abc123'
   })
 
   expect(infoMock).toHaveBeenCalledWith(
-    '✔️ CI and PR reviewers are not required for this operation - OK'
+    '✅ CI and PR reviewers are not required for this operation'
   )
 })
 
@@ -1192,8 +1115,7 @@ test('runs prechecks and finds that skip_ci is set', async () => {
       octokit
     )
   ).toStrictEqual({
-    message:
-      '⚠️ CI checks are not required for this operation but the PR has not been reviewed',
+    message: `### ⚠️ Cannot proceed with operation\n\n> CI checks are not required for this operation but the PR has not been reviewed`,
     status: false
   })
 })
@@ -1273,7 +1195,7 @@ test('runs prechecks and finds that the PR is NOT reviewed and CI checks have be
       octokit
     )
   ).toStrictEqual({
-    message: `✔️ CI and PR reviewers are not required for this operation - OK`,
+    message: `✅ CI and PR reviewers are not required for this operation`,
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
@@ -1315,7 +1237,7 @@ test('runs prechecks and finds the PR is approved and ci is passing', async () =
       octokit // octokit instance
     )
   ).toStrictEqual({
-    message: '✔️ PR is approved and all CI checks passed - OK',
+    message: '✅ PR is approved and all CI checks passed',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'
@@ -1358,7 +1280,7 @@ test('runs prechecks and finds the PR is approved and ci is passing', async () =
     )
   ).toStrictEqual({
     message:
-      '✔️ CI checked passsed and required reviewers have been disabled for this operation - OK',
+      '✅ CI checks are passing and reviews have been disabled for this operation',
     status: true,
     ref: 'test-ref',
     sha: 'abc123'

__tests__/functions/prechecks.test.js

@@ -1,14 +1,15 @@
 import {triggerCheck} from '../../src/functions/trigger-check'
+import {COLORS} from '../../src/functions/colors'
 import * as core from '@actions/core'
 
 const setOutputMock = jest.spyOn(core, 'setOutput')
-const infoMock = jest.spyOn(core, 'info')
+const debugMock = jest.spyOn(core, 'debug')
 
 beforeEach(() => {
   jest.clearAllMocks()
   jest.spyOn(core, 'setOutput').mockImplementation(() => {})
   jest.spyOn(core, 'saveState').mockImplementation(() => {})
-  jest.spyOn(core, 'info').mockImplementation(() => {})
+  jest.spyOn(core, 'debug').mockImplementation(() => {})
 })
 
 test('checks a message and finds a standard trigger', async () => {
@@ -23,17 +24,11 @@ test('checks a message and does not find trigger', async () => {
   const trigger = '.test'
   expect(await triggerCheck(body, trigger)).toBe(false)
   expect(setOutputMock).toHaveBeenCalledWith('comment_body', '.bad')
-  expect(infoMock).toHaveBeenCalledWith(
-    'Trigger ".test" not found in the comment body'
+  expect(debugMock).toHaveBeenCalledWith(
+    `trigger ${COLORS.highlight}${trigger}${COLORS.reset} not found in the comment body`
   )
 })
 
-test('checks a message and finds a global trigger', async () => {
-  const body = 'I want to .test'
-  const trigger = '.test'
-  expect(await triggerCheck(body, trigger)).toBe(false)
-})
-
 test('checks a message and finds a trigger with extra text', async () => {
   const trigger = '.test'
   expect(await triggerCheck('.test dev something', trigger)).toBe(true)
@@ -61,15 +56,15 @@ test('checks a message and finds a trigger with extra text', async () => {
   )
 })
 
-test('checks a message and does not find global trigger', async () => {
+test('checks a message and does not find a trigger', async () => {
   const body = 'I want to .ping a website'
   const trigger = '.test'
   expect(await triggerCheck(body, trigger)).toBe(false)
   expect(setOutputMock).toHaveBeenCalledWith(
     'comment_body',
     'I want to .ping a website'
   )
-  expect(infoMock).toHaveBeenCalledWith(
-    'Trigger ".test" not found in the comment body'
+  expect(debugMock).toHaveBeenCalledWith(
+    `trigger ${COLORS.highlight}${trigger}${COLORS.reset} not found in the comment body`
   )
 })