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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expected behaviour of snapshot property matchers #6455

Closed
darrenbritton opened this issue Jun 13, 2018 · 8 comments · Fixed by #6528
Closed

Expected behaviour of snapshot property matchers #6455

darrenbritton opened this issue Jun 13, 2018 · 8 comments · Fixed by #6528

Comments

@darrenbritton
Copy link

🐛 Bug Report

When using property matchers for snapshot testing, if a property matcher is defined within a nested object, then remaining properties are excluded from the generated snapshot.

To Reproduce

Steps to reproduce the behavior:

  1. create a property matcher for a nested object, example:
 expect({ data: { foo: 'foo', bar: 'bar' }}).toMatchSnapshot({ data: { foo: expect.any(String) }});
  1. resulting snapshot will not include bar: '' .e.g:
Object {
  "data": Object {
    "foo": Any<String>,
  },
}

Expected behavior

Expected the following snapshot to be generated:

Object {
  "data": Object {
    "foo": Any<String>,
	"bar": "bar",
  },
}

Link to repl or repo (highly encouraged)

recommended repl seems to be running Jest v22.1.2, so won't work here

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS High Sierra 10.13.4
    CPU: x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
  Binaries:
    Node: 8.9.0 - ~/.nvm/versions/node/v8.9.0/bin/node
    Yarn: 1.6.0 - /usr/local/bin/yarn
    npm: 5.5.1 - ~/.nvm/versions/node/v8.9.0/bin/npm
  npmPackages:
    jest: ^23.0.1 => 23.1.0 
@SimenB
Copy link
Member

SimenB commented Jun 13, 2018

PR most welcome 🙂

@rickhanlonii Told you somebody would need it! :D

@darrenbritton
Copy link
Author

Happy to give this a shot, any preference on the implementation? Should the default behaviour be updated, which will most likely break some existing use cases, or should this be a new method or something else entirely ?

@rickhanlonii
Copy link
Member

Ah, yes, the difficulty here is doing a performant deep merge correctly. I ran into issues with symbols and prototypes when trying to fix this, happy to see someone else give it a shot!

I would say this is a bug fix for the default behavior and push in a minor release (though that may require some snapshot updates, I think that's better than waiting for the next major)

@andys8
Copy link

andys8 commented Jun 22, 2018

Fixing this would be very welcome. Until I found this issue, I thought I'm missing something.

@darrenbritton
Copy link
Author

Pretty swamped in work but managed to take a look for a few hours. Tried utilising the deep merge from some other library's but with no success, I can't even get the relevant tests to pass locally, hard to say when I'll have the time to look at this again so if anyone else wants to give it a go, feel free!

@WaldoJeffers
Copy link

WaldoJeffers commented Jun 28, 2018

I might look into this. I'm having the same issue, and it can be misleading because the test passes, but the snapshot does not store what you think it does (it only stores the custom matchers), making the test quite inefficient.

@swcisel
Copy link
Contributor

swcisel commented Jun 28, 2018

@WaldoJeffers I have a PR out for this issue. Would you want to take a look at it and let me know what you think? I’d really appreciate any feedback people can give me, as it’s my first PR to this project.

@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 12, 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.

6 participants