Add a failing test for printSnapshotAndReceived incorrectly recognizing sometimes indented lines as changed #9863
Conversation
…ng sometimes indented lines as changed
| <t>+ Received + 2</> | ||
|
|
||
| <m>- **some plugin-generated string with custom leading whitespace**</> | ||
| <t>+ **some plugin-generated string with custom leading whitespace**</> |
There was a problem hiding this comment.
this should not be detected as changed because it's exactly the same line in both snapshot and received values
|
Thanks @Andarist, failing tests are the best bug reports ever! 😀 @pedrottimark @jeysal any ideas here? |
|
Just an additional note because it might not be super clear while skimming through my original post - this has changed between Jest 24 and Jest 25. Jest 24 did not report those false positives. |
|
Either I messed up the merge or this is fixed. @Andarist any idea? 😀 |
Codecov Report
@@ Coverage Diff @@
## main #9863 +/- ##
=======================================
Coverage 68.47% 68.48%
=======================================
Files 324 324
Lines 16968 16968
Branches 5060 5060
=======================================
+ Hits 11619 11620 +1
+ Misses 5317 5316 -1
Partials 32 32
Continue to review full report at Codecov.
|
|
@SimenB This PR was sent quite some time ago - there is a high chance of this being fixed now so perhaps this can be closed. I would probably recommend trying to check if there is already a test for this because if there is not then maybe it's still worth landing this PR so this test could act as a regression test for the future. I'm afraid that I won't have enough time to look into this myself though. |
|
I'm happy to land this as a regression test 🙂 |
|
Ah wait no, the test demonstrates the error, it wasn't a failing test. Makes sense! Ref #9863 (comment) |
|
Ah, so the problem is still here - I just didn't make it a failing test case back then but rather a test showcasing an invalid behavior, did I get this right (I'm refreshing my own memory about this issue)? |
|
yeah, seems like it to me 🙂 |
|
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
|
bump for stale bot |
|
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
|
bump for stale bot |
|
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
Summary
It has been reported to Emotion that each printed style property is being shown as changed in Jest 25 when anything changes between snapshots & received: emotion-js/emotion#1847
It turned out that we currently ignore
indentationbecause we useOldPlugininterface and we just print retrieved styles with always indented. We plan to fix this soon, but I believe the issue is still worth fixing in Jest itself - especially that the comment here:https://github.com/facebook/jest/blob/32aaff83f02c347ccd591727544002490fb4ee9a/packages/jest-snapshot/src/printSnapshot.ts#L316-L317
suggests that this was supposed to be handled.
So far I've just added a failing test and would like to first check-in if there is an interest in fixing this before I dive into this further. I've also actually tried to fix this quickly by calling
dedentLineson this:https://github.com/facebook/jest/blob/32aaff83f02c347ccd591727544002490fb4ee9a/packages/jest-snapshot/src/printSnapshot.ts#L324
but while it has fixed my issue it has changed 2 other tests and it would require further investigation why this is happening and how the fix could be improved.