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 10 #7960

Merged
merged 2 commits into from
Feb 22, 2019

Conversation

pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Feb 22, 2019

Summary

For .toHaveLength matcher:

  • Display matcher name in regular black instead of dim color
  • Call ensureExpectedIsNonNegativeInteger function with options argument
  • Display not following Expected length: label
  • Experiment to omit received value if it is equal to the not expected value (am happy if y’all can suggest as an alternative a concise explicit way to communicate that is reason why test fails)

For more information, see discussion with @jeysal in #7795

Test plan

  • Updated 14 snapshot tests
  • Rewrote 2 snapshot tests for .not
  • Added 4 snapshot tests for .rejects and .resolves

See also pictures in following comment

@pedrottimark
Copy link
Contributor Author

Example pictures baseline at left and improved at right

tohavelength

Auuu, there’s a double quote mark in my eye ;)

Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

Already conceptionally approved this in our discussion, found nothing too complain about in this implementation either :) great work!

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@a0834f8). Click here to learn what that means.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7960   +/-   ##
=========================================
  Coverage          ?   64.93%           
=========================================
  Files             ?      255           
  Lines             ?    10021           
  Branches          ?     1365           
=========================================
  Hits              ?     6507           
  Misses            ?     3200           
  Partials          ?      314
Impacted Files Coverage Δ
packages/expect/src/matchers.ts 97.33% <100%> (ø)
packages/jest-matcher-utils/src/index.ts 91.75% <20%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0834f8...12abff0. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Feb 22, 2019

Could we pad the quote character so it plays nice with emojis? Totally unrelated to the fix, of course

@jeysal
Copy link
Contributor

jeysal commented Feb 22, 2019

Could we pad the quote character so it plays nice with emojis? Totally unrelated to the fix, of course

I think doing this is (in general, perhaps not in some controlled environments) a dangerous path to go because it can wreck alignment in the monospace grid. Terminal emulators for good reasons render overlapping characters instead of doing padding magic.

@pedrottimark
Copy link
Contributor Author

Yeah, good thinking. The picture was humorous result from serious attempt at realistic failure because of incorrect expected length related to Unicode.

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

5 participants