-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add support for Jest Snapshots #5567
Conversation
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. 👍
We just noticed this change in a pull request review on github and I consider it a regression. Snapshot tests are often used to accompany meaningful changes. They are more of a blueprint than a fixture in the sense that they can be autogenerated easily and thus may change within a pull request by just adding Quite the contrary, the changes in the snapshots are the meaningful change of the test fixture and are thus to be reviewed specifically. As such I believe it is incorrect to mark them as autogenerated by default, as In github's case there is no way for us to opt out of the default behaviour on an org level for example, making this change even more grave. |
I just found: #3874 (comment) where it was reverted before? It seems like this PR follows the exact same premise: and then #4507 (comment) references |
@joscha Urgh. 😞 I'm so sorry about this. My memory—sharp as it is—isn't infallible, and relying on it to catch these types of scenarios isn't a good idea. We really need to add some sort of record to keep track of important reversions like this, as I predict something like this will happen again in future. In the mean time, I'll revert the change again and also add a regression test to assert that Jest snapshots are not marked generated by default. |
Thank you @Alhadis! |
See: #5567 (comment) References: #3579, #3874, #4507
See: #5567 (comment) References: #3579, #3874, #4507
Description
Support Jest Snapshot files.
Basically JavaScript, but has its own tm-scope to support the highlighting inside `backticks`.
Classify these as generated.
Checklist