TV-1914 - React Native CI Maintenance - #158
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:
Warning
|
| Layer / File(s) | Summary |
|---|---|
Shared Metro lifecycle and E2E integration scripts/metro.sh, scripts/run-e2e-*.sh, example/.detoxrc.js, example/e2e/jest.config.js |
The shared script reclaims ports, starts Metro, polls readiness, captures logs, and cleans up processes. Android and iOS E2E scripts use this lifecycle. Detox logging is configurable, and Jest enables forced process termination. |
CI execution and diagnostics .github/workflows/ci.yml, .github/workflows/publish_sample.yml, .github/actionlint.yaml |
CI and sample publishing workflows add concurrency controls and step or job timeouts. Android CI tracks the configured emulator and boot deadline. iOS workflows configure Metro, validate CocoaPods state, conditionally install pods, select labeled runners, and upload logs. |
Publishing and build log support .github/workflows/publish_npm.yml, scripts/build-android-debug.sh, .gitignore |
The npm publish job uses an Ubuntu runner and a job timeout. Android debug builds capture Gradle output. Generated Metro and Android assemble logs are ignored. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Sequence Diagram(s)
sequenceDiagram
participant E2EScript
participant metro_start
participant MetroServer
participant Detox
E2EScript->>metro_start: start Metro on the configured port
metro_start->>MetroServer: reclaim the port and launch Metro
metro_start->>MetroServer: poll the status endpoint
MetroServer-->>metro_start: readiness or process failure
metro_start-->>E2EScript: startup result and captured logs
E2EScript->>Detox: run E2E tests
Detox-->>E2EScript: return test status
Possibly related PRs
- Lucra-Sports/lucra-react-native-sdk#136: Related Detox, Android E2E, and emulator workflow changes.
Poem
A rabbit checks the CI stream,
Metro starts on ports kept clean.
Logs are saved when builds take flight,
Emulators boot within the limit.
Workflows stop before time can climb.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the primary changes to CI workflows and related React Native build and test scripts. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/publish_sample.yml (1)
97-97: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse the locked Ruby toolchain for CocoaPods.
example/Gemfile.lockrecords Bundler 2.5.9 and CocoaPods 1.15.2. Remove the manual installs and runbundle exec pod install --repo-updateso the release uses those locked versions.🤖 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/publish_sample.yml at line 97, Update the CocoaPods setup in the publish workflow by removing the manual Bundler installation and invoking CocoaPods through the locked Ruby dependencies with bundle exec pod install --repo-update. Ensure the workflow uses the versions recorded in example/Gemfile.lock, including Bundler 2.5.9 and CocoaPods 1.15.2.Source: Linters/SAST tools
.github/workflows/ci.yml (1)
192-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the CocoaPods version installed on the runner.
gem install cocoapods(also flagged by zizmor as an ad-hoc package install) pulls whatever is latest at run time, whilePodfile.lockpins an exactCOCOAPODS:version. A version drift between the two produces CocoaPods' well-known "lockfile version higher/lower than executable" warnings/failures and unnecessary lockfile diffs.♻️ Proposed fix
- name: Install cocoapods timeout-minutes: 15 run: | - gem install cocoapods + gem install cocoapods -v "$(grep -m1 'COCOAPODS:' example/ios/Podfile.lock | awk '{print $2}')"🤖 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/ci.yml around lines 192 - 196, Update the “Install cocoapods” workflow step to install the exact CocoaPods version declared by the repository’s Podfile.lock COCOAPODS entry instead of the unpinned latest gem. Keep the existing timeout and installation step structure unchanged.Source: Linters/SAST tools
🤖 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/ci.yml:
- Around line 69-76: Increase the CI job-level timeout-minutes values around the
Android and iOS workflow jobs so they exceed the summed critical-path step
timeouts, including cache-miss paths. Preserve the existing per-step timeout
settings and choose job budgets with enough margin to avoid premature whole-job
cancellation.
- Around line 98-110: Update the stale-emulator cleanup loop’s avd assignment to
tolerate failures from the adb emu avd name pipeline by appending the requested
non-failing fallback. Keep the existing DETOX_AVD_NAME and EMULATOR_PORT
matching and cleanup behavior unchanged so later devices are still processed.
In @.github/workflows/publish_sample.yml:
- Line 69: Add the custom ios label to the actionlint configuration under
self-hosted-runner.labels, creating .github/actionlint.yaml if needed, and
verify that an eligible self-hosted runner is assigned this label so the publish
workflow’s runs-on declaration remains valid and executable.
In `@scripts/run-e2e-android.sh`:
- Around line 2-8: Update the shell setup in scripts/run-e2e-android.sh lines
2-8 and scripts/run-e2e-ios.sh lines 2-7: enable errexit by adding -e to the
existing set -uo pipefail declaration, and guard each cd into the example
directory with || exit 1 so both E2E entry scripts fail fast when the directory
change fails.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 192-196: Update the “Install cocoapods” workflow step to install
the exact CocoaPods version declared by the repository’s Podfile.lock COCOAPODS
entry instead of the unpinned latest gem. Keep the existing timeout and
installation step structure unchanged.
In @.github/workflows/publish_sample.yml:
- Line 97: Update the CocoaPods setup in the publish workflow by removing the
manual Bundler installation and invoking CocoaPods through the locked Ruby
dependencies with bundle exec pod install --repo-update. Ensure the workflow
uses the versions recorded in example/Gemfile.lock, including Bundler 2.5.9 and
CocoaPods 1.15.2.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4b50403a-aeed-40c3-b524-237fc4d83f08
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/publish_npm.yml.github/workflows/publish_sample.yml.gitignorescripts/build-android-debug.shscripts/metro.shscripts/run-e2e-android.shscripts/run-e2e-ios.sh
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
195-223: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse the locked CocoaPods version.
example/Gemfile.locklocks CocoaPods to1.15.2. Removegem install cocoapodsand run both CocoaPods commands withbundle exec.🤖 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/ci.yml around lines 195 - 223, Update the CocoaPods setup in the workflow to rely on the version locked by example/Gemfile.lock: remove the standalone gem install cocoapods step, and invoke both pod repo remove and pod install through bundle exec.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 195-223: Update the CocoaPods setup in the workflow to rely on the
version locked by example/Gemfile.lock: remove the standalone gem install
cocoapods step, and invoke both pod repo remove and pod install through bundle
exec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c0630480-ae1c-4381-a9cd-52d76243fa9b
📒 Files selected for processing (5)
.github/actionlint.yaml.github/workflows/ci.yml.github/workflows/publish_sample.ymlscripts/run-e2e-android.shscripts/run-e2e-ios.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish_sample.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
example/e2e/jest.config.js (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid using unconditional
forceExitas the cleanup fix.
forceExit: truecan hide open Detox or native handles and truncate late diagnostics. Verify which handle still requires this option. Prefer fixing that cleanup path, or scopeforceExitto the CI workaround if it is temporary.🤖 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 `@example/e2e/jest.config.js` at line 12, Remove the unconditional forceExit setting from the Jest configuration and identify and fix the underlying Detox or native-handle cleanup path instead. If forceExit is required only for CI, scope it to the CI-specific configuration or conditional workaround rather than enabling it for all runs.
🤖 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 `@scripts/run-e2e-ios.sh`:
- Line 10: Align the Metro port in the script’s metro_start and Detox build
flow: use port 8081 to match the Xcode build default, or export
RCT_METRO_PORT=8082 before detox build so the built app embeds the same port; do
not set it only for detox test.
---
Nitpick comments:
In `@example/e2e/jest.config.js`:
- Line 12: Remove the unconditional forceExit setting from the Jest
configuration and identify and fix the underlying Detox or native-handle cleanup
path instead. If forceExit is required only for CI, scope it to the CI-specific
configuration or conditional workaround rather than enabling it for all runs.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 43e41356-461a-4083-b830-d36694810c6f
📒 Files selected for processing (4)
example/.detoxrc.jsexample/e2e/jest.config.jsscripts/run-e2e-android.shscripts/run-e2e-ios.sh
| yarn e2e:run-ios | ||
|
|
||
| DETOX_EXIT_CODE=$? | ||
| metro_start "${RCT_METRO_PORT:-8082}" || exit 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 \
'RCT_METRO_PORT|metro_start|reversePorts|8081|8082|packager|Metro' \
. \
--glob '*.sh' \
--glob '*.js' \
--glob '*.yml' \
--glob '*.yaml' \
--glob '*.xcconfig'Repository: Lucra-Sports/lucra-react-native-sdk
Length of output: 6215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(run-e2e-ios\.sh|\.detoxrc\.js|.*\.(xcodeproj|xcworkspace|pbxproj|xcconfig|plist|m|mm|swift|json|js|ts))$' | head -200
printf '%s\n' '--- iOS port and bundle URL references ---'
rg -n -C 6 \
'RCT_METRO_PORT|RCTBundleURLProvider|bundleURL|8081|8082|jsLocation|packager|Metro|e2e:run-ios' \
. \
--glob '!node_modules/**' \
--glob '!build/**' \
--glob '!dist/**' \
--glob '*.sh' \
--glob '*.js' \
--glob '*.ts' \
--glob '*.m' \
--glob '*.mm' \
--glob '*.swift' \
--glob '*.plist' \
--glob '*.pbxproj' \
--glob '*.xcconfig' \
--glob 'package.json'Repository: Lucra-Sports/lucra-react-native-sdk
Length of output: 15055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- iOS Detox configuration ---'
cat -n example/.detoxrc.js | sed -n '1,90p'
printf '%s\n' '--- iOS build and packager configuration ---'
cat -n example/ios/LucrasdkExample.xcodeproj/project.pbxproj | sed -n '300,335p'
cat -n example/ios/LucrasdkExample/AppDelegate.mm | sed -n '35,52p'
cat -n scripts/metro.sh | sed -n '72,105p'
cat -n scripts/run-e2e-ios.sh
printf '%s\n' '--- React Native versions and generated packager environment references ---'
rg -n -C 3 \
'"react-native"|"detox"|RCT_METRO_PORT|\.packager\.env|e2e:build-ios|run-e2e-ios' \
package.json example/package.json example/ios \
--glob '!Pods/**' \
--glob '!node_modules/**'Repository: Lucra-Sports/lucra-react-native-sdk
Length of output: 11139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete Detox configuration ---'
cat -n example/.detoxrc.js
printf '%s\n' '--- dependency versions ---'
rg -n -C 2 \
'"react-native"|"react-native-macos"|"detox"' \
package.json example/package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
printf '%s\n' '--- all iOS port assignments and packager environment writes ---'
rg -n -C 4 \
'RCT_METRO_PORT|packager\.env|jsBundleURLForBundleRoot|localhost:808[12]|808[12]' \
. \
--glob '!node_modules/**' \
--glob '!Pods/**' \
--glob '!build/**' \
--glob '!dist/**'Repository: Lucra-Sports/lucra-react-native-sdk
Length of output: 9006
Align the iOS Metro port before the Detox build.
The Xcode build phase defaults RCT_METRO_PORT to 8081, while this script starts Metro on 8082. Use 8081 here, or export RCT_METRO_PORT=8082 before detox build; exporting it only during detox test does not update the existing app binary.
🤖 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 `@scripts/run-e2e-ios.sh` at line 10, Align the Metro port in the script’s
metro_start and Detox build flow: use port 8081 to match the Xcode build
default, or export RCT_METRO_PORT=8082 before detox build so the built app
embeds the same port; do not set it only for detox test.
2 participants
https://lucrasports.atlassian.net/browse/TV-1914
Summary by CodeRabbit
Bug Fixes
Chores