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 positive CalledWith assertion fails #8771

Merged
merged 4 commits into from Aug 4, 2019

Conversation

@pedrottimark
Copy link
Collaborator

pedrottimark commented Aug 1, 2019

Summary

Fixes #7884

For positive toHaveBeen*CalledWith assertions:

  • Display matcher name in regular black instead of dim color
  • Replace sentences with Expected and Received labels
  • Display Number of calls at end of report
  • Display received call arguments on one line separated by commas with dim color if value is equal to expected, except for the following special cases:

Display line diff for received call which corresponds to expected call number:

  • not indented if only one received call
  • indented if multiple received calls

Residue: replace line diff with data-driven diff formatted on one line, when it becomes available

@bandersongit These improvements build on recent pull requests:

  • #8755 for negative toHaveBeen*CalledWith matchers
  • #8710 for toHave*ReturnedWith matchers
  • #8649 for simpler mock-spy matchers

Read the following descriptions with the qualifier if they exist:

matcher report displays calls
toHaveBeenCalledWith first 3 calls, including called with 0 arguments
toHaveBeenLastCalledWith last call and either nearest preceding arguments that are equal to expected; otherwise, next-to-last call
toHaveBeenNthCalledWith nth call and either nearest preceding/following arguments that are equal to expected; otherwise, adjacent calls

Questions

  • What do you think about comma following argument when report contains line diff?
  • Is dim color relevant to toHave*Returned matchers when received value is equal to expected?

Faithful reviewers, this is last in series of basic improvements to all matchers! Thank you ❤️

Test plan

Updated 34 snapshots

long name short name
6 toHaveBeenCalledWith toBeCalledWith
6 toHaveBeenLastCalledWith lastCalledWith
5 toHaveBeenNthCalledWith nthCalledWith

See also pictures in following comments

Example pictures baseline at left and improved at right

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Reports when there are multiple calls, which have primitive arguments:

7884

CalledWith false 4

NthCalled false 2

NthCalled false 9a

NthCalled false 9b

NthCalled false 9c

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Reports when the received function was not called:

CalledWith false 0

LastCalled false 0

NthCalled false 0

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Reports when there is only one call, which has primitive (or zero) arguments:

CalledWith false 1

NthCalled false 1

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Report when there is only one call, which has a line-diffable argument:

LastCalled false 1

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Reports when there are multiple calls, and the -> call has a line-diffable argument:

LastCalled false 2

LastCalled false 3

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Report for positive toHaveBeenCalledWith assertion is only which can display multiple line diffs:

Here is baseline report (which ignores option) versus improved report with --expand option:

CalledWith false 3

Here is improved report with default --no-expand option:

CalledWith false 3 no

@pedrottimark

This comment has been minimized.

Copy link
Collaborator Author

pedrottimark commented Aug 1, 2019

Here is improved report which fixes a mistake in positive toHaveNthReturnedWith for only one call when n is not 1

isOnlyCall

@jeysal
jeysal approved these changes Aug 1, 2019
Copy link
Collaborator

jeysal left a comment

Beautiful <3
Only looked through the snaps since it's quite a lot of code changes, but I think the output is perfect this way, including regarding your questions. Trailing comma FTW :D

@pedrottimark pedrottimark merged commit 2a004bf into facebook:master Aug 4, 2019
10 of 11 checks passed
10 of 11 checks passed
ci/circleci: test-jest-circus 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-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 #20190801.4 succeeded
Details
@pedrottimark pedrottimark deleted the pedrottimark:improve-mock-spy-7 branch Aug 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.