Merge pull request #281 from github/test-improvements

Test Improvements 🧪

Author: GrantBirki

__tests__/functions/help.test.js

@@ -132,3 +132,41 @@ test('successfully calls help with non-defaults', async () => {
     expect.stringMatching(/## 📚 Branch Deployment Help/)
   )
 })
+
+test('successfully calls help with non-defaults and unknown update_branch setting', async () => {
+  const inputs = {
+    trigger: '.deploy',
+    reaction: 'eyes',
+    environment: 'production',
+    stable_branch: 'main',
+    noop_trigger: '.noop',
+    lock_trigger: '.lock',
+    production_environments: 'production,production-eu,production-ap',
+    environment_targets: 'production,staging,development',
+    unlock_trigger: '.unlock',
+    help_trigger: '.help',
+    lock_info_alias: '.wcid',
+    global_lock_flag: '--global',
+    update_branch: 'bugzzz',
+    outdated_mode: 'default_branch',
+    required_contexts: 'cat',
+    allowForks: 'false',
+    skipCi: 'development',
+    skipReviews: 'development',
+    draft_permitted_targets: 'development',
+    admins: 'monalisa',
+    permissions: ['write', 'admin', 'maintain'],
+    allow_sha_deployments: false,
+    checks: 'required'
+  }
+
+  expect(await help(octokit, context, 123, inputs))
+
+  expect(debugMock).toHaveBeenCalledWith(
+    expect.stringMatching(/## 📚 Branch Deployment Help/)
+  )
+
+  expect(debugMock).toHaveBeenCalledWith(
+    expect.stringMatching(/Unknown value for update_branch/)
+  )
+})

__tests__/functions/lock.test.js

@@ -26,6 +26,9 @@ const lockBase64Monalisa =
 const lockBase64Octocat =
   'ewogICAgInJlYXNvbiI6ICJUZXN0aW5nIG15IG5ldyBmZWF0dXJlIHdpdGggbG90cyBvZiBjYXRzIiwKICAgICJicmFuY2giOiAib2N0b2NhdHMtZXZlcnl3aGVyZSIsCiAgICAiY3JlYXRlZF9hdCI6ICIyMDIyLTA2LTE0VDIxOjEyOjE0LjA0MVoiLAogICAgImNyZWF0ZWRfYnkiOiAib2N0b2NhdCIsCiAgICAic3RpY2t5IjogdHJ1ZSwKICAgICJlbnZpcm9ubWVudCI6ICJwcm9kdWN0aW9uIiwKICAgICJ1bmxvY2tfY29tbWFuZCI6ICIudW5sb2NrIHByb2R1Y3Rpb24iLAogICAgImdsb2JhbCI6IGZhbHNlLAogICAgImxpbmsiOiAiaHR0cHM6Ly9naXRodWIuY29tL3Rlc3Qtb3JnL3Rlc3QtcmVwby9wdWxsLzIjaXNzdWVjb21tZW50LTQ1NiIKfQo='
 
+const lockBase64OctocatNoReason =
+  'ewogICAgInJlYXNvbiI6IG51bGwsCiAgICAiYnJhbmNoIjogIm9jdG9jYXRzLWV2ZXJ5d2hlcmUiLAogICAgImNyZWF0ZWRfYXQiOiAiMjAyMi0wNi0xNFQyMToxMjoxNC4wNDFaIiwKICAgICJjcmVhdGVkX2J5IjogIm9jdG9jYXQiLAogICAgInN0aWNreSI6IHRydWUsCiAgICAiZW52aXJvbm1lbnQiOiAicHJvZHVjdGlvbiIsCiAgICAidW5sb2NrX2NvbW1hbmQiOiAiLnVubG9jayBwcm9kdWN0aW9uIiwKICAgICJnbG9iYWwiOiBmYWxzZSwKICAgICJsaW5rIjogImh0dHBzOi8vZ2l0aHViLmNvbS90ZXN0LW9yZy90ZXN0LXJlcG8vcHVsbC8yI2lzc3VlY29tbWVudC00NTYiCn0K'
+
 const lockBase64OctocatGlobal =
   'ewogICAgInJlYXNvbiI6ICJUZXN0aW5nIG15IG5ldyBmZWF0dXJlIHdpdGggbG90cyBvZiBjYXRzIiwKICAgICJicmFuY2giOiAib2N0b2NhdHMtZXZlcnl3aGVyZSIsCiAgICAiY3JlYXRlZF9hdCI6ICIyMDIyLTA2LTE0VDIxOjEyOjE0LjA0MVoiLAogICAgImNyZWF0ZWRfYnkiOiAib2N0b2NhdCIsCiAgICAic3RpY2t5IjogdHJ1ZSwKICAgICJlbnZpcm9ubWVudCI6IG51bGwsCiAgICAidW5sb2NrX2NvbW1hbmQiOiAiLnVubG9jayAtLWdsb2JhbCIsCiAgICAiZ2xvYmFsIjogdHJ1ZSwKICAgICJsaW5rIjogImh0dHBzOi8vZ2l0aHViLmNvbS90ZXN0LW9yZy90ZXN0LXJlcG8vcHVsbC8yI2lzc3VlY29tbWVudC00NTYiCn0K'
 
@@ -728,6 +731,74 @@ test('Determines that the lock request is coming from current owner of the lock
   )
 })
 
+test('checks a lock and finds that it is from another owner and that no reason was set - it was a lock for the production environment', async () => {
+  const octokit = {
+    rest: {
+      repos: {
+        getBranch: jest
+          .fn()
+          .mockReturnValueOnce({data: {commit: {sha: 'abc123'}}}),
+        get: jest.fn().mockReturnValue({data: {default_branch: 'main'}}),
+        getContent: jest
+          .fn()
+          .mockReturnValue({data: {content: lockBase64OctocatNoReason}})
+      }
+    }
+  }
+  expect(
+    await lock(octokit, context, ref, 123, true, environment)
+  ).toStrictEqual({
+    environment: 'production',
+    global: false,
+    globalFlag: '--global',
+    lockData: null,
+    status: false
+  })
+  expect(debugMock).toHaveBeenCalledWith(`detected lock env: ${environment}`)
+  expect(debugMock).toHaveBeenCalledWith(`detected lock global: false`)
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructed lock branch name: ${environment}-branch-deploy-lock`
+  )
+  expect(debugMock).toHaveBeenCalledWith(`no reason detected`)
+  expect(debugMock).toHaveBeenCalledWith(
+    `the lock was not claimed as it is owned by octocat`
+  )
+})
+
+test('checks a lock and finds that it is from another owner and that no reason was set - it was a lock for the production environment and sticky is set to false', async () => {
+  const octokit = {
+    rest: {
+      repos: {
+        getBranch: jest
+          .fn()
+          .mockReturnValueOnce({data: {commit: {sha: 'abc123'}}}),
+        get: jest.fn().mockReturnValue({data: {default_branch: 'main'}}),
+        getContent: jest
+          .fn()
+          .mockReturnValue({data: {content: lockBase64OctocatNoReason}})
+      }
+    }
+  }
+  expect(
+    await lock(octokit, context, ref, 123, false, environment)
+  ).toStrictEqual({
+    environment: 'production',
+    global: false,
+    globalFlag: '--global',
+    lockData: null,
+    status: false
+  })
+  expect(debugMock).toHaveBeenCalledWith(`detected lock env: ${environment}`)
+  expect(debugMock).toHaveBeenCalledWith(`detected lock global: false`)
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructed lock branch name: ${environment}-branch-deploy-lock`
+  )
+  expect(debugMock).toHaveBeenCalledWith(`no reason detected`)
+  expect(debugMock).toHaveBeenCalledWith(
+    `the lock was not claimed as it is owned by octocat`
+  )
+})
+
 test('Determines that the lock request is coming from current owner of the lock (GLOBAL lock) and exits - sticky', async () => {
   context.actor = 'octocat'
   context.payload.comment.body = '.lock --global'
@@ -967,9 +1038,9 @@ test('successfully obtains a deployment lock (sticky) by creating the branch and
   )
 })
 
-test('successfully obtains a deployment lock (sticky) by creating the branch and lock file with a --reason', async () => {
+test('successfully obtains a deployment lock (sticky) by creating the branch and lock file with a --reason and assuming a null environment to start (but it is production)', async () => {
   context.payload.comment.body = '.lock --reason because something is broken'
-  expect(await lock(octokit, context, ref, 123, true, null)).toStrictEqual(
+  expect(await lock(octokit, context, ref, 123, true)).toStrictEqual(
     createdLock
   )
   expect(debugMock).toHaveBeenCalledWith(`detected lock env: ${environment}`)

__tests__/functions/outdated-check.test.js

@@ -1,7 +1,9 @@
 import * as core from '@actions/core'
 import {isOutdated} from '../../src/functions/outdated-check'
+import {COLORS} from '../../src/functions/colors'
 
 const debugMock = jest.spyOn(core, 'debug')
+const warningMock = jest.spyOn(core, 'warning')
 
 var context
 var octokit
@@ -121,6 +123,23 @@ test('checks if the branch is out-of-date via commit comparison and finds that i
     outdated: true
   })
   expect(debugMock).toHaveBeenCalledWith('checking isOutdated with strict mode')
+  expect(warningMock).toHaveBeenCalledWith(
+    `The PR branch is behind the base branch by ${COLORS.highlight}1 commit${COLORS.reset}`
+  )
+})
+
+test('checks if the branch is out-of-date via commit comparison and finds that it is by many commits', async () => {
+  octokit.rest.repos.compareCommits = jest
+    .fn()
+    .mockReturnValue({data: {behind_by: 45}, status: 200})
+  expect(await isOutdated(context, octokit, data)).toStrictEqual({
+    branch: 'test-branch',
+    outdated: true
+  })
+  expect(debugMock).toHaveBeenCalledWith('checking isOutdated with strict mode')
+  expect(warningMock).toHaveBeenCalledWith(
+    `The PR branch is behind the base branch by ${COLORS.highlight}45 commits${COLORS.reset}`
+  )
 })
 
 test('checks if the branch is out-of-date via commit comparison and finds that it is only behind the stable branch', async () => {

__tests__/functions/post-deploy-message.test.js

@@ -2,7 +2,7 @@ import {postDeployMessage} from '../../src/functions/post-deploy-message'
 import * as core from '@actions/core'
 import dedent from 'dedent-js'
 
-// const debugMock = jest.spyOn(core, 'debug')
+const debugMock = jest.spyOn(core, 'debug')
 
 var context
 var environment
@@ -124,6 +124,28 @@ test('successfully constructs a post deploy message with the defaults during a d
   )
 })
 
+test('successfully constructs a post deploy message with the defaults during a deployment with an unknown status and the DEPLOY_MESSAGE_PATH is unset', async () => {
+  process.env.INPUT_DEPLOY_MESSAGE_PATH = ''
+  expect(
+    await postDeployMessage(
+      context, // context
+      environment, // environment
+      environment_url, // environment_url
+      'unknown', // status
+      noop, // noop
+      ref, // ref
+      approved_reviews_count // approved_reviews_count
+    )
+  ).toStrictEqual(
+    dedent(`
+      ### Deployment Results ⚠️
+
+      Warning: deployment status is unknown, please use caution`)
+  )
+
+  expect(debugMock).toHaveBeenCalledWith('deployMessagePath is not set - null')
+})
+
 test('successfully constructs a post deploy message with a custom env var', async () => {
   process.env.DEPLOY_MESSAGE = 'Deployed 1 shiny new server'
 

__tests__/functions/unlock-on-merge.test.js

@@ -68,6 +68,20 @@ test('successfully unlocks all environments on a pull request merge', async () =
   )
 })
 
+test('finds that no deployment lock is set so none are removed', async () => {
+  jest.spyOn(unlock, 'unlock').mockImplementation(() => {
+    return 'no deployment lock currently set - silent'
+  })
+
+  expect(
+    await unlockOnMerge(octokit, context, environment_targets)
+  ).toStrictEqual(true)
+  expect(debugMock).toHaveBeenCalledWith(
+    'unlock result for unlock-on-merge: no deployment lock currently set - silent'
+  )
+  expect(setOutputMock).toHaveBeenCalledWith('unlocked_environments', '')
+})
+
 test('only unlocks one environment because the other has no lock and the other is not associated with the pull request', async () => {
   checkLockFile.checkLockFile.mockImplementationOnce(() => {
     return {

dist/index.js

@@ -45,7 +45,10 @@
     ],
     "coverageThreshold": {
       "global": {
-        "lines": 100
+        "lines": 100,
+        "statements": 100,
+        "branches": 100,
+        "functions": 100
       }
     }
   },

dist/index.js.map

@@ -446,10 +446,8 @@ export async function environmentTargets(
     )
 
     return {environment: false, environmentUrl: null}
-  }
-
-  // If lockChecks is set to false, this request is for a branch deploy to check the body for an environment target
-  if (lockChecks === false) {
+  } else {
+    // If lockChecks is set to false, this request is for a branch deploy to check the body for an environment target
     const environmentObj = await onDeploymentChecks(
       environment_targets_sanitized,
       body,

package.json

@@ -11,13 +11,13 @@ export async function help(octokit, context, reactionId, inputs) {
   if (inputs.update_branch.trim() === 'warn') {
     update_branch_message =
       'This Action will warn if the branch is out of date with the base branch'
-  } else if (inputs.update_branch === 'force') {
+  } else if (inputs.update_branch.trim() === 'force') {
     update_branch_message =
       'This Action will force update the branch to the base branch if it is out of date'
-  } else if (inputs.update_branch === 'disabled') {
+  } else if (inputs.update_branch.trim() === 'disabled') {
     update_branch_message =
       'This Action will not update the branch to the base branch before deployment'
-  }
+  } else update_branch_message = 'Unknown value for update_branch'
 
   var required_contexts_message = defaultSpecificMessage
   if (inputs.required_contexts.trim() === 'false') {