feat: Upgrade to socket.io 1.3

This superseeds #1257 and #1258. Closes #1220.
karma-runner · May 27, 2015 · 603872c · 603872c
1 parent 9c5f817
commit 603872c
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 57 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()