Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnables Android E2E CI jobs, expands BrowserStack Android codegen tasks, updates PingTestRunner Gradle includes/scripts and APK packaging, bumps Detox, and pins/simplifies Xcode selection and simulator resolution in iOS CI workflows. ChangesAndroid E2E Testing Enablement
iOS Build Tooling Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test-ios.yml:
- Around line 116-124: The current extraction of AVAILABLE using awk '/Available
destinations/,/Ineligible destinations/' | head -n -1 is non-portable on macOS;
update the pipeline that sets AVAILABLE (which reads DESTINATIONS) to use a
single awk invocation that prints lines after the "Available destinations"
header and stops at "Ineligible destinations" if present (i.e., use awk state
flags to skip the two header/footer lines and tolerate a missing "Ineligible
destinations"), so DEST_LINE (which greps DESTINATIONS/AVAILABLE for
"platform:iOS Simulator" and "name:iPhone") receives the correct block on BSD
head.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d896e12c-3403-4dc2-b2f2-a1ff5b7b6e84
⛔ Files ignored due to path filters (2)
.yarn/install-state.gzis excluded by!**/.yarn/**,!**/*.gzyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.github/workflows/browserstack-e2e-android.yml.github/workflows/build-and-test-ios.yml.github/workflows/ci.yml.github/workflows/e2e-tests.ymlPingTestRunner/package.json
💤 Files with no reviewable changes (2)
- .github/workflows/e2e-tests.yml
- .github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test-ios.yml:
- Around line 87-88: Replace the hard-coded xcode-select step ("Select Xcode
version") with the runtime-aware "Resolve Xcode version" logic used in the
e2e-tests workflow (or add an equivalent preflight) so the job verifies a
provisioned iOS simulator runtime before switching Xcode; specifically, reuse
the resolver that determines an Xcode installation with matching simulator
runtimes (the same step that precedes "Resolve simulator destination") and run
that check before calling xcode-select, and then remove or narrow the use of
job-level continue-on-error so simulator/setup failures aren’t silently masked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3da92390-5376-4a9f-a296-3acc5021cb6c
⛔ Files ignored due to path filters (2)
.yarn/install-state.gzis excluded by!**/.yarn/**,!**/*.gzyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/browserstack-e2e-android.yml.github/workflows/build-and-test-ios.yml.github/workflows/ci.yml.github/workflows/e2e-tests.ymlPingTestRunner/android/app/build.gradlePingTestRunner/android/settings.gradlePingTestRunner/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-tests.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yml
- .github/workflows/browserstack-e2e-android.yml
- PingTestRunner/android/settings.gradle
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/build-and-test-ios.yml (2)
108-122: ⚡ Quick winHardcoded iOS version creates coupling risk; derive from XCODE_VERSION instead.
Lines 117, 119, and 120 hardcode
"iOS 26.4"three times. IfXCODE_VERSION(line 19) changes, these must be updated manually. The iOS SDK version should be derived fromXCODE_VERSIONto maintain consistency.Additionally, the UUID regex
[A-F0-9-]{36}on line 117 assumes uppercase hex digits. Whilesimctltypically outputs uppercase UUIDs, relying on case risks silent failure if output format changes.♻️ Proposed fix to derive iOS version and make UUID matching case-insensitive
# Resolve a simulator for Xcode 26.4.1 using xcrun simctl. # xcodebuild -showdestinations only returns placeholders on a fresh CI runner. - # Pick the first iPhone from the iOS 26.4 runtime (matches the active Xcode SDK). + # Pick the first iPhone from the iOS runtime matching the active Xcode SDK. # Sets DESTINATION and SIMULATOR_UDID in GITHUB_ENV for all subsequent steps. - name: Resolve simulator destination run: | + IOS_VERSION=$(echo "${{ env.XCODE_VERSION }}" | grep -oE '^[0-9]+\.[0-9]+') echo "--- xcrun simctl list devices available ---" xcrun simctl list devices available echo "-------------------------------------------" - SIMULATOR_UDID=$(xcrun simctl list devices "iOS 26.4" available | grep "iPhone" | grep -oE '[A-F0-9-]{36}' | head -1) - [ -n "$SIMULATOR_UDID" ] || { echo "No eligible iPhone simulator found for iOS 26.4"; exit 1; } - SIMULATOR_NAME=$(xcrun simctl list devices "iOS 26.4" available | grep "$SIMULATOR_UDID" | sed 's/ *(.*//' | xargs) - echo "Resolved: ${SIMULATOR_NAME} (iOS 26.4) [${SIMULATOR_UDID}]" + SIMULATOR_UDID=$(xcrun simctl list devices "iOS ${IOS_VERSION}" available | grep "iPhone" | grep -ioE '[A-F0-9-]{36}' | head -1) + [ -n "$SIMULATOR_UDID" ] || { echo "No eligible iPhone simulator found for iOS ${IOS_VERSION}"; exit 1; } + SIMULATOR_NAME=$(xcrun simctl list devices "iOS ${IOS_VERSION}" available | grep "$SIMULATOR_UDID" | sed 's/ *(.*//' | xargs) + echo "Resolved: ${SIMULATOR_NAME} (iOS ${IOS_VERSION}) [${SIMULATOR_UDID}]" echo "DESTINATION=platform=iOS Simulator,id=${SIMULATOR_UDID}" >> "$GITHUB_ENV" echo "SIMULATOR_UDID=${SIMULATOR_UDID}" >> "$GITHUB_ENV"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test-ios.yml around lines 108 - 122, The step "Resolve simulator destination" hardcodes "iOS 26.4" and assumes uppercase UUIDs; change it to derive an IOS_RUNTIME from XCODE_VERSION (e.g., take the first two dot-separated components of XCODE_VERSION to form "iOS <major>.<minor>") and use that variable instead of the literal string in all simctl calls and messages (affecting SIMULATOR_UDID and SIMULATOR_NAME resolution and the DESTINATION export), and make the UUID extraction case-insensitive by using a regex like [0-9A-Fa-f-]{36} or using grep -i/-oE so SIMULATOR_UDID=$(xcrun simctl list devices "$IOS_RUNTIME" available | grep "iPhone" | grep -oEi '[0-9A-Fa-f-]{36}' | head -1).
19-19: ⚡ Quick winXcode
26.4.1exists onmacos-26-xlargerunners (current image set); pinnedXCODE_VERSION: '26.4.1'should work.The
macos-26runner image docs list Xcode26.4.1as installed (and it’s the current default symlink), so setup shouldn’t fail for this pin. Consider adding a fallback strategy in case future runner image updates remove/change the installed version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test-ios.yml at line 19, The workflow pins XCODE_VERSION: '26.4.1' which currently exists on macos-26-xlarge but could be removed in future; update the GitHub Actions workflow to implement a fallback strategy around the XCODE_VERSION variable (e.g., try a prioritized list of versions or detect installed Xcode versions and pick the closest match) so job setup in the file will attempt the primary XCODE_VERSION '26.4.1' and then fall back to alternate versions or auto-detect if the primary is unavailable; reference and update the XCODE_VERSION usage and any setup-xcode or xcode-select steps so they honor the fallback/detection logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/browserstack-prep-ios-artifacts.yml:
- Line 42: The workflow uses the env var XCODE_VERSION with a default of
'26.1.1' which conflicts with the other iOS workflow that pins '26.4.1'; update
the default in this workflow to match the pinned version (use '26.4.1') or
otherwise read the centralized var so both workflows resolve the same
XCODE_VERSION, and if you intentionally need a different version, add a brief
comment explaining why BrowserStack prep requires '26.1.1'; locate the
XCODE_VERSION use in the workflow step (the YAML key XCODE_VERSION) and change
the default or add the explanatory comment accordingly.
- Line 66: The fallback loop that iterates Xcode installations (the for app in
"$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -V); do) now sorts
versions oldest-first; restore newest-first fallback by using reverse version
sort so the loop prefers the newest matching Xcode when the pinned XCODE_VERSION
is not suitable—update the sort invocation (used in the for loop that builds the
candidate list) back to sort -rV to ensure the most recent compatible Xcode with
the iOS simulator runtime is selected.
---
Nitpick comments:
In @.github/workflows/build-and-test-ios.yml:
- Around line 108-122: The step "Resolve simulator destination" hardcodes "iOS
26.4" and assumes uppercase UUIDs; change it to derive an IOS_RUNTIME from
XCODE_VERSION (e.g., take the first two dot-separated components of
XCODE_VERSION to form "iOS <major>.<minor>") and use that variable instead of
the literal string in all simctl calls and messages (affecting SIMULATOR_UDID
and SIMULATOR_NAME resolution and the DESTINATION export), and make the UUID
extraction case-insensitive by using a regex like [0-9A-Fa-f-]{36} or using grep
-i/-oE so SIMULATOR_UDID=$(xcrun simctl list devices "$IOS_RUNTIME" available |
grep "iPhone" | grep -oEi '[0-9A-Fa-f-]{36}' | head -1).
- Line 19: The workflow pins XCODE_VERSION: '26.4.1' which currently exists on
macos-26-xlarge but could be removed in future; update the GitHub Actions
workflow to implement a fallback strategy around the XCODE_VERSION variable
(e.g., try a prioritized list of versions or detect installed Xcode versions and
pick the closest match) so job setup in the file will attempt the primary
XCODE_VERSION '26.4.1' and then fall back to alternate versions or auto-detect
if the primary is unavailable; reference and update the XCODE_VERSION usage and
any setup-xcode or xcode-select steps so they honor the fallback/detection
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfdcf23a-d8b3-4b5a-88cc-70ad4fae9cc4
📒 Files selected for processing (2)
.github/workflows/browserstack-prep-ios-artifacts.yml.github/workflows/build-and-test-ios.yml
|
|
||
| env: | ||
| XCODE_VERSION: ${{ vars.XCODE_VERSION || '26.4.1' }} | ||
| XCODE_VERSION: ${{ vars.XCODE_VERSION || '26.1.1' }} |
There was a problem hiding this comment.
Inconsistent default Xcode version across iOS workflows.
This workflow defaults to 26.1.1 when vars.XCODE_VERSION is unset, while .github/workflows/build-and-test-ios.yml (line 19) pins to 26.4.1. Different Xcode versions can produce different build artifacts or test results, causing subtle inconsistencies if both workflows run in the same pipeline without explicit version coordination.
Consider aligning the defaults or documenting why BrowserStack preparation requires an older Xcode version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/browserstack-prep-ios-artifacts.yml at line 42, The
workflow uses the env var XCODE_VERSION with a default of '26.1.1' which
conflicts with the other iOS workflow that pins '26.4.1'; update the default in
this workflow to match the pinned version (use '26.4.1') or otherwise read the
centralized var so both workflows resolve the same XCODE_VERSION, and if you
intentionally need a different version, add a brief comment explaining why
BrowserStack prep requires '26.1.1'; locate the XCODE_VERSION use in the
workflow step (the YAML key XCODE_VERSION) and change the default or add the
explanatory comment accordingly.
| | grep -i "iOS" | grep -v "unavailable" \ | ||
| | grep -oE "[0-9]+\.[0-9]+" | sort -u) | ||
| for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -rV); do | ||
| for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -V); do |
There was a problem hiding this comment.
Sort order change prefers oldest Xcode instead of newest in fallback.
Changing from sort -rV (reverse version order, newest first) to sort -V (forward version order, oldest first) reverses the fallback preference. When the pinned XCODE_VERSION isn't found or lacks the iOS simulator runtime, the loop now selects the oldest matching Xcode instead of the newest.
Preferring older toolchains increases the risk of encountering known bugs, missing optimizations, or deprecated behavior. Fallback logic should typically prefer the newest compatible version.
🔄 Proposed fix to restore newest-first fallback
- for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -V); do
+ for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -rV); do📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -V); do | |
| for app in "$PINNED" $(ls -d /Applications/Xcode*.app 2>/dev/null | sort -rV); do |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/browserstack-prep-ios-artifacts.yml at line 66, The
fallback loop that iterates Xcode installations (the for app in "$PINNED" $(ls
-d /Applications/Xcode*.app 2>/dev/null | sort -V); do) now sorts versions
oldest-first; restore newest-first fallback by using reverse version sort so the
loop prefers the newest matching Xcode when the pinned XCODE_VERSION is not
suitable—update the sort invocation (used in the for loop that builds the
candidate list) back to sort -rV to ensure the most recent compatible Xcode with
the iOS simulator runtime is selected.
1 participant
Summary by CodeRabbit