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

Question regarding stack trace. #878

Closed
jaridmargolin opened this issue Dec 2, 2016 · 6 comments
Closed

Question regarding stack trace. #878

jaridmargolin opened this issue Dec 2, 2016 · 6 comments

Comments

@jaridmargolin
Copy link

Currently chai implements some magic to hide internal stack traces on AssertionError's. Cool stuffs btw! I am however curious why the stack includes the actual assert implementation. Does it provide additional value to the user? There may be a very obvious reason, so please bare with any possible stupidity on my part :)

Example:

AssertionError: expected false to be true
      at Function.assert.isTrue (node_modules/chai/lib/chai/interface/assert.js:332:31)
      at Context.<anonymous> (test/integration/index.js:13:12)

99.99% of the time I am looking for:

at Context.<anonymous> (test/integration/index.js:13:12)

In fact, in the few years I have been using chai, I not once followed a stacktrace to view the internal assert implementation.


Just as a little background, I'm working on a small util to pretty up stack traces for a testing tool I am developing:

https://cldup.com/fovVPUi28W.png

@keithamus
Copy link
Member

Chai's core is pretty solid - we code pretty defensively, but we don't have much influence over plugins, which can cause genuine errors within the assertion. Sometimes it is actually useful to read the code of the assertion that failed. I've certainly - more than a handful of times - read the code of the assertion that's throwing to check what is going wrong.

Your utility looks super useful - and I've been thinking a lot about doing the exact same thing somehow in chai or perhaps up the stack in the test runner (like mocha). Please let us know when you've finished developing this! I'd love to take a look ❤️

@jaridmargolin
Copy link
Author

Hmmm ok.

So if I want to display the code frame at the point in which the assert was called (which I believe is more useful as I write tests), I am going to have to sniff for the chai AssertError and shift off the first entry in the call stack.

I see it as the only solution, but it arrises the following issues:

  1. If I put it in the core stacktrace styling lib, the library becomes coupled to chai.
  2. I could pull the chai specific functionality out into a separate module, but then I don't see it actually ever being implemented.

Ideally I was hoping it could just work out of the box. Ex: mocha --require flvr

Am I overlooking any possible solutions?


Quick example of what the output looks like after quickly hacking in the shift:

https://cldup.com/vcen70ISmf.png

@meeber
Copy link
Contributor

meeber commented Dec 2, 2016

@jaridmargolin I'm gonna take a look at this stuff this weekend. Note that there's a related issue in chai@4.0.0-canary.1 with the proxy-related frames showing up in the stack trace.

@jaridmargolin
Copy link
Author

@meeber 👍

Additionally, the more I think about this... maybe extracting out various addons isn't the end of the world. As long as the plugins are automatically loaded from node_modules to reduce implementation friction.

This would allow for third party addons to be tailored towards whatever tools the consumer decides to use. For example, in the above mocha run, an addon could enable filtering out mocha entries in the stack trace (which again, I feel like 99.99% of the time is not necessary to display).

@meeber
Copy link
Contributor

meeber commented Dec 8, 2016

Let's compare the stack traces between the different interfaces in Chai v4.0.

Tests:

it("assert", function () {
  assert.isTrue(false);
});

it("expect", function () {
  expect(false).to.be.true;
});

it("should", function () {
  false.should.be.true;
});

Results:

  1)  assert:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true

      at Object.getProperty [as get] (node_modules/chai/lib/chai/utils/proxify.js:67:20)
      at Function.assert.isTrue (node_modules/chai/lib/chai/interface/assert.js:335:31)
      at Context.<anonymous> (test/index.js:8:10)

  2)  expect:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true

      at Object.getProperty [as get] (node_modules/chai/lib/chai/utils/proxify.js:67:20)
      at Context.<anonymous> (test/index.js:12:22)

  3)  should:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true

      at Object.getProperty [as get] (node_modules/chai/lib/chai/utils/proxify.js:67:20)
      at Context.<anonymous> (test/index.js:16:18)

The reason the assert interface has an extra frame compared to should and expect is because each assertion on the assert interface is a wrapper around the actual assertion object that's created to perform the assertion. Currently, the only frames removed from the Assertion error are those beginning with this inner, wrapped assertion. But it wouldn't be too difficult to make Chai use the outer wrapper function as the starting point to remove frames instead. Doing so would make the assert stack traces more consistent with the expect and should by removing the implementation frames.

My initial impression is that we should implement such a change; most of the time, the implementation frames aren't needed, and in the rare occasion that they are, they can be enabled using the includeStack config value.

Regarding the extra proxy frames in v4.0: These are easily removed by using the proxy getter as the first frame to remove instead of the object getter. I'll submit a PR for that in a day or two.

@jaridmargolin
Copy link
Author

jaridmargolin commented Dec 8, 2016

Oh... Great! Thanks for looking into @meeber

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

No branches or pull requests

3 participants