feat(server): allow 'exit' listeners to set exit code (#3541)

matz3 · web-flow · commit 7a94d33ab60b · 2020-08-17T15:09:10.000-07:00
Currently it seems that it's not possible for reporters to set the exit
code asynchronously.
Within the "onRunComplete" event, the results.exitCode must be modified
synchronously, otherwise the updated value is not taken into account.

With this change, the reporters (or any other plugin) can pass an exit
code to the callback of the 'exit' event.
diff --git a/lib/server.js b/lib/server.js
@@ -142,6 +142,35 @@ class Server extends KarmaEventEmitter {
     return this._fileList ? this._fileList.changeFile(path) : Promise.resolve()
   }
 
+  emitExitAsync (code) {
+    const name = 'exit'
+    let pending = this.listeners(name).length
+    const deferred = helper.defer()
+
+    function resolve () {
+      deferred.resolve(code)
+    }
+
+    try {
+      this.emit(name, (newCode) => {
+        if (newCode && typeof newCode === 'number') {
+          // Only update code if it is given and not zero
+          code = newCode
+        }
+        if (!--pending) {
+          resolve()
+        }
+      })
+
+      if (!pending) {
+        resolve()
+      }
+    } catch (err) {
+      deferred.reject(err)
+    }
+    return deferred.promise
+  }
+
   async _start (config, launcher, preprocess, fileList, capturedBrowsers, executor, done) {
     if (config.detached) {
       this._detach(config, done)
@@ -296,7 +325,8 @@ class Server extends KarmaEventEmitter {
 
     this.on('stop', function (done) {
       this.log.debug('Received stop event, exiting.')
-      return disconnectBrowsers().then(done)
+      disconnectBrowsers()
+      done()
     })
 
     if (config.singleRun) {
@@ -354,28 +384,29 @@ class Server extends KarmaEventEmitter {
         }
       })
 
-      let removeAllListenersDone = false
-      const removeAllListeners = () => {
-        if (removeAllListenersDone) {
-          return
+      this.emitExitAsync(code).catch((err) => {
+        this.log.error('Error while calling exit event listeners\n' + err.stack || err)
+        return 1
+      }).then((code) => {
+        socketServer.sockets.removeAllListeners()
+        socketServer.close()
+
+        let removeAllListenersDone = false
+        const removeAllListeners = () => {
+          if (removeAllListenersDone) {
+            return
+          }
+          removeAllListenersDone = true
+          webServer.removeAllListeners()
+          processWrapper.removeAllListeners()
+          done(code || 0)
         }
-        removeAllListenersDone = true
-        webServer.removeAllListeners()
-        processWrapper.removeAllListeners()
-        done(code || 0)
-      }
 
-      return this.emitAsync('exit').then(() => {
-        return new Promise((resolve, reject) => {
-          socketServer.sockets.removeAllListeners()
-          socketServer.close()
-          const closeTimeout = setTimeout(removeAllListeners, webServerCloseTimeout)
+        const closeTimeout = setTimeout(removeAllListeners, webServerCloseTimeout)
 
-          webServer.close(() => {
-            clearTimeout(closeTimeout)
-            removeAllListeners()
-            resolve()
-          })
+        webServer.close(() => {
+          clearTimeout(closeTimeout)
+          removeAllListeners()
         })
       })
     }
diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js
@@ -306,6 +306,110 @@ describe('server', () => {
         expect(await exitCode()).to.have.equal(15)
       })
 
+      it('given on run_complete with exit event listener (15)', async () => {
+        mockProcess(process)
+
+        await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, (exitCode) => {
+          resolveExitCode(exitCode)
+        })
+
+        // last non-zero exit code will be taken
+        server.on('exit', (done) => {
+          setTimeout(() => done(30))
+        })
+        server.on('exit', (done) => {
+          setTimeout(() => done(15))
+        })
+        server.on('exit', (done) => {
+          setTimeout(() => done(0))
+        })
+
+        // Provided run_complete exitCode will be overridden by exit listeners
+        server.emit('run_complete', browserCollection, { exitCode: 5 })
+
+        function mockProcess (process) {
+          sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev))
+        }
+        expect(await exitCode()).to.have.equal(15)
+      })
+
+      it('given on run_complete with exit event listener (0)', async () => {
+        mockProcess(process)
+
+        await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, (exitCode) => {
+          resolveExitCode(exitCode)
+        })
+
+        // exit listeners can't set exit code back to 0
+        server.on('exit', (done) => {
+          setTimeout(() => done(0))
+        })
+
+        server.emit('run_complete', browserCollection, { exitCode: 15 })
+
+        function mockProcess (process) {
+          sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev))
+        }
+        expect(await exitCode()).to.have.equal(15)
+      })
+
+      it('1 on run_complete with exit event listener throws', async () => {
+        mockProcess(process)
+
+        await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, (exitCode) => {
+          resolveExitCode(exitCode)
+        })
+
+        server.on('exit', (done) => {
+          throw new Error('async error from exit event listener')
+        })
+
+        server.emit('run_complete', browserCollection, { exitCode: 0 })
+
+        function mockProcess (process) {
+          sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev))
+        }
+        expect(await exitCode()).to.have.equal(1)
+      })
+
+      it('1 on run_complete with exit event listener rejects', async () => {
+        mockProcess(process)
+
+        await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, (exitCode) => {
+          resolveExitCode(exitCode)
+        })
+
+        function onExit (done) {
+          // Need to remove listener to prevent endless loop via unhandledRejection handler
+          // which again calls disconnectBrowsers to fire the 'exit' event
+          server.off('exit', onExit)
+          return Promise.reject(new Error('async error from exit event listener'))
+        }
+        server.on('exit', onExit)
+
+        server.emit('run_complete', browserCollection, { exitCode: 0 })
+
+        function mockProcess (process) {
+          sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev))
+        }
+        expect(await exitCode()).to.have.equal(1)
+      })
+
+      it('0 on server stop', async () => {
+        mockProcess(process)
+
+        await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, (exitCode) => {
+          resolveExitCode(exitCode)
+        })
+
+        server.stop()
+
+        function mockProcess (process) {
+          sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev))
+        }
+        expect(await exitCode()).to.have.equal(0)
+      })
+
       it('1 on browser_process_failure (singleRunBrowserNotCaptured)', async () => {
         mockProcess(process)
 
