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: Display equal values for ReturnedWith similar to CalledWith #8791

Merged
merged 2 commits into from
Aug 10, 2019

Conversation

pedrottimark
Copy link
Contributor

Summary

Display received value in dim color when it is equal to expected

Test plan

Updated 12 snapshots

long name short name
0 toHaveReturnedWith toReturnWith
1 toHaveLastReturnedWith lastReturnedWith
5 toHaveNthReturnedWith nthReturnedWith

See also pictures in following comments

Example pictures baseline at left and improved at right if it changed

@pedrottimark
Copy link
Contributor Author

negative toHaveReturnedWith assertions

ReturnWIth true 1 3 4

ReturnWith true 3 4 4

positive toHaveReturnedWith assertions

ReturnWith false 1 1

ReturnWith false 2 3

ReturnWith false 4 4

@pedrottimark
Copy link
Contributor Author

negative toHaveLastReturnedWith assertions

LastReturn true 1 3

positive toHaveLastReturnedWith assertions

LastReturn false 0 1b

LastReturn false 0 3

LastReturn false 1 3

@pedrottimark
Copy link
Contributor Author

negative toHaveNthReturnedWith assertions

NthReturn true 2 2

NthReturn true 10 12

positive toHaveNthReturnedWith assertions

NthReturn false 1 1

NthReturn false 1 2

NthReturn false 9 10 - 1

NthReturn false 9 10 - 2

NthReturn false 9 12

@pedrottimark pedrottimark requested a review from jeysal August 6, 2019 20:10
@pedrottimark pedrottimark merged commit 9a15d46 into jestjs:master Aug 10, 2019
@pedrottimark pedrottimark deleted the improve-mock-spy-8 branch August 10, 2019 22:49
@jeysal
Copy link
Contributor

jeysal commented Aug 11, 2019

@pedrottimark Question: For not matchers, wouldn't it make more sense to show the equal values in red? To me, the red means "wrong" rather than "different"

jeysal added a commit to felipepastorelima/jest that referenced this pull request Aug 11, 2019
* upstream/master:
  expect: Display equal values for ReturnedWith similar to CalledWith (jestjs#8791)
  Clearer messages for Node assert errors (jestjs#8792)
  use babel-plugin-replace-ts-export-assignment package (jestjs#8805)
  jest-matcher-utils: Add color options to matcherHint (jestjs#8795)
  chore: Make sure copyright header comment includes license (jestjs#8783)
@pedrottimark
Copy link
Contributor Author

@jeysal That question caused me to wait until after we evaluated the convention for CalledWith because dim for matching versus red for not matching:

  • can show contrast for negative LastReturnedWith or NthReturnedWith assertion fails, and maybe help decide whether the reason is incorrect call number in test, if preceding or adjacent context value also matches (dim) or not (red)
  • cannot show contrast for negative ReturnedWith assertion because report displays only matching values, so I am open to viewpoint that consistency with the other matchers causes confusion or distraction that we can avoid with usual green versus red contrast for this matcher

@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 11, 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.

4 participants