Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRemoved the repository's E2E GitHub Actions workflow file and updated the example RN harness config to add CI-derived iOS simulator defaults and reduce the CI bridge timeout to 120000 ms. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
5b2ec7b to
04ad6b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/e2e-tests.yml:
- Around line 57-66: The Boot iOS Simulator step may leave DEVICE_ID empty
causing later cryptic failures; add a guard after determining DEVICE_ID (the
variable used to set SIMULATOR_DEVICE_ID and passed to xcrun simctl boot) that
checks if DEVICE_ID is empty and if so echo a clear error message and exit with
non-zero status (fail fast) instead of proceeding; ensure the check is placed
before the xcrun simctl boot "$DEVICE_ID" || true line so that failures are
explicit and the GitHub Actions job stops when no simulator is found.
- Around line 49-55: The cache key uses hashFiles(...) with glob patterns that
don't exist, causing cache misses; update the patterns in the
xcode-deriveddata-e2e cache key (the key construction using hashFiles in the
Cache Xcode DerivedData step) to match real iOS sources by replacing the
non-existent src/**/*.swift, src/**/*.mm, src/**/*.cpp, and
nitrogen/generated/ios/** patterns with
packages/react-native-nitro-device-info/ios/** and
packages/react-native-nitro-device-info/src/DeviceInfo.nitro.ts so the hashFiles
call produces a real hash and the xcode-deriveddata-e2e-${{ runner.os }}-... key
becomes specific.
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yml (1)
91-94: Piping totail -20discards build diagnostics needed for debugging failures.GitHub Actions' default
pipefailensures a failedxcodebuildstill fails the step, so correctness is fine. However, only the last 20 lines of output are preserved — the actual compiler error or linker failure is likely earlier in the log and will be invisible.Consider using
teeto preserve the full log as an artifact, or a tool likexcbeautify/xcprettythat filters noise while keeping errors visible.Example: preserve full log and show tail
- build | tail -20 + build | tee /tmp/xcodebuild.log | tail -50Then optionally upload
/tmp/xcodebuild.logas an artifact on failure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/e2e-tests.yml:
- Around line 119-127: The pre-bundling step is ineffective because the workflow
builds with -configuration Debug so AppDelegate uses
RCTBundleURLProvider.sharedSettings().jsBundleURL(forBundleRoot: "index") to
load from Metro, not the pre-bundled Bundle.main.url(forResource: "main",
withExtension: "jsbundle"); either remove the "Pre-bundle Metro JS" step
entirely or change the build invocation to use -configuration Release so the app
will load ios/build/main.jsbundle at runtime (update the workflow build step
that currently sets -configuration Debug accordingly).
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yml (1)
107-117: Build step improvements look solid, but verifytaildoesn't mask build errors.
set -o pipefailon line 107 ensures xcodebuild's exit code propagates through the| tail -40pipe — this is correct. However, note that only the last 40 lines of output will be visible. If xcodebuild fails with an error earlier in the output, the relevant error message may be truncated and not visible in the CI logs, making debugging harder.Consider using
xcprettyor increasing thetailline count (e.g.,tail -100) to ensure error diagnostics aren't lost, or alternatively capture the full log to a file and upload it as an artifact on failure.
Summary by CodeRabbit