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

produces edited stack that starts with error name #699

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

jpbochi
Copy link
Contributor

@jpbochi jpbochi commented Jan 15, 2021

I realized I introduced an issue in my previous PR (#690). This is the fix.

The result of the issue is that assertion messages would be output twice. Below is the output of a failed test on a private repo of mine using jest. Jest was able to point to the right place in the code where the failed .expect happened, but the assertion message was duplicated.

    expected 418 "I'm a Teapot", got 400 "Bad Request"expected 418 "I'm a Teapot", got 400 "Bad Request"

      580 |       });
      581 |       return sendRequest(token)
    > 582 |       .expect(418)

After some digging, I realized that the new stack being built started with error.message, and it should really start with error.toString(). See the real-life example below. The error stack starts with Error: and then it continues with its .message.

> new Error('line two\nline one').stack
'Error: line two\n' +
  'line one\n' +
  '    at REPL48:1:1\n' +
…

I verified this code against tests running in another repo and displays assertion messages correctly. As far as I could test, only jest was impacted, but I would not be surprised that this impacts mocha and others, too.

Again, I'm so sorry about this. @niftylettuce, can you please review this and release a patch version? Thank you.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 483

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.447%

Totals Coverage Status
Change from base Build 481: 0.0%
Covered Lines: 146
Relevant Lines: 150

💛 - Coveralls

@niftylettuce niftylettuce merged commit 6f3928f into ladjs:master Jan 15, 2021
@niftylettuce
Copy link
Collaborator

v6.1.1 published and I have deprecated v6.1.0

@jpbochi
Copy link
Contributor Author

jpbochi commented Jan 15, 2021

thank you very much :)

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.

3 participants