Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request improves simulator setup and testing scripts by explicitly invoking them with bash, refining runtime detection to match major and minor versions, and cleaning up existing unavailable simulators before creation. Feedback on these changes suggests optimizing version extraction using Bash's built-in regex matching instead of subshells, correcting backslash escaping in parameter expansion for runtime filtering, and making the UUID regex case-insensitive to robustly handle lowercase characters.
| if [[ -n "$target_major" ]]; then | ||
| runtime=$(xcrun simctl list runtimes available | grep "^${platform_prefix} ${target_major}\." | grep -o "com\.apple\.CoreSimulator\.SimRuntime\.${platform_prefix}-[0-9-]*" | tail -n 1 || true) | ||
| if [[ -n "$target_major_minor" ]]; then | ||
| runtime=$(xcrun simctl list runtimes available | grep -E "^[[:space:]]*${platform_prefix} ${target_major_minor//./\.}([^0-9]|$)" | grep -o "com\.apple\.CoreSimulator\.SimRuntime\.${platform_prefix}-[0-9-]*" | tail -n 1 || true) |
There was a problem hiding this comment.
In Bash parameter expansion replacement strings, a single backslash escapes the following character (e.g., \\. becomes .). To produce a literal backslash in the expanded string (so that grep receives \\. to match a literal dot), you must double the backslash: ${target_major_minor//./\\\\.}.
| runtime=$(xcrun simctl list runtimes available | grep -E "^[[:space:]]*${platform_prefix} ${target_major_minor//./\.}([^0-9]|$)" | grep -o "com\.apple\.CoreSimulator\.SimRuntime\.${platform_prefix}-[0-9-]*" | tail -n 1 || true) | |
| runtime=$(xcrun simctl list runtimes available | grep -E "^[[:space:]]*${platform_prefix} ${target_major_minor//./\\.}([^0-9]|$)" | grep -o "com\.apple\.CoreSimulator\.SimRuntime\.${platform_prefix}-[0-9-]*" | tail -n 1 || true) |
| sdk_ver=$(xcrun --sdk "$sdk_name" --show-sdk-version 2>/dev/null || true) | ||
| if [[ -n "$sdk_ver" ]]; then | ||
| target_major="${sdk_ver%%.*}" | ||
| target_major_minor=$(echo "$sdk_ver" | grep -oE '^[0-9]+\.[0-9]+' || true) |
There was a problem hiding this comment.
Spawning subshells with echo and grep is inefficient and unnecessary in Bash. We can use Bash's built-in regular expression matching ([[ ... =~ ... ]]) to extract the major and minor version numbers directly and more efficiently.
| target_major_minor=$(echo "$sdk_ver" | grep -oE '^[0-9]+\.[0-9]+' || true) | |
| [[ "$sdk_ver" =~ ^[0-9]+\.[0-9]+ ]] && target_major_minor="${BASH_REMATCH[0]}" |
| echo "✅ Simulator '$sim_name' already exists and is available." | ||
| else | ||
| # Clean up any existing (but unavailable) simulators with the same name to avoid duplicates | ||
| xcrun simctl list devices | grep -F "${sim_name} (" | grep -oE '[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}' | while read -r uuid; do |
There was a problem hiding this comment.
UUIDs in simctl output can sometimes contain lowercase letters depending on the environment or how they were generated. Using [0-9a-fA-F] is more robust and prevents potential matching failures.
| xcrun simctl list devices | grep -F "${sim_name} (" | grep -oE '[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}' | while read -r uuid; do | |
| xcrun simctl list devices | grep -F "${sim_name} (" | grep -oE '[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}' | while read -r uuid; do |
Follow-up to #1882 to harden simulator provisioning when multiple point-release runtimes or unavailable duplicate devices exist on shared macOS runners.
This PR updates
scripts/setup_simulator.sh,.github/actions/setup_simulator/action.yml, andscripts/test.shto:major.minorSDK runtime matching (e.g.,iOS 26.4for Xcode 26.4 onmacos-26where26.2,26.4, and26.5runtimes are installed side-by-side) before falling back to major-version matching.grep -Fqdevice matching, tolerate leading whitespace insimctl list runtimes, explicitly invokesetup_simulator.shviabash, and only auto-runsetup_simulator.shfromscripts/test.shwhenDEVICEuses aFirebase-prefix.