-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
.toMatchSnapshot(?string) #2094
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
Conversation
|
I just banged this out & going to bed. I'll check feedback in the morning. |
|
Woohoo! It's not super obvious to me from the diff, but it looks to me like the passed named overrides the entire snapshot name. That means that the snapshot from one test could override the snapshot from another test, right? I think the actual behavior I'd want is for the passed string to be append and used in place of the autoincrementing number. Let me know if I'm misunderstanding the diff! |
|
@jlfwong You know what, you could be right! My use-case is solving the name here: I figured that:
Are you recommending that In my scenario:
|
|
Yep, that is my suggestion -- I would want to be able to use the same snapshot "name" across tests. Although I'm not even a contributor in this repo, so we'll have to wait and see what the owners say! |
|
Awaiting feedback from @cpojer or someone else on next steps. (FYI, Yarn borked the build.) |
|
Hey! I'm sorry, I'm about to embark on a vacation and I will get back to this next week. Overall, lgtm though so happy to merge it when I'm back and make a release :) |
d5f4fa9 to
12b6cbe
Compare
* toMatchSnapshot accepts a custom testName * Update toMatchSnapshot test to accept custom name * Update toMatchSnapshot documentation regarding custom name * Add a note to the API docs.
I find the context of the test name and the snapshot name very useful in cases where you have multiple different Meanwhile I also added the option to add a testName to `toThrowErrorMatchingSnapshot`. I inspired from jestjs#2094, but couldn't really find back where the exact tests are I need to change for this to work. Let me know if I'm on the correct path
* feat(snapshot): concatenate name of test and snapshot I find the context of the test name and the snapshot name very useful in cases where you have multiple different Meanwhile I also added the option to add a testName to `toThrowErrorMatchingSnapshot`. I inspired from #2094, but couldn't really find back where the exact tests are I need to change for this to work. Let me know if I'm on the correct path * update test * fix: make sure currentTestName exists
|
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
In reference to #1392, changes
.toMatchSnapshot()to.toMatchSnapshot(?string).This is primarily so that dynamic tests can have more meaningful snapshot names, compared to having the same name with a different numeric suffix.
See: c46d1b7.
Test plan
npm run watchnpm run jest ./integration_tests/__tests__/toMatchSnapshot-test.jsSee: 8d05bec.