-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Call jest-diff and pretty-format more precisely in toHaveProperty matcher #4445
Conversation
packages/expect/src/matchers.js
Outdated
? diff(value, result.value, { | ||
expand: this.expand, | ||
}) | ||
: ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in one line for readability?
? diff(value, result.value, {expand: this.expand})
: '';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good call.
(hasEndProp | ||
? `Received:\n` + | ||
` ${printReceived(result.value)}` + | ||
(diffString ? `\n\nDifference:\n\n${diffString}` : '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\n\nDifference
is this double newline intended? Old code has only one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see from snapshot that it actually looks better now 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: Good eyes! Yes, the double newline is following pattern of toEqual
matcher.
EDIT: It’s not yet clear to me if eager evaluation of diff caused perf problem for examples in #3847 |
…cher (jestjs#4445) * Call diff more precisely in toHaveProperty matcher * Write diff call on one line for readability
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. |
Summary
EDITED:
diff
if and only ifvaluePassed && hasEndProp
pretty-format
for received property value, not for its parent objectlastTraversedObject
if and only iftraversedPath
non-empty and incompleteTest plan
Updated 3 snapshots