From 98a4fbfb566a1635be1b980ee63c6189c10c0965 Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Wed, 7 Sep 2016 08:53:09 -0700 Subject: [PATCH] feat(reporter): add config formatError function Allows a `formatError` config function. The function receives the entire error message as its only argument. It returns the new error message. Fixes #2119. --- docs/config/01-configuration-file.md | 17 +++++++ lib/cli.js | 14 ++++++ lib/config.js | 4 ++ lib/reporter.js | 10 +++- test/unit/cli.spec.js | 12 +++++ test/unit/config.spec.js | 10 ++++ test/unit/fixtures/format-error-property.js | 3 ++ test/unit/fixtures/format-error-root.js | 4 ++ test/unit/reporter.spec.js | 54 +++++++++++++++++---- 9 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 test/unit/fixtures/format-error-property.js create mode 100644 test/unit/fixtures/format-error-root.js diff --git a/docs/config/01-configuration-file.md b/docs/config/01-configuration-file.md index b95b3f44c..09087bb95 100644 --- a/docs/config/01-configuration-file.md +++ b/docs/config/01-configuration-file.md @@ -588,6 +588,23 @@ Additional reporters, such as `growl`, `junit`, `teamcity` or `coverage` can be Note: Just about all additional reporters in Karma (other than progress) require an additional library to be installed (via NPM). +## formatError +**Type:** Function + +**Default:** `undefined` + +**CLI:** `--format-error ./path/to/formatFunction.js` + +**Arguments:** + + * `msg` - The entire assertion error and stack trace as a string. + +**Returns:** A new error message string. + +**Description:** Format assertion errors and stack traces. Useful for removing vendors and compiled sources. + +The CLI option should be a path to a file that exports the format function. This can be a function exported at the root of the module or an export named `formatError`. + ## restartOnFileChange **Type:** Boolean diff --git a/lib/cli.js b/lib/cli.js index 25a9e988a..a86f837b9 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -41,6 +41,20 @@ var processArgs = function (argv, options, fs, path) { options.failOnEmptyTestSuite = options.failOnEmptyTestSuite === 'true' } + if (helper.isString(options.formatError)) { + try { + var required = require(options.formatError) + } catch (err) { + console.error('Could not require formatError: ' + options.formatError, err) + } + // support exports.formatError and module.exports = function + options.formatError = required.formatError || required + if (!helper.isFunction(options.formatError)) { + console.error('Format error must be a function, got: ' + typeof options.formatError) + process.exit(1) + } + } + if (helper.isString(options.logLevel)) { var logConstant = constant['LOG_' + options.logLevel.toUpperCase()] if (helper.isDefined(logConstant)) { diff --git a/lib/config.js b/lib/config.js index 182e42842..235c4bb75 100644 --- a/lib/config.js +++ b/lib/config.js @@ -165,6 +165,10 @@ var normalizeConfig = function (config, configFilePath) { throw new TypeError('Invalid configuration: browsers option must be an array') } + if (config.formatError && !helper.isFunction(config.formatError)) { + throw new TypeError('Invalid configuration: formatError option must be a function.') + } + var defaultClient = config.defaultClient || {} Object.keys(defaultClient).forEach(function (key) { var option = config.client[key] diff --git a/lib/reporter.js b/lib/reporter.js index 4b52d9a49..d44c8788f 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -8,7 +8,8 @@ var log = require('./logger').create('reporter') var MultiReporter = require('./reporters/multi') var baseReporterDecoratorFactory = require('./reporters/base').decoratorFactory -var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) { +var createErrorFormatter = function (config, emitter, SourceMapConsumer) { + var basePath = config.basePath var lastServedFiles = [] emitter.on('file_list_modified', function (files) { @@ -92,12 +93,17 @@ var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) { msg = indentation + msg.replace(/\n/g, '\n' + indentation) } + // allow the user to format the error + if (config.formatError) { + msg = config.formatError(msg) + } + return msg + '\n' } } var createReporters = function (names, config, emitter, injector) { - var errorFormatter = createErrorFormatter(config.basePath, emitter, SourceMapConsumer) + var errorFormatter = createErrorFormatter(config, emitter, SourceMapConsumer) var reporters = [] // TODO(vojta): instantiate all reporters through DI diff --git a/test/unit/cli.spec.js b/test/unit/cli.spec.js index a4af1c659..7696cb823 100644 --- a/test/unit/cli.spec.js +++ b/test/unit/cli.spec.js @@ -133,6 +133,18 @@ describe('cli', () => { expect(mockery.process.exit).to.have.been.calledWith(1) }) + it('should parse format-error into a function', () => { + // root export + var options = processArgs(['--format-error', '../../test/unit/fixtures/format-error-root']) + var formatErrorRoot = require('../../test/unit/fixtures/format-error-root') + expect(options.formatError).to.equal(formatErrorRoot) + + // property export + options = processArgs(['--format-error', '../../test/unit/fixtures/format-error-property']) + var formatErrorProperty = require('../../test/unit/fixtures/format-error-property').formatError + expect(options.formatError).to.equal(formatErrorProperty) + }) + it('should parse browsers into an array', () => { var options = processArgs(['--browsers', 'Chrome,ChromeCanary,Firefox']) expect(options.browsers).to.deep.equal(['Chrome', 'ChromeCanary', 'Firefox']) diff --git a/test/unit/config.spec.js b/test/unit/config.spec.js index cb186e50e..11294332f 100644 --- a/test/unit/config.spec.js +++ b/test/unit/config.spec.js @@ -331,6 +331,16 @@ describe('config', () => { expect(invalid).to.throw('Invalid configuration: browsers option must be an array') }) + + it('should validate that the formatError option is a function', () => { + var invalid = function () { + normalizeConfigWithDefaults({ + formatError: 'lodash/identity' + }) + } + + expect(invalid).to.throw('Invalid configuration: formatError option must be a function.') + }) }) describe('createPatternObject', () => { diff --git a/test/unit/fixtures/format-error-property.js b/test/unit/fixtures/format-error-property.js new file mode 100644 index 000000000..72380fbd7 --- /dev/null +++ b/test/unit/fixtures/format-error-property.js @@ -0,0 +1,3 @@ +exports.formatError = function formatErrorProperty (msg) { + return msg +} diff --git a/test/unit/fixtures/format-error-root.js b/test/unit/fixtures/format-error-root.js new file mode 100644 index 000000000..a17fb97e9 --- /dev/null +++ b/test/unit/fixtures/format-error-root.js @@ -0,0 +1,4 @@ +// a valid --format-error file +module.exports = function formatErrorRoot (msg) { + return msg +} diff --git a/test/unit/reporter.spec.js b/test/unit/reporter.spec.js index 197898903..ab55cd312 100644 --- a/test/unit/reporter.spec.js +++ b/test/unit/reporter.spec.js @@ -2,6 +2,7 @@ import {EventEmitter} from 'events' import {loadFile} from 'mocks' import path from 'path' import _ from 'lodash' +import sinon from 'sinon' import File from '../../lib/file' @@ -15,10 +16,43 @@ describe('reporter', () => { describe('formatError', () => { var emitter var formatError = emitter = null + var sandbox beforeEach(() => { emitter = new EventEmitter() - formatError = m.createErrorFormatter('', emitter) + formatError = m.createErrorFormatter({ basePath: '' }, emitter) + sandbox = sinon.sandbox.create() + }) + + it('should call config.formatError if defined', () => { + var spy = sandbox.spy() + formatError = m.createErrorFormatter({ basePath: '', formatError: spy }, emitter) + formatError() + + expect(spy).to.have.been.calledOnce + }) + + it('should not call config.formatError if not defined', () => { + var spy = sandbox.spy() + formatError() + + expect(spy).not.to.have.been.calledOnce + }) + + it('should pass the error message as the first config.formatError argument', () => { + var ERROR = 'foo bar' + var spy = sandbox.spy() + formatError = m.createErrorFormatter({ basePath: '', formatError: spy }, emitter) + formatError(ERROR) + + expect(spy.firstCall.args[0]).to.equal(ERROR) + }) + + it('should display the error returned by config.formatError', () => { + var formattedError = 'A new error' + formatError = m.createErrorFormatter({ basePath: '', formatError: () => formattedError }, emitter) + + expect(formatError('Something', '\t')).to.equal(formattedError + '\n') }) it('should indent', () => { @@ -51,7 +85,7 @@ describe('reporter', () => { // TODO(vojta): enable once we serve source under urlRoot it.skip('should handle non default karma service folders', () => { - formatError = m.createErrorFormatter('', '/_karma_/') + formatError = m.createErrorFormatter({ basePath: '' }, '/_karma_/') expect(formatError('file http://localhost:8080/_karma_/base/usr/a.js and http://127.0.0.1:8080/_karma_/base/home/b.js')).to.be.equal('file usr/a.js and home/b.js\n') }) @@ -65,7 +99,7 @@ describe('reporter', () => { }) it('should restore base paths', () => { - formatError = m.createErrorFormatter('/some/base', emitter) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter) expect(formatError('at http://localhost:123/base/a.js?123')).to.equal('at a.js\n') }) @@ -121,7 +155,7 @@ describe('reporter', () => { MockSourceMapConsumer.LEAST_UPPER_BOUND = 2 it('should rewrite stack traces', (done) => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/a.js'), new File('/some/base/b.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.js'} servedFiles[1].sourceMap = {content: 'SOURCE MAP b.js'} @@ -136,7 +170,7 @@ describe('reporter', () => { }) it('should rewrite stack traces to the first column when no column is given', (done) => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/a.js'), new File('/some/base/b.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.js'} servedFiles[1].sourceMap = {content: 'SOURCE MAP b.js'} @@ -151,7 +185,7 @@ describe('reporter', () => { }) it('should rewrite relative url stack traces', (done) => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/a.js'), new File('/some/base/b.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.js'} servedFiles[1].sourceMap = {content: 'SOURCE MAP b.js'} @@ -167,7 +201,7 @@ describe('reporter', () => { it('should resolve relative urls from source maps', (done) => { sourceMappingPath = 'original/' // Note: relative path. - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/path/a.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.fancyjs'} @@ -181,7 +215,7 @@ describe('reporter', () => { }) it('should fall back to non-source-map format if originalPositionFor throws', (done) => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/a.js'), new File('/some/base/b.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.js'} servedFiles[1].sourceMap = {content: 'SOURCE MAP b.js'} @@ -196,7 +230,7 @@ describe('reporter', () => { }) it('should not try to use source maps when no line is given', (done) => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) var servedFiles = [new File('/some/base/a.js'), new File('/some/base/b.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP a.js'} servedFiles[1].sourceMap = {content: 'SOURCE MAP b.js'} @@ -216,7 +250,7 @@ describe('reporter', () => { var servedFiles = null beforeEach(() => { - formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + formatError = m.createErrorFormatter({ basePath: '/some/base' }, emitter, MockSourceMapConsumer) servedFiles = [new File('C:/a/b/c.js')] servedFiles[0].sourceMap = {content: 'SOURCE MAP b.js'} })