Repository navigation
fix(ci): add setup_simulator action and standardize Xcode 26 matrix - #1882
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a standardized simulator setup mechanism by adding a .github/actions/setup_simulator/action.yml composite action and a corresponding scripts/setup_simulator.sh script. It also updates scripts/test.sh to automatically invoke the setup script when the required simulator is missing. The review feedback suggests several improvements to make the scripts more robust: explicitly invoking the setup script with bash to handle potential permission issues, using grep -Fq for literal string matching, handling potential leading whitespace in simctl output, cleaning up existing but unavailable duplicate simulators, and restricting automatic setup in test.sh to only Firebase- prefixed devices.
ncooke3
left a comment
There was a problem hiding this comment.
I'll look into addressing above feedback in follow-ups.
Also:
- Script execution permissions: In
.github/actions/setup_simulator/action.ymlandscripts/test.sh, explicitly prependbashwhen invoking./scripts/setup_simulator.shto preventPermission deniedfailures if the script was committed without executable (+x) permissions. - Simulator cleanup & duplicate naming: In
scripts/setup_simulator.sh,xcrun simctl list devices availablemisses existing simulators that are in an unavailable state. Creating a new one can result in duplicate devices with identical names, causing destination resolution ambiguity in Xcode. Clean up unavailable devices matching${sim_name}prior to creation. - Literal matching in grep checks: Use
grep -Fqinstead ofgrep -qwhen checking for device and simulator strings insetup_simulator.shandtest.shto avoid regex parsing quirks on device names. - Leading whitespace in runtime parsing: In
setup_simulator.sh, update the regex matchingxcrun simctl list runtimes availablefrom^${platform_prefix}to^[[:space:]]*${platform_prefix}to guard against format changes or indented output across different Xcode toolchains. - Tracking issue for temporary override: Add a reminder/issue in
quickstart-iosto removecontinue-on-error: ${{ matrix.xcode == '26.2' }}infirestore.ymloncefirebase-ios-sdk 13.2.0lands with upstream fix #16944. - Guarding custom simulator creation: In
scripts/test.sh, restrict the automaticsetup_simulator.shfallback so it only triggers ifDEVICEstarts withFirebase-. If a caller provides a custom device name that is missing, running the default setup script will not create it and will fail anyway. - Increased CI runner minutes: Monitor CI runner consumption now that
fail-fast: falseis enabled across all 14 matrix workflows, as builds will run all permutations to completion even on early failures. - Verification of
firebaseai.ymltoolchain requirements: Confirm that droppingXcode 26.6/iOS 26.5in favor of standardizing onXcode 26.2andXcode 26.4does not remove required coverage for any Swift 6 / modern AI SDK APIs.
|
@paulb777 , the firestore Xcode 26.2 failure is expected. It will be fixed by Andrew's 26.2 trait workaround in upcoming firebase-ios-sdk@13.2.0 |
cedc027 to
2a6c0d1
Compare
GitHub Actions macOS runners vary in pre-installed simulator device types across Xcode releases (e.g., iPhone 15 Pro vs. iPhone 16/17 Pro), causing hardcoded device names in
scripts/test.shand workflow files to fail when upgrading Xcode.This PR adopts the
setup_simulatorcomposite action pattern fromfirebase-ios-sdkto dynamically provision standardized simulator names (Firebase-iPhone-15-Pro,Firebase-Apple-TV-4K-Gen-2,Firebase-Apple-Watch-Ultra-2) matched to the active Xcode SDK runtime, and standardizes all product workflows on Xcode 26.2 (macos-15) and Xcode 26.4 (macos-26).