From 19590e1f66fd6c3b0d3fc9e90000c705198e0e70 Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Sun, 1 Dec 2013 14:12:57 -0800 Subject: [PATCH] feat: add `browserDisconnectTolerance` config option This adds a new config option `browserDisconnectTolerance`. docs/config/01-configuration-file.md for more information. When `browserDisconnectTolerance` is set to 1 and a browser disconnects during a test run, Karma will restart the browser and run the tests again. Only if the browser disconnects again, it will be treated as a failure. Based on #777. --- docs/config/01-configuration-file.md | 12 +++++++++ lib/browser.js | 12 +++++++++ lib/config.js | 6 +++++ lib/launcher.js | 17 +++++++++++- lib/server.js | 20 +++++++++----- test/unit/browser.spec.coffee | 40 ++++++++++++++++++++++++++++ test/unit/launcher.spec.coffee | 18 +++++++++++++ 7 files changed, 118 insertions(+), 7 deletions(-) diff --git a/docs/config/01-configuration-file.md b/docs/config/01-configuration-file.md index 089aedad2..afd296064 100644 --- a/docs/config/01-configuration-file.md +++ b/docs/config/01-configuration-file.md @@ -83,6 +83,18 @@ paths defined in `files` and `exclude`. If the `basePath` configuration is a relative path then it will be resolved to the `__dirname` of the configuration file. +## browserDisconnectTolerance +**Type:** Number + +**Default:** `0` + +**Description:** The number of disconnections tolerated. + +The `disconnectTolerance` value represents the maximum number of tries a browser will attempt in case of disconnection. +Usually any disconnection is considered as a failure, but this option allows to define a tolerance level when there is +a flaky network link between the karma server and the browsers. + + ## browsers **Type:** Array diff --git a/lib/browser.js b/lib/browser.js index 82e872684..c529af359 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -43,6 +43,7 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter, this.name = name; this.state = READY; this.lastResult = new Result(); + this.disconnectsCount = 0; this.init = function() { collection.add(this); @@ -90,6 +91,7 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter, }; this.onStart = function(info) { + this.lastResult = new Result(); this.lastResult.total = info.total; if (info.total === null) { @@ -118,6 +120,7 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter, var self = this; var disconnect = function() { self.state = DISCONNECTED; + self.disconnectsCount++; log.warn('Disconnected'); collection.remove(self); }; @@ -147,6 +150,15 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter, log.debug('New connection %s, forgetting %s.', newSocket.id, socket.id); // TODO(vojta): this should only remove this browser.onDisconnect listener socket.removeAllListeners('disconnect'); + } else if (this.state === DISCONNECTED) { + this.state = READY; + log.info('Connected on socket %s with id %s', newSocket.id, this.id); + collection.add(this); + + // TODO(vojta): move to collection + emitter.emit('browsers_change', collection); + + emitter.emit('browser_register', this); } socket = newSocket; diff --git a/lib/config.js b/lib/config.js index 3bf5b58f3..89a8749ba 100644 --- a/lib/config.js +++ b/lib/config.js @@ -113,6 +113,11 @@ var normalizeConfig = function(config, configFilePath) { config.autoWatch = false; } + if (!config.singleRun && config.browserDisconnectTolerance) { + log.debug('browserDisconnectTolerance set to 0, because of singleRun'); + config.browserDisconnectTolerance = 0; + } + if (helper.isString(config.reporters)) { config.reporters = config.reporters.split(','); } @@ -208,6 +213,7 @@ var Config = function() { args: [] }; this.browserDisconnectTimeout = 2000; + this.browserDisconnectTolerance = 0; }; var CONFIG_SYNTAX_HELP = ' module.exports = function(config) {\n' + diff --git a/lib/launcher.js b/lib/launcher.js index c2bf6d771..3ec9ee312 100644 --- a/lib/launcher.js +++ b/lib/launcher.js @@ -4,10 +4,11 @@ var baseBrowserDecoratorFactory = require('./launchers/Base').decoratorFactory; var Launcher = function(emitter, injector) { var browsers = []; + var lastUrl; this.launch = function(names, hostname, port, urlRoot) { - var url = 'http://' + hostname + ':' + port + urlRoot; var browser; + var url = lastUrl = 'http://' + hostname + ':' + port + urlRoot; names.forEach(function(name) { var locals = { @@ -60,6 +61,20 @@ var Launcher = function(emitter, injector) { }; + this.restart = function(id) { + for (var i = 0; i < browsers.length; i++) { + if (browsers[i].id === id) { + browsers[i].kill(function() { + browsers[i].start(lastUrl); + }); + return true; + } + } + + return false; + }; + + this.killAll = function(callback) { log.debug('Disconnecting all browsers'); diff --git a/lib/server.js b/lib/server.js index ae23764e1..55989dcb2 100644 --- a/lib/server.js +++ b/lib/server.js @@ -135,14 +135,22 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file if (config.singleRun) { globalEmitter.on('browser_complete', function(completedBrowser) { - singleRunDoneBrowsers[completedBrowser.id] = true; + if (completedBrowser.lastResult.disconnected && completedBrowser.disconnectsCount <= config.browserDisconnectTolerance) { + log.info('Restarting %s (%d of %d attempts)', completedBrowser.name, completedBrowser.disconnectsCount, config.browserDisconnectTolerance); + if (!launcher.restart(completedBrowser.id)) { + singleRunDoneBrowsers[completedBrowser.id] = true; + emitRunCompleteIfAllBrowsersDone(); + } + } else { + singleRunDoneBrowsers[completedBrowser.id] = true; - if (launcher.kill(completedBrowser.id)) { - // workaround to supress "disconnect" warning - completedBrowser.state = browser.Browser.STATE_DISCONNECTED; - } + if (launcher.kill(completedBrowser.id)) { + // workaround to supress "disconnect" warning + completedBrowser.state = browser.Browser.STATE_DISCONNECTED; + } - emitRunCompleteIfAllBrowsersDone(); + emitRunCompleteIfAllBrowsersDone(); + } }); globalEmitter.on('browser_process_failure', function(browserLauncher) { diff --git a/test/unit/browser.spec.coffee b/test/unit/browser.spec.coffee index 46815ff7d..e23fd218d 100644 --- a/test/unit/browser.spec.coffee +++ b/test/unit/browser.spec.coffee @@ -299,6 +299,15 @@ describe 'browser', -> expect(browser.state).to.equal b.Browser.STATE_EXECUTING + it 'should reconnect a disconnected browser', -> + browser = new b.Browser 'id', 'Chrome 25.0', collection, emitter, socket, null, 10 + browser.state = b.Browser.STATE_DISCONNECTED + + browser.onReconnect new e.EventEmitter + + expect(browser.isReady()).to.equal true + + #========================================================================== # browser.Browser.onResult #========================================================================== @@ -427,6 +436,37 @@ describe 'browser', -> expect(spy).to.have.been.calledWith browser + it 'restarting a disconnected browser', -> + timer = createMockTimer() + browser = new b.Browser 'fake-id', 'Chrome 31.0', collection, emitter, socket, timer, 10 + browser.init() + + browser.execute() + socket.emit 'start', {total: 10} + socket.emit 'result', {success: true, suite: [], log: []} + socket.emit 'result', {success: false, suite: [], log: []} + socket.emit 'result', {skipped: true, suite: [], log: []} + socket.emit 'disconnect' + timer.wind 10 # wait-for reconnecting delay + expect(browser.state).to.equal b.Browser.STATE_DISCONNECTED + expect(browser.disconnectsCount).to.equal 1 + + newSocket = new e.EventEmitter + emitter.on 'browser_register', -> browser.execute() + + # reconnect on a new socket (which triggers re-execution) + browser.onReconnect newSocket + expect(browser.state).to.equal b.Browser.STATE_EXECUTING + newSocket.emit 'start', {total: 11} + socket.emit 'result', {success: true, suite: [], log: []} + + # expected cleared last result (should not include the results from previous run) + expect(browser.lastResult.total).to.equal 11 + expect(browser.lastResult.success).to.equal 1 + expect(browser.lastResult.failed).to.equal 0 + expect(browser.lastResult.skipped).to.equal 0 + + #============================================================================ # browser.Collection #============================================================================ diff --git a/test/unit/launcher.spec.coffee b/test/unit/launcher.spec.coffee index f2b9ad437..02b223874 100644 --- a/test/unit/launcher.spec.coffee +++ b/test/unit/launcher.spec.coffee @@ -75,6 +75,24 @@ describe 'launcher', -> expect(browser.start).to.have.been.calledWith 'http://whatever:1234/root/' + describe 'restart', -> + it 'should kill and start the browser with the original url', -> + l.launch ['Fake'], 'localhost', 1234, '/root/' + browser = FakeBrowser._instances.pop() + browser.start.reset() + + returnedValue = l.restart lastGeneratedId + expect(returnedValue).to.equal true + expect(browser.kill).to.have.been.called + browser.kill.callArg 0 # killing is done + expect(browser.start).to.have.been.calledWith 'http://localhost:1234/root/' + + + it 'should return false if the browser was not launched by launcher (manual)', -> + l.launch [], 'localhost', 1234, '/' + expect(l.restart 'manual-id').to.equal false + + describe 'kill', -> it 'should kill browser with given id', -> killSpy = sinon.spy()