Merge pull request #344 from github/unlock-on-merge-improvements

Unlock on Merge branch check improvements

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

@@ -1,7 +1,9 @@
 import * as core from '@actions/core'
 import * as unlock from '../../src/functions/unlock'
 import * as checkLockFile from '../../src/functions/check-lock-file'
+import * as checkBranch from '../../src/functions/lock'
 import {unlockOnMerge} from '../../src/functions/unlock-on-merge'
+import {COLORS} from '../../src/functions/colors'
 
 const setOutputMock = jest.spyOn(core, 'setOutput')
 const infoMock = jest.spyOn(core, 'info')
@@ -27,6 +29,9 @@ beforeEach(() => {
       link: 'https://github.com/corp/test/pull/123#issuecomment-123456789'
     }
   })
+  jest.spyOn(checkBranch, 'checkBranch').mockImplementation(() => {
+    return true
+  })
 
   context = {
     eventName: 'pull_request',
@@ -54,13 +59,13 @@ test('successfully unlocks all environments on a pull request merge', async () =
     await unlockOnMerge(octokit, context, environment_targets)
   ).toStrictEqual(true)
   expect(infoMock).toHaveBeenCalledWith(
-    '🔓 removed lock - environment: staging'
+    `🔓 removed lock - environment: ${COLORS.highlight}staging${COLORS.reset}`
   )
   expect(infoMock).toHaveBeenCalledWith(
-    '🔓 removed lock - environment: development'
+    `🔓 removed lock - environment: ${COLORS.highlight}development${COLORS.reset}`
   )
   expect(infoMock).toHaveBeenCalledWith(
-    '🔓 removed lock - environment: production'
+    `🔓 removed lock - environment: ${COLORS.highlight}production${COLORS.reset}`
   )
   expect(setOutputMock).toHaveBeenCalledWith(
     'unlocked_environments',
@@ -95,14 +100,41 @@ test('only unlocks one environment because the other has no lock and the other i
   expect(
     await unlockOnMerge(octokit, context, environment_targets)
   ).toStrictEqual(true)
-  expect(debugMock).toHaveBeenCalledWith(
-    '⏩ lock for PR 111 (env: production) is not associated with PR 123 - skipping...'
+  expect(infoMock).toHaveBeenCalledWith(
+    `⏩ lock for PR ${COLORS.info}111${COLORS.reset} (env: ${COLORS.highlight}production${COLORS.reset}) is not associated with PR ${COLORS.info}123${COLORS.reset} - skipping...`
   )
-  expect(debugMock).toHaveBeenCalledWith(
-    '⏩ no lock found for environment development - skipping...'
+  expect(infoMock).toHaveBeenCalledWith(
+    `⏩ no lock file found for environment ${COLORS.highlight}development${COLORS.reset} - skipping...`
+  )
+  expect(infoMock).toHaveBeenCalledWith(
+    `🔓 removed lock - environment: ${COLORS.highlight}staging${COLORS.reset}`
+  )
+})
+
+test('only unlocks one environment because the other is not associated with the pull request and the other has no lock branch', async () => {
+  checkLockFile.checkLockFile.mockImplementationOnce(() => {
+    return {
+      link: 'https://github.com/corp/test/pull/111#issuecomment-123456789'
+    }
+  })
+  checkBranch.checkBranch.mockImplementationOnce(() => {
+    return true
+  })
+  checkBranch.checkBranch.mockImplementationOnce(() => {
+    return false
+  })
+
+  expect(
+    await unlockOnMerge(octokit, context, environment_targets)
+  ).toStrictEqual(true)
+  expect(infoMock).toHaveBeenCalledWith(
+    `⏩ lock for PR ${COLORS.info}111${COLORS.reset} (env: ${COLORS.highlight}production${COLORS.reset}) is not associated with PR ${COLORS.info}123${COLORS.reset} - skipping...`
+  )
+  expect(infoMock).toHaveBeenCalledWith(
+    `⏩ no lock branch found for environment ${COLORS.highlight}development${COLORS.reset} - skipping...`
   )
   expect(infoMock).toHaveBeenCalledWith(
-    '🔓 removed lock - environment: staging'
+    `🔓 removed lock - environment: ${COLORS.highlight}staging${COLORS.reset}`
   )
 })
 

src/functions/lock.js

@@ -245,7 +245,7 @@ async function findReason(context, sticky) {
 // :param context: The GitHub Actions event context
 // :param branchName: The name of the branch to check
 // :return: true if the branch exists, false if not
-async function checkBranch(octokit, context, branchName) {
+export async function checkBranch(octokit, context, branchName) {
   core.debug(`checking if branch ${branchName} exists...`)
   // Check if the lock branch already exists
   try {

src/functions/unlock-on-merge.js

@@ -2,7 +2,9 @@ import * as core from '@actions/core'
 import {unlock} from './unlock'
 import {LOCK_METADATA} from './lock-metadata'
 import {checkLockFile} from './check-lock-file'
+import {checkBranch} from './lock'
 import {constructValidBranchName} from './valid-branch-name'
+import {COLORS} from './colors'
 
 // Helper function to automatically find, and release a deployment lock when a pull request is merged
 // :param octokit: the authenticated octokit instance
@@ -31,14 +33,27 @@ export async function unlockOnMerge(octokit, context, environment_targets) {
     // construct the lock branch name for this environment
     var lockBranch = `${constructValidBranchName(environment)}-${LOCK_METADATA.lockBranchSuffix}`
 
+    // Check if the lock branch exists
+    const branchExists = await checkBranch(octokit, context, lockBranch)
+
+    // if the lock branch does not exist at all, then there is no lock to release
+    if (!branchExists) {
+      core.info(
+        `⏩ no lock branch found for environment ${COLORS.highlight}${environment}${COLORS.reset} - skipping...`
+      )
+      continue
+    }
+
     // attempt to fetch the lockFile for this branch
     var lockFile = await checkLockFile(octokit, context, lockBranch)
 
     // check to see if the lockFile exists and if it does, check to see if it has a link property
     if (lockFile && lockFile?.link) {
       // if the lockFile has a link property, find the PR number from the link
       var prNumber = lockFile.link.split('/pull/')[1].split('#issuecomment')[0]
-      core.info(`🔍 checking lock for PR ${prNumber} (env: ${environment})`)
+      core.info(
+        `🔍 checking lock for PR ${COLORS.info}${prNumber}${COLORS.reset} (env: ${COLORS.highlight}${environment}${COLORS.reset})`
+      )
 
       // if the PR number matches the PR number of the merged pull request, then this lock is associated with the merged pull request
       if (prNumber === context.payload.pull_request.number.toString()) {
@@ -60,16 +75,19 @@ export async function unlockOnMerge(octokit, context, environment_targets) {
 
         // log the result and format the output as it will always be a string ending with '- silent'
         var resultFmt = result.replace('- silent', '')
-        core.info(`🔓 ${resultFmt.trim()} - environment: ${environment}`)
+        core.info(
+          `🔓 ${resultFmt.trim()} - environment: ${COLORS.highlight}${environment}${COLORS.reset}`
+        )
       } else {
-        core.debug(
-          `⏩ lock for PR ${prNumber} (env: ${environment}) is not associated with PR ${context.payload.pull_request.number} - skipping...`
+        core.info(
+          `⏩ lock for PR ${COLORS.info}${prNumber}${COLORS.reset} (env: ${COLORS.highlight}${environment}${COLORS.reset}) is not associated with PR ${COLORS.info}${context.payload.pull_request.number}${COLORS.reset} - skipping...`
         )
       }
     } else {
-      core.debug(
-        `⏩ no lock found for environment ${environment} - skipping...`
+      core.info(
+        `⏩ no lock file found for environment ${COLORS.highlight}${environment}${COLORS.reset} - skipping...`
       )
+      continue
     }
   }