refactor(runner): remove runnerPort

This changes the runner to communicate through HTTP. BREAKING CHANGE: `runnerPort` is merged with `port` if you are using `karma run` with custom `--runer-port`, please change that to `--port`.
karma-runner · Jul 25, 2013 · ca4c4d8 · ca4c4d8
1 parent 0e284a9
commit ca4c4d8
Show file tree

Hide file tree

Showing 7 changed files with 175 additions and 88 deletions.
diff --git a/lib/config.js b/lib/config.js
@@ -268,6 +268,9 @@ var Config = function() {
   this.loggers = [constant.CONSOLE_APPENDER];
   this.transports = ['websocket', 'flashsocket', 'xhr-polling', 'jsonp-polling'];
   this.plugins = ['karma-*'];
+  this.client = {
+    args: []
+  };
 
   // TODO(vojta): remove in 0.10
   this.junitReporter = {

diff --git a/lib/middleware/runner.js b/lib/middleware/runner.js
@@ -0,0 +1,58 @@
+/**
+ * Runner middleware is reponsible for communication with `karma run`.
+ *
+ * It basically triggers a test run and streams stdout back.
+ */
+
+var log = require('../logger').create();
+var constant = require('../constants');
+var json = require('connect').json();
+
+var createRunnerMiddleware = function(emitter, fileList, capturedBrowsers, reporter,
+    /* config.hostname */ hostname, /* config.port */ port, /* config.urlRoot */ urlRoot, config) {
+
+  return function(request, response, next) {
+
+    if (request.url !== '/__run__' && request.url !== urlRoot + 'run') {
+      return next();
+    }
+
+    log.debug('Execution (fired by runner)');
+    response.writeHead(200);
+
+    if (!capturedBrowsers.length) {
+      var url = 'http://' + hostname + ':' + port + urlRoot;
+
+      return response.end('No captured browser, open ' + url + '\n');
+    }
+
+    json(request, response, function() {
+      if (!capturedBrowsers.areAllReady([])) {
+        response.write('Waiting for previous execution...\n');
+      }
+
+      emitter.once('run_start', function() {
+        var responseWrite = response.write.bind(response);
+
+        reporter.addAdapter(responseWrite);
+
+        // clean up, close runner response
+        emitter.once('run_complete', function(browsers, results) {
+          reporter.removeAdapter(responseWrite);
+          response.end(constant.EXIT_CODE + results.exitCode);
+        });
+      });
+
+      var clientArgs = request.body.args;
+      log.debug('Setting client.args to ', clientArgs);
+      config.client.args = clientArgs;
+
+      log.debug('Refreshing all the files / patterns');
+      fileList.refresh();
+    });
+  };
+};
+
+
+// PUBLIC API
+exports.create = createRunnerMiddleware;
diff --git a/lib/runner.js b/lib/runner.js
@@ -1,4 +1,4 @@
-var net = require('net');
+var http = require('http');
 
 var constant = require('./constants');
 var helper = require('./helper');
@@ -22,34 +22,41 @@ var parseExitCode = function(buffer, defaultCode) {
 };
 
 
+// TODO(vojta): read config file (port, host, urlRoot)
 exports.run = function(config, done) {
-  var port = config.runnerPort || constant.DEFAULT_RUNNER_PORT;
-  var socket = net.connect(port);
+  done = helper.isFunction(done) ? done : process.exit;
+
   var exitCode = 1;
+  var options = {
+    hostname: 'localhost',
+    path: '/__run__',
+    // TODO(vojta): remove runnerPort in 0.11
+    port: config.port || config.runnerPort || constant.DEFAULT_PORT,
+    method: 'POST',
+    headers: {
+      'Content-Type': 'application/json'
+    }
+  };
 
-  // Make done callback optional so it's backwards compatible
-  if (! helper.isFunction(done)) {
-    done = process.exit;
-  }
+  var request = http.request(options, function(response) {
+    response.on('data', function(buffer) {
+      exitCode = parseExitCode(buffer, exitCode);
+      process.stdout.write(buffer);
+    });
 
-  // TODO(vojta): error when no Karma listening on this port
-  socket.on('data', function(buffer) {
-    exitCode = parseExitCode(buffer, exitCode);
-    process.stdout.write(buffer);
+    response.on('end', function() {
+      done(exitCode);
+    });
   });
 
-  socket.on('error', function(e) {
+  request.on('error', function(e) {
     if (e.code === 'ECONNREFUSED') {
-      console.error('There is no server listening on port %d', port);
+      console.error('There is no server listening on port %d', options.port);
       done(1);
     } else {
       throw e;
     }
   });
 
-  socket.on('close', function() {
-    done(exitCode);
-  });
-
-  socket.write(JSON.stringify({args: config.clientArgs}) + '\0');
+  request.end(JSON.stringify({args: config.clientArgs}));
 };
diff --git a/lib/server.js b/lib/server.js
@@ -1,5 +1,4 @@
 var io = require('socket.io');
-var net = require('net');
 var di = require('di');
 
 var cfg = require('./config');
@@ -22,7 +21,7 @@ var log = logger.create();
 
 // TODO(vojta): get this whole mess under test
 var start = function(injector, config, launcher, globalEmitter, preprocess, fileList, webServer,
-    resultReporter, capturedBrowsers, done) {
+    capturedBrowsers, done) {
 
   config.frameworks.forEach(function(framework) {
     injector.get('framework:' + framework);
@@ -67,7 +66,6 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   var executionScheduled = false;
   var pendingCount = 0;
   var runningBrowsers;
-  var clientConfig = {args: config.clientArgs};
 
   globalEmitter.on('browsers_change', function() {
     // TODO(vojta): send only to interested browsers
@@ -101,7 +99,7 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
       pendingCount = capturedBrowsers.length;
       runningBrowsers = capturedBrowsers.clone();
       globalEmitter.emit('run_start', runningBrowsers);
-      socketServer.sockets.emit('execute', clientConfig);
+      socketServer.sockets.emit('execute', config.client);
       return true;
     } else {
       log.info('Delaying execution, these browsers are not ready: ' + nonReady.join(', '));
@@ -137,70 +135,6 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
     tryExecution();
   });
 
-
-  // listen on port, waiting for runner
-  var runnerServer = net.createServer(function (socket) {
-    var buf = '';
-    socket.on('data', function(data) {
-      buf += data;
-
-      // data is followed by a NUL byte, so keep buffering until that's present
-      if (buf[buf.length - 1] !== '\0') {
-        return;
-      }
-
-      // strip the NUL byte and parse
-      clientConfig = JSON.parse(buf.substr(0, buf.length - 1));
-      buf = '';
-      log.debug('Execution (fired by runner)');
-
-      if (!capturedBrowsers.length) {
-        var url = 'http://' + config.hostname + ':' + config.port + config.urlRoot;
-
-        log.warn('No captured browser, open ' + url);
-        socket.end('No captured browser, open ' + url + '\n');
-        return;
-      }
-
-      if (!capturedBrowsers.areAllReady([])) {
-        socket.write('Waiting for previous execution...\n');
-      }
-
-      globalEmitter.once('run_start', function() {
-        var socketWrite = socket.write.bind(socket);
-
-        resultReporter.addAdapter(socketWrite);
-
-        // clean up, close runner socket
-        globalEmitter.once('run_complete', function(browsers, results) {
-          resultReporter.removeAdapter(socketWrite);
-          socket.end(constant.EXIT_CODE + results.exitCode);
-        });
-      });
-
-      log.debug('Refreshing all the files / patterns');
-      fileList.refresh();
-    });
-  });
-
-  runnerServer.on('error', function(e) {
-    if (e.code === 'EADDRINUSE') {
-      log.warn('Port %d in use', config.runnerPort);
-      config.runnerPort++;
-      runnerServer.listen(config.runnerPort);
-    } else {
-      throw e;
-    }
-  });
-
-  runnerServer.listen(config.runnerPort);
-
-  runnerServer.on('listening', function() {
-    if (config.runnerPort !== constant.DEFAULT_RUNNER_PORT) {
-      log.info('To run via this server, use "karma run --runner-port %d"', config.runnerPort);
-    }
-  });
-
   var disconnectBrowsers = function(code) {
     // Slightly hacky way of removing disconnect listeners
     // to suppress "browser disconnect" warnings
@@ -239,7 +173,7 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   });
 };
 start.$inject = ['injector', 'config', 'launcher', 'emitter', 'preprocess', 'fileList',
-    'webServer', 'reporter', 'capturedBrowsers', 'done'];
+    'webServer', 'capturedBrowsers', 'done'];
 
 
 exports.start = function(cliOptions, done) {

diff --git a/lib/web-server.js b/lib/web-server.js
@@ -4,6 +4,7 @@ var path = require('path');
 var connect = require('connect');
 
 var common = require('./middleware/common');
+var runnerMiddleware = require('./middleware/runner');
 var karmaMiddleware = require('./middleware/karma');
 var sourceFilesMiddleware = require('./middleware/source-files');
 var proxyMiddleware = require('./middleware/proxy');
@@ -45,6 +46,7 @@ var createWebServer = function(injector) {
 
   var handler = connect()
       .use(connect.compress(compressOptions))
+      .use(injector.invoke(runnerMiddleware.create))
       .use(injector.invoke(karmaMiddleware.create))
       .use(injector.invoke(sourceFilesMiddleware.create))
       // TODO(vojta): extract the proxy into a plugin

diff --git a/test/unit/middleware/runner.spec.coffee b/test/unit/middleware/runner.spec.coffee
@@ -0,0 +1,79 @@
+describe 'middleware.runner', ->
+
+  mocks = require 'mocks'
+  HttpResponseMock = mocks.http.ServerResponse
+  HttpRequestMock = mocks.http.ServerRequest
+
+  EventEmitter = require('events').EventEmitter
+  Browser = require('../../../lib/browser').Browser
+  BrowserCollection = require('../../../lib/browser').Collection
+  MultReporter = require('../../../lib/reporters/Multi')
+  createRunnerMiddleware = require('../../../lib/middleware/runner').create
+
+  handler = nextSpy = response = mockReporter = capturedBrowsers = emitter = config = null
+
+  beforeEach ->
+    mockReporter =
+      adapters: []
+      write: (msg) -> @adapters.forEach (adapter) -> adapter msg
+
+    emitter = new EventEmitter
+    capturedBrowsers = new BrowserCollection emitter
+    fileListMock =
+      refresh: -> emitter.emit 'run_start'
+    nextSpy = sinon.spy()
+    response = new HttpResponseMock
+    config = {client: {}}
+
+    handler = createRunnerMiddleware emitter, fileListMock, capturedBrowsers,
+        new MultReporter([mockReporter]), 'localhost', 8877, '/', config
+
+
+  it 'should trigger test run and stream the reporter', (done) ->
+    capturedBrowsers.add new Browser
+    sinon.stub capturedBrowsers, 'areAllReady', -> true
+
+    response.once 'end', ->
+      expect(nextSpy).to.not.have.been.called
+      expect(response).to.beServedAs 200, 'result\x1FEXIT0'
+      done()
+
+    handler new HttpRequestMock('/__run__'), response, nextSpy
+
+    mockReporter.write 'result'
+    emitter.emit 'run_complete', capturedBrowsers, {exitCode: 0}
+
+
+  it 'should not run if there is no browser captured', (done) ->
+    response.once 'end', ->
+      expect(nextSpy).to.not.have.been.called
+      expect(response).to.beServedAs 200, 'No captured browser, open http://localhost:8877/\n'
+      done()
+
+    handler new HttpRequestMock('/__run__'), response, nextSpy
+
+
+  it 'should parse body and set client.args', (done) ->
+    capturedBrowsers.add new Browser
+    sinon.stub capturedBrowsers, 'areAllReady', -> true
+
+    emitter.once 'run_start', ->
+      expect(config.client.args).to.deep.equal ['arg1', 'arg2']
+      done()
+
+    request = new HttpRequestMock '/__run__', {
+      'content-type': 'application/json'
+      'content-length': 1
+    }
+    request.setEncoding = -> null
+
+    handler request, response, nextSpy
+
+    request.emit 'data', '{"args": ["arg1", "arg2"]}'
+    request.emit 'end'
+
+
+  it 'should ignore other urls', (done) ->
+    handler new HttpRequestMock('/something'), response, ->
+      expect(response).to.beNotServed()
+      done()
diff --git a/test/unit/web-server.spec.coffee b/test/unit/web-server.spec.coffee
@@ -36,7 +36,11 @@ describe 'web-server', ->
 
     injector = new di.Injector [{
       config: ['value', {basePath: '/base/path', urlRoot: '/'}]
-      customFileHandlers: ['value', customFileHandlers]
+      customFileHandlers: ['value', customFileHandlers],
+      emitter: ['value', null],
+      fileList: ['value', null],
+      capturedBrowsers: ['value', null],
+      reporter: ['value', null]
     }]
 
     server = injector.invoke m.createWebServer