feat: allow browser to reconnect during the test run

vojtajina · vojtajina · commit cbe2851baa55 · 2013-08-21T01:57:33.000-07:00
When a browser disconnects during a test run, Karma waits for reconnecting (configurable by `browserDisconnectTimeout`). If the browser reconnects in this timeout frame, nothing happpens - Karma replies the events (results) and the test run continues. If the browser does not reconnect in the timeout frame, Karma fails the build. This should solve the connection issues with IE on polling. - add browserDisconnectTimeout config property (defaults to 2000) Internal changes: - `Browser.isReady` is a function now, as browser has multiple states - `BrowserCollection.setAllIsReadyTo` -> `setAllToExecuting` - remove `Browser.launchId`, we use `Browser.id` instead Closes #82 Closes #590
diff --git a/lib/browser.js b/lib/browser.js
@@ -16,33 +16,57 @@ var Result = function() {
 };
 
 
-var Browser = function(id, collection, emitter) {
-  var log = logger.create(id);
+// The browser is ready to execute tests.
+var READY = 1;
+
+// The browser is executing the tests/
+var EXECUTING = 2;
+
+// The browser is not executing, but temporarily disconnected (waiting for reconnecting).
+var READY_DISCONNECTED = 3;
+
+// The browser is executing the tests, but temporarily disconnect (waiting for reconnecting).
+var EXECUTING_DISCONNECTED = 4;
+
+// The browser got permanently disconnected (being removed from the collection and destroyed).
+var DISCONNECTED = 5;
+
+
+var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter, socket, timer,
+    /* config.browserDisconnectTimeout */ disconnectDelay) {
+
+  var name = helper.browserFullNameToShort(fullName);
+  var log = logger.create(name);
 
   this.id = id;
-  this.name = id;
-  this.fullName = null;
-  this.isReady = true;
+  this.fullName = fullName;
+  this.name = name;
+  this.state = READY;
   this.lastResult = new Result();
 
+  this.init = function() {
+    collection.add(this);
 
-  this.toString = function() {
-    return this.name;
-  };
+    events.bindAll(this, socket);
 
-  this.onRegister = function(info) {
-    this.launchId = info.id;
-    this.fullName = info.name;
-    this.name = helper.browserFullNameToShort(this.fullName);
-    log = logger.create(this.name);
-    log.info('Connected on socket id ' + this.id);
+    log.info('Connected on socket %s', socket.id);
 
-    emitter.emit('browser_register', this);
+    // TODO(vojta): move to collection
     emitter.emit('browsers_change', collection);
+
+    emitter.emit('browser_register', this);
+  };
+
+  this.isReady = function() {
+    return this.state === READY;
+  };
+
+  this.toString = function() {
+    return this.name;
   };
 
   this.onError = function(error) {
-    if (this.isReady) {
+    if (this.isReady()) {
       return;
     }
 
@@ -51,7 +75,7 @@ var Browser = function(id, collection, emitter) {
   };
 
   this.onInfo = function(info) {
-    if (this.isReady) {
+    if (this.isReady()) {
       return;
     }
 
@@ -70,11 +94,11 @@ var Browser = function(id, collection, emitter) {
   };
 
   this.onComplete = function(result) {
-    if (this.isReady) {
+    if (this.isReady()) {
       return;
     }
 
-    this.isReady = true;
+    this.state = READY;
     this.lastResult.totalTimeEnd();
 
     if (!this.lastResult.success) {
@@ -85,21 +109,50 @@ var Browser = function(id, collection, emitter) {
     emitter.emit('browser_complete', this, result);
   };
 
+  var self = this;
+  var disconnect = function() {
+    self.state = DISCONNECTED;
+    log.warn('Disconnected');
+    collection.remove(self);
+  };
+
+  var pendingDisconnect;
   this.onDisconnect = function() {
-    if (!this.isReady) {
-      this.isReady = true;
-      this.lastResult.totalTimeEnd();
-      this.lastResult.disconnected = true;
-      emitter.emit('browser_complete', this);
+    if (this.state === READY) {
+      disconnect();
+    } else if (this.state === EXECUTING) {
+      log.debug('Disconnected during run, waiting for reconnecting.');
+      this.state = EXECUTING_DISCONNECTED;
+
+      pendingDisconnect = timer.setTimeout(function() {
+        self.lastResult.totalTimeEnd();
+        self.lastResult.disconnected = true;
+        disconnect();
+        emitter.emit('browser_complete', self);
+      }, disconnectDelay);
     }
+  };
 
-    log.warn('Disconnected');
-    collection.remove(this);
+  this.onReconnect = function(newSocket) {
+    if (this.state === EXECUTING_DISCONNECTED) {
+      this.state = EXECUTING;
+      log.debug('Reconnected.');
+    } else if (this.state === EXECUTING || this.state === READY) {
+      log.debug('New connection, forgetting the old one.');
+      // TODO(vojta): this should only remove this browser.onDisconnect listener
+      socket.removeAllListeners('disconnect');
+    }
+
+    socket = newSocket;
+    events.bindAll(this, newSocket);
+    if (pendingDisconnect) {
+      timer.clearTimeout(pendingDisconnect);
+    }
   };
 
   this.onResult = function(result) {
     // ignore - probably results from last run (after server disconnecting)
-    if (this.isReady) {
+    if (this.isReady()) {
       return;
     }
 
@@ -119,11 +172,17 @@ var Browser = function(id, collection, emitter) {
     return {
       id: this.id,
       name: this.name,
-      isReady: this.isReady
+      isReady: this.state === READY
     };
   };
 };
 
+Browser.STATE_READY = READY;
+Browser.STATE_EXECUTING = EXECUTING;
+Browser.STATE_READY_DISCONNECTED = READY_DISCONNECTED;
+Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED;
+Browser.STATE_DISCONNECTED = DISCONNECTED;
+
 
 var Collection = function(emitter, browsers) {
   browsers = browsers || [];
@@ -146,23 +205,29 @@ var Collection = function(emitter, browsers) {
     return true;
   };
 
-  this.setAllIsReadyTo = function(value) {
-    var change = false;
+  this.getById = function(browserId) {
+    for (var i = 0; i < browsers.length; i++) {
+      if (browsers[i].id === browserId) {
+        return browsers[i];
+      }
+    }
+
+    return null;
+  };
+
+  this.setAllToExecuting = function() {
     browsers.forEach(function(browser) {
-      change = change || browser.isReady !== value;
-      browser.isReady = value;
+      browser.state = EXECUTING;
     });
 
-    if (change) {
-      emitter.emit('browsers_change', this);
-    }
+    emitter.emit('browsers_change', this);
   };
 
   this.areAllReady = function(nonReadyList) {
     nonReadyList = nonReadyList || [];
 
     browsers.forEach(function(browser) {
-      if (!browser.isReady) {
+      if (!browser.isReady()) {
         nonReadyList.push(browser);
       }
     });
@@ -222,12 +287,3 @@ Collection.$inject = ['emitter'];
 exports.Result = Result;
 exports.Browser = Browser;
 exports.Collection = Collection;
-
-exports.createBrowser = function(socket, collection, emitter) {
-  var browser = new Browser(socket.id, collection, emitter);
-
-  events.bindAll(browser, socket);
-  collection.add(browser);
-
-  return browser;
-};
diff --git a/lib/config.js b/lib/config.js
@@ -271,6 +271,7 @@ var Config = function() {
   this.client = {
     args: []
   };
+  this.browserDisconnectTimeout = 2000;
 
   // TODO(vojta): remove in 0.10
   this.junitReporter = {
diff --git a/lib/events.js b/lib/events.js
@@ -15,6 +15,41 @@ var bindAllEvents = function(object, context) {
   }
 };
 
+
+var bufferEvents = function(emitter, eventsToBuffer) {
+  var listeners = [];
+  var eventsToReply = [];
+  var genericListener = function() {
+    eventsToReply.push(Array.prototype.slice.call(arguments));
+  };
+
+  eventsToBuffer.forEach(function(eventName) {
+    var listener = genericListener.bind(null, eventName);
+    listeners.push(listener);
+    emitter.on(eventName, listener);
+  });
+
+  return function() {
+    if (!eventsToReply) {
+      return;
+    }
+
+    // remove all buffering listeners
+    listeners.forEach(function(listener, i) {
+      emitter.removeListener(eventsToBuffer[i], listener);
+    });
+
+    // reply
+    eventsToReply.forEach(function(args) {
+      events.EventEmitter.prototype.emit.apply(emitter, args);
+    });
+
+    // free-up
+    listeners = eventsToReply = null;
+  };
+};
+
+
 // TODO(vojta): log.debug all events
 var EventEmitter = function() {
   this.bind = bindAllEvents;
@@ -38,6 +73,8 @@ var EventEmitter = function() {
 
 util.inherits(EventEmitter, events.EventEmitter);
 
+
 // PUBLISH
 exports.EventEmitter = EventEmitter;
 exports.bindAll = bindAllEvents;
+exports.bufferEvents = bufferEvents;
diff --git a/lib/executor.js b/lib/executor.js
@@ -18,8 +18,8 @@ var Executor = function(capturedBrowsers, config, emitter) {
     if (capturedBrowsers.areAllReady(nonReady)) {
       log.debug('All browsers are ready, executing');
       executionScheduled = false;
-      capturedBrowsers.setAllIsReadyTo(false);
       capturedBrowsers.clearResults();
+      capturedBrowsers.setAllToExecuting();
       pendingCount = capturedBrowsers.length;
       runningBrowsers = capturedBrowsers.clone();
       emitter.emit('run_start', runningBrowsers);
diff --git a/lib/server.js b/lib/server.js
@@ -14,7 +14,8 @@ var Launcher = require('./launcher').Launcher;
 var FileList = require('./file-list').List;
 var reporter = require('./reporter');
 var helper = require('./helper');
-var EventEmitter = require('./events').EventEmitter;
+var events = require('./events');
+var EventEmitter = events.EventEmitter;
 var Executor = require('./executor');
 
 var log = logger.create();
@@ -60,8 +61,8 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   });
 
   globalEmitter.on('browser_register', function(browser) {
-    if (browser.launchId) {
-      launcher.markCaptured(browser.launchId);
+    if (browser.id) {
+      launcher.markCaptured(browser.id);
     }
 
     // TODO(vojta): This is lame, browser can get captured and then crash (before other browsers get
@@ -78,8 +79,31 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   });
 
   socketServer.sockets.on('connection', function (socket) {
-    log.debug('New browser has connected on socket ' + socket.id);
-    browser.createBrowser(socket, capturedBrowsers, globalEmitter);
+    log.debug('A browser has connected on socket ' + socket.id);
+
+    var replySocketEvents = events.bufferEvents(socket, ['info', 'error', 'result', 'complete']);
+
+    socket.on('register', function(info) {
+      var newBrowser;
+
+      if (info.id) {
+        newBrowser = capturedBrowsers.getById(info.id);
+      }
+
+      if (newBrowser) {
+        newBrowser.onReconnect(socket);
+      } else {
+        newBrowser = injector.createChild([{
+          id: ['value', info.id || null],
+          fullName: ['value', info.name],
+          socket: ['value', socket]
+        }]).instantiate(browser.Browser);
+
+        newBrowser.init();
+      }
+
+      replySocketEvents();
+    });
   });
 
   if (config.autoWatch) {
@@ -167,7 +191,8 @@ exports.start = function(cliOptions, done) {
     customScriptTypes: ['value', []],
     reporter: ['factory', reporter.createReporters],
     capturedBrowsers: ['type', browser.Collection],
-    args: ['value', {}]
+    args: ['value', {}],
+    timer: ['value', {setTimeout: setTimeout, clearTimeout: clearTimeout}]
   }];
 
   // load the plugins
diff --git a/static/karma.src.js b/static/karma.src.js
@@ -250,9 +250,6 @@ var Karma = function(socket, context, navigator, location) {
     }
   });
 
-  // cancel execution
-  socket.on('disconnect', clearContext);
-
   // report browser name, id
   socket.on('connect', function() {
     socket.emit('register', {
diff --git a/test/unit/browser.spec.coffee b/test/unit/browser.spec.coffee
diff --git a/test/unit/events.spec.coffee b/test/unit/events.spec.coffee

Original file line number	Diff line number	Diff line change
`@@ -250,9 +250,6 @@ var Karma = function(socket, context, navigator, location) {`
`250`	`250`	`}`
`251`	`251`	`});`
`252`	`252`
`253`		`- // cancel execution`
`254`		`- socket.on('disconnect', clearContext);`
`255`		`-`
`256`	`253`	`// report browser name, id`
`257`	`254`	`socket.on('connect', function() {`
`258`	`255`	`socket.emit('register', {`