Skip to content

Commit

Permalink
feat: Upgrade to socket.io 1.3
Browse files Browse the repository at this point in the history
This superseeds #1257 and #1258.
Closes #1220.
  • Loading branch information
dignifiedquire committed May 27, 2015
1 parent 9c5f817 commit 603872c
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 57 deletions.
15 changes: 4 additions & 11 deletions client/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ var Karma = function(socket, iframe, opener, navigator, location) {
var queryParams = util.parseQueryParams(location.search);
var browserId = queryParams.id || util.generateId('manual-');
var returnUrl = queryParams['return_url' + ''] || null;
var currentTransport;

var resultsBufferLimit = 1;
var resultsBufferLimit = 50;
var resultsBuffer = [];

this.VERSION = constant.VERSION;
Expand Down Expand Up @@ -104,7 +103,6 @@ var Karma = function(socket, iframe, opener, navigator, location) {

this.stringify = stringify;


var clearContext = function() {
reloadingContext = true;
navigateContextTo('about:blank');
Expand All @@ -114,7 +112,7 @@ var Karma = function(socket, iframe, opener, navigator, location) {
// we are not going to execute at all
this.error = function(msg, url, line) {
hasError = true;
socket.emit('error', url ? msg + '\nat ' + url + (line ? ':' + line : '') : msg);
socket.emit('karma_error', url ? msg + '\nat ' + url + (line ? ':' + line : '') : msg);
this.complete();
return false;
};
Expand Down Expand Up @@ -218,14 +216,9 @@ var Karma = function(socket, iframe, opener, navigator, location) {

// report browser name, id
socket.on('connect', function() {
currentTransport = socket.socket.transport.name;

// TODO(vojta): make resultsBufferLimit configurable
if (currentTransport === 'websocket' || currentTransport === 'flashsocket') {
socket.io.engine.on('upgrade', function() {
resultsBufferLimit = 1;
} else {
resultsBufferLimit = 50;
}
});

socket.emit('register', {
name: navigator.userAgent,
Expand Down
15 changes: 7 additions & 8 deletions client/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ var util = require('./util');
var KARMA_URL_ROOT = require('./constants').KARMA_URL_ROOT;


// connect socket.io
// https://github.com/LearnBoost/Socket.IO/wiki/Configuring-Socket.IO
var socket = io.connect('http://' + location.host, {
'reconnection delay': 500,
'reconnection limit': 2000,
'resource': KARMA_URL_ROOT.substr(1) + 'socket.io',
'sync disconnect on unload': true,
'max reconnection attempts': Infinity
// Connect to the server using socket.io http://socket.io
var socket = io('http://' + location.host, {
reconnectionDelay: 500,
reconnectionDelayMax: Infinity,
timeout: 2000,
path: '/' + KARMA_URL_ROOT.substr(1) + 'socket.io',
'sync disconnect on unload': true
});

// instantiate the updater of the view
Expand Down
2 changes: 1 addition & 1 deletion lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter,
return this.name;
};

this.onError = function(error) {
this.onKarmaError = function(error) {
if (this.isReady()) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ var Config = function() {
this.urlRoot = '/';
this.reportSlowerThan = 0;
this.loggers = [constant.CONSOLE_APPENDER];
this.transports = ['websocket', 'flashsocket', 'xhr-polling', 'jsonp-polling'];
this.transports = ['polling', 'websocket'];
this.plugins = ['karma-*'];
this.defaultClient = this.client = {
args: [],
Expand Down
4 changes: 2 additions & 2 deletions lib/launchers/process.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ var ProcessLauncher = function(spawn, tempDir, timer) {

var errorOutput = '';

self._process.on('close', function(code) {
self._process.on('exit', function(code) {
self._onProcessExit(code, errorOutput);
});

self._process.on('error', function(err) {
self._process.on('karma_error', function(err) {
if (err.code === 'ENOENT') {
self._retryLimit = -1;
errorOutput = 'Can not find the binary ' + cmd + '\n\t' +
Expand Down
25 changes: 11 additions & 14 deletions lib/server.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var io = require('socket.io');
var Server = require('socket.io');
var di = require('di');

var cfg = require('./config');
Expand Down Expand Up @@ -87,7 +87,7 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
}
});

var EVENTS_TO_REPLY = ['start', 'info', 'error', 'result', 'complete'];
var EVENTS_TO_REPLY = ['start', 'info', 'karma_error', 'result', 'complete'];
socketServer.sockets.on('connection', function(socket) {
log.debug('A browser has connected on socket ' + socket.id);

Expand Down Expand Up @@ -200,11 +200,13 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
// to suppress "browser disconnect" warnings
// TODO(vojta): change the client to not send the event (if disconnected by purpose)
var sockets = socketServer.sockets.sockets;
Object.getOwnPropertyNames(sockets).forEach(function(key) {
var socket = sockets[key];

sockets.forEach(function(socket) {
socket.removeAllListeners('disconnect');
if (!socket.disconnected) {
socket.disconnect();
// Disconnect asynchronously. Socket.io mutates the `sockets.sockets` array
// underneath us so this would skip every other browser/socket.
process.nextTick(socket.disconnect.bind(socket));
}
});

Expand All @@ -230,11 +232,6 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
clearTimeout(closeTimeout);
removeAllListeners();
});

// shutdown socket.io flash transport, if defined
if (socketServer.flashPolicyServer) {
socketServer.flashPolicyServer.close();
}
});
};

Expand All @@ -258,16 +255,16 @@ start.$inject = ['injector', 'config', 'launcher', 'emitter', 'preprocess', 'fil


var createSocketIoServer = function(webServer, executor, config) {
var server = io.listen(webServer, {
var server = new Server(webServer, {
// avoid destroying http upgrades from socket.io to get proxied websockets working
'destroy upgrade': false,
destroyUpgrade: false,
// socket.io has a timeout (15s by default) before destroying a store (a data structure where
// data associated with a socket are stored). Unfortunately this timeout is not cleared
// properly on socket.io shutdown and this timeout prevents karma from exiting cleanly.
// We change this timeout to 0 to make Karma exit just after all tests were executed.
'client store expiration': 0,
//'client store expiration': 0,
logger: logger.create('socket.io', constant.LOG_ERROR),
resource: config.urlRoot + 'socket.io',
path: config.urlRoot + 'socket.io/',
transports: config.transports
});

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@
],
"dependencies": {
"di": "~0.0.1",
"socket.io": "0.9.16",
"socket.io": "~1.3.5",
"chokidar": ">=0.8.2",
"glob": "~3.2.7",
"minimatch": "~0.2",
Expand Down
10 changes: 7 additions & 3 deletions test/client/karma.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('Karma', function() {


it('should buffer results when polling', function() {
setTransportTo('xhr-polling');
setTransportTo('polling');

// emit 49 results
for (var i = 1; i < 50; i++) {
Expand All @@ -138,7 +138,7 @@ describe('Karma', function() {


it('should buffer results when polling', function() {
setTransportTo('xhr-polling');
setTransportTo('polling');

// emit 40 results
for (var i = 1; i <= 40; i++) {
Expand All @@ -161,6 +161,8 @@ describe('Karma', function() {
log.push('start');
});

setTransportTo('websocket');

// adapter didn't call info({total: x})
k.result();
expect(log).toEqual(['start', 'result']);
Expand All @@ -178,6 +180,8 @@ describe('Karma', function() {

socket.on('start', spyStart);

setTransportTo('websocket');

k.info({total: 321});
k.result();
expect(log).toEqual(['start', 'result']);
Expand Down Expand Up @@ -237,7 +241,7 @@ describe('Karma', function() {
var spyResult = jasmine.createSpy('onResult');
socket.on('result', spyResult);

setTransportTo('xhr-polling');
setTransportTo('polling');

// emit 40 results
for (var i = 0; i < 40; i++) {
Expand Down
17 changes: 14 additions & 3 deletions test/client/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,26 @@ var Emitter = function() {
var MockSocket = function() {
Emitter.call(this);

this.socket = {transport: {name: 'websocket'}};
var transportName = 'websocket';

this.io = {
engine: {
on: function(event, cb) {
if (event === 'upgrade' && transportName === 'websocket') {
//setTimeout(cb, 0);
cb();
}
}
}
};

this.disconnect = function() {
this.emit('disconnect');
};

// MOCK API
this._setTransportNameTo = function(transportName) {
this.socket.transport.name = transportName;
this._setTransportNameTo = function(name) {
transportName = name;
};
};

Expand Down
10 changes: 5 additions & 5 deletions test/unit/browser.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ describe 'Browser', ->


#==========================================================================
# Browser.onError
# Browser.onKarmaError
#==========================================================================
describe 'onError', ->
describe 'onKarmaError', ->

beforeEach ->
browser = new Browser 'fake-id', 'full name', collection, emitter, socket
Expand All @@ -79,7 +79,7 @@ describe 'Browser', ->
emitter.on 'browser_error', spy
browser.state = Browser.STATE_EXECUTING

browser.onError()
browser.onKarmaError()
expect(browser.lastResult.error).to.equal true
expect(spy).to.have.been.called

Expand All @@ -89,7 +89,7 @@ describe 'Browser', ->
emitter.on 'browser_error', spy
browser.state = Browser.STATE_READY

browser.onError()
browser.onKarmaError()
expect(browser.lastResult.error).to.equal false
expect(spy).not.to.have.been.called

Expand Down Expand Up @@ -275,7 +275,7 @@ describe 'Browser', ->
socket.emit 'result', {success: true}
expect(browser.lastResult.success).to.equal 1

socket.emit 'error', {}
socket.emit 'karma_error', {}
expect(browser.lastResult.error).to.equal true

# should be ignored, keep executing
Expand Down
16 changes: 8 additions & 8 deletions test/unit/launchers/process.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ describe 'launchers/process.js', ->
emitter.on 'browser_process_failure', failureSpy

launcher.start 'http://host:9876/'
mockSpawn._processes[0].emit 'error', {code: 'ENOENT'}
mockSpawn._processes[0].emit 'close', 1
mockSpawn._processes[0].emit 'karma_error', {code: 'ENOENT'}
mockSpawn._processes[0].emit 'exit', 1
mockTempDir.remove.callArg 1

scheduleNextTick ->
Expand Down Expand Up @@ -111,7 +111,7 @@ describe 'launchers/process.js', ->
expect(mockSpawn._processes[0].kill).to.have.been.called

# process exits
mockSpawn._processes[0].emit 'close', 0
mockSpawn._processes[0].emit 'exit', 0
mockTempDir.remove.callArg 1

killingLauncher.done ->
Expand All @@ -133,7 +133,7 @@ describe 'launchers/process.js', ->

# expect killing browser
expect(browserProcess.kill).to.have.been.called
browserProcess.emit 'close', 0
browserProcess.emit 'exit', 0
mockTempDir.remove.callArg 1
mockSpawn.reset()

Expand All @@ -158,7 +158,7 @@ describe 'launchers/process.js', ->

# expect killing browser
expect(browserProcess.kill).to.have.been.called
browserProcess.emit 'close', 0
browserProcess.emit 'exit', 0
mockTempDir.remove.callArg 1
mockTempDir.remove.reset()

Expand All @@ -174,7 +174,7 @@ describe 'launchers/process.js', ->

# expect killing browser
expect(browserProcess.kill).to.have.been.called
browserProcess.emit 'close', 0
browserProcess.emit 'exit', 0
mockTempDir.remove.callArg 1
mockTempDir.remove.reset()

Expand All @@ -190,7 +190,7 @@ describe 'launchers/process.js', ->

# expect killing browser
expect(browserProcess.kill).to.have.been.called
browserProcess.emit 'close', 0
browserProcess.emit 'exit', 0
mockTempDir.remove.callArg 1
mockTempDir.remove.reset()

Expand All @@ -211,7 +211,7 @@ describe 'launchers/process.js', ->
mockSpawn.reset()

# crash
browserProcess.emit 'close', 1
browserProcess.emit 'exit', 1
mockTempDir.remove.callArg 1
mockTempDir.remove.reset()

Expand Down

0 comments on commit 603872c

Please sign in to comment.