From b7f7dea8b93b262efd1568abbb0264967c567fa5 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 21 Jul 2020 11:33:48 +0200 Subject: [PATCH] feat(server): allow 'exit' listeners to set exit code 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. --- lib/server.js | 71 ++++++++++++++++++-------- test/unit/server.spec.js | 104 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 20 deletions(-) diff --git a/lib/server.js b/lib/server.js index 798f3e3c7..fb07b77b9 100644 --- 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 index 826f18c70..8bfd39200 100644 --- 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)