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

remove assert message from stack #6061

Merged
merged 2 commits into from
Apr 24, 2018
Merged

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Apr 24, 2018

Summary

We've merged node 10 compat into master (#6055), but because we've landed breaking changes leading up to a v23, we cannot release it.

I think the changes to the message from the assert module (a diff view with swapped + and - from what Jest uses) are large enough that it will confuse users of Jest, so we should make a patch release of Jest 22 with just the removal of that diff.

In this PR, I've basically cherry picked #6055 on top of the 22.4.2 release commit. I've also setup a https://github.com/facebook/jest/tree/22.x branch which this can be merged to.

See this screenshot for example of the output this PR cleans up: image

Test plan

Green CI. node@10.0.0-rc.2 also passes with this change.

@codecov-io
Copy link

codecov-io commented Apr 24, 2018

Codecov Report

Merging #6061 into 22.x will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             22.x    #6061      +/-   ##
==========================================
- Coverage   62.32%   62.31%   -0.02%     
==========================================
  Files         216      216              
  Lines        7905     7907       +2     
  Branches        4        4              
==========================================
  Hits         4927     4927              
- Misses       2977     2979       +2     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-jasmine2/src/assert_support.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83a51cc...987f86f. Read the comment docs.

@cpojer cpojer merged commit 1ea05fb into jestjs:22.x Apr 24, 2018
@SimenB SimenB deleted the node-10-error-compat branch April 24, 2018 09:04
@SimenB
Copy link
Member Author

SimenB commented Apr 24, 2018

@cpojer @mjesun do you think you could do a patch release from the 22 branch now that node 10 is out?

@mjesun
Copy link
Contributor

mjesun commented Apr 24, 2018

I can do that, but I will need a couple of reminders tomorrow 😄

@mjesun
Copy link
Contributor

mjesun commented Apr 24, 2018

@SimenB 22.4.3 and I patch this on top? As 22.4.4?

@SimenB
Copy link
Member Author

SimenB commented Apr 25, 2018

Isn't 22.4.2 the latest? But yeah, just a patch release from the 22 branch (not from master as it contains breaking changes)

jeysal added a commit to spockjs/spockjs that referenced this pull request Jun 7, 2018
jestjs/jest#6061 causes Jest to omit
AssertionError messages in the test output.
This means that without runner-jest converting AssertionErrors
to regular Errors, Jest output now isn't very helpful at all.
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants