Skip to content

Commit

Permalink
feat: allow browser to reconnect during the test run
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@vojtajina
vojtajina committed Aug 21, 2013
1 parent 09866f9 commit cbe2851
Show file tree
Hide file tree
Showing 8 changed files with 424 additions and 127 deletions.
146 changes: 101 additions & 45 deletions lib/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -51,7 +75,7 @@ var Browser = function(id, collection, emitter) {
};

this.onInfo = function(info) {
if (this.isReady) {
if (this.isReady()) {
return;
}

Expand All @@ -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) {
Expand All @@ -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;
}

Expand All @@ -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 || [];
Expand All @@ -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);
}
});
Expand Down Expand Up @@ -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;
};
1 change: 1 addition & 0 deletions lib/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ var Config = function() {
this.client = {
args: []
};
this.browserDisconnectTimeout = 2000;

// TODO(vojta): remove in 0.10
this.junitReporter = {
Expand Down
37 changes: 37 additions & 0 deletions lib/events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,6 +73,8 @@ var EventEmitter = function() {

util.inherits(EventEmitter, events.EventEmitter);


// PUBLISH
exports.EventEmitter = EventEmitter;
exports.bindAll = bindAllEvents;
exports.bufferEvents = bufferEvents;
2 changes: 1 addition & 1 deletion lib/executor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
37 changes: 31 additions & 6 deletions lib/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions static/karma.src.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Loading

0 comments on commit cbe2851

Please sign in to comment.