From 2d2916551453724c6aa7ddd1dcb2dd9c6f9da7e2 Mon Sep 17 00:00:00 2001 From: Christian Budde Christensen Date: Tue, 5 Jan 2016 19:16:12 +0100 Subject: [PATCH 1/2] feat(logging): Add colors and log-level options to run-command Add colors and log-level arguments to run argument. Refactor log-setup functions for server and init. Correct bug in server where log-level was ignored before `parseConfig` Closing #1067 --- lib/cli.js | 3 +++ lib/init.js | 13 ++----------- lib/logger.js | 24 ++++++++++++++++++++++++ lib/runner.js | 2 ++ lib/server.js | 8 +------- 5 files changed, 32 insertions(+), 18 deletions(-) diff --git a/lib/cli.js b/lib/cli.js index 520dd43d3..d8198abcf 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -185,6 +185,9 @@ var describeRun = function () { .describe('fail-on-empty-test-suite', 'Fail on empty test suite.') .describe('no-fail-on-empty-test-suite', 'Do not fail on empty test suite.') .describe('help', 'Print usage.') + .describe('log-level', ' Level of logging.') + .describe('colors', 'Use colors when reporting and printing logs.') + .describe('no-colors', 'Do not use colors when reporting or printing logs.') } var describeCompletion = function () { diff --git a/lib/init.js b/lib/init.js index 51e1eed7e..a257e05b2 100755 --- a/lib/init.js +++ b/lib/init.js @@ -6,7 +6,6 @@ var exec = require('child_process').exec var helper = require('./helper') var logger = require('./logger') -var constant = require('./constants') var log = logger.create('init') @@ -211,21 +210,13 @@ var processAnswers = function (answers, basePath, testMainFile) { } exports.init = function (config) { - var useColors = true - var logLevel = constant.LOG_INFO + logger.setupFromConfig(config) + var colorScheme = COLOR_SCHEME.ON if (helper.isDefined(config.colors)) { colorScheme = config.colors ? COLOR_SCHEME.ON : COLOR_SCHEME.OFF - useColors = config.colors - } - - if (helper.isDefined(config.logLevel)) { - logLevel = config.logLevel } - - logger.setup(logLevel, useColors) - // need to be registered before creating readlineInterface process.stdin.on('keypress', function (s, key) { sm.onKeypress(key) diff --git a/lib/logger.js b/lib/logger.js index 455081dbe..210242996 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -43,6 +43,29 @@ var setup = function (level, colors, appenders) { }) } +// Setup the logger by passing in the config object. The function sets the +// `colors` and `logLevel` if they are defined. It takes two arguments: +// +// setupFromConfig(config, appenders) +// +// * `config`: *Object* The configuration object. +// * `appenders`: *Array* This will be passed as appenders to log4js +// to allow for fine grained configuration of log4js. For more information +// see https://github.com/nomiddlename/log4js-node. +var setupFromConfig = function (config, appenders) { + var useColors = true + var logLevel = constant.LOG_INFO + + if (helper.isDefined(config.colors)) { + useColors = config.colors + } + + if (helper.isDefined(config.logLevel)) { + logLevel = config.logLevel + } + setup(logLevel, useColors, appenders) +} + // Create a new logger. There are two optional arguments // * `name`, which defaults to `karma` and // If the `name = 'socket.io'` this will create a special wrapper @@ -60,3 +83,4 @@ var create = function (name, level) { exports.create = create exports.setup = setup +exports.setupFromConfig = setupFromConfig diff --git a/lib/runner.js b/lib/runner.js index 41c43bf75..45c8a2467 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -32,6 +32,8 @@ var parseExitCode = function (buffer, defaultCode, failOnEmptyTestSuite) { // TODO(vojta): read config file (port, host, urlRoot) exports.run = function (config, done) { + logger.setupFromConfig(config) + done = helper.isFunction(done) ? done : process.exit config = cfg.parseConfig(config.configFile, config) diff --git a/lib/server.js b/lib/server.js index 892a1b55e..4c8084419 100644 --- a/lib/server.js +++ b/lib/server.js @@ -39,17 +39,11 @@ function createSocketIoServer (webServer, executor, config) { return server } -function setupLogger (level, colors) { - var logLevel = logLevel || constant.LOG_INFO - var logColors = helper.isDefined(colors) ? colors : true - logger.setup(logLevel, logColors, [constant.CONSOLE_APPENDER]) -} - // Constructor var Server = function (cliOptions, done) { EventEmitter.call(this) - setupLogger(cliOptions.logLevel, cliOptions.colors) + logger.setupFromConfig(cliOptions) this.log = logger.create() From 486c4f31c43fbbd63450b0aa6c64d3cb4c78ada4 Mon Sep 17 00:00:00 2001 From: Christian Budde Christensen Date: Thu, 7 Jan 2016 21:01:13 +0100 Subject: [PATCH 2/2] feat(logging): Send color option to server Delay the decision on color/no-color reporter to write. This is done by letting the individual adapter decide the color and providing a default color to the base adapter. Closing #1067 --- lib/middleware/runner.js | 4 ++-- lib/reporter.js | 6 ++++-- lib/reporters/base.js | 14 ++++++++++---- lib/reporters/dots.js | 4 ++-- lib/reporters/dots_color.js | 4 ++-- lib/reporters/progress.js | 4 ++-- lib/reporters/progress_color.js | 4 ++-- lib/runner.js | 3 ++- test/unit/reporters/base.spec.js | 30 +++++++++++++++++++++++++++++- 9 files changed, 55 insertions(+), 18 deletions(-) diff --git a/lib/middleware/runner.js b/lib/middleware/runner.js index 377b1e39d..00f03e201 100644 --- a/lib/middleware/runner.js +++ b/lib/middleware/runner.js @@ -33,9 +33,10 @@ var createRunnerMiddleware = function (emitter, fileList, capturedBrowsers, repo response.write('Waiting for previous execution...\n') } + var data = request.body emitter.once('run_start', function () { var responseWrite = response.write.bind(response) - + responseWrite.colors = data.colors reporter.addAdapter(responseWrite) // clean up, close runner response @@ -46,7 +47,6 @@ var createRunnerMiddleware = function (emitter, fileList, capturedBrowsers, repo }) }) - var data = request.body log.debug('Setting client.args to ', data.args) config.client.args = data.args diff --git a/lib/reporter.js b/lib/reporter.js index fca44ec85..9bcbab017 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -91,8 +91,10 @@ var createReporters = function (names, config, emitter, injector) { // TODO(vojta): instantiate all reporters through DI names.forEach(function (name) { if (['dots', 'progress'].indexOf(name) !== -1) { - var Cls = require('./reporters/' + name + (config.colors ? '_color' : '')) - return reporters.push(new Cls(errorFormatter, config.reportSlowerThan)) + var Cls = require('./reporters/' + name) + var ClsColor = require('./reporters/' + name + '_color') + reporters.push(new Cls(errorFormatter, config.reportSlowerThan, config.colors)) + return reporters.push(new ClsColor(errorFormatter, config.reportSlowerThan, config.colors)) } var locals = { diff --git a/lib/reporters/base.js b/lib/reporters/base.js index 9316c766e..9aaefc612 100644 --- a/lib/reporters/base.js +++ b/lib/reporters/base.js @@ -2,7 +2,7 @@ var util = require('util') var helper = require('../helper') -var BaseReporter = function (formatError, reportSlow, adapter) { +var BaseReporter = function (formatError, reportSlow, useColors, adapter) { this.adapters = [adapter || process.stdout.write.bind(process.stdout)] this.onRunStart = function () { @@ -46,9 +46,15 @@ var BaseReporter = function (formatError, reportSlow, adapter) { this.write = function () { var msg = util.format.apply(null, Array.prototype.slice.call(arguments)) - + var self = this this.adapters.forEach(function (adapter) { - adapter(msg) + if (!helper.isDefined(adapter.colors)) { + adapter.colors = useColors + } + + if (adapter.colors === self.USE_COLORS) { + return adapter(msg) + } }) } @@ -136,7 +142,7 @@ BaseReporter.decoratorFactory = function (formatError, reportSlow) { } } -BaseReporter.decoratorFactory.$inject = ['formatError', 'config.reportSlowerThan'] +BaseReporter.decoratorFactory.$inject = ['formatError', 'config.reportSlowerThan', 'config.colors'] // PUBLISH module.exports = BaseReporter diff --git a/lib/reporters/dots.js b/lib/reporters/dots.js index a0369cacf..f06b094af 100644 --- a/lib/reporters/dots.js +++ b/lib/reporters/dots.js @@ -1,7 +1,7 @@ var BaseReporter = require('./base') -var DotsReporter = function (formatError, reportSlow) { - BaseReporter.call(this, formatError, reportSlow) +var DotsReporter = function (formatError, reportSlow, useColors) { + BaseReporter.call(this, formatError, reportSlow, useColors) var DOTS_WRAP = 80 diff --git a/lib/reporters/dots_color.js b/lib/reporters/dots_color.js index ab42637cb..a583857ca 100644 --- a/lib/reporters/dots_color.js +++ b/lib/reporters/dots_color.js @@ -1,8 +1,8 @@ var DotsReporter = require('./dots') var BaseColorReporter = require('./base_color') -var DotsColorReporter = function (formatError, reportSlow) { - DotsReporter.call(this, formatError, reportSlow) +var DotsColorReporter = function (formatError, reportSlow, useColors) { + DotsReporter.call(this, formatError, reportSlow, useColors) BaseColorReporter.call(this) } diff --git a/lib/reporters/progress.js b/lib/reporters/progress.js index 9b5f21086..7a39d8053 100644 --- a/lib/reporters/progress.js +++ b/lib/reporters/progress.js @@ -1,7 +1,7 @@ var BaseReporter = require('./base') -var ProgressReporter = function (formatError, reportSlow) { - BaseReporter.call(this, formatError, reportSlow) +var ProgressReporter = function (formatError, reportSlow, useColors) { + BaseReporter.call(this, formatError, reportSlow, useColors) this.writeCommonMsg = function (msg) { this.write(this._remove() + msg + this._render()) diff --git a/lib/reporters/progress_color.js b/lib/reporters/progress_color.js index 7d59f783e..b27f2e7eb 100644 --- a/lib/reporters/progress_color.js +++ b/lib/reporters/progress_color.js @@ -1,8 +1,8 @@ var ProgressReporter = require('./progress') var BaseColorReporter = require('./base_color') -var ProgressColorReporter = function (formatError, reportSlow) { - ProgressReporter.call(this, formatError, reportSlow) +var ProgressColorReporter = function (formatError, reportSlow, useColors) { + ProgressReporter.call(this, formatError, reportSlow, useColors) BaseColorReporter.call(this) } diff --git a/lib/runner.js b/lib/runner.js index 45c8a2467..5f3c264e7 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -73,6 +73,7 @@ exports.run = function (config, done) { removedFiles: config.removedFiles, changedFiles: config.changedFiles, addedFiles: config.addedFiles, - refresh: config.refresh + refresh: config.refresh, + colors: config.colors })) } diff --git a/test/unit/reporters/base.spec.js b/test/unit/reporters/base.spec.js index 0eee0cd20..49d17b4ea 100644 --- a/test/unit/reporters/base.spec.js +++ b/test/unit/reporters/base.spec.js @@ -16,7 +16,7 @@ describe('reporter', function () { beforeEach(function () { adapter = sinon.spy() - reporter = new m.BaseReporter(null, null, adapter) + reporter = new m.BaseReporter(null, null, false, adapter) return reporter }) @@ -29,6 +29,34 @@ describe('reporter', function () { return expect(anotherAdapter).to.have.been.calledWith('some') }) + it('should omit adapters not using the right color', function () { + var anotherAdapter = sinon.spy() + anotherAdapter.colors = true + reporter.adapters.push(anotherAdapter) + reporter.write('some') + expect(adapter).to.have.been.calledWith('some') + return expect(anotherAdapter).to.not.have.been.called + }) + + it('should not call non-colored adapters when wrong default setting', function () { + var reporter = new m.BaseReporter(null, null, true, adapter) + var anotherAdapter = sinon.spy() + reporter.adapters.push(anotherAdapter) + reporter.write('some') + expect(adapter).to.not.have.been.called + return expect(anotherAdapter).to.not.have.been.called + }) + + it('should call colored adapters regardless of default setting', function () { + var reporter = new m.BaseReporter(null, null, true, adapter) + var anotherAdapter = sinon.spy() + reporter.adapters.push(anotherAdapter) + adapter.colors = false + reporter.write('some') + expect(adapter).to.have.been.calledWith('some') + return expect(anotherAdapter).to.not.have.been.called + }) + it('should format', function () { reporter.write('Success: %d Failure: %d', 10, 20)