-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Prevent drifting inline snapshots #8424 #8492
Prevent drifting inline snapshots #8424 #8492
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Fixes jestjs#8424 by avoiding indenting inline snapshot if second line of inline snapshot already has been indented.
0417ae6
to
ef5a9e3
Compare
Codecov Report
@@ Coverage Diff @@
## master #8492 +/- ##
==========================================
+ Coverage 62.25% 62.26% +0.01%
==========================================
Files 266 266
Lines 10746 10749 +3
Branches 2625 2628 +3
==========================================
+ Hits 6690 6693 +3
Misses 3471 3471
Partials 585 585
Continue to review full report at Codecov.
|
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.
I like it, thanks!
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.
Awesome, this really bugged me
Also added a short comment to the code describing its purpose.
@scotthovestadt mind taking a look since you build the indentation feature initially? |
@scotthovestadt any news on this??? Drifting inline snapshots are driving us nuts. We are using a lot of them in our project to verify calls to mocks and all unnecessary changes in code creates lots of disturbance when reviewing code. So, we would really like this PR to be merged. |
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.
Thank you for tracking this down and fixing it!
Fix has been merged but not released: jestjs/jest#8492
* chore: patch jest snapshot bug Fix has been merged but not released: jestjs/jest#8492 * chore: bump package-locks
Has this been released? I'm still seeing inline snapshot drifting on 24.9.0 ... |
it's in 24.9 yeah (just check the changelog). if you're still seeing drifting snapshots, please open up a new issue with reproduction steps |
We are using jest 24.9.0 we don't have any problems with drifting inline-snapshots. |
* CSS Module Support * Fix Server-Side Render of CSS Modules * Fix Jest Snapshots jestjs/jest#8492 * Fix snapshots * Add test for CSS module edit without remounting * Add tests for dev and production style being applied * Add missing TODOs * Include/exclude should only be applied to issuer, not the CSS file itself * Add CSS modules + node_modules tests * Test that content is correct * Create Multi Module Suite * Add client-side navigation support for CSS * Add tests for client-side nav * Add some delays * Try another fix * Increase timeout to 3 minutes * Fix test * Give all unique directories
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. |
Fixes #8424 by avoiding indenting inline snapshot if second line of inline snapshot already has been indented.
Summary
As described in #8424, existing and unmodified inline snapshot drifts every time inline snapshots are updated.
This pull request fixes this behavior.
Test plan