Skip to content
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

Add support for snapshotting multiple devices in one go #38

Merged

Conversation

stefanceriu
Copy link
Contributor

@stefanceriu stefanceriu commented Feb 23, 2024

Let me just start by saying that we're absolutely delighted by Prefire so thank you very much for making it happen! 👏

We've been using it for quite some time now in Element X with great success and we would like to take it one step further.

We're currently also using UI tests to snapshot multiple devices and languages but those are more cumbersome to setup and run so we would like to switch them to Prefire too.

We can do so by building custom ViewImageConfigs and simulate other devices no matter what simulator you're running on, which is what this PR aims to add. Different languages can be handled by adding extra xctestplan configurations and running the generated tests multiple times + a small tweak to the tests template to include the language in the screenshot name e.g. https://github.com/element-hq/element-x-ios/blob/develop/UITests/Sources/Application.swift#L74

It seems to be working well for the Prefire Example project and I would try it out in Element X but I'm blocked on #37. I do have another implementation of this based on Prefire 1 which worked just fine so I have no reason to believe it won't do the job.

Hope this all makes sense and that I'm not missing anything.

P.S. I also tried removing checkEnvironments entirely and let you run on any simulator but there's weird text rendering kerning issues between devices that I wasn't able to figure out. For example the iPhone 15 family works no matter which one you chose but iPads an iPhone SE renders text slightly differently, enough to break the tests.

@woutergoossens
Copy link

Really like the PR, would be a great addition ;)

@stefanceriu stefanceriu force-pushed the stefan/multipleDeviceSupportPart2 branch from 0d168b4 to 7ef3ca1 Compare February 26, 2024 08:07
@stefanceriu
Copy link
Contributor Author

Alrighty, with 37 out of the way I had no problem setting this up in Element X, just a few configuration changes. We now have 1000 new shiny snapshots element-hq/element-x-ios@6ea320e

It's especially nice to see one in which the size classes actually matter, like this split view test we have: iPad vs iPhone

Otherwise, I've updated the version and binary. Anything else I need to do before we can get this merged?

@BarredEwe
Copy link
Owner

Let's add information about the new feature in Readme.md?)

@stefanceriu
Copy link
Contributor Author

Oh, of course. I've added something, have a look please

@stefanceriu
Copy link
Contributor Author

Bump 👋

@BarredEwe BarredEwe merged commit 3e9aa58 into BarredEwe:main Mar 2, 2024
3 checks passed
@BarredEwe
Copy link
Owner

@stefanceriu Congratulations on the completed Merge Request 🥳

Thank you so much for such a cool feature! ❤️

@BarredEwe BarredEwe added the enhancement New feature or request label Mar 2, 2024
@stefanceriu
Copy link
Contributor Author

Yey, it was my absolute pleasure! Thank you too! 🥳

@stefanceriu stefanceriu deleted the stefan/multipleDeviceSupportPart2 branch March 2, 2024 19:10
@markst
Copy link
Contributor

markst commented Sep 2, 2024

👏

@markst
Copy link
Contributor

markst commented Sep 3, 2024

#73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants