-
-
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
expect: Highlight substring differences when matcher fails, part 2 #8528
Conversation
Display as diff if either string is multiline: Notice middle dot instead of highlight to avoid misunderstanding when trailing space is not a change: Support single or multiline substring diff for equality comparison of error message: But report for partial match of substring or regexp does not have any highlight: A following #8528 (comment) about rough edges explains why not. |
@@ -3080,6 +3097,26 @@ Expected value: <green>\\"\\\\\\"That <inverse>cat </>cartoon\\\\\\"\\"</> | |||
Received value: <red>\\"\\\\\\"That cartoon\\\\\\"\\"</>" | |||
`; | |||
|
|||
exports[`.toHaveProperty() {pass: false} expect({"children": ["Roses are red. | |||
Violets are blue. | |||
Testing with Jest is good for you."], "props": null, "type": "pre"}).toHaveProperty('children,0', "Roses are red, violets are blue. |
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.
😀
Realistic example to replace space with newline: [By mistake, I reversed expected and received in the added test which is subset of this example] With default With Artificial example to replace space with newline and the opposite: By the way, except for ↵ to solve very edge case of empty common line at beginning or end, it turned out that displaying an symbol for line breaks was often confusing and complicated the code. |
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.
Awesome, as always! 🚀
Should we stick all of these diff utils in jest-diff
instead? jest-matcher-utils
are exposed as this.utils
in custom matchers, and I think it'd make sense to group these diff utils onto this.diff
as a sort of a namespace thing. Thoughts?
And now for rough edge cases for semantic cleanup algorithm.
In the following picture, By the way, the relative length of match versus mismatch is the reason that diff was hit-or-miss in my tests for report when partial match fails, like
In the following picture, the period at the end survived the cleanup: If nothing survives the cleanup, then the report falls back to a line diff: |
@SimenB Super question about packaging! It is especially helpful that you specifically ask about the context object for matchers. The reason I exported only |
Codecov Report
@@ Coverage Diff @@
## master #8528 +/- ##
==========================================
+ Coverage 60.56% 63.14% +2.58%
==========================================
Files 269 271 +2
Lines 11045 11265 +220
Branches 2690 2742 +52
==========================================
+ Hits 6689 7113 +424
+ Misses 3770 3537 -233
- Partials 586 615 +29
Continue to review full report at Codecov.
|
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.
LGTM. Agree with @SimenB on extracting some (especially copied) logic to jest-diff
Is the following code in getType.isPrimitive = (value: unknown) => Object(value) !== value;
export = getType; |
Until we've migrated to default exports, yeah it probably is :/ |
Yup, until the next major |
Thank y’all for suggestion about packaging. Here is minimal interface for now:
In part 3, snapshot matchers also call |
Faithful reviewers, moving files and making changes confused Changed Files:
|
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.
Looks awesome!
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
Add specific diff with highlight to
printDiffOrStringify
for multiline stringsUnless string length is > 20K, it supersedes generic line diff:
toBe
toEqual
toStrictEqual
toHaveProperty(path, value)
Also highlight differences in
toThrow(object)
report if message is not equalTo avoid ambiguity with inverse highlight to mean changed, display middle dot for spaces at end of line for multiline strings in generic print functions:
printExpected
printReceived
Friendly reviewers, take your time and enjoy with a cup or mug of your favorite beverage :)
Test plan
Added unit tests to
jest-matcher-utils
package to achieve coverage:getAlignedDiffs.test.ts
has 24 snapshotsjoinAlignedDiffs.test.ts
has 6 snapshotsprint.test.ts
has 6 snapshots plus another testIn
expect
package:toBe
toEqual
toStrictEqual
toHaveProperty(path, value)
toThrow(object)
See also pictures in following comments
Example pictures baseline at left and improved at right