Skip to content

Commit

Permalink
feat(reporter): add config formatError function
Browse files Browse the repository at this point in the history
Allows a `formatError` config function. The function receives the entire error message as its only argument. It returns the new error message.

Fixes #2119.
  • Loading branch information
levithomason authored and dignifiedquire committed Sep 7, 2016
1 parent 96f8f14 commit 98a4fbf
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 12 deletions.
17 changes: 17 additions & 0 deletions docs/config/01-configuration-file.md
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions lib/cli.js
Expand Up @@ -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)) {
Expand Down
4 changes: 4 additions & 0 deletions lib/config.js
Expand Up @@ -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]
Expand Down
10 changes: 8 additions & 2 deletions lib/reporter.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions test/unit/cli.spec.js
Expand Up @@ -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'])
Expand Down
10 changes: 10 additions & 0 deletions test/unit/config.spec.js
Expand Up @@ -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', () => {
Expand Down
3 changes: 3 additions & 0 deletions test/unit/fixtures/format-error-property.js
@@ -0,0 +1,3 @@
exports.formatError = function formatErrorProperty (msg) {
return msg
}
4 changes: 4 additions & 0 deletions test/unit/fixtures/format-error-root.js
@@ -0,0 +1,4 @@
// a valid --format-error file
module.exports = function formatErrorRoot (msg) {
return msg
}
54 changes: 44 additions & 10 deletions test/unit/reporter.spec.js
Expand Up @@ -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'

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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')
})

Expand All @@ -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')
})

Expand Down Expand Up @@ -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'}
Expand All @@ -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'}
Expand All @@ -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'}
Expand All @@ -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'}

Expand All @@ -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'}
Expand All @@ -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'}
Expand All @@ -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'}
})
Expand Down

0 comments on commit 98a4fbf

Please sign in to comment.