expect: clarify errors with .not and equal Expected and Received

[jeysal]

Summary

Currently, errors generated using matchers with .not mostly only show the fact that not is being used in the expect(received).not.toEqual(expected) pseudocode. This leads to (IMO) confusing errors like:

Expected: 42
Received: 42

My idea is to replace the Expected: 42 with what the matcher really expects in this case: Anything other than 42.
Expected and Received are swapped so that "anything else" comes after "42"

The concrete phrasing may be subject to debate ofc, not sure about the "length" and "thrown substring" ones.
Would love to hear what @pedrottimark in particular thinks about this way of handling .not matchers.

Note: There are matchers that can cause Expected and Received to be the same without using .not, e.g. toBeGreaterThan. I also find errors from those somewhat weird, but did not touch them for this PR.

Test plan

Updated snapshots

[pedrottimark]

Thank you for moving forward when I was stuck :)

Here are some questions that have been in my mind:

• Can report omit Expected because it is redundant? Maybe for .not.toHaveLength but not as a general pattern.
• How to balance information that looks weird when correctly written test fails (especially during first phase of TDD) with information that might be useful to debug incorrectly written test? For example, if tester includes .not by mistake or computes incorrect expected value.
• How to repeat that assertion is negative without forcing testers to stop and read? To say it another way, in a way that allows testers to selectively ignore information that is not relevant to them.
• How to make sure that improvements for redundant literal expected values are not regressions for more subtle regexp, substring, or asymmetric matcher?
• What to do for number matchers because .not means opposite direction of interval?

I am working on some pictures of an alternative pattern that we can compare and contrast.

[pedrottimark]

Here are pictures of a pattern for y’all to discuss:

• include explicit not in label for expected value
• inverse highlight occurrence of expected value in received value when possible

Current or near future baseline is at left and improved is at right.

.not.toContain when expected is string:

.not.toEqual when expected is asymmetric matcher:

.not.toEqual when expected is literal array:

.not.toHaveLength seems clear enough (to me :) without Received length:

.not.toMatch when expected is regular expression:

[pedrottimark]

Here are pictures of a pattern for numeric matchers to include in label for expected value either:

• not
• comparison operator

Current or near future baseline is at left and improved is at right.

.toBeCloseTo

.toBeGreaterThanOrEqual

.toBeLessThan

Although I didn’t (yet) provide picture of equal expected and received, this pattern might make it look less weird.

[jeysal]

I had the same idea about showing >, <= etc. for number comparisons - after experimenting a bit I chose to focus on the not parts first.
Note that, unfortunately, not on number comparisons is not as simple as you made it on the right side.
"Not >" is not the same as "<=" because of NaN.

I love the not.toContain / toMatch!

For the ones where Expected and Received are the same (toEqual, toHaveLength) I think I still prefer my solution of not printing the value twice, but instead using any other value etc.
EDIT: Not toHaveLength is not one of those with your idea.

To answer your question

• Can report omit Expected

Omitting it completely would make it worse IMO, I think avoiding duplication with Received (as was the goal with my implementation) is a good intermediate route

With almost all of them, getting the phrasing right is hard - trade-offs between being concise vs. fluent/correct English.

On your question

• How to balance information

I'm not sure, what information would you consider useful in the two cases respectively?

[pedrottimark]

Good point about NaN for number matchers. To be correct is also to be more consistent:

Thank you for patience with my mind going outside scope of this pull request :)

[jeysal]

You should definitely make some PRs with your number, match/pattern and asymmetric matcher improvements.

About to{Be{,InstanceOf},{,Strict}Equal,HaveLength,Throw} (except toThrow string because that is covered by your pattern improvements): Do you prefer putting not after them (I think it sounds awkward) - I'd close this PR then in favor of your upcoming implementation - or would you like to use the any other x style of this PR in some form for those matchers?

[pedrottimark]

Good question, what did I mean? When a test fails, we need useful information to decide:

1. is the assertion incorrect, especially the expected value?
2. why is the received value computed by code incorrect?
3. are both incorrect?

I agree with your first comment that .not in matcher hint is not enough (pardon pun :)

With an Expected … not: pattern for labels:

• it seems like enough-but-not-too-much of visual difference compared to positive matcher
• it is still possible to align the values

It seems like any other … to display expected relative to received:

• can be misunderstood: if expected value is incorrect, then received is correct (see 3 above)
• can seem like double negative: when test fails in first phase of TDD, received is often incorrect

Can we prototype some potential solutions which display:

• Expected … not:
• some non-redundant description of received value that is counterpart to any other …

My gut feeling is when a report provides additional useful information about received value, to omit the directly redundant received value is an alternative to consider.

For example, if .not.toHaveLength displays Expected length not: and Received array: or Received string: then you have useful information to decide:

• is the expected (not) value incorrect?
• why is the received value incorrect?

For a test like .not.toBe referential equality, I think only Expected not: is worth considering.

Thanks for your patience with my slow cooker brain. If I sound stubborn, please forgive me.

[pedrottimark]

For .not.toEqual do you think we can use equality of serialized values as heuristic to distinguish:

• expected which has some abstraction like asymmetric matcher, therefore you might need both expected and received to decide why the test failed
• ordinary deep equality and as I suggest in previous comment, can we think of a short descriptive phrase to say received is same as expected?

[jeysal]

So the pattern you're suggesting would be:

expect(received).not.toEqual(expected);

Expected not: ["asdf"]
Received a value equal to the one that was not expected

And you're looking for a concise phrasing for the Received line?

[pedrottimark]

Yes, can we follow that path of prototyping for a while and then weigh the pros and cons?

[pedrottimark]

I am thankful that you saw problem with reports for .not matchers. They remind me of relevance question that the best reviewer of my technical writing often asked: “why are you telling me this?”

[pedrottimark]

Can you proposed some phrasing patterns about received value for matchers:

• .not.toBe
• .not.toEqual if serializations of expected and received are equal
• .not.toStrictEqual if … are equal
• .not.toHave{BeenCalled|Returned}Times
• .not.toHave{BeenCalled|Returned}With if … are equal
• .not.toHaveLength
• .not.toHaveProperty(keyPath) related to key path
• .not.toHaveProperty(keyPath, value) related to value if … are equal
• .not.toMatchObject if … are equal
• .not.toThrow(object) related to message
• .not.toThrow(class/constructor) related to class/constructor

In preceding negative matchers, when serializations of expected and received are not equal, then report can display expected and received values.

When we have a pattern that scales to the variety of Jest matchers (and community matchers, for bonus points :) then we can move forward with several small and independent pull requests.

Current report seems okay:

• with Received: only for positive and negative matchers: .toBeDefined, .toBeFalsy, .toBeNaN, .toBeNull, .toBeTruthy, .toBeUndefined

Proposed improvement seems okay:

• with comparison operator and not if negative for number matchers: .toBeCloseTo, .toBeGreaterThan, .toBeGreaterThanOrEqual, .toBeLessThan, .toBeLessThanOrEqual
• with inverse highlight for not expected item or substring for matchers: .not.toContain, .not.ToContainEqual, .not.toMatch, not.toThrow(string), .not.toThrow(regexp)

[jeysal]

I'll list a few options with their downsides. Not sure we'll find something that does not allow for any complaints, but maybe something with only downsides we can live with.

---

Expected: ["Alice", "Bob", "Eve"]
Received: ["Alice", "Bob", "Eve"]

Expected length: 3
Received array:  ["Alice", "Bob", "Eve"]

(Current message)
Confusing because it looks like the matcher got what it expected, so why did it throw...

---

Received: ["Alice", "Bob", "Eve"]
Expected: any different value

Received array: ["Alice", "Bob", "Eve"]
Expected:       any other length

(My initial proposal)
Swaps Received and Expected

---

Expected not: ["Alice", "Bob", "Eve"]
Received:     ["Alice", "Bob", "Eve"]

Expected length not: 3
Received array:      ["Alice", "Bob", "Eve"]

(Your initial proposal)
Broken English

---

Expected: not ["Alice", "Bob", "Eve"]
Received:     ["Alice", "Bob", "Eve"]

Expected length: not 3
Received array:      ["Alice", "Bob", "Eve"]

Something after the colon that is not the printed value, might not be a problem because the color highlighting still distinguishes the printed value?

---

Expected: value !== ["Alice", "Bob", "Eve"]
Received: 			["Alice", "Bob", "Eve"]

Expected: length !== 3
Received array:      ["Alice", "Bob", "Eve"]

Messy (+problem of the one above), but on the other hand more in line with the proposed number matchers

---

Unexpected: ["Alice", "Bob", "Eve"]
Received:   ["Alice", "Bob", "Eve"]

Unexpected length: 3
Received array:    ["Alice", "Bob", "Eve"]

Sounds like "unexpected token", not clear what was expected instead.

---

Not expected: ["Alice", "Bob", "Eve"]
Received:     ["Alice", "Bob", "Eve"]

Not expected length: 3
Received array:      ["Alice", "Bob", "Eve"]

Does not work well with "Not expected value", "Not expected length", ...

---

Expected value different from: ["Alice", "Bob", "Eve"]
Received: 				       ["Alice", "Bob", "Eve"]

Expected length other than: 3
Received array:             ["Alice", "Bob", "Eve"]

Long

---

Expected: Not<["Alice", "Bob", "Eve"]>
Received: ["Alice", "Bob", "Eve"]

Expected length: Not<3>
Received array:  ["Alice", "Bob", "Eve"]

Hard to read depending on value, conflates .not with actual asymmetric matchers

[pedrottimark]

Bravo! Expected: not … is small change that makes big difference to my eyes.

Do you also have any ideas for phrasing pattern when received value is referential or deep equal to (not) expected value?

[pedrottimark]

Here are updated pictures for two number matchers:

[jeysal]

Closing in favor of Mark's PRs

[github-actions]

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.