diff --git a/client/karma.js b/client/karma.js index ff4217c62..97b3b62fe 100644 --- a/client/karma.js +++ b/client/karma.js @@ -14,6 +14,11 @@ function Karma (socket, iframe, opener, navigator, location) { var resultsBufferLimit = 50 var resultsBuffer = [] + // 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. + var socketReconnect = false + this.VERSION = constant.VERSION this.config = {} @@ -227,20 +232,27 @@ function Karma (socket, iframe, opener, navigator, location) { this.complete() }.bind(this)) - // report browser name, id + // 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 }) var info = { name: navigator.userAgent, - id: browserId + id: browserId, + isSocketReconnect: socketReconnect } if (displayName) { info.displayName = displayName } socket.emit('register', info) }) + + socket.on('reconnect', function () { + socketReconnect = true + }) } module.exports = Karma diff --git a/lib/browser.js b/lib/browser.js index 81865395a..f4389c172 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -7,8 +7,8 @@ const logger = require('./logger') const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests. 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 reconnecting). -const DISCONNECTED = 'DISCONNECTED' // The browser got permanently disconnected (being removed from the collection and destroyed). +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. class Browser { constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) { @@ -121,15 +121,21 @@ class Browser { reconnect (newSocket) { if (this.state === EXECUTING_DISCONNECTED) { - this.log.debug(`Reconnected on ${newSocket.id}.`) + 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(`New connection ${newSocket.id} (already have ${this.getActiveSocketsIds()})`) + this.log.debug(`Rebinding to new socket ${newSocket.id} (already have ` + + `${this.getActiveSocketsIds()})`) } else if (this.state === DISCONNECTED) { - this.log.info(`Connected on socket ${newSocket.id} with id ${this.id}`) + this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`) this.setState(CONNECTED) - this.collection.add(this) + // 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) } diff --git a/lib/server.js b/lib/server.js index 9624d4c6b..235929a54 100644 --- a/lib/server.js +++ b/lib/server.js @@ -232,10 +232,20 @@ class Server extends KarmaEventEmitter { 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) - // We are restarting a previously disconnected browser. - if (newBrowser.state === Browser.STATE_DISCONNECTED && config.singleRun) { + // 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) } } else { diff --git a/test/client/karma.spec.js b/test/client/karma.spec.js index 78745962f..d07995a99 100644 --- a/test/client/karma.spec.js +++ b/test/client/karma.spec.js @@ -147,6 +147,15 @@ describe('Karma', function () { assert(spyInfo.called) }) + it('should mark "register" event for reconnected socket', function () { + socket.on('register', sinon.spy(function (info) { + assert(info.isSocketReconnect === true) + })) + + socket.emit('reconnect') + socket.emit('connect') + }) + it('should report browser id', function () { windowLocation.search = '?id=567' socket = new MockSocket() diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index 447bb02cc..33eca8013 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -297,6 +297,19 @@ describe('Browser', () => { expect(browser.isConnected()).to.equal(true) }) + + 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) + + browser.state = Browser.STATE_DISCONNECTED + + browser.reconnect(mkSocket()) + + expect(collection.length).to.equal(1) + }) }) describe('onResult', () => { diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index 276618b12..b24ef5f6e 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -2,6 +2,7 @@ const Server = require('../../lib/server') const BundleUtils = require('../../lib/utils/bundle-utils') const NetUtils = require('../../lib/utils/net-utils') const BrowserCollection = require('../../lib/browser_collection') +const Browser = require('../../lib/browser') describe('server', () => { let mockConfig @@ -10,6 +11,7 @@ describe('server', () => { let fileListOnReject let mockLauncher let mockWebServer + let mockServerSocket let mockSocketServer let mockBoundServer let mockExecutor @@ -17,6 +19,7 @@ describe('server', () => { let server = mockConfig = browserCollection = webServerOnError = null let fileListOnResolve = fileListOnReject = mockLauncher = null let mockFileList = mockWebServer = mockSocketServer = mockExecutor = doneSpy = null + let mockSocketEventListeners = new Map() // Use regular function not arrow so 'this' is mocha 'this'. beforeEach(function () { @@ -67,6 +70,13 @@ describe('server', () => { kill: () => true } + mockServerSocket = { + id: 'socket-id', + on: (name, handler) => mockSocketEventListeners.set(name, handler), + emit: () => {}, + removeListener: () => {} + } + mockSocketServer = { close: () => {}, flashPolicyServer: { @@ -74,7 +84,7 @@ describe('server', () => { }, sockets: { sockets: {}, - on: () => {}, + on: (name, handler) => handler(mockServerSocket), emit: () => {}, removeAllListeners: () => {} } @@ -277,4 +287,44 @@ describe('server', () => { } }) }) + + describe('reconnecting browser', () => { + let mockBrowserSocket + + beforeEach(() => { + browserCollection = new BrowserCollection(server) + server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) + + 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, null, 0) + 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) + }) + }) }) diff --git a/yarn.lock b/yarn.lock index 8833be7d0..75469fc1c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2795,6 +2795,11 @@ flat-cache@^1.2.1: graceful-fs "^4.1.2" write "^0.2.1" +flatted@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/flatted/-/flatted-2.0.0.tgz#55122b6536ea496b4b44893ee2608141d10d9916" + integrity sha512-R+H8IZclI8AAkSBRQJLVOsxwAoHd6WC40b4QTNWIjzAa6BXOBfQcM587MXDTVPeYaopFNWHUFLx7eNmHDSxMWg== + for-in@^1.0.1, for-in@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80" @@ -4216,11 +4221,6 @@ json-stringify-safe@^5.0.1, json-stringify-safe@~5.0.1: resolved "https://registry.yarnpkg.com/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz#1296a2d58fd45f19a0f6ce01d65701e2c735b6eb" integrity sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus= -json3@^3.3.2: - version "3.3.2" - resolved "https://registry.yarnpkg.com/json3/-/json3-3.3.2.tgz#3c0434743df93e2f5c42aee7b19bcb483575f4e1" - integrity sha1-PAQ0dD35Pi9cQq7nsZvLSDV19OE= - jsonfile@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/jsonfile/-/jsonfile-3.0.1.tgz#a5ecc6f65f53f662c4415c7675a0331d0992ec66" @@ -4583,6 +4583,11 @@ lodash@^4.0.0, lodash@^4.14.0, lodash@^4.16.6, lodash@^4.17.2, lodash@^4.17.4, l resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae" integrity sha1-eCA6TRwyiuHYbcpkYONptX9AVa4= +lodash@^4.17.5: + version "4.17.11" + resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d" + integrity sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg== + lodash@~4.3.0: version "4.3.0" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.3.0.tgz#efd9c4a6ec53f3b05412429915c3e4824e4d25a4" @@ -4641,10 +4646,13 @@ lower-case@^1.1.1: resolved "https://registry.yarnpkg.com/lower-case/-/lower-case-1.1.4.tgz#9a2cabd1b9e8e0ae993a4bf7d5875c39c42e8eac" integrity sha1-miyr0bno4K6ZOkv31YdcOcQujqw= -lru-cache@2.2.x: - version "2.2.4" - resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.2.4.tgz#6c658619becf14031d0d0b594b16042ce4dc063d" - integrity sha1-bGWGGb7PFAMdDQtZSxYELOTcBj0= +lru-cache@4.1.x: + version "4.1.5" + resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-4.1.5.tgz#8bbe50ea85bed59bc9e33dcab8235ee9bcf443cd" + integrity sha512-sWZlbEP2OsHNkXrMl5GYk/jKk70MBng6UU4YI/qGDYbgf6YbP4EvmqISbXCoJiRKs+1bSpFHVgQxvJ17F2li5g== + dependencies: + pseudomap "^1.0.2" + yallist "^2.1.2" lru-cache@^4.0.1: version "4.1.1" @@ -6277,10 +6285,10 @@ signal-exit@^3.0.0, signal-exit@^3.0.2: resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.2.tgz#b5fdc08f1287ea1178628e415e25132b73646c6d" integrity sha1-tf3AjxKH6hF4Yo5BXiUTK3NkbG0= -sinon-chai@^2.7.0: - version "2.14.0" - resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-2.14.0.tgz#da7dd4cc83cd6a260b67cca0f7a9fdae26a1205d" - integrity sha512-9stIF1utB0ywNHNT7RgiXbdmen8QDCRsrTjw+G9TgKt1Yexjiv8TOWZ6WHsTPz57Yky3DIswZvEqX8fpuHNDtQ== +sinon-chai@^3.0.0: + version "3.3.0" + resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-3.3.0.tgz#8084ff99451064910fbe2c2cb8ab540c00b740ea" + integrity sha512-r2JhDY7gbbmh5z3Q62pNbrjxZdOAjpsqW/8yxAZRSqLZqowmfGZPGUZPFf3UX36NLis0cv8VEM5IJh9HgkSOAA== sinon@^6.1.5: version "6.3.5" @@ -7149,12 +7157,12 @@ use@^3.1.0: dependencies: kind-of "^6.0.2" -useragent@2.2.1: - version "2.2.1" - resolved "https://registry.yarnpkg.com/useragent/-/useragent-2.2.1.tgz#cf593ef4f2d175875e8bb658ea92e18a4fd06d8e" - integrity sha1-z1k+9PLRdYdei7ZY6pLhik/QbY4= +useragent@2.3.0: + version "2.3.0" + resolved "https://registry.yarnpkg.com/useragent/-/useragent-2.3.0.tgz#217f943ad540cb2128658ab23fc960f6a88c9972" + integrity sha512-4AoH4pxuSvHCjqLO04sU6U/uE65BYza8l/KKBS0b0hnUPWi+cQ2BpeTEwejCSx9SPV5/U03nniDTrWx5NrmKdw== dependencies: - lru-cache "2.2.x" + lru-cache "4.1.x" tmp "0.0.x" util-arity@^1.0.2: