fix: restarted browsers not running tests

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

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)
     }
 

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 {

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()

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', () => {

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,13 +11,15 @@ describe('server', () => {
   let fileListOnReject
   let mockLauncher
   let mockWebServer
+  let mockServerSocket
   let mockSocketServer
   let mockBoundServer
   let mockExecutor
   let doneSpy
   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,14 +70,21 @@ describe('server', () => {
       kill: () => true
     }
 
+    mockServerSocket = {
+      id: 'socket-id',
+      on: (name, handler) => mockSocketEventListeners.set(name, handler),
+      emit: () => {},
+      removeListener: () => {}
+    }
+
     mockSocketServer = {
       close: () => {},
       flashPolicyServer: {
         close: () => {}
       },
       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)
+    })
+  })
 })

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: