From ca4c4d88b9a4a1992f7975aa32b37a008394847b Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Tue, 23 Jul 2013 14:57:01 -0700 Subject: [PATCH] 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`. --- lib/config.js | 3 + lib/middleware/runner.js | 58 ++++++++++++++++++ lib/runner.js | 43 ++++++++------ lib/server.js | 72 +--------------------- lib/web-server.js | 2 + test/unit/middleware/runner.spec.coffee | 79 +++++++++++++++++++++++++ test/unit/web-server.spec.coffee | 6 +- 7 files changed, 175 insertions(+), 88 deletions(-) create mode 100644 lib/middleware/runner.js create mode 100644 test/unit/middleware/runner.spec.coffee diff --git a/lib/config.js b/lib/config.js index bee75cc32..f14982724 100644 --- 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 new file mode 100644 index 000000000..1a195c3ff --- /dev/null +++ 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 index 16e8fe138..c1814c7ed 100644 --- 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 index d443d923b..3bf3cb311 100644 --- 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 index 0588536fd..972bbf59e 100644 --- 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 new file mode 100644 index 000000000..140ca6772 --- /dev/null +++ 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 index 9a2f25333..e42466971 100644 --- 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