Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Socket io 1.3 #1404

Closed
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ before_script:
- sh -e /etc/init.d/xvfb start
- npm install -g grunt-cli
- rm -rf node_modules/karma
- cd node_modules && ln -nsf ../ karma && cd ../
- export $(openssl aes-256-cbc -pass env:CREDENTIALS_PASS -d -in credentials)

script:
Expand Down
19 changes: 8 additions & 11 deletions client/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ 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;
this.config = {};

// Expose for testing purposes as there is no global socket.io
// registry anymore.
this.socket = socket;

var childWindow = null;
var navigateContextTo = function(url) {
if (self.config.useIframe === false) {
Expand Down Expand Up @@ -104,7 +107,6 @@ var Karma = function(socket, iframe, opener, navigator, location) {

this.stringify = stringify;


var clearContext = function() {
reloadingContext = true;
navigateContextTo('about:blank');
Expand All @@ -114,7 +116,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 +220,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
4 changes: 2 additions & 2 deletions docs/config/01-configuration-file.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,10 @@ on whether all tests passed or any tests failed.
## transports
**Type:** Array

**Default:** `['websocket', 'flashsocket', 'xhr-polling', 'jsonp-polling']`
**Default:** `['polling', 'websocket']`

**Description:** An array of allowed transport methods between the browser and testing server. This configuration setting
is handed off to [socket.io](https://github.com/LearnBoost/Socket.IO/wiki/Configuring-Socket.IO) (which manages the communication
is handed off to [socket.io](http://socket.io/) (which manages the communication
between browsers and the testing server).

## client.useIframe
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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line does not look correct. The error event in this case is coming from the child process, indicating that spawn failed. It is different from the error event emitted by the socket, which was renamed from "error" to "karma_error".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right, thanks a lot fixed in 45a6922

if (err.code === 'ENOENT') {
self._retryLimit = -1;
errorOutput = 'Can not find the binary ' + cmd + '\n\t' +
Expand Down
23 changes: 4 additions & 19 deletions lib/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,6 @@ var log4js = require('log4js');
var helper = require('./helper');
var constant = require('./constants');

// Special Wrapper for Socket.io :(
var LogWrapper = function(name, level) {
this.logger = log4js.getLogger(name);
this.logger.setLevel(level);
};
['error', 'warn', 'info', 'debug'].forEach(function(level) {
LogWrapper.prototype[level] = function() {
this.logger[level].apply(this.logger, arguments);
};
});

// #### Public Functions

// Setup the logger by passing in the configuration options. It needs
Expand Down Expand Up @@ -61,15 +50,11 @@ var setup = function(level, colors, appenders) {
// to be used as a logger for socket.io.
// * `level`, which defaults to the global level.
var create = function(name, level) {
if (name === 'socket.io') {
return new LogWrapper('socket.io', level);
} else {
var logger = log4js.getLogger(name || 'karma');
if (helper.isDefined(level)) {
logger.setLevel(level);
}
return logger;
var logger = log4js.getLogger(name || 'karma');
if (helper.isDefined(level)) {
logger.setLevel(level);
}
return logger;
};


Expand Down
29 changes: 10 additions & 19 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,10 @@ 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,
// 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,
logger: logger.create('socket.io', constant.LOG_ERROR),
resource: config.urlRoot + 'socket.io',
destroyUpgrade: false,
path: config.urlRoot + 'socket.io/',
transports: config.transports
});

Expand Down
6 changes: 4 additions & 2 deletions 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 Expand Up @@ -199,6 +199,8 @@
"grunt-jscs": "^1.8.0",
"grunt-mocha-test": "^0.12.7",
"grunt-npm": "0.0.2",
"grunt-simple-mocha": "*",
"karma-browserify": "^4.1.2",
"karma-browserstack-launcher": "*",
"karma-chrome-launcher": "*",
"karma-coffee-preprocessor": "*",
Expand All @@ -207,7 +209,7 @@
"karma-firefox-launcher": "*",
"karma-growl-reporter": "*",
"karma-html2js-preprocessor": "*",
"karma-jasmine": "^0.1.5",
"karma-jasmine": "~0.3.5",
"karma-junit-reporter": "*",
"karma-live-preprocessor": "*",
"karma-mocha": "*",
Expand Down
11 changes: 4 additions & 7 deletions test/client/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,19 @@ module.exports = function(config) {
// base path, that will be used to resolve files and exclude
basePath: '../..',

frameworks: ['jasmine', 'commonjs'],
frameworks: ['browserify', 'mocha'],

// list of files / patterns to load in the browser
files: [
'client/*.js',
'test/client/*.js'
],

// list of files to exclude
exclude: [
'client/main.js'
],

preprocessors: {
'client/*.js': ['commonjs'],
'test/client/*.js': ['commonjs']
'test/client/*.js': ['browserify']
},

// use dots reporter, as travis terminal does not support escaping sequences
Expand Down Expand Up @@ -72,11 +69,11 @@ module.exports = function(config) {
reportSlowerThan: 500,

plugins: [
'karma-jasmine',
'karma-mocha',
'karma-chrome-launcher',
'karma-firefox-launcher',
'karma-junit-reporter',
'karma-commonjs'
'karma-browserify'
]
});
};
Loading