-
Notifications
You must be signed in to change notification settings - Fork 117
[iOS Screenshots CI] Fix screenshots generation GitHub Action #15822
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
base: trunk
Are you sure you want to change the base?
Conversation
…ine upload rejected: `step` may only contain alphanumeric characters, slashes, dashes or underscores`.
…tifacts: GET ...: 400 Bad Request: Multiple artifacts were found for query: `screenshot-artifacts.tar`. Try scoping by the job ID or name.`.
🚨 Missing path option in the cache plugin to restore 🚨 Error: running "plugin cache post-checkout" shell hook: The plugin cache post-checkout hook exited with status 1`.
…/Products/Debug-iphonesimulator/'`.
… SCREENSHOT_LANGUAGE: unbound variable`.
WooCommerceScreenshots-Runner (3506) encountered an error (Failed to load the test bundle. (Underlying Error: The bundle “WooCommerceScreenshots” couldn’t be loaded. The bundle couldn’t be loaded. Try reinstalling the bundle.`.
… S3_BUCKET: unbound variable` error.
…e in macOS Sonoma and above.
9b6b3c5
to
91cbef9
Compare
… generate-screenshots` error.
… error (Failed to load the test bundle. (Underlying Error: The bundle “WooCommerceScreenshots” couldn’t be loaded` error.
…eenshots. Run `git lfs install && git lfs fetch && git lfs pull` to fix this.` error.
…fonts in Buildkite at least. Try using Helvetica Neue which is more likely to exist.
name: screenshot-runner | ||
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/WooCommerceScreenshots-Runner.app | ||
name: screenshot-build-products | ||
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗒️ There are more files required for later steps than just the WooCommerce.app
and WooCommerceScreenshots-Runner.app
. Otherwise, errors like WooCommerceScreenshots-Runner (19200) encountered an error (Failed to load the test bundle. (Underlying Error: The bundle “WooCommerceScreenshots” couldn’t be loaded
are thrown.
… available in CI macOS images.
…s look good for all locales.
|
CONFIGURE_ENCRYPTION_KEY: ${{ secrets.CONFIGURE_ENCRYPTION_KEY }} | ||
|
||
jobs: | ||
build: | ||
name: Build Application | ||
if: contains(github.event.pull_request.labels.*.name, 'generate screenshots') | ||
runs-on: macos-latest | ||
runs-on: macos-15 | ||
|
||
steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not part of this PR, but an improvement idea:
we should be able to remove the Install Screenshot Gems
step and use this env var at the job level instead (which will make the ruby/setup-ruby@v1
step to already use the screenshots
group without a separate step).
env:
BUNDLE_WITH: screenshots
name: screenshot-runner | ||
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/WooCommerceScreenshots-Runner.app | ||
name: screenshot-build-products | ||
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not worth it for the storage reclaim per se, but maybe worth it to make it clearer that these are discardable adding something like:
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/ | |
path: fastlane/DerivedData/Build/Products/Debug-iphonesimulator/ | |
retention-days: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats on getting it up and working again! 👍
Approving to unblock; just added a couple of minor comments.
And I agree it's best to at some point migrate this to Buildkite and integrate it with the rest of the release automation.
Version |
Closes WOOMOB-721
Why
The iOS screenshots generation CI job via the
generate-screenshots
GitHub label in a PR has been broken for a while since it was added in 2021. The team has been relying on manually generating the raw and promo screenshots and upload them to the App Store. This PR fixes the CI job which is a GitHub Action that public repos have unlimited access for free. We might still want to migrate to Buildkite for better performance in the future, but since this is a job required rather infrequently and without cost, the slow time (3h6m) and 403MB for just the promo screenshots for 17 locales are fine for now.How
Helvetica Neue
instead, sinceProxima Nova
doesn't seem to be available in CI macOS image even though it is supposed to be included in macOS Sonoma+Known issue
When running the CI job with all locales, there were some flaky locales that didn't pass
testScreenshots
for the first time. I also noticed the last screenshot, showing a push notification, is missing the push notification for a subset of locales from the promo-screenshots artifact. For example, theit
5th screenshot in both tablet and phone does not have the notification. I hadn't seen this flakiness locally, and will look into this more separately in WOOMOB-722.Test plan
Example screenshots
Feel free to download en-US screenshots here from GHA artifacts and compare them with the App Store screenshots.
RELEASE-NOTES.txt
if necessary.