Do not highlight matched asymmetricMatcher in diffs

[WeiAnAn]

Summary

This fixes #6184.

My implementation is traverse the expected object. If property is asymmetric matcher, check is matched actual or not, then replace actual property with asymmetric matcher.

Code:

test('asymmetricMatcher', () => {
  expect({
    a: 1,
    b: 2,
  }).toEqual({
    a: expect.any(Number),
    b: 1,
  });
});

Result:

Test plan

Add tests with different types of asymmetric matchers.

[jeysal]

I've only had a chance to glance over this quickly, but it looks like this might actually be a decent workaround that is worth it considering the extra maintenance cost, even though of course in the long run it would still be nice to properly solve all cases with better integration of expect/pretty-format/jest-diff as proposed in the issue threads.
One thing we'd definitely need to do here is properly recurse when replacing the asymmatchers, or doing it while deep-copying right away.
We should also consider hiding this behind a jest-diff option since it's special behavior for certain assertions, not standard diffing - expect.any(String) is still not the same thing as 'asdf' after all.
Would love to hear @SimenB's and @pedrottimark's thoughts on this!

[SimenB]

Exciting! Definitely needs @pedrottimark's review. I agree with @jeysal that this behavior needs to be behind an options, as we've tried to keep jest-diff relatively decoupled from Jest itself and usable as a generic diffing module. I like how small this change is, though!

[WeiAnAn]

We can add highlightAsymmetricMatcherInDiff option with default value true.
But it is a little bit long, any other suggestion?

[jeysal]

I suggest ignoreAsymmetricMatches, default false, expect sets it to true.

One thing we'd definitely need to do here is properly recurse when replacing the asymmatchers, or doing it while deep-copying right away.

Do you think this would be easily possible? Test cases would be an asymmatcher deeper down the structure, and probably something nested like expect.objectContaining({a: expect.any(String)})

[pedrottimark]

@WeiAnAn Thank you for contributing to Jest! Here are a few first impressions:

1. To keep specifics out of jest-diff package, can you see if the special logic can move to printDiffOrStringify function in jest-matcher-utils package, before it calls a diff function?
2. In addition to Do not highlight matched asymmetricMatcher in diffs #9257 (comment) by Tim about matchers deeper in the structure, can you please include a test of an object which has circular references, see https://github.com/facebook/jest/blob/master/packages/jest-matcher-utils/src/__tests__/index.test.ts#L51-L55
3. From viewpoint of developer experience, what do you think about the opposite transformation to replace the asymmetric matcher in the expected object with the received value?

  Object {
    "a": 1,
-   "b": 2,
+   "b": 1,
  }

The deepCyclicCopy function in jest-util package is new to me.

Outside the scope of this pull request, we might need to compare it to current utility functions which have some problems with recursive replacements in reports when the following matchers fail:

• toMatchObject
• toMatchSnapshot(properties)
• toMatchInlineSnapshot(properties, snapshot)

P.S. to Simen and Tim: Last week I returned to prototyping a future “relevant comparison” diff and this PR reminded me to add asymmetric matchers to the realistic examples :)

The path property displays received value which passes expect.any(String) matcher

Full color at left and simulated protanopia (no red, 1% of males) at right:

[WeiAnAn]

@jeysal Thank you point out the issue of nested object and circular object. I will add these test cases.

@pedrottimark Thanks for your review.

To keep specifics out of jest-diff package, can you see if the special logic can move to printDiffOrStringify function in jest-matcher-utils package, before it calls a diff function?

Move replacing logic to jest-matcher-utils is better than place it in jest-diff. It should be done before starting diffing.

From viewpoint of developer experience, what do you think about the opposite transformation to replace the asymmetric matcher in the expected object with the received value?

I think if user use asymmetricMatcher, it means user only focus on asymmetricMatcher's condition.
But sometimes, user may also need received value.
Maybe we can keep both expected and received.

  Object {
    "a": Any<Number>(1),
-   "b": 2,
+   "b": 1,
  }

[jeysal]

Any(1)

Omg yes having pretty-format serialize a value that represents an asymmatcher with its matched value would be awesome! But we might want to serialize just the asymmatcher for now and do that in a follow up because it can be separated easily and reduces the size of this PR which will probably already get big with more tests and recursion etc.

[pedrottimark]

@WeiAnAn @jeysal Yes, I agree for this PR, let’s keep the original design to replace received value with serialization of asymmetric matcher.

Tim, related to your #9257 (comment) and mine in #9239 (comment)

• it seems like there is room to brainstorm about future improvements in report template and asymmetric matcher API to include optional concise reason in context when it fails (for example, expect.any(Number) fails because received has String constructor)
• in contrast to Any<Number> I am experimenting with valid ECMAScript as much as possible (for example, new TypeError('"x" is read-only') instead of [TypeError: "x" is read-only])

[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.