Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorten stack traces #545

Closed
haraldrudell opened this issue Aug 22, 2012 · 52 comments
Closed

Shorten stack traces #545

haraldrudell opened this issue Aug 22, 2012 · 52 comments
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal

Comments

@haraldrudell
Copy link

80% of the AssertionError stack trace is uninteresting mocha internal code

Suggestion:
suppress all frames from Test.Runnable.run and previous

Suggestion:

  1. Reformat the stack trace so the first text is base-file:line:column, then Object.function, then directory.
  2. Directory can be shortened to be relative to the deploy folder (ie. where app.js is)

I am using node assert, ui exports and spec reporter

@tj
Copy link
Contributor

tj commented Aug 22, 2012

yeah I'd like to have this too, at least as a default (so I can still get those callsites if it really is a mocha bug)

+1

@rauchg
Copy link
Contributor

rauchg commented Aug 22, 2012

Also highlighting the test that failed in the stack trace would be sweet

@tj
Copy link
Contributor

tj commented Aug 22, 2012

that too, I think I have a branch with that going

@haraldrudell
Copy link
Author

I implemented the short stack traces in the assert-wrap I use in mochawrapper 0.0.22

It also has a variant of character escaping you might find interesting.

@haraldrudell
Copy link
Author

I have since written a stack trace parser here: https://github.com/haraldrudell/haraldutil/blob/master/lib/stacktraceparser.js
It could make this improvement easier to accomplish.

@aronwoost
Copy link

+1

3 similar comments
@mistaecko
Copy link

+1

@eddiemonge
Copy link

+1

@rschmukler
Copy link

+1

@andrew-luhring
Copy link

+1
#815 (comment)

How do we move this forward? I just looked at mocha code for the last 2 hours trying to figure out how to shorten the output to a single line of

      at Context.<anonymous> (./public/tests/_prettyfy.js:10:25)

and the closest I came was removing the stack trace completely by removing "stack" from line 205 of mocha/lib/reporters/base.js like this:

    // ( line ~201 )
    // indent stack trace without msg
    stack = stack.slice(index ? index + 1 : index)
      .replace(/^/gm, '  ');

    console.error(fmt
    , (i + 1)
    , test.fullTitle()
    , msg
    //, stack <====== commenting this dude out.
    );
  });

which will output this:

Running "Mocha" task
 1   -_-_,------,
 1   -_-_|   /\_/\ 
 0   -_-^|__( x .x) 
     -_-  ""  "" 

  1 passing (15ms)
  1 failing

  1) mocha will fail:
     Error: expected 1 to equal 0
%s

...
So... close...
I don't know where the %s comes from but it kind of looks cool if you get a bunch of them in a line

%s%s%s%s%s%s____✬
%s%s%s%s%s%s%s%s%s%s____✬
%s%s%s%s%s%s%s%s%s____✬
%s%s%s%s%s%s____✬

#pewpewpew

p.s.
I'm using mocha because of nyancat and growl-- whoever made the decision to add those two reporters as defaults is a genius.

@lynndylanhurley
Copy link

+1

I'm getting pages and pages of this:

screen shot 2014-04-11 at 2 39 36 pm

@tj
Copy link
Contributor

tj commented Apr 11, 2014

@lynndylanhurley looks like you're using one of the long stacktrace libs? Maybe we could dedupe and display at <something> (x 1500) haha, just truncating isn't overly useful

@lynndylanhurley
Copy link

@visionmedia - de-duping would be great 👍

@LittleG
Copy link

LittleG commented May 10, 2014

I made a quick fix without file modification, might not good for every situation since it removes an argument from console error

console.oldError = console.error;
console.error = function (args) {
    if (typeof arguments.stack !== 'undefined') {
        console.oldError.call(console, arguments.stack);
    } else {
        if (typeof arguments[4] !== 'undefined') arguments[4] = "...";
        console.oldError.apply(console, arguments);
    }
 }

@asheb
Copy link

asheb commented May 11, 2014

@LittleG: Nice hack! Thank you. Made it filter stacktrace lines.

@jackfranklin
Copy link

I stumbled upon this today and thanks to @LittleG I was able to tweak my setup. @LittleG's solution removes the entire stack trace, but I updated it so it just shows the first 4 lines of it:

console.oldError = console.error;
console.error = function (args) {
  if (typeof arguments.stack !== 'undefined') {
    console.oldError.call(console, arguments.stack);
  } else {
    if (typeof arguments[4] !== 'undefined') {
      var traceToShow = arguments[4].split('\n').slice(0, 4);
      arguments[4] = traceToShow.join('\n');
    }
    console.oldError.apply(console, arguments);
  }
}

@kfatehi
Copy link

kfatehi commented Jul 25, 2014

@LittleG and @jackfranklin thanks, that was great!

I've modified it slightly to further reduce noise.
The use of "global" is to support mocha -w code reloading.

/* Make stack traces shorter and more relevant
 * by cutting full path to app and reducing height */
var path = require('path');
var appRoot = path.resolve(__dirname, '..')+'/';
console.oldError = global.oldError || console.error;
console.error = function (args) {
  if (typeof arguments.stack !== 'undefined') {
    console.oldError.call(console, arguments.stack);
  } else {
    if (typeof arguments[4] !== 'undefined') {
      var traceToShow = arguments[4].split('\n').slice(0, 4);
      arguments[4] = traceToShow.join('\n').replace(RegExp(appRoot, 'g'), '');
    }
    console.oldError.apply(console, arguments);
  }
}
global.oldError = console.oldError;

@kfatehi
Copy link

kfatehi commented Jul 25, 2014

ok cool, you guys can use/extend this now, built from this discussion ...
https://www.npmjs.org/package/simple-stacktrace / https://github.com/keyvanfatehi/js-simple-stacktrace

@kfatehi
Copy link

kfatehi commented Jul 28, 2014

v0.2.0 now ships with a bin and prefers that you use it as a pipe

e.g. mocha -w --recursive test/unit -R min 2>&1 | simple-stacktrace

works pretty damn well

@pelicanorojo
Copy link

Hi, I do something similar to the keyvanfatehi solution:

I'm developing a package for httprequesttests ( mocha + chai + superagent ) and put this in the index.js, also give the users the opportunity of change errorStackDepth by config.

var
oldconsoleError = console.error,
errorStackDepth = 1;

console.error = function () {//fmt, i, title, msg, stack
var
args = Array.prototype.slice.call( arguments, 0 ),
stack = args[ 4 ];

if ( stack ) {
    args[ 4 ] = stack.split( '\n' ).slice( 0, errorStackDepth ).join( '\n' );
}

oldconsoleError.apply( console, args );

};

@rstacruz
Copy link
Contributor

Here's a more elegant solution to cleaning up stack traces. simple add -r mocha-clean to your invocation (or to the test/mocha.opts file).

https://github.com/rstacruz/mocha-clean

@andrew-luhring
Copy link

you're my hero

On Wed, Sep 24, 2014 at 11:51 PM, Rico Sta. Cruz notifications@github.com
wrote:

Here's a more elegant solution to cleaning up stack traces.

https://github.com/rstacruz/mocha-clean

Reply to this email directly or view it on GitHub:
#545 (comment)

@boneskull boneskull added type: feature enhancement proposal status: accepting prs Mocha can use your help with this one! labels Nov 7, 2014
@boneskull
Copy link
Contributor

I'd like to see this in Mocha. Too much garbage in the stack.

@dasilvacontin
Copy link
Contributor

+1

@rstacruz
Copy link
Contributor

rstacruz commented Nov 8, 2014

@boneskull, would you be interested in merging mocha-clean features into mocha?

@boneskull
Copy link
Contributor

@rstacruz Looks cool, I'll have to take a closer look and see what kind of effort it would take.

@dasilvacontin
Copy link
Contributor

I want a burrito. Let's see.

@ghost
Copy link

ghost commented Feb 1, 2015

I think a lot of us stopped at @rstacruz's fix and enjoyed our burritos then. I'd happily try out your fork and enjoy a burrito deluxe.

@a8m
Copy link
Contributor

a8m commented Mar 9, 2015

I think we can close this issue @travisjeffery.

@amir-rahnama
Copy link

This is not fixed at all.

module.js:339
    throw err;
    ^
Error: Cannot find module '../../lib/camelize'
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/Users/ara/dev/iteam/dm-crawler/tests/unit/lib/camelize.js:5:16)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at /Users/ara/dev/iteam/dm-crawler/node_modules/mocha/lib/mocha.js:216:27
    at Array.forEach (native)
    at Mocha.loadFiles (/Users/ara/dev/iteam/dm-crawler/node_modules/mocha/lib/mocha.js:213:14)
    at Mocha.run (/Users/ara/dev/iteam/dm-crawler/node_modules/mocha/lib/mocha.js:453:10)
    at Object.<anonymous> (/Users/ara/dev/iteam/dm-crawler/node_modules/mocha/bin/_mocha:393:18)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:134:18)
    at node.js:961:3
npm ERR! Test failed.  See above for more details.

all the trace is hugely irrelevant to my code.

@rstacruz
Copy link
Contributor

haha.. i forgot how this was resolved... but i think it was rolled back at some point

@danielstjules
Copy link
Contributor

Admittedly, it was partially rolled back and it may have been a knee-jerk reaction on my part to some of the issues we were seeing at that time. We do partial stack trace filtering in an attempt to remove Mocha's internals from the results. However, we don't hide any code living in other modules (under node_modules). Maybe this is something that could be revisited.

@danielstjules danielstjules reopened this Feb 10, 2016
@adamreisnz
Copy link

+1 please re-implement this.

@rstacruz
Copy link
Contributor

in the mean time, mocha-clean is still around.

@adamreisnz
Copy link

Yep, I am using it now. Just a shame really that this simple plugin does exactly what is needed, yet fails to have been merged into mocha and that this issue has been around since August 2012... :(

@danielstjules
Copy link
Contributor

It was merged, but the filtering was minimized after complaints that test output was no longer helpful in debugging. I think the simple issue is that it shouldn't have been the default behavior, and should instead be opt-in.

@adamreisnz
Copy link

Agreed that it can be opt-in. I believe mocha-clean also has options to exclude files in node_modules from being filtered out, I assume this could be tweaked to exclude certain other directories as well. Or simply only ignore mocha internals.

Perhaps a .mocharc would provide more flexibility than .mochaopts in terms of configuration.

@danielstjules
Copy link
Contributor

There's a --full-trace flag which will result in even longer stack traces. I thought the core issues had been mostly resolved, though edge cases exist (e.g. when using compilers, etc). See below:

$ cat example.js
var assert = require('assert');

it('test', function() {
  function foo() {
    assert(false);
  }

  foo();
});

And running it:

$ mocha example.js


  1) test

  0 passing (10ms)
  1 failing

  1)  test:

      AssertionError: false == true
      + expected - actual

      -false
      +true

      at foo (example.js:5:5)
      at Context.<anonymous> (example.js:8:3)

For comparison:

dstjules:~
$ mocha --full-trace example.js


  1) test

  0 passing (9ms)
  1 failing

  1)  test:

      AssertionError: false == true
      + expected - actual

      -false
      +true

      at foo (/Users/dstjules/example.js:5:5)
      at Context.<anonymous> (/Users/dstjules/example.js:8:3)
      at callFn (/usr/local/lib/node_modules/mocha/lib/runnable.js:315:21)
      at Test.Runnable.run (/usr/local/lib/node_modules/mocha/lib/runnable.js:308:7)
      at Runner.runTest (/usr/local/lib/node_modules/mocha/lib/runner.js:422:10)
      at /usr/local/lib/node_modules/mocha/lib/runner.js:533:12
      at next (/usr/local/lib/node_modules/mocha/lib/runner.js:342:14)
      at /usr/local/lib/node_modules/mocha/lib/runner.js:352:7
      at next (/usr/local/lib/node_modules/mocha/lib/runner.js:284:14)
      at Immediate._onImmediate (/usr/local/lib/node_modules/mocha/lib/runner.js:320:5)
      at processImmediate [as _immediateCallback] (timers.js:357:17)

@danielstjules
Copy link
Contributor

So, it looks like the original issue has been resolved. That is:

80% of the AssertionError stack trace is uninteresting mocha internal code

This can be seen in my previous comment.

As for #545 (comment) part of the stack trace could be cleaned, but only half the noise is from mocha itself:

$ cat example.js
var failed = require('failed');

$ node example.js
module.js:318
    throw err;
          ^
Error: Cannot find module 'failed'
    at Function.Module._resolveFilename (module.js:316:15)
    at Function.Module._load (module.js:258:25)
    at Module.require (module.js:345:17)
    at require (module.js:364:17)
    at Object.<anonymous> (/Users/dstjules/example.js:1:76)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:428:10)
    at Module.load (module.js:335:32)
    at Function.Module._load (module.js:290:12)
    at Function.Module.runMain (module.js:451:10)

@boneskull
Copy link
Contributor

I'm not really sure we should be in the business of filtering stack traces. It's too easy to filter too much or not enough. When you add something like trace and/or clarify you start stepping on toes.

I propose an alternative: highlight each line of the stack that is within the code under test or the test files itself. JetBrains' reporter does this. Hopefully, that won't be too difficult to implement.

It could look something like the following:

2016-03-16_2202

Opinions?

@ghost
Copy link

ghost commented Mar 20, 2016

It's too easy to filter too much or not enough.

Not really.

  • Too much: more than the part of the stack trace that is mocha internals
  • Too little: including mocha internals portion in the stack trace

By default, cleaner output. Provide a flag for full stack traces. It's what tj said even he wanted.

My opinion is that mocha-clean solved this for me and it's rather silly that mocha's default output is still cluttered.

@rhendric
Copy link

Filtering sounds all well and good, but I for one would love to have an option to either mute stack traces or truncate the stack trace to a maximum number of lines (instead of or in addition to the kind of filtering I see discussed here), for those of us that make extensive use of recursion, or use libraries like jsverify that do so. When your stack traces are hundreds of lines long, even if it's all your code, more than the first, oh, thirty lines or so is not likely to be helpful in fixing a test. As I mentioned in #2114, I don't know whether to suggest this be a reporter option or a core mocha option; guidance would be appreciated.

@Lujaw
Copy link

Lujaw commented Jul 26, 2016

+1

@boneskull
Copy link
Contributor

long-since done

@vincentntang
Copy link

I have mocha running in the browser to display tests following a setup I wrote here https://medium.com/@Kagerjay/portable-javascript-unit-testing-with-mocha-and-chai-e2e468dbeccc,

I ended up just using CSS to limit the size of the stacktrace on the browser.

Its a lazy solution but it works

.error {
  max-height: 30px !important;
}

Example of how the stacktrace looks on my browser now

vztctsi 1

@eKoopmans
Copy link

Tested under: Browser (Chrome 69.0.3497.100)

Issue: mocha.js internal stack trace isn't hidden when the file has a different name (e.g. mocha.min.js)


Hi @boneskull, I appreciate that the internal stack trace has been hidden for the standard use case, but I've encountered that any change to the filename of mocha.js (e.g. to mocha.min.js, as in @vincentntang's example above) prevents that filtering from happening.

I think there's a simple solution, at least within browsers, by detecting the name of the script using document.currentScript:

// Gets entire path and name of the current script.
var filename = document.currentScript.src;

A possible downside would be a loss of granularity on heavily bundled apps. Another approach could be to simply include mocha.min.js (or regex mocha.*\.js to also match files with version numbers) in the filtering. For the time being, @rstacruz's mocha-clean is necessary for anyone using a version with an altered filename.

P.S. I did test and the minified version is properly filtered when its name is changed to mocha.js, so it's exclusively a filename issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests