fix: Stop and finalize build on exec termination (#332)

* Fix incorrect run check

This property is a function and not a getter. Without invoking it as a function,
it's value is always truthy.

* Stop the agent service when the exec process is terminated

* Do not handle stopping exec multiple times

When the process was terminated, all of the signal handlers were triggered
causing multiple logs to be outputted. Skipping stopping the agent once another
event has stopped it causes subsequent events to skip straight to exiting, and
never finalizes builds.

Exiting is tracked so that if it is already being handled, there is no need to
handle exiting again.

* Correct test for PERCY_ENABLED

The previous test tested that PERCY_ENABLED _did not_ work. That is, it tested
that a warning was still thrown even though it was disabled.

That test has been corrected to assert that _nothing_ is thrown when disabled.

* Fix comment typo

src/commands/exec.ts

@@ -1,4 +1,4 @@
-import {flags} from '@oclif/command'
+import { flags } from '@oclif/command'
 import * as spawn from 'cross-spawn'
 import { DEFAULT_CONFIGURATION } from '../configuration/configuration'
 import ConfigurationService from '../services/configuration-service'
@@ -32,12 +32,13 @@ export default class Exec extends PercyCommand {
     }),
   }
 
+  // helps prevent exiting before the agent service has stopped
+  private exiting = false
+
   async run() {
     await super.run()
 
-    const {argv} = this.parse(Exec)
-    const {flags} = this.parse(Exec)
-
+    const { argv, flags } = this.parse(Exec)
     const command = argv.shift()
 
     if (!command) {
@@ -54,14 +55,23 @@ export default class Exec extends PercyCommand {
     }
 
     // Even if Percy will not run, continue to run the subprocess
-    const spawnedProcess = spawn(command, argv, {stdio: 'inherit'})
+    const spawnedProcess = spawn(command, argv, { stdio: 'inherit' })
+    spawnedProcess.on('exit', (code) => this.stop(code))
+
+    // Receiving any of these events should stop the agent and exit
+    process.on('SIGHUP', () => this.stop())
+    process.on('SIGINT', () => this.stop())
+    process.on('SIGTERM', () => this.stop())
+  }
 
-    spawnedProcess.on('exit', async (code: any) => {
-      if (this.percyWillRun()) {
-        await this.agentService.stop()
-      }
+  private async stop(exitCode?: number | null) {
+    if (this.exiting) { return }
+    this.exiting = true
+
+    if (this.percyWillRun()) {
+      await this.agentService.stop()
+    }
 
-      process.exit(code)
-    })
+    process.exit(exitCode || 0)
   }
 }

src/commands/percy-command.ts

@@ -22,7 +22,7 @@ export default class PercyCommand extends Command {
   }
 
   async run() {
-    if (this.percyEnabled && !this.percyTokenPresent()) {
+    if (this.percyEnabled() && !this.percyTokenPresent()) {
       this.warn('Skipping visual tests. PERCY_TOKEN was not provided.')
     }
   }

test/commands/percy-command.test.ts

@@ -14,15 +14,13 @@ describe('percy-command', () => {
     .stub(process, 'env', {PERCY_ENABLE: '0', PERCY_TOKEN: ''})
     .stderr()
     .command(['percy-command'])
-    .do((output) => expect(output.stderr).to.contain(
-      'Warning: Skipping visual tests. PERCY_TOKEN was not provided.',
-    ))
-    .it('warns about PERCY_TOKEN to be set')
+    .do((output) => expect(output.stderr).to.eql(''))
+    .it('outputs no warnings when PERCY_ENABLED is 0')
 
   test
-    .stub(process, 'env', {PERCY_ENABLE: '0', PERCY_TOKEN: 'ABC'})
+    .stub(process, 'env', {PERCY_TOKEN: 'ABC'})
     .stderr()
     .command(['percy-command'])
     .do((output) => expect(output.stderr).to.eql(''))
-    .it('outputs no errors')
+    .it('outputs no errors when PERCY_TOKEN is set')
 })