From 603872c9dbb9a0809e4d348c18720f20dfde8856 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Fri, 15 May 2015 22:29:57 +0200 Subject: [PATCH] feat: Upgrade to socket.io 1.3 This superseeds #1257 and #1258. Closes #1220. --- client/karma.js | 15 ++++----------- client/main.js | 15 +++++++-------- lib/browser.js | 2 +- lib/config.js | 2 +- lib/launchers/process.js | 4 ++-- lib/server.js | 25 +++++++++++-------------- package.json | 2 +- test/client/karma.spec.js | 10 +++++++--- test/client/mocks.js | 17 ++++++++++++++--- test/unit/browser.spec.coffee | 10 +++++----- test/unit/launchers/process.spec.coffee | 16 ++++++++-------- 11 files changed, 61 insertions(+), 57 deletions(-) diff --git a/client/karma.js b/client/karma.js index b1a5a4058..30070151c 100644 --- a/client/karma.js +++ b/client/karma.js @@ -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; @@ -104,7 +103,6 @@ var Karma = function(socket, iframe, opener, navigator, location) { this.stringify = stringify; - var clearContext = function() { reloadingContext = true; navigateContextTo('about:blank'); @@ -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; }; @@ -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, diff --git a/client/main.js b/client/main.js index 11fba2934..e891f0849 100644 --- a/client/main.js +++ b/client/main.js @@ -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 diff --git a/lib/browser.js b/lib/browser.js index 43b296caa..22aa69fe5 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -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; } diff --git a/lib/config.js b/lib/config.js index 02cd8ec36..4942c86af 100644 --- a/lib/config.js +++ b/lib/config.js @@ -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: [], diff --git a/lib/launchers/process.js b/lib/launchers/process.js index 067cfae87..a4cc61154 100644 --- a/lib/launchers/process.js +++ b/lib/launchers/process.js @@ -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' + diff --git a/lib/server.js b/lib/server.js index 6bdf30b5c..9f6eac6b3 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1,4 +1,4 @@ -var io = require('socket.io'); +var Server = require('socket.io'); var di = require('di'); var cfg = require('./config'); @@ -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); @@ -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)); } }); @@ -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(); - } }); }; @@ -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 }); diff --git a/package.json b/package.json index 6322f2aed..56fef0a76 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/test/client/karma.spec.js b/test/client/karma.spec.js index e866e33c3..c6d3d7516 100644 --- a/test/client/karma.spec.js +++ b/test/client/karma.spec.js @@ -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++) { @@ -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++) { @@ -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']); @@ -178,6 +180,8 @@ describe('Karma', function() { socket.on('start', spyStart); + setTransportTo('websocket'); + k.info({total: 321}); k.result(); expect(log).toEqual(['start', 'result']); @@ -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++) { diff --git a/test/client/mocks.js b/test/client/mocks.js index 67a746d1d..f3909cec9 100644 --- a/test/client/mocks.js +++ b/test/client/mocks.js @@ -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; }; }; diff --git a/test/unit/browser.spec.coffee b/test/unit/browser.spec.coffee index 53771c606..5a70c8a4e 100644 --- a/test/unit/browser.spec.coffee +++ b/test/unit/browser.spec.coffee @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/test/unit/launchers/process.spec.coffee b/test/unit/launchers/process.spec.coffee index bd0254f1d..65edf9a5e 100644 --- a/test/unit/launchers/process.spec.coffee +++ b/test/unit/launchers/process.spec.coffee @@ -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 -> @@ -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 -> @@ -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() @@ -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() @@ -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() @@ -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() @@ -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()