From e18b6c52ede40a8da629310fa07054229c429fdc Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 21 Jul 2020 11:33:48 +0200 Subject: [PATCH 1/3] feat(server): allow 'exit' listeners to set exit code --- lib/server.js | 51 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/lib/server.js b/lib/server.js index 597404ce0..650adfeb1 100644 --- a/lib/server.js +++ b/lib/server.js @@ -147,6 +147,32 @@ 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) + } + + 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() + } + + return deferred.promise + } + async _start (config, launcher, preprocess, fileList, capturedBrowsers, executor, done) { if (config.detached) { this._detach(config, done) @@ -356,21 +382,22 @@ class Server extends KarmaEventEmitter { } }) - let removeAllListenersDone = false - const removeAllListeners = () => { - if (removeAllListenersDone) { - return - } - removeAllListenersDone = true - webServer.removeAllListeners() - processWrapper.removeAllListeners() - done(code || 0) - } - - return this.emitAsync('exit').then(() => { + return this.emitExitAsync(code).then((code) => { return new Promise((resolve, reject) => { socketServer.sockets.removeAllListeners() socketServer.close() + + let removeAllListenersDone = false + const removeAllListeners = () => { + if (removeAllListenersDone) { + return + } + removeAllListenersDone = true + webServer.removeAllListeners() + processWrapper.removeAllListeners() + done(code || 0) + } + const closeTimeout = setTimeout(removeAllListeners, webServerCloseTimeout) webServer.close(() => { From 837a0af3c2e8b1b8093ee43ad76cc3f2f89ae457 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 21 Jul 2020 12:48:01 +0200 Subject: [PATCH 2/3] chore(npm): update npmignore to allow install via GH url --- .npmignore | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.npmignore b/.npmignore index 7ca4e10e8..cbb83331c 100644 --- a/.npmignore +++ b/.npmignore @@ -3,10 +3,10 @@ tmp test -tasks +#tasks /tools/ docs -client +#client logo integration-tests @@ -16,8 +16,8 @@ Gruntfile.coffee credentials Karma.sublime-* -static/karma.src.js -static/karma.wrapper +#static/karma.src.js +#static/karma.wrapper test-results.xml thesis.pdf mocha-watch.sh From 5a2e2f0c5250e70bb7055cd79a2df8410599fe9b Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 28 Jul 2020 16:40:10 +0200 Subject: [PATCH 3/3] chore(test): add server exit code unit tests --- lib/server.js | 68 +++++++++++++------------ test/unit/server.spec.js | 104 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 32 deletions(-) diff --git a/lib/server.js b/lib/server.js index 650adfeb1..abfe4e197 100644 --- a/lib/server.js +++ b/lib/server.js @@ -156,20 +156,23 @@ class Server extends KarmaEventEmitter { deferred.resolve(code) } - this.emit(name, (newCode) => { - if (newCode && typeof newCode === 'number') { - // Only update code if it is given and not zero - code = newCode - } - if (!--pending) { + 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() } - }) - - if (!pending) { - resolve() + } catch (err) { + deferred.reject(err) } - return deferred.promise } @@ -324,7 +327,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) { @@ -382,29 +386,29 @@ class Server extends KarmaEventEmitter { } }) - return this.emitExitAsync(code).then((code) => { - return new Promise((resolve, reject) => { - socketServer.sockets.removeAllListeners() - socketServer.close() - - let removeAllListenersDone = false - const removeAllListeners = () => { - if (removeAllListenersDone) { - return - } - removeAllListenersDone = true - webServer.removeAllListeners() - processWrapper.removeAllListeners() - done(code || 0) + 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) + } - 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 a7761bdd8..46d29c15d 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -312,6 +312,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)