diff --git a/client/karma.js b/client/karma.js index 4442589cd..0f1e05ede 100644 --- a/client/karma.js +++ b/client/karma.js @@ -41,9 +41,11 @@ function Karma (socket, iframe, opener, navigator, location, document) { } } - // This variable will be set to "true" whenever the socket lost connection and was able to - // reconnect to the Karma server. This will be passed to the Karma server then, so that - // Karma can differentiate between a socket client reconnect and a full browser reconnect. + // To start we will signal the server that we are not reconnecting. If the socket loses + // connection and was able to reconnect to the Karma server we will get a + // second 'connect' event. There we will pass 'true' and that will be passed to the + // Karma server then, so that Karma can differentiate between a socket client + // econnect and a full browser reconnect. var socketReconnect = false this.VERSION = constant.VERSION @@ -299,9 +301,6 @@ function Karma (socket, iframe, opener, navigator, location, document) { info.displayName = displayName } socket.emit('register', info) - }) - - socket.on('reconnect', function () { socketReconnect = true }) } diff --git a/lib/browser.js b/lib/browser.js index c3ab8cc21..aa8795792 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -11,7 +11,8 @@ const EXECUTING_DISCONNECTED = 'EXECUTING_DISCONNECTED' // The browser is execut const DISCONNECTED = 'DISCONNECTED' // The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution. class Browser { - constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) { + constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, + noActivityTimeout, singleRun, clientConfig) { this.id = id this.fullName = fullName this.name = helper.browserFullNameToShort(fullName) @@ -19,6 +20,8 @@ class Browser { this.disconnectsCount = 0 this.activeSockets = [socket] this.noActivityTimeout = noActivityTimeout + this.singleRun = singleRun + this.clientConfig = clientConfig this.collection = collection this.emitter = emitter this.socket = socket @@ -94,9 +97,8 @@ class Browser { } } - onDisconnect (reason, disconnectedSocket) { + onSocketDisconnect (reason, disconnectedSocket) { helper.arrayRemove(this.activeSockets, disconnectedSocket) - if (this.activeSockets.length) { this.log.debug(`Disconnected ${disconnectedSocket.id}, still have ${this.getActiveSocketsIds()}`) return @@ -119,24 +121,27 @@ class Browser { } } - reconnect (newSocket) { - if (this.state === EXECUTING_DISCONNECTED) { + 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() + } + } 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()})`) - } else if (this.state === DISCONNECTED) { - this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`) - this.setState(CONNECTED) - - // Since the disconnected browser is already part of the collection and we want to - // make sure that the server can properly handle the browser like it's the first time - // connecting this browser (as we want a complete new execution), we need to emit the - // following events: - this.emitter.emit('browsers_change', this.collection) - this.emitter.emit('browser_register', this) } if (!this.activeSockets.some((s) => s.id === newSocket.id)) { @@ -161,8 +166,8 @@ class Browser { this.refreshNoActivityTimeout() } - execute (config) { - this.activeSockets.forEach((socket) => socket.emit('execute', config)) + execute () { + this.activeSockets.forEach((socket) => socket.emit('execute', this.clientConfig)) this.setState(CONFIGURING) this.refreshNoActivityTimeout() } @@ -172,10 +177,14 @@ class Browser { } disconnect (reason) { - this.log.warn(`Disconnected (${this.disconnectsCount} times)${reason || ''}`) - this.setState(DISCONNECTED) + this.log.warn(`Disconnected (${this.disconnectsCount} times) ${reason || ''}`) this.disconnectsCount++ - this.emitter.emit('browser_error', this, `Disconnected${reason || ''}`) + this.emitter.emit('browser_error', this, `Disconnected ${reason || ''}`) + this.remove() + } + + remove () { + this.setState(DISCONNECTED) this.collection.remove(this) } @@ -201,7 +210,7 @@ class Browser { bindSocketEvents (socket) { // TODO: check which of these events are actually emitted by socket - socket.on('disconnect', (reason) => this.onDisconnect(reason, socket)) + socket.on('disconnect', (reason) => this.onSocketDisconnect(reason, socket)) socket.on('start', (info) => this.onStart(info)) socket.on('karma_error', (error) => this.onKarmaError(error)) socket.on('complete', (result) => this.onComplete(result)) @@ -246,9 +255,11 @@ class Browser { Browser.factory = function ( id, fullName, /* capturedBrowsers */ collection, emitter, socket, timer, /* config.browserDisconnectTimeout */ disconnectDelay, - /* config.browserNoActivityTimeout */ noActivityTimeout -) { - return new Browser(id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) + /* config.browserNoActivityTimeout */ noActivityTimeout, + /* config.singleRun */ singleRun, + /* config.client */ clientConfig) { + return new Browser(id, fullName, collection, emitter, socket, timer, + disconnectDelay, noActivityTimeout, singleRun, clientConfig) } Browser.STATE_CONNECTED = CONNECTED diff --git a/lib/server.js b/lib/server.js index 229c21b19..804745088 100644 --- a/lib/server.js +++ b/lib/server.js @@ -261,27 +261,12 @@ class Server extends KarmaEventEmitter { }) socket.on('register', (info) => { - let newBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null - - if (newBrowser) { - // By default if a browser disconnects while still executing, we assume that the test - // execution still continues because just the socket connection has been terminated. Now - // since we know whether this is just a socket reconnect or full client reconnect, we - // need to update the browser state accordingly. This is necessary because in case a - // browser crashed and has been restarted, we need to start with a fresh execution. - if (!info.isSocketReconnect) { - newBrowser.setState(Browser.STATE_DISCONNECTED) - } - - newBrowser.reconnect(socket) + const knownBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null - // Since not every reconnected browser is able to continue with its previous execution, - // we need to start a new execution in case a browser has restarted and is now idling. - if (newBrowser.state === Browser.STATE_CONNECTED && config.singleRun) { - newBrowser.execute(config.client) - } + if (knownBrowser) { + knownBrowser.reconnect(socket, info.isSocketReconnect) } else { - newBrowser = this._injector.createChild([{ + const newBrowser = this._injector.createChild([{ id: ['value', info.id || null], fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)], socket: ['value', socket] @@ -290,7 +275,7 @@ class Server extends KarmaEventEmitter { newBrowser.init() if (config.singleRun) { - newBrowser.execute(config.client) + newBrowser.execute() singleRunBrowsers.add(newBrowser) } } @@ -334,8 +319,7 @@ class Server extends KarmaEventEmitter { singleRunDoneBrowsers[completedBrowser.id] = true if (launcher.kill(completedBrowser.id)) { - // workaround to supress "disconnect" warning - completedBrowser.state = Browser.STATE_DISCONNECTED + completedBrowser.remove() } emitRunCompleteIfAllBrowsersDone() diff --git a/static/karma.js b/static/karma.js index 4b791bc85..9a66d9f0d 100644 --- a/static/karma.js +++ b/static/karma.js @@ -51,9 +51,11 @@ function Karma (socket, iframe, opener, navigator, location, document) { } } - // This variable will be set to "true" whenever the socket lost connection and was able to - // reconnect to the Karma server. This will be passed to the Karma server then, so that - // Karma can differentiate between a socket client reconnect and a full browser reconnect. + // To start we will signal the server that we are not reconnecting. If the socket loses + // connection and was able to reconnect to the Karma server we will get a + // second 'connect' event. There we will pass 'true' and that will be passed to the + // Karma server then, so that Karma can differentiate between a socket client + // econnect and a full browser reconnect. var socketReconnect = false this.VERSION = constant.VERSION @@ -309,9 +311,6 @@ function Karma (socket, iframe, opener, navigator, location, document) { info.displayName = displayName } socket.emit('register', info) - }) - - socket.on('reconnect', function () { socketReconnect = true }) } diff --git a/test/client/karma.spec.js b/test/client/karma.spec.js index 567339f48..21966464f 100644 --- a/test/client/karma.spec.js +++ b/test/client/karma.spec.js @@ -204,11 +204,13 @@ describe('Karma', function () { }) it('should mark "register" event for reconnected socket', function () { + // First connect. + socket.emit('connect') + socket.on('register', sinon.spy(function (info) { assert(info.isSocketReconnect === true) })) - - socket.emit('reconnect') + // Reconnect socket.emit('connect') }) diff --git a/test/e2e/reconnecting.feature b/test/e2e/reconnecting.feature index 829cbaec7..246d4e0da 100644 --- a/test/e2e/reconnecting.feature +++ b/test/e2e/reconnecting.feature @@ -21,6 +21,7 @@ Feature: Passing Options When I start Karma Then it passes with: """ + LOG: '============== START TEST ==============' ..... Chrome Headless """ diff --git a/test/e2e/support/reconnecting/test.js b/test/e2e/support/reconnecting/test.js index a3b682e1b..74216c7fe 100644 --- a/test/e2e/support/reconnecting/test.js +++ b/test/e2e/support/reconnecting/test.js @@ -6,27 +6,26 @@ describe('plus', function () { } 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) - socket().disconnect() - - done() + setTimeout(() => { + socket().disconnect() + done() + }, 500) }) it('should work', function () { expect(plus(1, 2)).toBe(3) }) - it('should re-connect', function (done) { + it('should re-connect', function () { expect(4).toBe(4) - // Emit reconnect, so Karma will not start new test run after reconnecting. - socket().emit('reconnect') socket().connect() - - done() }) it('should work', function () { diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index ec889b083..15b150a59 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -215,7 +215,7 @@ describe('Browser', () => { }) }) - describe('onDisconnect', () => { + describe('onSocketDisconnect', () => { let timer = null beforeEach(() => { @@ -227,7 +227,7 @@ describe('Browser', () => { it('should remove from parent collection', () => { expect(collection.length).to.equal(1) - browser.onDisconnect('socket.io-reason', socket) + browser.onSocketDisconnect('socket.io-reason', socket) expect(collection.length).to.equal(0) }) @@ -236,7 +236,7 @@ describe('Browser', () => { emitter.on('browser_complete', spy) browser.state = Browser.STATE_EXECUTING - browser.onDisconnect('socket.io-reason', socket) + browser.onSocketDisconnect('socket.io-reason', socket) timer.wind(20) expect(browser.lastResult.disconnected).to.equal(true) @@ -248,7 +248,7 @@ describe('Browser', () => { emitter.on('browser_complete', spy) browser.state = Browser.STATE_CONNECTED - browser.onDisconnect('socket.io-reason', socket) + browser.onSocketDisconnect('socket.io-reason', socket) expect(spy).not.to.have.been.called }) }) @@ -261,8 +261,8 @@ describe('Browser', () => { browser.init() browser.state = Browser.STATE_EXECUTING - browser.onDisconnect('socket.io-reason', socket) - browser.reconnect(mkSocket()) + browser.onSocketDisconnect('socket.io-reason', socket) + browser.reconnect(mkSocket(), true) timer.wind(10) expect(browser.state).to.equal(Browser.STATE_EXECUTING) @@ -275,7 +275,7 @@ describe('Browser', () => { browser.init() browser.state = Browser.STATE_EXECUTING - browser.reconnect(mkSocket()) + browser.reconnect(mkSocket(), true) // still accept results on the old socket socket.emit('result', { success: true }) @@ -293,7 +293,7 @@ describe('Browser', () => { browser = new Browser('id', 'Chrome 25.0', collection, emitter, socket, null, 10) browser.state = Browser.STATE_DISCONNECTED - browser.reconnect(mkSocket()) + browser.reconnect(mkSocket(), true) expect(browser.isConnected()).to.equal(true) }) @@ -306,7 +306,7 @@ describe('Browser', () => { browser.state = Browser.STATE_DISCONNECTED - browser.reconnect(mkSocket()) + browser.reconnect(mkSocket(), false) expect(collection.length).to.equal(1) }) @@ -387,13 +387,18 @@ describe('Browser', () => { describe('execute and start', () => { it('should emit execute and change state to CONFIGURING', () => { const spyExecute = sinon.spy() - const config = {} - browser = new Browser('fake-id', 'full name', collection, emitter, socket) + const timer = undefined + const disconnectDelay = 0 + const noActivityTimeout = 0 + const singleRun = false + const clientConfig = {} + browser = new Browser('fake-id', 'full name', collection, emitter, socket, + timer, disconnectDelay, noActivityTimeout, singleRun, clientConfig) socket.on('execute', spyExecute) - browser.execute(config) + browser.execute() expect(browser.state).to.equal(Browser.STATE_CONFIGURING) - expect(spyExecute).to.have.been.calledWith(config) + expect(spyExecute).to.have.been.calledWith(clientConfig) }) it('should emit start and change state to EXECUTING', () => { @@ -417,7 +422,7 @@ describe('Browser', () => { expect(browser.isConnected()).to.equal(false) const newSocket = mkSocket() - browser.reconnect(newSocket) + browser.reconnect(newSocket, true) expect(browser.isConnected()).to.equal(false) newSocket.emit('result', { success: false, suite: [], log: [] }) @@ -466,7 +471,7 @@ describe('Browser', () => { emitter.on('browser_register', () => browser.execute()) // reconnect on a new socket (which triggers re-execution) - browser.reconnect(newSocket) + 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) @@ -487,13 +492,14 @@ describe('Browser', () => { 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) + browser.reconnect(newSocket, true) // Disconnect the second connection... - browser.onDisconnect('socket.io-reason', newSocket) + 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) @@ -512,7 +518,7 @@ describe('Browser', () => { browser.execute() // A second connection... - browser.reconnect(socket) + browser.reconnect(socket, true) socket.emit('result', { success: true, suite: [], log: [] }) socket.emit('complete') diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index 8bfd39200..01121213e 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -45,7 +45,8 @@ describe('server', () => { singleRun: true, logLevel: 'OFF', plugins: [], - browserDisconnectTolerance: 0 + browserDisconnectTolerance: 0, + browserNoActivityTimeout: 0 } server = new Server(mockConfig, doneSpy) @@ -432,7 +433,7 @@ describe('server', () => { resolveExitCode(exitCode) }) - server.emit('browser_complete_with_no_more_retries', { id: 'fake' }) + server.emit('browser_complete_with_no_more_retries', { id: 'fake', remove: () => {} }) function mockProcess (process) { sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev)) @@ -449,7 +450,7 @@ describe('server', () => { resolveExitCode(exitCode) }) - server.emit('browser_complete_with_no_more_retries', { id: 'fake' }) + server.emit('browser_complete_with_no_more_retries', { id: 'fake', remove: () => {} }) function mockProcess (process) { sinon.stub(process, 'kill').callsFake((pid, ev) => process.emit(ev)) @@ -476,7 +477,7 @@ describe('server', () => { 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, null, 0) + mockBrowserSocket, undefined, 0, 0, true, {}) const registerFn = mockSocketEventListeners.get('register') browser.init()