From 2e45262693c23863b112917b910d8e3d6f8f0de3 Mon Sep 17 00:00:00 2001 From: johnjbarton Date: Wed, 10 Feb 2021 15:30:14 -0800 Subject: [PATCH] feat(server): remove socket reconnect support This feature has been trouble and there are cases it cannot work. Any socket disconnect during a test can lose data. The server will disconnect if the client does not reply. Combined it makes any long running or large test error prone. Better to bail and restart the test. Fixes #3652 --- client/karma.js | 3 - client/main.js | 6 +- lib/browser.js | 147 +++++-------------- lib/server.js | 27 ++-- static/karma.js | 6 +- test/e2e/reconnecting.feature | 27 ---- test/e2e/support/reconnecting/plus.js | 5 - test/e2e/support/reconnecting/test.js | 34 ----- test/unit/browser.spec.js | 202 ++++++-------------------- test/unit/browser_collection.spec.js | 2 +- test/unit/server.spec.js | 41 ------ 11 files changed, 105 insertions(+), 395 deletions(-) delete mode 100644 test/e2e/reconnecting.feature delete mode 100644 test/e2e/support/reconnecting/plus.js delete mode 100644 test/e2e/support/reconnecting/test.js diff --git a/client/karma.js b/client/karma.js index 0cc7556e0..d64fe155f 100644 --- a/client/karma.js +++ b/client/karma.js @@ -282,9 +282,6 @@ function Karma (updater, socket, iframe, opener, navigator, location, document) this.complete() }.bind(this)) - // Report the browser name and Id. Note that this event can also fire if the connection has - // been temporarily lost, but the socket reconnected automatically. Read more in the docs: - // https://socket.io/docs/client-api/#Event-%E2%80%98connect%E2%80%99 socket.on('connect', function () { socket.io.engine.on('upgrade', function () { resultsBufferLimit = 1 diff --git a/client/main.js b/client/main.js index 6ca6e5d8b..9a04b3608 100644 --- a/client/main.js +++ b/client/main.js @@ -12,8 +12,10 @@ var BROWSER_SOCKET_TIMEOUT = constants.BROWSER_SOCKET_TIMEOUT // Connect to the server using socket.io https://socket.io/ var socket = io(location.host, { - reconnectionDelay: 500, - reconnectionDelayMax: Infinity, + // We can't support reconnection without message replay: any messages sent + // after disconnect are lost. + reconnection: false, + // At this timeout the client disconnects and the server sees 'transport close' timeout: BROWSER_SOCKET_TIMEOUT, path: KARMA_PROXY_PATH + KARMA_URL_ROOT.substr(1) + 'socket.io', 'sync disconnect on unload': true diff --git a/lib/browser.js b/lib/browser.js index aa8795792..5adbfcac9 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -4,22 +4,19 @@ const BrowserResult = require('./browser_result') const helper = require('./helper') const logger = require('./logger') -const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests. +const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests, or finished testing. const CONFIGURING = 'CONFIGURING' // The browser has been told to execute tests; it is configuring before tests execution. const EXECUTING = 'EXECUTING' // The browser is executing the tests. -const EXECUTING_DISCONNECTED = 'EXECUTING_DISCONNECTED' // The browser is executing the tests, but temporarily disconnect (waiting for socket reconnecting). -const DISCONNECTED = 'DISCONNECTED' // The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution. +/* The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution. */ +const DISCONNECTED = 'DISCONNECTED' class Browser { - constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, - noActivityTimeout, singleRun, clientConfig) { + constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, pingTimeout, singleRun, clientConfig) { this.id = id this.fullName = fullName this.name = helper.browserFullNameToShort(fullName) this.lastResult = new BrowserResult() this.disconnectsCount = 0 - this.activeSockets = [socket] - this.noActivityTimeout = noActivityTimeout this.singleRun = singleRun this.clientConfig = clientConfig this.collection = collection @@ -27,16 +24,16 @@ class Browser { this.socket = socket this.timer = timer this.disconnectDelay = disconnectDelay + // Set onto the socket when it was created. + this.pingTimeout = pingTimeout this.log = logger.create(this.name) - this.noActivityTimeoutId = null - this.pendingDisconnect = null this.setState(CONNECTED) } init () { - this.log.info(`Connected on socket ${this.socket.id} with id ${this.id}`) + this.log.info(`Binding socket ${this.socket.id} to browser with id ${this.id}`) this.bindSocketEvents(this.socket) this.collection.add(this) @@ -53,7 +50,6 @@ class Browser { this.lastResult.error = true } this.emitter.emit('browser_error', this, error) - this.refreshNoActivityTimeout() } onInfo (info) { @@ -70,8 +66,6 @@ class Browser { } else if (!helper.isDefined(info.dump)) { this.emitter.emit('browser_info', this, info) } - - this.refreshNoActivityTimeout() } onStart (info) { @@ -82,78 +76,45 @@ class Browser { this.lastResult = new BrowserResult(info.total) this.setState(EXECUTING) this.emitter.emit('browser_start', this, info) - this.refreshNoActivityTimeout() } onComplete (result) { - if (this.isNotConnected()) { - this.setState(CONNECTED) - this.lastResult.totalTimeEnd() - - this.emitter.emit('browsers_change', this.collection) - this.emitter.emit('browser_complete', this, result) - - this.clearNoActivityTimeout() + if (this.state !== EXECUTING) { + this.log.warn(`Unexpected:'complete' event arrived while in state ${this.state}`) } + this.lastResult.totalTimeEnd() + this.setState(CONNECTED) + this.socket.disconnect(true) } onSocketDisconnect (reason, disconnectedSocket) { - helper.arrayRemove(this.activeSockets, disconnectedSocket) - if (this.activeSockets.length) { - this.log.debug(`Disconnected ${disconnectedSocket.id}, still have ${this.getActiveSocketsIds()}`) + if (this.state === DISCONNECTED) { return } - - if (this.isConnected()) { - this.disconnect(`Client disconnected from CONNECTED state (${reason})`) - } else if ([CONFIGURING, EXECUTING].includes(this.state)) { - this.log.debug(`Disconnected during run, waiting ${this.disconnectDelay}ms for reconnecting.`) - this.setState(EXECUTING_DISCONNECTED) - - this.pendingDisconnect = this.timer.setTimeout(() => { - this.lastResult.totalTimeEnd() - this.lastResult.disconnected = true - this.disconnect(`reconnect failed before timeout of ${this.disconnectDelay}ms (${reason})`) - this.emitter.emit('browser_complete', this) - }, this.disconnectDelay) - - this.clearNoActivityTimeout() - } - } - - reconnect (newSocket, clientSaysReconnect) { - if (!clientSaysReconnect || this.state === DISCONNECTED) { - this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`) - this.setState(CONNECTED) - - // The disconnected browser is already part of the collection. - // Update the collection view in the UI (header on client.html) - this.emitter.emit('browsers_change', this.collection) - // Notify the launcher - this.emitter.emit('browser_register', this) - // Execute tests if configured to do so. - if (this.singleRun) { - this.execute() + // Done with this socket + disconnectedSocket.disconnect() + disconnectedSocket.removeAllListeners() + + if (this.state !== CONNECTED) { + // If we loaded the context or began testing, `disconnect` loses data. + this.emitter.emit('browser_error', this, `Disconnected from state ${this.state} with reason: ${reason || ''}`) + if (reason === 'ping timeout') { + this.log.info('Check for a test that crashes the browser or tab.') + this.log.info(`For large JS or tests that take longer than ${this.pingTimeout}ms, increase 'config.pingTimeout'`) + } else if (reason === 'transport close') { + this.log.info('The client may have disconnected.') } - } else if (this.state === EXECUTING_DISCONNECTED) { - this.log.debug('Lost socket connection, but browser continued to execute. Reconnected ' + - `on socket ${newSocket.id}.`) - this.setState(EXECUTING) - } else if ([CONNECTED, CONFIGURING, EXECUTING].includes(this.state)) { - this.log.debug(`Rebinding to new socket ${newSocket.id} (already have ` + - `${this.getActiveSocketsIds()})`) - } - - if (!this.activeSockets.some((s) => s.id === newSocket.id)) { - this.activeSockets.push(newSocket) - this.bindSocketEvents(newSocket) - } + this.lastResult.totalTimeEnd() + this.lastResult.disconnected = true - if (this.pendingDisconnect) { - this.timer.clearTimeout(this.pendingDisconnect) + this.timer.setTimeout(() => { + // If we disconnect during a test the server can retry the browser + this.disconnectsCount++ + this.finishDisconnect() + }, this.disconnectDelay) + } else { + this.finishDisconnect() } - - this.refreshNoActivityTimeout() } onResult (result) { @@ -163,23 +124,15 @@ class Browser { this.lastResult.add(result) this.emitter.emit('spec_complete', this, result) } - this.refreshNoActivityTimeout() } execute () { - this.activeSockets.forEach((socket) => socket.emit('execute', this.clientConfig)) + this.socket.emit('execute', this.clientConfig) this.setState(CONFIGURING) - this.refreshNoActivityTimeout() - } - - getActiveSocketsIds () { - return this.activeSockets.map((s) => s.id).join(', ') } - disconnect (reason) { - this.log.warn(`Disconnected (${this.disconnectsCount} times) ${reason || ''}`) - this.disconnectsCount++ - this.emitter.emit('browser_error', this, `Disconnected ${reason || ''}`) + finishDisconnect () { + this.emitter.emit('browser_complete', this) this.remove() } @@ -188,26 +141,6 @@ class Browser { this.collection.remove(this) } - refreshNoActivityTimeout () { - if (this.noActivityTimeout) { - this.clearNoActivityTimeout() - - this.noActivityTimeoutId = this.timer.setTimeout(() => { - this.lastResult.totalTimeEnd() - this.lastResult.disconnected = true - this.disconnect(`, because no message in ${this.noActivityTimeout} ms.`) - this.emitter.emit('browser_complete', this) - }, this.noActivityTimeout) - } - } - - clearNoActivityTimeout () { - if (this.noActivityTimeout && this.noActivityTimeoutId) { - this.timer.clearTimeout(this.noActivityTimeoutId) - this.noActivityTimeoutId = null - } - } - bindSocketEvents (socket) { // TODO: check which of these events are actually emitted by socket socket.on('disconnect', (reason) => this.onSocketDisconnect(reason, socket)) @@ -246,7 +179,6 @@ class Browser { state: this.state, lastResult: this.lastResult, disconnectsCount: this.disconnectsCount, - noActivityTimeout: this.noActivityTimeout, disconnectDelay: this.disconnectDelay } } @@ -255,17 +187,16 @@ class Browser { Browser.factory = function ( id, fullName, /* capturedBrowsers */ collection, emitter, socket, timer, /* config.browserDisconnectTimeout */ disconnectDelay, - /* config.browserNoActivityTimeout */ noActivityTimeout, + /* config.pingTimeout */ pingTimeout, /* config.singleRun */ singleRun, /* config.client */ clientConfig) { return new Browser(id, fullName, collection, emitter, socket, timer, - disconnectDelay, noActivityTimeout, singleRun, clientConfig) + disconnectDelay, pingTimeout, singleRun, clientConfig) } Browser.STATE_CONNECTED = CONNECTED Browser.STATE_CONFIGURING = CONFIGURING Browser.STATE_EXECUTING = EXECUTING -Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED Browser.STATE_DISCONNECTED = DISCONNECTED module.exports = Browser diff --git a/lib/server.js b/lib/server.js index cec1ecd0b..2fd6a0435 100644 --- a/lib/server.js +++ b/lib/server.js @@ -295,20 +295,19 @@ class Server extends KarmaEventEmitter { const knownBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null if (knownBrowser) { - knownBrowser.reconnect(socket, info.isSocketReconnect) - } else { - const newBrowser = this._injector.createChild([{ - id: ['value', info.id || null], - fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)], - socket: ['value', socket] - }]).invoke(Browser.factory) - - newBrowser.init() - - if (config.singleRun) { - newBrowser.execute() - singleRunBrowsers.add(newBrowser) - } + this.log.debug(`A previously captured browser has connected (${info.id} with reconnect ${info.isSocketReconnect})`) + } + const newBrowser = this._injector.createChild([{ + id: ['value', info.id || null], + fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)], + socket: ['value', socket] + }]).invoke(Browser.factory) + + newBrowser.init() + + if (config.singleRun) { + newBrowser.execute() + singleRunBrowsers.add(newBrowser) } replySocketEvents() diff --git a/static/karma.js b/static/karma.js index 95be74c59..2db69a6cf 100644 --- a/static/karma.js +++ b/static/karma.js @@ -334,8 +334,10 @@ var BROWSER_SOCKET_TIMEOUT = constants.BROWSER_SOCKET_TIMEOUT // Connect to the server using socket.io https://socket.io/ var socket = io(location.host, { - reconnectionDelay: 500, - reconnectionDelayMax: Infinity, + // We can't support reconnection without message replay: any messages sent + // after disconnect are lost. + reconnection: false, + // At this timeout the client disconnects and the server sees 'transport close' timeout: BROWSER_SOCKET_TIMEOUT, path: KARMA_PROXY_PATH + KARMA_URL_ROOT.substr(1) + 'socket.io', 'sync disconnect on unload': true diff --git a/test/e2e/reconnecting.feature b/test/e2e/reconnecting.feature deleted file mode 100644 index 246d4e0da..000000000 --- a/test/e2e/reconnecting.feature +++ /dev/null @@ -1,27 +0,0 @@ -Feature: Passing Options - In order to use Karma - As a person who wants to write great tests - I want to the browser to reconnect to Karma when it gets disconnected. - - Scenario: Manual disconnect from the browser - Given a configuration with: - """ - files = ['reconnecting/test.js', 'reconnecting/plus.js']; - browsers = ['ChromeHeadlessNoSandbox']; - plugins = [ - 'karma-jasmine', - 'karma-chrome-launcher' - ]; - client = { - jasmine: { - random: false - } - }; - """ - When I start Karma - Then it passes with: - """ - LOG: '============== START TEST ==============' - ..... - Chrome Headless - """ diff --git a/test/e2e/support/reconnecting/plus.js b/test/e2e/support/reconnecting/plus.js deleted file mode 100644 index d7fd9e84e..000000000 --- a/test/e2e/support/reconnecting/plus.js +++ /dev/null @@ -1,5 +0,0 @@ -/* eslint-disable no-unused-vars */ -// Some code under test -function plus (a, b) { - return a + b -} diff --git a/test/e2e/support/reconnecting/test.js b/test/e2e/support/reconnecting/test.js deleted file mode 100644 index 74216c7fe..000000000 --- a/test/e2e/support/reconnecting/test.js +++ /dev/null @@ -1,34 +0,0 @@ -/* globals plus */ -describe('plus', function () { - // super hacky way to get the actual socket to manipulate it... - function socket () { - return window.parent.karma.socket - } - - it('should pass', function () { - // In flaky fails we probably get two starts. - console.log('============== START TEST ==============') - expect(1).toBe(1) - }) - - it('should disconnect', function (done) { - expect(2).toBe(2) - setTimeout(() => { - socket().disconnect() - done() - }, 500) - }) - - it('should work', function () { - expect(plus(1, 2)).toBe(3) - }) - - it('should re-connect', function () { - expect(4).toBe(4) - socket().connect() - }) - - it('should work', function () { - expect(plus(3, 2)).toBe(5) - }) -}) diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index 15b150a59..f816926a7 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -16,6 +16,7 @@ describe('Browser', () => { const s = new e.EventEmitter() socketId = socketId + 1 s.id = socketId + s.disconnect = () => {} return s } @@ -174,35 +175,20 @@ describe('Browser', () => { sinon.stub(Date, 'now') Date.now.returns(12345) browser = new Browser('fake-id', 'full name', collection, emitter, socket) + collection.add(browser) }) afterEach(() => { Date.now.restore() }) - it('should set isConnected to true', () => { - browser.state = Browser.STATE_EXECUTING - browser.onComplete() - expect(browser.isConnected()).to.equal(true) - }) - - it('should fire "browsers_change" event', () => { + it('should disconnect the socket', () => { const spy = sinon.spy() - emitter.on('browsers_change', spy) + socket.disconnect = spy browser.state = Browser.STATE_EXECUTING browser.onComplete() - expect(spy).to.have.been.calledWith(collection) - }) - - it('should ignore if browser not executing', () => { - const spy = sinon.spy() - emitter.on('browsers_change', spy) - emitter.on('browser_complete', spy) - - browser.state = Browser.STATE_CONNECTED - browser.onComplete() - expect(spy).not.to.have.been.called + expect(spy).to.have.been.called }) it('should set totalTime', () => { @@ -220,6 +206,7 @@ describe('Browser', () => { beforeEach(() => { timer = createMockTimer() + socket.disconnect = sinon.spy() browser = new Browser('fake-id', 'full name', collection, emitter, socket, timer, 10) browser.init() }) @@ -229,86 +216,54 @@ describe('Browser', () => { browser.onSocketDisconnect('socket.io-reason', socket) expect(collection.length).to.equal(0) + expect(socket.disconnect).to.have.been.called }) - it('should complete if browser executing', () => { + it('should send "browser_complete"', () => { const spy = sinon.spy() emitter.on('browser_complete', spy) - browser.state = Browser.STATE_EXECUTING - browser.onSocketDisconnect('socket.io-reason', socket) - timer.wind(20) - - expect(browser.lastResult.disconnected).to.equal(true) expect(spy).to.have.been.called }) - it('should not complete if browser not executing', () => { + it('should error if browser executing', () => { const spy = sinon.spy() emitter.on('browser_complete', spy) - browser.state = Browser.STATE_CONNECTED - - browser.onSocketDisconnect('socket.io-reason', socket) - expect(spy).not.to.have.been.called - }) - }) - - describe('reconnect', () => { - it('should cancel disconnecting', () => { - const timer = createMockTimer() - - browser = new Browser('id', 'Chrome 19.0', collection, emitter, socket, timer, 10) - browser.init() + const errorSpy = sinon.spy() + emitter.on('browser_error', errorSpy) browser.state = Browser.STATE_EXECUTING browser.onSocketDisconnect('socket.io-reason', socket) - browser.reconnect(mkSocket(), true) + timer.wind(20) - timer.wind(10) - expect(browser.state).to.equal(Browser.STATE_EXECUTING) + expect(spy).to.have.been.called + expect(errorSpy).to.have.been.called }) - it('should ignore disconnects on old sockets, but accept other messages', () => { - // IE on polling sometimes reconnect on another socket (before disconnecting) - - browser = new Browser('id', 'Chrome 19.0', collection, emitter, socket, null, 0) - browser.init() - browser.state = Browser.STATE_EXECUTING - - browser.reconnect(mkSocket(), true) - - // still accept results on the old socket - socket.emit('result', { success: true }) - expect(browser.lastResult.success).to.equal(1) - - socket.emit('karma_error', {}) - expect(browser.lastResult.error).to.equal(true) - - // should be ignored, keep executing - socket.emit('disconnect', 'socket.io reason') - expect(browser.state).to.equal(Browser.STATE_EXECUTING) - }) + it('should not error if browser is connected', () => { + const spy = sinon.spy() + emitter.on('browser_complete', spy) + const errorSpy = sinon.spy() + emitter.on('browser_error', errorSpy) - it('should reconnect a disconnected browser', () => { - browser = new Browser('id', 'Chrome 25.0', collection, emitter, socket, null, 10) browser.state = Browser.STATE_DISCONNECTED - browser.reconnect(mkSocket(), true) - - expect(browser.isConnected()).to.equal(true) + browser.onSocketDisconnect('socket.io-reason', socket) + expect(spy).not.to.have.been.called + expect(errorSpy).not.to.have.been.called }) - it('should not add a disconnected browser to the collection multiple times', () => { - browser = new Browser('id', 'Chrome 25.0', collection, emitter, socket, null, 10) - browser.init() - - expect(collection.length).to.equal(1) + it('should not error if browser is disconnected', () => { + const spy = sinon.spy() + emitter.on('browser_complete', spy) + const errorSpy = sinon.spy() + emitter.on('browser_error', errorSpy) browser.state = Browser.STATE_DISCONNECTED - browser.reconnect(mkSocket(), false) - - expect(collection.length).to.equal(1) + browser.onSocketDisconnect('socket.io-reason', socket) + expect(spy).not.to.have.been.called + expect(errorSpy).not.to.have.been.called }) }) @@ -389,11 +344,11 @@ describe('Browser', () => { const spyExecute = sinon.spy() const timer = undefined const disconnectDelay = 0 - const noActivityTimeout = 0 + const pingTimeout = 17 const singleRun = false const clientConfig = {} browser = new Browser('fake-id', 'full name', collection, emitter, socket, - timer, disconnectDelay, noActivityTimeout, singleRun, clientConfig) + timer, disconnectDelay, pingTimeout, singleRun, clientConfig) socket.on('execute', spyExecute) browser.execute() @@ -412,39 +367,23 @@ describe('Browser', () => { }) describe('scenario:', () => { - it('reconnecting during the run', () => { - const timer = createMockTimer() - browser = new Browser('fake-id', 'full name', collection, emitter, socket, timer, 10) - browser.init() - browser.state = Browser.STATE_EXECUTING - socket.emit('result', { success: true, suite: [], log: [] }) - socket.emit('disconnect', 'socket.io reason') - expect(browser.isConnected()).to.equal(false) - - const newSocket = mkSocket() - browser.reconnect(newSocket, true) - expect(browser.isConnected()).to.equal(false) - - newSocket.emit('result', { success: false, suite: [], log: [] }) - newSocket.emit('complete') - expect(browser.isConnected()).to.equal(true) - expect(browser.lastResult.success).to.equal(1) - expect(browser.lastResult.failed).to.equal(1) + beforeEach(() => { + socket.disconnect = sinon.spy() }) - it('disconecting during the run', () => { + it('disconnecting during the run', () => { const spy = sinon.spy() emitter.on('browser_complete', spy) + const spyBrowserError = sinon.spy() + emitter.on('browser_error', spyBrowserError) const timer = createMockTimer() browser = new Browser('fake-id', 'full name', collection, emitter, socket, timer, 10) + browser.init() browser.state = Browser.STATE_EXECUTING socket.emit('result', { success: true, suite: [], log: [] }) socket.emit('disconnect', 'socket.io reason') - const spyBrowserError = sinon.spy() - emitter.on('browser_error', spyBrowserError) - timer.wind(10) expect(browser.lastResult.disconnected).to.equal(true) expect(spy).to.have.been.calledWith(browser) @@ -466,67 +405,9 @@ describe('Browser', () => { timer.wind(10) // wait-for reconnecting delay expect(browser.state).to.equal(Browser.STATE_DISCONNECTED) expect(browser.disconnectsCount).to.equal(1) - - const newSocket = mkSocket() - emitter.on('browser_register', () => browser.execute()) - - // reconnect on a new socket (which triggers re-execution) - browser.reconnect(newSocket, false) - expect(browser.state).to.equal(Browser.STATE_CONFIGURING) - newSocket.emit('start', { total: 11 }) - expect(browser.state).to.equal(Browser.STATE_EXECUTING) - 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) - }) - - it('keeping multiple active sockets', () => { - // If there is a new connection (socket) for an already connected browser, - // we need to keep the old socket, in the case that the new socket will disconnect. - browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10) - browser.init() - expect(browser.state).to.equal(Browser.STATE_CONNECTED) - - browser.execute() - expect(browser.state).to.equal(Browser.STATE_CONFIGURING) - - // A second connection... - const newSocket = mkSocket() - browser.reconnect(newSocket, true) - - // Disconnect the second connection... - browser.onSocketDisconnect('socket.io-reason', newSocket) - expect(browser.state).to.equal(Browser.STATE_CONFIGURING) - socket.emit('start', { total: 1 }) - expect(browser.state).to.equal(Browser.STATE_EXECUTING) - - // It should still be listening on the old socket. - socket.emit('result', { success: true, suite: [], log: [] }) - expect(browser.lastResult.success).to.equal(1) }) - it('complete only once after reconnect on the same socket', () => { - // If there is a new connection on the same socket, - // we should emit complete message only once. - browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10) - browser.onComplete = sinon.spy() - browser.init() - browser.execute() - - // A second connection... - browser.reconnect(socket, true) - - socket.emit('result', { success: true, suite: [], log: [] }) - socket.emit('complete') - - expect(browser.onComplete.callCount).to.equal(1) - }) - - it('disconnect when no message during the run', () => { + it('disconnect when pingTimeout is exceeded during the run', () => { const timer = createMockTimer() browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, timer, 10, 20) browser.init() @@ -538,7 +419,12 @@ describe('Browser', () => { socket.emit('start', { total: 11 }) socket.emit('result', { success: true, suite: [], log: [] }) + // Simulate ping timeout timer.wind(20) + socket.emit('disconnect', 'ping timeout') + // Simulate retry + timer.wind(10) + expect(browser.state).to.equal(Browser.STATE_DISCONNECTED) expect(browser.disconnectsCount).to.equal(1) expect(spyBrowserComplete).to.have.been.called diff --git a/test/unit/browser_collection.spec.js b/test/unit/browser_collection.spec.js index fc7ad5937..bd224913e 100644 --- a/test/unit/browser_collection.spec.js +++ b/test/unit/browser_collection.spec.js @@ -96,7 +96,7 @@ describe('BrowserCollection', () => { it('return all non-ready browsers', () => { const browsers = [new Browser(), new Browser(), new Browser()] browsers[0].state = Browser.STATE_EXECUTING - browsers[1].state = Browser.STATE_EXECUTING_DISCONNECTED + browsers[1].state = Browser.STATE_DISCONNECTED browsers[2].state = Browser.STATE_CONNECTED browsers.forEach((browser) => collection.add(browser)) diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index fceaebe4c..d8b899700 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -1,7 +1,6 @@ const Server = require('../../lib/server') const NetUtils = require('../../lib/utils/net-utils') const BrowserCollection = require('../../lib/browser_collection') -const Browser = require('../../lib/browser') const cfg = require('../../lib/config') const logger = require('../../lib/logger') @@ -425,44 +424,4 @@ describe('server', () => { }) }) }) - - describe('reconnecting browser', () => { - let mockBrowserSocket - - beforeEach(async () => { - browserCollection = new BrowserCollection(server) - await server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneStub) - - mockBrowserSocket = { - id: 'browser-socket-id', - on: () => {}, - emit: () => {} - } - }) - - it('should re-configure disconnected browser which has been restarted', () => { - const testBrowserId = 'my-id' - const browser = new Browser(testBrowserId, 'Chrome 19.0', browserCollection, server, - mockBrowserSocket, undefined, 0, 0, true, {}) - const registerFn = mockSocketEventListeners.get('register') - - browser.init() - browserCollection.add(browser) - - // We assume that our browser was running when it disconnected randomly. - browser.setState(Browser.STATE_EXECUTING_DISCONNECTED) - - // We now simulate a "connect" event from the Karma client where it registers - // a previous browser that disconnected while executing. Usually if it was just a - // socket.io reconnect, we would not want to restart the execution, but since this is - // a complete reconnect, we want to configure the browser and start a new execution. - registerFn({ - name: 'fake-name', - id: testBrowserId, - isSocketReconnect: false - }) - - expect(browser.state).to.equal(Browser.STATE_CONFIGURING) - }) - }) })