Change fail messages for toBeGreaterThan, toBeLessThan matchers #7134
Conversation
Also changed toBeGreaterThanOrEqual, toBeLessThanOrEqual Better reflects the fail condition Closes #7105
packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #7134 +/- ##
=======================================
Coverage 66.43% 66.43%
=======================================
Files 253 253
Lines 10658 10658
Branches 4 3 -1
=======================================
Hits 7081 7081
Misses 3576 3576
Partials 1 1
Continue to review full report at Codecov.
|
|
Updated, thanks for the tips and direction @rickhanlonii @natealcedo. |
| @@ -587,462 +587,462 @@ Received: <red>undefined</>" | |||
| exports[`.toBeGreaterThan(), .toBeLessThan(), .toBeGreaterThanOrEqual(), .toBeLessThanOrEqual() equal numbers: [-Infinity, -Infinity] 1`] = ` | |||
| "<dim>expect(</><red>received</><dim>).</>not<dim>.toBeGreaterThanOrEqual(</><green>expected</><dim>)</> | |||
|
|
|||
| Expected: <green>-Infinity</> | |||
| Expected value not to be greater than or equal: <green>-Infinity</> | |||
| Received: <red>-Infinity</>" | |||
There was a problem hiding this comment.
Hm, now we don't align those 2 values so it's actually harder to compare. How about
Expected: 0 not to be greater than or equal to
Received: 0
There was a problem hiding this comment.
I don't agree this makes the assertion error any clearer. Why should we repeat the name of the matcher when listing out the values given to Jest? With this change we say "to be greater than" et.al. twice, which (IMO):
- feels redundant (we already have
expect(received).toBeGreaterThan(expected)in the error) - distracts from the assertion error (it doesn't align, I have to read more text to figure out what went wrong)
- is very inconsistent with how other matchers are formatted in Jest (I can't think of a single case where it's not
Expected: *value*\nReceived: *value*when comparing values)
jeffreypriebe
deleted the
feature/improve-toBeGreaterThan-fail-message
branch
|
@SimenB what about |
|
Hah, would you look at that! I think that's redundant as well 😅 However, that means this PR wasn't as inconsistent with the other matchers as I thought. Maybe we can land this together with an overall look at more matcher, like @pedrottimark is doing in #7152? If all matchers followed the same general way of reporting assertion errors, I'd be happy to remove my objection to this PR |
|
Yes, reports for the following matchers will get more concise labels in coming weeks:
|
|
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. |
Also changed toBeGreaterThanOrEqual, toBeLessThanOrEqual
Better reflects the fail condition
Closes #7105
Summary
When testing with toBeGreaterThan matcher, getting 'Expected: ${expected}' text doesn't jibe with mental expectation.
Test plan
Test snapshots updated.
Note: current master has failing tests, this PR only touches the relevant snapshots.