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 17 #8349

Merged
merged 6 commits into from Apr 21, 2019

Conversation

Projects
None yet
4 participants
@pedrottimark
Copy link
Collaborator

commented Apr 19, 2019

Summary

For toHaveProperty matcher:

  • Display matcher name in regular black instead of dim color
  • Display promise rejects or resolves in matcher hint
  • Replace obscure pass must be initialized error with expected path must not be an empty array
  • 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
  • Display unique report for edge case

When positive assertion fails:

  • If received path is not complete: Display value of last received property
  • If received path is complete: Display either diff (without Difference label) or stringify values, not both

Residue:

  • chore to rewrite getPath as iterative algorithm and always return hasEndProp
  • possible future breaking change (in Jest 26) to remove the edge case that expected value undefined also matches absence of a property with the key path

Test plan

Snapshots for toHaveProperty

  • updated 6 for error
  • updated 24 for fail
  • updated 23 and added 2 for pass

See also pictures in following comment.

Example pictures baseline at left and improved at right

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2019

Negative assertion edge case

The cost of edge case for double meaning of undefined expected value in positive assertion, is Jest got some ’splainin’ to do why the negative assertion must fail

Friendly reviewers, your critique is especially welcome for this message:

Because a positive assertion passes for expected value undefined if the property does not exist, therefore this negative assertion fails unless the property does exist and has a defined value

Show both paths and values:

true undefined unintuitive 1 3

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2019

Negative assertion

Show received value of unexpected property if no expected value:

true array false

true string false

Omit received value if its serialization is equal to expected value:

true array true omit

true string true omit

Show received value if its serialization is not equal to expected value:

true array true show

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2019

Positive assertion

If it has no expected value, then the received path must be incomplete:

false false primitive 0

If the received path is empty, display [] even if expected is string, because "" means [""]

false false object 1

false false object 2

If it has expected value, then the received path might be complete:

false true complete diff

Note context specific - Expected value because - Expected seemed ambiguous

false true complete stringify

If it has expected value, then the received path might be incomplete:

false true incomplete stringify

false true incomplete TDD

This last test represents TDD updated expected path before updating tested code

@SimenB

SimenB approved these changes Apr 21, 2019

SimenB and others added some commits Apr 21, 2019

Update packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
Yes, thank you for improving grammar

Co-Authored-By: pedrottimark <pedrottimark@gmail.com>

@pedrottimark pedrottimark merged commit cd11240 into facebook:master Apr 21, 2019

11 checks passed

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-6 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 #20190421.10 succeeded
Details

@pedrottimark pedrottimark deleted the pedrottimark:improve-report-17 branch Apr 21, 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.