fix: Environment variables from the GitHub action context were not pa…

…ssed through to the underlying Octopus CLI

fix: StdError and the process exit code returned by the CLI are now shown in Github Action runs
refactor: Sync codebase with create-release-action

closes #248 #249

.prettierrc.json

@@ -1,10 +1,11 @@
 {
-  "printWidth": 80,
+  "printWidth": 120,
   "tabWidth": 2,
   "useTabs": false,
   "semi": false,
   "singleQuote": true,
   "trailingComma": "none",
-  "bracketSpacing": false,
-  "arrowParens": "avoid"
+  "bracketSpacing": true,
+  "arrowParens": "avoid",
+  "endOfLine": "auto"
 }

__tests__/integration/cleanup-helper.ts

@@ -3,7 +3,7 @@ type Callback = () => unknown
 export class CleanupHelper {
   #actions: Callback[] = []
 
-  add(callback: Callback): void {
+  add(callback: Callback) {
     this.#actions.push(callback)
   }
 
@@ -15,11 +15,15 @@ export class CleanupHelper {
     toExecute.reverse()
     this.#actions = []
 
-    for (const a of toExecute) {
+    for (let a of toExecute) {
       try {
         const result = a()
-        if (result && result instanceof Promise) {
-          await result
+        if (result && typeof result === 'object') {
+          const resultObj: any = result
+          if (typeof resultObj?.then === 'function') {
+            // it's a promise!
+            await resultObj
+          }
         }
       } catch (e: unknown) {
         console.error(`ERROR DURING CLEANUP!\n${e}`)

__tests__/integration/integration.test.ts

@@ -1,11 +1,7 @@
-import {makeInputParameters} from '../../src/input-parameters'
-import {
-  Client,
-  ClientConfiguration,
-  Repository
-} from '@octopusdeploy/api-client'
-import {randomBytes} from 'crypto'
-import {CleanupHelper} from './cleanup-helper'
+import { makeInputParameters } from '../../src/input-parameters'
+import { Client, ClientConfiguration, Repository } from '@octopusdeploy/api-client'
+import { randomBytes } from 'crypto'
+import { CleanupHelper } from './cleanup-helper'
 import {
   GuidedFailureMode,
   PackageRequirement,
@@ -14,24 +10,44 @@ import {
   StartTrigger,
   TenantedDeploymentMode
 } from '@octopusdeploy/message-contracts'
-import {RunConditionForAction} from '@octopusdeploy/message-contracts/dist/runConditionForAction'
-import {RunbookEnvironmentScope} from '@octopusdeploy/message-contracts/dist/runbookEnvironmentScope'
-import {setOutput} from '@actions/core'
-import {OctopusCliWrapper} from '../../src/octopus-cli-wrapper'
+import { RunConditionForAction, RunbookEnvironmentScope } from '@octopusdeploy/message-contracts'
+import { setOutput } from '@actions/core'
+import { runRunbook } from '../../src/octopus-cli-wrapper'
+import { CaptureOutput } from '../test-helpers'
+import { platform, tmpdir } from 'os'
+import { chmodSync, mkdirSync, rmSync, writeFileSync } from 'fs'
+import { join as pathJoin } from 'path'
 
 const octoExecutable = process.env.OCTOPUS_TEST_CLI_PATH || 'octo' // if 'octo' isn't in your system path, you can override it for tests here
 
+const isWindows = platform().includes('win')
+
 const apiClientConfig: ClientConfiguration = {
-  apiKey:
-    process.env.OCTOPUS_TEST_APIKEY || 'API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX',
+  apiKey: process.env.OCTOPUS_TEST_APIKEY || 'API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX',
   apiUri: process.env.OCTOPUS_TEST_URL || 'http://localhost:8050'
 }
 
+// experimental. Should probably be a custom jest matcher
+function expectMatchAll(actual: string[], expected: (string | RegExp)[]) {
+  for (let i = 0; i < Math.min(expected.length, actual.length); i++) {
+    const a = actual[i]
+    const e = expected[i]
+    if (e instanceof RegExp) {
+      expect(a).toMatch(e)
+    } else {
+      expect(a).toEqual(e)
+    }
+  }
+  expect(actual.length).toEqual(expected.length)
+}
+
 describe('integration tests', () => {
   const runId = randomBytes(16).toString('hex')
+
   const globalCleanup = new CleanupHelper()
+
   const localProjectName = `project${runId}`
-  const standardInput = makeInputParameters({
+  const standardInputParameters = makeInputParameters({
     project: localProjectName,
     apiKey: apiClientConfig.apiKey,
     server: apiClientConfig.apiUri
@@ -54,12 +70,9 @@ describe('integration tests', () => {
       LifecycleId: lifeCycle.Id,
       ProjectGroupId: projectGroup.Id
     })
-    standardInput.project = project.Id
+    standardInputParameters.project = project.Id
     globalCleanup.add(async () => repository.projects.del(project))
-    const deploymentProcess = await repository.deploymentProcesses.get(
-      project.DeploymentProcessId,
-      undefined
-    )
+    const deploymentProcess = await repository.deploymentProcesses.get(project.DeploymentProcessId, undefined)
     deploymentProcess.Steps = [
       {
         Condition: RunCondition.Success,
@@ -68,7 +81,7 @@ describe('integration tests', () => {
         StartTrigger: StartTrigger.StartAfterPrevious,
         Id: '',
         Name: `step1-${runId}`,
-        Properties: {'Octopus.Action.TargetRoles': 'deploy'},
+        Properties: { 'Octopus.Action.TargetRoles': 'deploy' },
         Actions: [
           {
             Id: '',
@@ -101,10 +114,7 @@ describe('integration tests', () => {
         ]
       }
     ]
-    await repository.deploymentProcesses.saveToProject(
-      project,
-      deploymentProcess
-    )
+    await repository.deploymentProcesses.saveToProject(project, deploymentProcess)
 
     const runbook = await repository.runbooks.create({
       ProjectId: project.Id,
@@ -119,10 +129,7 @@ describe('integration tests', () => {
       }
     })
     globalCleanup.add(async () => repository.runbooks.del(runbook))
-    const runbookProcess = await repository.runbookProcess.get(
-      runbook.RunbookProcessId,
-      undefined
-    )
+    const runbookProcess = await repository.runbookProcess.get(runbook.RunbookProcessId, undefined)
     runbookProcess.Steps = [
       {
         Condition: RunCondition.Success,
@@ -131,7 +138,7 @@ describe('integration tests', () => {
         StartTrigger: StartTrigger.StartAfterPrevious,
         Id: '',
         Name: `Run a Script`,
-        Properties: {'Octopus.Action.TargetRoles': 'deploy'},
+        Properties: { 'Octopus.Action.TargetRoles': 'deploy' },
         Actions: [
           {
             Id: '',
@@ -171,39 +178,146 @@ describe('integration tests', () => {
     snapshot = await repository.runbookSnapshots.create(snapshot, {
       publish: true
     })
-    standardInput.runbook = runbook.Id
+    standardInputParameters.runbook = runbook.Id
     const env = await repository.environments.create({
       Name: `Test-${runId}`
     })
     globalCleanup.add(async () => repository.environments.del(env))
-    standardInput.environments = env.Id
-    // Added sometime to wait for the runbook to finish running before cleanup
-    globalCleanup.add(async () => new Promise(r => setTimeout(r, 2500)))
+    standardInputParameters.environments = env.Id
+
+    globalCleanup.add(async () => {
+      // Added some time to wait for the runbook to finish running before cleanup
+      return new Promise(r => setTimeout(r, 2500))
+    })
   })
 
   afterAll(async () => {
     if (process.env.GITHUB_ACTIONS) {
-      setOutput('gha_selftest_project_name', standardInput.project)
-      setOutput('gha_selftest_environments', standardInput.environments)
-      setOutput('gha_selftest_runbook', standardInput.runbook)
+      setOutput('gha_selftest_project_name', standardInputParameters.project)
+      setOutput('gha_selftest_environments', standardInputParameters.environments)
+      setOutput('gha_selftest_runbook', standardInputParameters.runbook)
     } else {
       await globalCleanup.cleanup()
     }
   })
 
   test('can run runbook', async () => {
-    const messages: string[] = []
-    const w = new OctopusCliWrapper(
-      standardInput,
-      {},
-      m => messages.push(m),
-      m => messages.push(m)
-    )
-    await w.runRunbook(octoExecutable)
-    console.log('Got: ', messages)
+    const output = new CaptureOutput()
+    await runRunbook({ parameters: standardInputParameters, env: {} }, output, octoExecutable)
+
+    console.log('Got: ', output.getAllMessages())
     // The CLI outputs a diffrent amount of inputs with diffrent values
     // everytime it runs. As we will be moving away from
     // the CLI we will just check that the CLI outpus something.
-    expect(messages.length).toBeGreaterThan(0)
+    expect(output.infos.length).toBeGreaterThan(0)
+  })
+
+  test('fails with error if CLI executable not found', async () => {
+    const output = new CaptureOutput()
+    try {
+      await runRunbook({ parameters: standardInputParameters, env: {} }, output, 'not-octo')
+      throw new Error('should not get here: expecting runRunbook to throw an exception')
+    } catch (err: any) {
+      expect(err.message).toMatch(
+        // regex because the error prints the underlying nodejs error which has different text on different platforms, and we're not worried about
+        // asserting on that
+        new RegExp(
+          "Octopus CLI executable missing. Ensure you have added the 'OctopusDeploy/install-octopus-cli-action@v1' step to your GitHub actions workflow"
+        )
+      )
+    }
+
+    expect(output.getAllMessages()).toEqual([])
+  })
+
+  test('fails picks up stderr from executable as well as return codes', async () => {
+    const output = new CaptureOutput()
+
+    let tmpDirPath = pathJoin(tmpdir(), runId)
+    mkdirSync(tmpDirPath)
+
+    let exePath: string
+    if (isWindows) {
+      const fileContents =
+        '@echo off\n' + 'echo An informational Message\n' + 'echo An error message 1>&2\n' + 'exit /b 37'
+      exePath = pathJoin(tmpDirPath, 'erroring_executable.cmd')
+      writeFileSync(exePath, fileContents)
+    } else {
+      const fileContents = 'echo An informational Message\n' + '>&2 echo "An error message "\n' + '(exit 37)'
+      exePath = pathJoin(tmpDirPath, 'erroring_executable.sh')
+      writeFileSync(exePath, fileContents)
+      chmodSync(exePath, '755')
+    }
+
+    const expectedExitCode = 37
+    try {
+      await runRunbook({ parameters: standardInputParameters, env: {} }, output, exePath)
+      throw new Error('should not get here: expecting runRunbook to throw an exception')
+    } catch (err: any) {
+      expect(err.message).toMatch(
+        new RegExp(`The process .*erroring_executable.* failed with exit code ${expectedExitCode}`)
+      )
+    } finally {
+      rmSync(tmpDirPath, { recursive: true })
+    }
+
+    expect(output.infos).toEqual(['An informational Message'])
+    expect(output.warns).toEqual(['An error message ']) // trailing space is deliberate because of windows bat file
+  })
+
+  test('fails with error if CLI returns an error code', async () => {
+    const output = new CaptureOutput()
+
+    const expectedExitCode = isWindows ? 4294967295 : 255 // Process should return -1 which maps to 4294967295 on windows or 255 on linux
+    const cliInputs = {
+      parameters: makeInputParameters({
+        // no project
+        apiKey: apiClientConfig.apiKey,
+        server: apiClientConfig.apiUri
+      }),
+      env: {}
+    }
+
+    try {
+      await runRunbook(cliInputs, output, octoExecutable)
+      throw new Error('should not get here: expecting runRunbook to throw an exception')
+    } catch (err: any) {
+      expect(err.message).toMatch(
+        // regex because when run locally the output logs 'octo' but in GHA it logs '/opt/hostedtoolcache/octo/9.1.3/x64/octo'
+        new RegExp(`The process .*octo.* failed with exit code ${expectedExitCode}`)
+      )
+    }
+
+    expect(output.warns).toEqual([])
+    console.log('Got: ', output.getAllMessages())
+    expect(output.infos.length).toBeGreaterThan(0)
+  })
+
+  test('fails with error if CLI returns an error code (bad auth)', async () => {
+    const output = new CaptureOutput()
+
+    const expectedExitCode = isWindows ? 4294967291 : 2 // Process should return -3 which maps to 4294967291 on windows or 2 on linux
+
+    const cliInputs = {
+      parameters: makeInputParameters({
+        project: localProjectName,
+        apiKey: apiClientConfig.apiKey + 'ZZZ',
+        server: apiClientConfig.apiUri
+      }),
+      env: {}
+    }
+
+    try {
+      await runRunbook(cliInputs, output, octoExecutable)
+      throw new Error('should not get here: expecting runRunbook to throw an exception')
+    } catch (err: any) {
+      expect(err.message).toMatch(
+        // regex because when run locally the output logs 'octo' but in GHA it logs '/opt/hostedtoolcache/octo/9.1.3/x64/octo'
+        new RegExp(`The process .*octo.* failed with exit code ${expectedExitCode}`)
+      )
+    }
+
+    expect(output.warns).toEqual([])
+    expect(output.infos[output.infos.length - 1]).toEqual('Exit code: -5')
   })
 })

__tests__/test-helpers.ts

@@ -0,0 +1,22 @@
+import { CliOutput } from '../src/cli-util'
+
+export class CaptureOutput implements CliOutput {
+  infos: string[]
+  warns: string[]
+
+  constructor() {
+    this.infos = []
+    this.warns = []
+  }
+
+  info(message: string) {
+    this.infos.push(message)
+  }
+  warn(message: string) {
+    this.warns.push(message)
+  }
+
+  getAllMessages(): string[] {
+    return this.infos.concat(this.warns)
+  }
+}