-
Notifications
You must be signed in to change notification settings - Fork 36
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
feat: Added a new CLI flag: --snapshot-patch-pycharm-diff #864
feat: Added a new CLI flag: --snapshot-patch-pycharm-diff #864
Conversation
Cool idea! You did what I wanted to but didn't have time to. You're right tho, it's kinda messy. Would this logic be better suited to living in PyCharm or does it absolutely need to be on the syrupy side? |
I have no idea how Jetbrains manages their scripts, but the script in question is just a class that generically receives an "expected" and "actual" objects and formats them. It doubt Jetbrains would rush into adding extra code handling specifically for objects of a single library. Unless they offically add support for custom diff calculations (like a Regardless, I guess it wouldn't hurt to ask around and check their stance on this issue. |
5d80870
to
814a158
Compare
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## main #864 +/- ##
==========================================
- Coverage 98.31% 98.11% -0.20%
==========================================
Files 20 21 +1
Lines 1545 1595 +50
==========================================
+ Hits 1519 1565 +46
- Misses 26 30 +4 |
(Only relevant for Pycharm users) If the flag is passed, Syrupy will import and extend Pycharm's default diff script to provide more meaning full diffs when viewing snapshots mismatches in Pycharm's diff viewer
814a158
to
a4dfcf5
Compare
I've added test coverage and will get this merged. Thank you for the contribution! |
🎉 This PR is included in version 4.7.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
I couldn't determine any contributions to add, did you specify any contributions? |
@all-contributors add @UltimateLobster for code |
I've put up a pull request to add @UltimateLobster! 🎉 |
(Only relevant for Pycharm users) If the flag is passed, Syrupy will import and extend Pycharm's default diff script to provide more meaning full diffs when viewing snapshots mismatches in Pycharm's diff viewer
Description
When running snapshots tests in Pycharm (in fact, when running tests in general in Pycharm), it has this cool feature where you can see the difference between the
expected
and theactual
objects in a diff viewer:After running
test_example
, and looking at Pycharm's console, we can see a link:By clicking the
<Click to see difference>
link we can easily see the difference and know what the problem was:However, lets run some snapshot tests this time:
This time, Pycharm will show an incorrect difference that is hard to parse out:
This happens because Pycharm only sees an
actual
and anexpected
object, it doesn't have a way to know that theactual
objects needs further parsing before the diff calculation.This PR adds a new flag: --snapshot-patch-pycharm-diff. If the flag is active, Syrupy will attempt to import Pycharm's built-in
teamcity-messages
package (which Pycharm automatically adds to PYTHONPATH when running scripts in order to make it importable). If the import was successful, theteamcity.diff_tools.EqualsAssertionError
object (which is responsible to calculating the string representation of theexpected
andactual
objects in the diff view) will be patched.The patched object will behave the same as before. However, if either the
expected
object or theactual
objects are instances ofsyrupy.assertions.SnapshotAssertion
, theexpected
andactual
objects will be replaced by the snapshot's recalled data and asserted data respectively.Now, if we activate the new flag, the diff will be meaningful again:
Related Issues
This PR should resolve #675 as well as the second part of #597
Checklist
Additional Comments
As you might see in the checklist, there are a few missing parts in this PR. This is because I wasn't sure how to go about certain parts of the checklist.