Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not highlight passing asymmetric matchers in diffs #6184

Closed
MarkyMarkMcDonald opened this issue May 15, 2018 · 9 comments 路 Fixed by #9257
Closed

Do not highlight passing asymmetric matchers in diffs #6184

MarkyMarkMcDonald opened this issue May 15, 2018 · 9 comments 路 Fixed by #9257

Comments

@MarkyMarkMcDonald
Copy link

MarkyMarkMcDonald commented May 15, 2018

馃殌 Feature Proposal

This is another idea related to more helpful failure diffs in the same vein as #6170.
The diff of a failing expectation currently highlights nested usages of expect.objectContaining and expect.anything because they are not "equal" to whatever was received. I propose only printing out a diff if the matcher fails.

Motivation

Both this issue and #6170 are the product of a pairing session where we had a hard time tracking down what the actual error was when an existing test started failing. I think that test could have been broken up into several more specific tests or used chained expectations, but it would have been nice to pinpoint the failing property right away.

Example

Here's an example of a failing test that uses a combination of matchers:

https://repl.it/repls/DifficultSvelteMonitors
image

The only property that doesn't match is "rank" due to a type mismatch, but it's hard to see that when scanning the failure diff because the nested matchers are displaying.

The desired approach would look like this:
image

Pitch

Why does this feature belong in the Jest core platform?

This is a change to the default reporter that will help identify the cause of failures faster.

@rickhanlonii
Copy link
Member

Oh this is great, so basically when doing the diff, also compare the asymmetric matchers

What do you think @pedrottimark?

@pedrottimark
Copy link
Contributor

Yes, serialize-then-diff is barrier to solve some obvious problems. See also #7027

This has been in my slow cooker for a while with limited progress. Happy to hear your thoughts.

@grosto
Copy link
Contributor

grosto commented Feb 9, 2019

This one sounds like fun. I can give it a try if nobody is actively working on it.

@pedrottimark
Copy link
Contributor

@grosto Super! As a first step, so we can discuss what a solution looks like independent of how to achieve it, can you and anyone else subscribed to this issue either:

  • identify existing tests which display confusing diff when they fail
  • develop some baseline tests to illustrate problems

Tomorrow I will share some thoughts about what and how for you to critique.

@jeysal
Copy link
Contributor

jeysal commented Feb 10, 2019

Well the minimal one is

expect({
  a: 'a',
  b: 'b',
}).toEqual({
  a: 'x',
  b: expect.any(String),
})

@grosto
Copy link
Contributor

grosto commented Feb 11, 2019

@pedrottimark Sounds great!

I am a bit busy for the next 2-3 days. As soon as I find the time, I will make a PR with simple tests which demonstrate the current diff issues.

@dortamiguel

This comment has been minimized.

@markmsmith
Copy link

I can offer another use case in support of the output being more specific.
When using expect.objectContaining it can be really hard to see what's changed when the structure is really large, such as when checking the headers of a Restify Request or Response object.
It would be much easier to see the mismatch if the output only printed the fields that you're attempting to partially match on.
For example:

 expect(myFnSpy).toHaveBeenLastCalledWith(
  expect.objectContaining({
    env: 'test',
    req: expect.objectContaining({
      method: 'POST',
      headers: expect.objectContaining({
        'x-amzn-trace-id': expect.any(String),
        'x-forwarded-proto': 'https',
        'x-forwarded-for': 'some_client_ip'
      })
    })
  })
);

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants