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

Printing and Snapshot Fixes #1752

Merged
merged 1 commit into from
Sep 23, 2016
Merged

Printing and Snapshot Fixes #1752

merged 1 commit into from
Sep 23, 2016

Conversation

cpojer
Copy link
Member

@cpojer cpojer commented Sep 21, 2016

Summary
This diff:

  • Updates pretty-format to 4.2.x.
  • Removes function names from snapshots. This has been a frequent source of issues, most recently with a Node.js update and istanbul's coverage plugin. Generally I believe that it is some signal to keep the function names in the snapshot but at the same time it isn't something visible in the UI so we shouldn't include them. Fixes Coverage flag breaks snapshot test #1740.
  • Changes jest-diff to use pretty-format instead of our own JSON.stringify serialization. Fixes Printing Issues #1640 and Use pretty-format instead of jest's stringify #1727.
  • If two objects are compared (toEqual) and serialize to the same value because of calls to toJSON, we now try to create a second diff that doesn't call toJSON. The worst case here is that we'll do twice as much work trying to come up with a diff for the user but it seems like an exceptionally rare case that two objects would look identical in both passes and the best case is that the second diff will actually provide signal to the user. Fixes Disable toJSON in pretty-format it diff is identical. #1728.

Test plan
jest

@@ -68,6 +68,12 @@ test('oneline strings', () => {
expect(stripAnsi(diff('123456789', '234567890'))).toBe(null);
});

test('falls back to not call toJSON if objects look identical', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably worth adding a test case where two objects have the same internal structure as well

'[' + typeof obj + ']'
);
return prettyFormat(object, {
min: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@cpojer cpojer force-pushed the master branch 2 times, most recently from 96a043d to 580f945 Compare September 23, 2016 01:08
…print without toJSON if objects appear similar.
@cpojer cpojer merged commit 6b90297 into jestjs:master Sep 23, 2016
mthmulders pushed a commit to mthmulders/jest that referenced this pull request Oct 10, 2016
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
@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 14, 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.

Coverage flag breaks snapshot test Disable toJSON in pretty-format it diff is identical. Printing Issues
2 participants