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

Add support for suppressing stack traces #1095

Closed
wants to merge 119 commits into from

Conversation

tandrewnichols
Copy link

I run mocha tests with --watch in a small tmux pane, but even using --reporter min, the stack trace makes it impossible to see the actual error messages. For instant feedback, it's nice to get ONLY the error message. If you're running mocha with --watch, you're likely to know, already, where the error in the code is.

@waylonflinn
Copy link

👍
Thanks for working on this. Some of my stack traces are uber-nasty. :D

@tandrewnichols
Copy link
Author

This actually needs a little more work. I've been using it for a few months now, and I've noticed that occasionally, it prints out "%s", which is part of the formatting pattern. So I must've missed one of the stack print outs.

@jbnicolai
Copy link

@tandrewnichols happy to merge this if you could confirm the stability and add a small test case.

@tandrewnichols
Copy link
Author

As for stability, I've been using this branch in all my repos for months (see github.com/tandrewnichols/{tinder,varity,indeed,task-master,pedestrian,kindly,file-manifest,etc.}) with no issues other than the rare "%s" I mentioned above. I can add a test and resolve the conflicts here soon. Thanks for the feedback.

There is one other issue . . . I'm not sure if it's related to this branch or if it's a general mocha problem. When I put .only on a describe while running mocha with --watch, mocha will correctly rerun only that one test, but when I remove the .only, it doesn't rerun ALL the tests. I have to restart the process to get it to pick up the change. Is that a known general mocha problem (possibly even addressed already, as I haven't pulled upstream into this branch in a while) or is that something specific to my changes?

@tandrewnichols
Copy link
Author

@jbnicolai I don't have any experience writing tests with make. What I've got works, though maybe it's not the best way. It doesn't, for instance, report a success in green like the other tests. I couldn't figure out a way to make the tests fail and still assert that the output was correct. So I'm open to suggestions for improving the test.

But if you're satisfied, this is ready to go.

@tandrewnichols
Copy link
Author

@boneskull @travisjeffery @jbnicolai - Not to be annoying, but . . .

I've been using my branch of this in a number of projects but because my package dependency points to a git repo, another library I'm using that specifies a peerDependency for mocha fails (see npm/npm#3218). Which breaks basically everything I do in travis. Which is a way of saying, can we please just merge this so I can go back to a blissful CI experience? (But also, I've merged master and resolved conflicts once already, so I'd like to get this off my plate.)

@travisjeffery
Copy link
Contributor

i wanna fix some things first

@boneskull
Copy link
Contributor

@tandrewnichols Out of curiousity, does shrinkwrapping resolve the dependency issue?

@tandrewnichols
Copy link
Author

I haven't tried it, but I would be really surprised if it did because the version of mocha that I'm using is actually the version specified by the other module's peerDependencies. So it shouldn't be throwing that error at all. It's only because of that bug mentioned above that it's a problem. I was able to find another way around it though, which is to also fork that other module, remove it's peerDependency, and point my repos to that.

@travisjeffery
Copy link
Contributor

ah alright looks good to me, can some update the pr so we have a clean a merge? thanks!

@dasilvacontin
Copy link
Contributor

And maybe squash commits along the way.

Christopher Hiller and others added 27 commits December 15, 2014 13:29
I think it's better, anyway.  I was becoming frustrated by seeing my `Date`s and `Function`s represented as objects within the diffs.  Even more frustrating is when the diff result *appears to be exactly the same* but the assertion failed anyway.  So this is my attempt at making the diffs more friendly and understandable.

Summary of changes:

- Added inner function `emptyRepresentation()`, which is called in the case that your `Object`, `Function` or `Array` has nothing "in" it.  It attempts to make a unique representation of that empty value.
- Added private API function `exports.type()`, which can be used to deduce the type of any given value.  *I exported this function mainly for testing purposes*
- Modified `exports.stringify()` to avoid `JSON.stringify()` in circumstances where `JSON.stringify()` may not be much help.  My only concern here is whether or not we should display a string in double-quotes, as to coincide with `JSON.stringify()` behavior.
  - If `value` is `undefined` or `null`, return `'[undefined]'` or `'[null]'`, respectively.   *It's possible to have strings with these same values, which may lead to confusion, but this is an edge case.*
  - If `value` is a `Date`, return ISO8601 representation w/ formatting
  - If `value` is not an `Object`, `Function` or `Array`, return result of `value.toString()`.
  - If `value` is an *empty* `Object`, `Function` or `Array`, return result of function `emptyRepresentation()`.
  - If `value` has properties, call `exports.canonicalize()` on it, then return result of `JSON.stringify()`.
- Modified `exports.canonicalize()` to support the above strategy, with a couple slight differences.  The strategy is as follows.  If the value...
  - has already been seen, return string `'[Circular]'`
  - is `undefined`, return string `'[undefined]'`
  - is `null`, return value `null`.  This differs from the `stringify()` behavior, as to provide a valid JSON representation.
  - is some other primitive, return the value
  - is a Date, return wrapped ISO8601 representation
  - is not a primitive or an `Array`, `Object`, or `Function`, return the value of the Thing's `toString()` method
  - is a non-empty `Array`, `Object`, or `Function`, return the result of calling this function again.
  - is an empty `Array`, `Object`, or `Function`, return the result of calling `emptyRepresentation()`
- Added many tests
Color output only if standard and error output are ttys.
@tandrewnichols
Copy link
Author

Hah, something is all janked up in the history. Sorry to leave it in a bad state, but frankly, I just don't care. It took so long for you to say yes to this that I don't even use this feature anymore. If someone wants to pull it down, resolve the conflicts, squash, and whatever else, go for it.

@danielstjules
Copy link
Contributor

Hi @tandrewnichols, sorry this took a bit to get the green light. Really appreciate the work you did on this. Another PR was merged (#1564) that ended up achieving similar behavior, which can be seen in the example below:

$ cat example.js
it('test', function(done) {
  done(new Error('Something went wrong!'));
});

$ mocha example.js

  

  0 passing (5ms)
  1 failing

  1)  test:
     Error: Something went wrong!
      at Context.<anonymous> (example.js:2:8)

But thanks again for your work on this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.