Skip to content

Users should be able to define a manualScale instead of being locked into the mainScreen.scale - #15

Closed
regnerjr wants to merge 3 commits into
uber:masterfrom
regnerjr:manual-scale-support
Closed

Users should be able to define a manualScale instead of being locked into the mainScreen.scale#15
regnerjr wants to merge 3 commits into
uber:masterfrom
regnerjr:manual-scale-support

Conversation

@regnerjr

Copy link
Copy Markdown

In order to stay flexible, users of this library "need" to upload two version of there reference images to keep tests passing on various environments (Other Developers, CI, etc)

This proposal would not lock users in to always using the "one true simulator" to run their tests on.

We've recently adopted Snapshot testing at my shop and I don't really want to be the the code review czar ensuring every new PR which adds a snapshot has one at @2x and @3x. Other cancers are the size of our git repo growing over time to support 2 screenshots for each new test.

Currently the framework uses [[UIScreen mainScreen] scale] for saving and comparing snapshots of views and layers.

If the view or layer are meant to be displayed at a different scale that doesn't match the main screen's one, the screenshots get resized (minified / magnified) and the comparisons are not correct any more from the point of view of the user, since the images are different from what will be shown to the user.

This PR tries to remove this limitation for those special cases by allowing the test developer to manually override the scale that will be used.


This work was originally proposed by @djrtl as facebookarchive/ios-snapshot-test-case#166


Thanks for doing the original implementation @djrtl, I hope I did not overstep by re-opening this here now that this lib is under new ownership.

MatteoMecucci-TomTom and others added 2 commits February 21, 2018 15:10
* Manual scale wasn't propagated properly to image capture and reference image selection methods
@CLAassistant

CLAassistant commented Feb 21, 2018

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 3 committers have signed the CLA.

❌ BorysZhdanov-TomTom
❌ regnerjr
❌ MatteoMecucci-TomTom
You have signed the CLA already but the status is still pending? Let us recheck it.

This better reflects the duality between it an `UIScreen.scale` which
has type `CGFloat`
@djrtl

djrtl commented Feb 22, 2018

Copy link
Copy Markdown

@regnerjr of course no problem, go ahead 👍

@alanzeino

Copy link
Copy Markdown
Collaborator

I'll take a look at this soon.

@alanzeino

Copy link
Copy Markdown
Collaborator

Is this resolved here?

facebookarchive/ios-snapshot-test-case#166 (comment)

@m-ruhl

m-ruhl commented Mar 22, 2018

Copy link
Copy Markdown

@alanzeino
The described problem seams to be fixed.
Unfortunately with the scaling the images have most of the time little differences, and are failing therefore.

@regnerjr

Copy link
Copy Markdown
Author

Thanks for the support on this Alan. 👍 Really glad you are supporting this project.

I'm thinking perhaps a better approach (instead of having a fixed size window and trying to do all of our snapshotting in there) will be to do snapshots for all devices we support.
(Current project is iPhone only, only Portrait orientation).
I've been thinking of adopting the technique of running the snapshot tests in their own, test target and making a build for each device and testing all 4 configs iPhoneX, iPhone 5.5", iPhone 4.7", and iPhone 4".

Glad we've got a fix for that issue sorted out (each just needs it's own build folder) #37

Cheers. Keep up the good work.

@regnerjr regnerjr closed this Jul 31, 2018
@Weizzz

Weizzz commented Aug 27, 2020

Copy link
Copy Markdown

It seems the capability for using manualScale is no longer possible and it's back to using [[UIScreen mainScreen] scale].

@alanzeino

Copy link
Copy Markdown
Collaborator

It seems the capability for using manualScale is no longer possible and it's back to using [[UIScreen mainScreen] scale].

What did you see when debugging the manualScale?

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.

8 participants