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

expect: Improve report when matcher fails, part 16 #8306

Merged
merged 4 commits into from Apr 13, 2019

Conversation

Projects
None yet
4 participants
@pedrottimark
Copy link
Collaborator

commented Apr 10, 2019

Summary

For toEqual and toStrictEqual matcher:

  • Display matcher name in regular black instead of dim color
  • Display promise rejects or resolves in matcher hint
  • Display // deep equality in dim color at end of matcher hint
  • Omit redundant or distracting information, as described below

When negative assertion fails:

  • Display not between expected label and value
  • Display received value only if it has different serialization

When positive assertion fails:

  • Display either diff (without Difference label) or Expected/Received values, not both
  • Display Received: has same serialization instead of redundant value

Moved code added in #8281 from toBe matcher to print.ts file and edit message string

@jeysal after we have improved reports for all matchers, let’s discuss whether to move the additional logic in shouldPrintDiff to jest-matcher-utils package

Two positive reports changed from diff to Expected/Received:

  • strings which are not both multi-line
  • received array and expected asymmetric matcher do not have same getType value

Residue: stringify (that is, pretty-format with min option)

  • omits constructor name for array or object, so in edge case of received value constructed from different class than expected value, it should have a different serialization
  • which is especially confusing for Number(0) and String('abc')

Test plan

Updated snapshots:

file matcher
4 expect extend toEqual positive hint
2 e2e failures toEqual positive hint including rejects and resolves
1 jest-matcher-utils index toEqual positive hint
5 expect matchers toBe positive display Received: has same serialization
42 expect matchers toEqual positive remove Difference and display Received: has same serialization
48 expect matchers toEqual negative omit redundant Received value
1 expect matchers toStrictEqual positive remove Difference
1 expect matchers toStrictEqual negative omit redundant Received value

See also pictures in following comment with baseline at left and improved at right

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 10, 2019

For negative toEqual when received value has different serialization:

toEqual true !==

For negative toEqual when received value has same serialization:

toEqual true ===

For positive toEqual when received value has different serialization:

toEqual false falsey !==

For positive toEqual when received value has same serialization:

toEqual false falsey ===

For positive toBe when received value has same serialization:

toBe false falsey serialization ===

For positive toEqual which has diff:

toEqual false truthy

For positive toStrictEqual which has diff:

toStrictEqual false truthy

@SimenB

SimenB approved these changes Apr 11, 2019

Copy link
Collaborator

left a comment

Oh yeah! Especially the "not" changes are sweet

@@ -325,7 +325,7 @@ exports[`.toBe() fails for: [] and [] 1`] = `
<dim>If it should pass with deep equality, replace \\"toBe\\" with \\"toStrictEqual\\"</>

Expected: <green>[]</>
Received value has no visual difference"
Received: has same serialization"

This comment has been minimized.

Copy link
@SimenB

SimenB Apr 11, 2019

Collaborator

"serialises to the same string"?

This comment has been minimized.

Copy link
@pedrottimark

pedrottimark Apr 11, 2019

Author Collaborator

Yes, let’s prefer a verb to a noun. What do you think about the following:

Expected: []
Received: different instance that serializes to the same string

And this reminds me to think about a sentence for toEqual and toStrictEqual in ExpectAPI.md to explain that different functions are not deep equal, see #8166

This comment has been minimized.

Copy link
@pedrottimark

pedrottimark Apr 13, 2019

Author Collaborator

I decided on the shorter statement suggested by Simen.

The called function cannot assume that the longer statement is true.

@scotthovestadt
Copy link
Contributor

left a comment

if this is merged soon it can go into the next release :)

@pedrottimark pedrottimark merged commit c7bcefb into facebook:master Apr 13, 2019

10 of 11 checks passed

ci/circleci: test-node-6 Your tests failed on CircleCI
Details
ci/circleci: lint-and-typecheck Your tests passed on CircleCI!
Details
ci/circleci: test-browser Your tests passed on CircleCI!
Details
ci/circleci: test-jest-circus Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
ci/circleci: test-node-8 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
facebook.jest #20190413.8 succeeded
Details

@pedrottimark pedrottimark deleted the pedrottimark:improve-report-16 branch Apr 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.