Skip to content

fix(ci): harden simulator setup script and runtime matching - #1886

Open
ncooke3 wants to merge 1 commit into
mainfrom
nc.ci.sim-hardening
Open

ncooke3 wants to merge 1 commit into
mainfrom
nc.ci.sim-hardening

Conversation

@ncooke3

@ncooke3 ncooke3 commented Oct 28, 2026

Copy link
Copy Markdown
Member

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, and scripts/test.sh to:

  • Prefer exact major.minor SDK runtime matching (e.g., iOS 26.4 for Xcode 26.4 on macos-26 where 26.2, 26.4, and 26.5 runtimes are installed side-by-side) before falling back to major-version matching.
  • Delete any existing unavailable simulators with the same name prior to creation to prevent duplicate-destination ambiguity.
  • Use literal grep -Fq device matching, tolerate leading whitespace in simctl list runtimes, explicitly invoke setup_simulator.sh via bash, and only auto-run setup_simulator.sh from scripts/test.sh when DEVICE uses a Firebase- prefix.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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//./\\\\.}.

Suggested change
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant