feat(reporter): add config formatError function #2313

levithomason · 2016-08-15T22:42:11Z

Fixes #2119. This PR allows a formatError config function. The function receives the entire error message as its only argument. It returns the new error message.

levithomason · 2016-08-15T22:43:38Z

lib/cli.js

@@ -41,6 +41,10 @@ var processArgs = function (argv, options, fs, path) {
    options.failOnEmptyTestSuite = options.failOnEmptyTestSuite === 'true'
  }

+  if (helper.isString(options.formatError)) {
+    options.formatError = options.formatError = require(options.formatError)
+  }


CLI support via requiring a path that exports a formatError() function.

levithomason · 2016-08-15T22:51:04Z

I'd fix/add tests if I could, but they are failing when I run them on master as well:

Error: Cannot find module 'karma/lib/preprocessor'

The file is clearly there, so I'm not sure what is going on. I'm running node v6.2.1/npm v3.9.3.

Full output:

Running "test:unit" (test) task

Running "mochaTest:unit" (mochaTest) task


  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․

  454 passing (2s)
  3 pending

Running "test:client" (test) task
Running /Users/levithomason/src/karma/bin/karma start test/client/karma.conf.js --single-run --no-auto-watch --reporters=dots
failed to add custom browserify preprocessor
{ Error: Cannot find module 'karma/lib/preprocessor'
  at Function.Module._resolveFilename (module.js:440:15)
  at Function.Module._load (module.js:388:25)
  at Module.require (module.js:468:17)
  at require (internal/module.js:20:19)
  at Object.<anonymous> (/Users/levithomason/src/karma/node_modules/karma-browserify/lib/preprocessor.js:3:20)
  at Module._compile (module.js:541:32)
  at Object.Module._extensions..js (module.js:550:10)
  at Module.load (/Users/levithomason/src/karma/node_modules/coffee-script/lib/coffee-script/register.js:45:36)
  at tryModuleLoad (module.js:417:12)
  at Function.Module._load (module.js:409:3)
  at Module.require (module.js:468:17)
  at require (internal/module.js:20:19)
  at Object.<anonymous> (/Users/levithomason/src/karma/node_modules/karma-browserify/index.js:29:44)
  at Module._compile (module.js:541:32)
  at Object.Module._extensions..js (module.js:550:10)
  at Module.load (/Users/levithomason/src/karma/node_modules/coffee-script/lib/coffee-script/register.js:45:36)
  at tryModuleLoad (module.js:417:12)
  at Function.Module._load (module.js:409:3)
  at Module.require (module.js:468:17)
  at require (internal/module.js:20:19)
  at requirePlugin (/Users/levithomason/src/karma/lib/plugin.js:15:20)
  at /Users/levithomason/src/karma/lib/plugin.js:30:9
  at Array.forEach (native)
  at Object.exports.resolve (/Users/levithomason/src/karma/lib/plugin.js:27:11)
  at new Server (/Users/levithomason/src/karma/lib/server.js:90:35)
  at Object.exports.run (/Users/levithomason/src/karma/lib/cli.js:266:7)
  at Object.<anonymous> (/Users/levithomason/src/karma/bin/karma:3:23)
  at Module._compile (module.js:541:32)
  at Object.Module._extensions..js (module.js:550:10)
  at Module.load (module.js:458:32)
  at tryModuleLoad (module.js:417:12)
  at Function.Module._load (module.js:409:3)
  at Function.Module.runMain (module.js:575:10)
  at startup (node.js:160:18)
  at node.js:456:3
 code: 'MODULE_NOT_FOUND' }

levithomason · 2016-08-15T23:08:21Z

It might be worth it to include an option for removing node_modules, and another for removing compiled sources. This are the most common use cases and complaints. We could accept an array or a function. There would be a filter function each option and we'd just map through them. It might look something like:

// some pre-bundled filters
{ formatError: ['remove-node-modules', 'remove-compiled-sources'] }

// or your own function
{ formatError: (msg) => msg }

--format-error remove-node-modules,remove-compiled-sources
--format-error ./some-error-formatter.js

kahwee · 2016-08-16T04:02:05Z

Hi @levithomason, I encountered the issue with Error: Cannot find module 'karma/lib/preprocessor' as well and I think one of the steps I did managed to resolve this.

Most likely this is it:

npm run init

levithomason · 2016-08-16T04:34:09Z

Hm, I've never used npm init except to initialize a package.json. Gave it a shot anyway, no luck though.

kahwee · 2016-08-16T04:49:57Z

It's npm run init. I forgot you have to run grunt browserify as well.

There are similar instructions in the contribution guide http://karma-runner.github.io/1.0/dev/making-changes.html

Try this:

$ npm install
$ rm -rf node_modules/karma
$ cd node_modules
$ ln -s ../ karma
$ cd ../
$ grunt browserify

dignifiedquire · 2016-08-16T09:11:44Z

Yes, to get all the tests being actually run properly make sure to do the linking part as mentioned above.

levithomason · 2016-08-16T18:30:06Z

@dignifiedquire I believe it's ready for review. Tests and linting passed locally, just rebased to master.

levithomason · 2016-08-19T18:07:34Z

@dignifiedquire I understand busy. Do you have any ETA on this PR?

dignifiedquire · 2016-08-24T12:47:40Z

Sorry for the delay, will review this week

levithomason · 2016-08-24T15:06:26Z

Much appreciated, thanks.

levithomason · 2016-08-25T00:20:24Z

Rebased to latest master.

dignifiedquire · 2016-08-27T09:26:18Z

lib/cli.js

@@ -41,6 +41,16 @@ var processArgs = function (argv, options, fs, path) {
    options.failOnEmptyTestSuite = options.failOnEmptyTestSuite === 'true'
  }

+  if (helper.isString(options.formatError)) {
+    var required = require(options.formatError)


It would be good to wrap this into a try catch and provide a helpful error message if it fails.

levithomason · 2016-08-30T19:53:06Z

@dignifiedquire ready for review

levithomason · 2016-08-31T00:12:50Z

Tests pass locally and the CI failure seem odd Uncaught CancellationError: cancellation error.

levithomason · 2016-08-31T00:21:51Z

Retried the build via PR close/open to no avail, still getting this:

The command "if [ "${VALIDATE_COMMIT_MSG}" != "TRUE" ]; then npm run travis; else ./scripts/validate-commit-msg.sh "${TRAVIS_COMMIT}"; fi" exited with 1.

I edited the commit that removed the package.json git hooks and force pushed it. I'm wondering if CI is somehow confused on the force pushed history?

dignifiedquire · 2016-09-01T08:52:19Z

Looking at the CI failures they look like valid test failures

levithomason · 2016-09-01T15:05:26Z

Hm, the only change was replacing the package.json hooks. Also, tests pass locally. So I'm not sure what else to try there. Ideas?

dignifiedquire · 2016-09-01T15:23:20Z

Restarted them, lets see what happens

levithomason · 2016-09-01T16:12:11Z

@dignifiedquire looks like we're good to go.

levithomason · 2016-09-07T15:37:06Z

@dignifiedquire bump, this is labeled needs feedback but actually needs review.

dignifiedquire · 2016-09-07T15:53:23Z

@levithomason sorry about that, forgot that it was already green :)

levithomason · 2016-09-07T15:54:25Z

No worries, thanks for the quick response and merge. When do you suspect this will get released?

dignifiedquire · 2016-09-07T15:55:19Z

I think I will make a release later today

levithomason · 2016-09-07T15:56:21Z

Fantastic, thanks much. This is going to improve our workflow dramatically.

donaldpipowitch · 2016-09-07T17:13:23Z

This is awesome!

levithomason reviewed Aug 15, 2016
View reviewed changes

levithomason closed this Aug 16, 2016

levithomason deleted the feature/format-error branch August 16, 2016 15:33

levithomason restored the feature/format-error branch August 16, 2016 15:42

levithomason reopened this Aug 16, 2016

levithomason force-pushed the feature/format-error branch 3 times, most recently from b78e306 to caec040 Compare August 16, 2016 18:29

dignifiedquire self-assigned this Aug 16, 2016

dignifiedquire added the needs: review label Aug 16, 2016

This was referenced Aug 19, 2016

Feature Request: Filter 3rd party libs demerzel3/karma-sourcemap-loader#29

Closed

Not working with Karma rstacruz/mocha-clean#5

Open

docs(formatError): document formatError config

24cb3aa

levithomason force-pushed the feature/format-error branch from caec040 to ce2cbdc Compare August 24, 2016 22:21

dignifiedquire reviewed Aug 27, 2016
View reviewed changes

dignifiedquire added needs: feedback-from-author and removed needs: review labels Aug 27, 2016

levithomason added 4 commits August 30, 2016 12:52

feat(cli): add --format-error option

ce2e830

feat(reporter): implement config.formatError

1f0fa01

test(reporter): update existing tests

2c3dfae

test(reporter): add config.formatError tests

a216cc6

levithomason force-pushed the feature/format-error branch from 9691a14 to 2c673da Compare August 30, 2016 19:52

levithomason closed this Aug 31, 2016

levithomason reopened this Aug 31, 2016

levithomason force-pushed the feature/format-error branch from 2c673da to c88366e Compare September 1, 2016 15:29

levithomason added 3 commits September 1, 2016 08:51

test(cli): test --format-error

b5d4188

feat(config): add formatError option

54768a1

test(config): test formatError option validation

1f5d55f

levithomason force-pushed the feature/format-error branch from c88366e to 1f5d55f Compare September 1, 2016 15:51

dignifiedquire merged commit 98a4fbf into karma-runner:master Sep 7, 2016

dignifiedquire removed the needs: feedback-from-author label Sep 7, 2016

dignifiedquire removed their assignment Sep 7, 2016

levithomason deleted the feature/format-error branch September 7, 2016 15:54

levithomason mentioned this pull request Sep 10, 2016

chore(karma): clean and format stacktraces Semantic-Org/Semantic-UI-React#490

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(reporter): add config formatError function #2313

feat(reporter): add config formatError function #2313

levithomason commented Aug 15, 2016

levithomason Aug 15, 2016

levithomason commented Aug 15, 2016

levithomason commented Aug 15, 2016 •

edited

Loading

kahwee commented Aug 16, 2016

levithomason commented Aug 16, 2016

kahwee commented Aug 16, 2016

dignifiedquire commented Aug 16, 2016

levithomason commented Aug 16, 2016

levithomason commented Aug 19, 2016

dignifiedquire commented Aug 24, 2016

levithomason commented Aug 24, 2016

levithomason commented Aug 25, 2016

dignifiedquire Aug 27, 2016

levithomason commented Aug 30, 2016

levithomason commented Aug 31, 2016

levithomason commented Aug 31, 2016

dignifiedquire commented Sep 1, 2016

levithomason commented Sep 1, 2016

dignifiedquire commented Sep 1, 2016

levithomason commented Sep 1, 2016

levithomason commented Sep 7, 2016

dignifiedquire commented Sep 7, 2016

levithomason commented Sep 7, 2016

dignifiedquire commented Sep 7, 2016

levithomason commented Sep 7, 2016

donaldpipowitch commented Sep 7, 2016

feat(reporter): add config formatError function #2313

feat(reporter): add config formatError function #2313

Conversation

levithomason commented Aug 15, 2016

levithomason Aug 15, 2016

Choose a reason for hiding this comment

levithomason commented Aug 15, 2016

levithomason commented Aug 15, 2016 • edited Loading

kahwee commented Aug 16, 2016

levithomason commented Aug 16, 2016

kahwee commented Aug 16, 2016

dignifiedquire commented Aug 16, 2016

levithomason commented Aug 16, 2016

levithomason commented Aug 19, 2016

dignifiedquire commented Aug 24, 2016

levithomason commented Aug 24, 2016

levithomason commented Aug 25, 2016

dignifiedquire Aug 27, 2016

Choose a reason for hiding this comment

levithomason commented Aug 30, 2016

levithomason commented Aug 31, 2016

levithomason commented Aug 31, 2016

dignifiedquire commented Sep 1, 2016

levithomason commented Sep 1, 2016

dignifiedquire commented Sep 1, 2016

levithomason commented Sep 1, 2016

levithomason commented Sep 7, 2016

dignifiedquire commented Sep 7, 2016

levithomason commented Sep 7, 2016

dignifiedquire commented Sep 7, 2016

levithomason commented Sep 7, 2016

donaldpipowitch commented Sep 7, 2016

levithomason commented Aug 15, 2016 •

edited

Loading