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.
karma-runner · Sep 7, 2016 · 98a4fbf · 98a4fbf
1 parent 96f8f14
commit 98a4fbf
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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'}
         })