Add Mac Catalyst and iPad/iPhone app on Mac Paywall Validation Screenshot Generation #5371
Conversation
Just testing how Emerge will handle Mac variants.
…lly running in the correct environment.
…e call te emerge.
…scheme, and add a host application to run them. This is in preparation for having these run on Mac Catalyst and Mac Native platforms, which will require a host application.
* Add the RevenueCatUITestsDev as the default build target for the RevenueCatUITestsDev scheme. * Update the Fastfile to use scan, and point at the proper workspace.
…s running on Mac.
Just add the new condition as an "or" in record-and-push-paywall-template-screenshots.
|
Failing iOS 17 tests seem unrelated to this change. |
…alls/send-validation-screenshots-to-emerge
Co-authored-by: JayShortway <29483617+JayShortway@users.noreply.github.com>
…github.com:RevenueCat/purchases-ios into mac-paywalls/send-validation-screenshots-to-emerge
| # "iphone-app-on-mac", | ||
| # "ipad-app-on-mac", |
There was a problem hiding this comment.
Are these supposed to be commented out? I ask because the PR title says that it also adds iPad/iPhone app on Mac validation
There was a problem hiding this comment.
They work locally, but we have not been able to figure out how to get them to run on CI yet because of codesigning issues(@joshdholtz spent a bunch of time on this and wasn't able to get it working). I still think it's useful to include the logic in the commit, because I can run them locally to verify those platforms. But they are commented out so that the CI build doesn't fail on them.
| targeted_device_families = { | ||
| "mac-catalyst-optimized-for-mac" => "1,2,3,4,6", | ||
| "mac-catalyst-scaled-to-match-ipad" => "1,2,3,4" | ||
| } |
There was a problem hiding this comment.
I'm seeing here that 3 is for Apple TV and 4 is for Apple watch. Why are these included in the targeted device families?
There was a problem hiding this comment.
I included those platforms because they were included in the target before this change, and so I was trying not to mess with anything that was already in there.
Ideally the logic here would be to just add/remove that 6 from the list of targets that was included in the build settings already. But it seemed like a much bigger change to read the build settings here and modify them, than to just pass in a hard-coded override. So I felt like it was a better approach to simply include the full list as it was before this change.
|
@ajpallares whoops I had set it to auto-merge when all the checks passed, so when I approved the Emerge screenshot diff it merged! But feel free to leave any requests for changes and I'll open a follow-up PR! |
|
@clindsay3 No problem! I was going to approve anyway after reading the comments. So it's all good! Thanks for the explanations! |
Motivation
Adds validation screenshot generation for our newly supported Mac platforms, so we can have a clear path towards parity with other platforms, and then avoid regressions in the future.
I'm not sure if having two separate "iPhone App running on Mac" and "iPad App running on Mac" variations is truly necessary, but it felt like it couldn't hurt to include them separately. If it becomes burdensome in the future and we find that in practice there are no actual differences between the two, we could remove one of them in the future.
Description
Updates our paywall validation screenshot generation so that it executes in 4 new contexts: