Skip to content

Make image diffing strategies custom scale aware #336

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

Merged

Conversation

moritzsternemann
Copy link
Contributor

This PR resolves the issue described in #324 and partially in #243.

When overriding the scale of image snapshots by setting the displayScale property of the passed trait collection, failure diffs were not rendered correctly because the screens scale was used to interpret the snapshot stored on disk.

To resolve this issue, I introduced a new scale parameter to the image diffing strategies which falls back to the display scale if it's not set or the default undefined value 0.0 of the UITraitCollection.

@ffittschen
Copy link

@mbrandonw @stephencelis do you have an estimate when you will have time to review this PR? Due to this scaling issue the diffs are unusable for us at the moment.

@rcarver
Copy link

rcarver commented Aug 9, 2020

@ffittschen Thanks for this! I merged it into a fork and it worked perfectly.

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to us! Just one small thing if you have time. If not we can make the change after merging.

@stephencelis stephencelis merged commit 641ad58 into pointfreeco:master Sep 1, 2020
regexident pushed a commit to regexident/swift-snapshot-testing that referenced this pull request Sep 18, 2020
* Add scale parameter to image diffing strategies

* Use updated diffing in CALayer, UIView, UIViewController and SwiftUIView strategies

* Pass scale through to CGPath and UIBezierPath strategies

* Re-generate project file
@moritzsternemann
Copy link
Contributor Author

Hey @stephencelis, I just noticed this was merged into master, not into main. Should I open a new PR against main or are you going to handle getting this into the right branch?

YkmLo added a commit to YkmLo/swift-snapshot-testing that referenced this pull request Sep 25, 2020
[Update Fork] Make image diffing strategies custom scale aware (pointfreeco#336)
andreyz added a commit to andreyz/swift-snapshot-testing that referenced this pull request Dec 18, 2020
…_with_test_plans' into main

* origin/master:
  Make image diffing strategies custom scale aware (pointfreeco#336)

* fork1/rtl_support_with_test_plans:
  Update README
  Make configuration directory a subdirectory of file name directory
  Change snapshot directory url when SNAPSHOT_CONFIGURATION_NAME environment variable is available
  Remove layout direction trait override
dflems pushed a commit to dflems/swift-snapshot-testing that referenced this pull request Mar 26, 2021
* Add scale parameter to image diffing strategies

* Use updated diffing in CALayer, UIView, UIViewController and SwiftUIView strategies

* Pass scale through to CGPath and UIBezierPath strategies

* Re-generate project file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants