ci: add PR gate and react-native-harness E2E workflows - #6
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds React Native Harness configuration, WebView E2E smoke tests, a local HTTP test server, Android execution scripting, and GitHub Actions workflows for host, native, Android, and iOS validation. ChangesHarness E2E and native CI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant App as Native example app
participant Harness as React Native Harness
participant Server as e2e-server.mjs
CI->>App: build and launch simulator/emulator app
Harness->>Server: poll health and request test page
Server-->>Harness: readiness, HTML, or HTTP error response
Harness-->>CI: report E2E result and failure artifact
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 7
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
24-25: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout steps. These jobs only read the repo and never push, butactions/checkoutdefaults to persisting theGITHUB_TOKENin.git/config, where the many subsequent untrusted build steps (yarn install,pod install,gradlew, npm lifecycle scripts) can read it. Disabling persistence removes that exfiltration surface (zizmorartipacked).
.github/workflows/ci.yml#L24-L25: addwith: persist-credentials: falseto the checkout step..github/workflows/ci.yml#L49-L50: same..github/workflows/ci.yml#L83-L84: same..github/workflows/ci.yml#L111-L112: same..github/workflows/ci.yml#L165-L166: same..github/workflows/ci.yml#L219-L220: same..github/workflows/e2e.yml#L29-L30: same..github/workflows/e2e.yml#L133-L134: same.🤖 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 24 - 25, Update every actions/checkout@v4 step to configure persist-credentials: false under its with options: .github/workflows/ci.yml lines 24-25, 49-50, 83-84, 111-112, 165-166, and 219-220, plus .github/workflows/e2e.yml lines 29-30 and 133-134.Source: Linters/SAST tools
example/scripts/run-harness-android-ci.sh (1)
39-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFail fast when the e2e-server never becomes healthy. Both readiness loops
breakon success but do nothing when the 30s budget is exhausted, so a server that failed to start falls through to the harness run and surfaces only as an opaque bridge timeout — wasting the fullTEST_TIMEOUTand obscuring the real cause.
example/scripts/run-harness-android-ci.sh#L39-L42: track readiness in a flag andexit 1(after echoing a clear message) if the loop finishes without a 200 from/health..github/workflows/e2e.yml#L214-L217: apply the same fail-fast check to the inline iOS readiness loop before invokingyarn test:e2e:ios.🤖 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/scripts/run-harness-android-ci.sh` around lines 39 - 42, Update the Android readiness loop in example/scripts/run-harness-android-ci.sh (lines 39-42) to track whether /health succeeds with HTTP 200, then echo a clear failure message and exit 1 when all 30 attempts are exhausted. Apply the same readiness flag, success check, message, and exit behavior to the inline iOS loop in .github/workflows/e2e.yml (lines 214-217) before yarn test:e2e:ios 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 @.github/workflows/e2e.yml:
- Around line 91-100: Remove the -no-snapshot-save option from the
emulator-options in the “Create AVD and generate snapshot” step. Leave the other
emulator options unchanged and retain -no-snapshot-save only on the test run
step.
In `@example/e2e-server.mjs`:
- Around line 32-34: Update the request handler in e2e-server.mjs to handle `/`
explicitly with the existing 200 HTML response, while preserving the `/health`
and `/notfound` routes; change the fallback branch to return HTTP 404 for all
unknown paths.
- Line 8: Use the resolved E2E port consistently across the server and harness:
update example/e2e-server.mjs at lines 8-8 as the source of the port value,
derive the WebView host URL from that same value in
example/src/__tests__/e2eServer.ts at lines 8-9, and update the readiness
polling in example/scripts/run-harness-android-ci.sh at lines 38-41 to target
the resolved port instead of 8099.
In `@example/jest.config.js`:
- Around line 2-3: Remove the global react-native-harness preset and
harness-specific testMatch from the Jest configuration used by the default test
command, so host suites such as App.test.tsx remain discoverable. Move those
settings into the test:e2e* Jest invocation or its dedicated configuration while
preserving Harness test matching for end-to-end runs.
In `@example/rn-harness.config.mjs`:
- Around line 28-29: Align the AVD identifier used by the React Native Harness
configuration with the workflow’s emulator name: update the `device`
configuration around `androidEmulator` to use the workflow’s default `test`, or
configure the workflow’s `reactivecircus/android-emulator-runner` step with
`avd-name: e2e_avd`. Ensure both sides reference the same AVD without adding
creation or rename logic.
In `@example/scripts/run-harness-android-ci.sh`:
- Around line 45-54: Define an absolute artifacts directory or crash-log path
before the working directory changes, then update the setup, artifact handling,
and cleanup logic around the relevant initialization, line 30, and cleanup() to
use it. Ensure cleanup writes to the same absolute path expected by the
workflow’s failure-upload step, regardless of the current directory.
In `@example/src/__tests__/postmessage.harness.tsx`:
- Around line 9-22: Add an assertion after rendering in the “mounts with a
hybridRef and onMessage wired” test that waits for ref.current to become
non-null, using waitFor and expect. Keep the existing render and unmount flow
unchanged.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 24-25: Update every actions/checkout@v4 step to configure
persist-credentials: false under its with options: .github/workflows/ci.yml
lines 24-25, 49-50, 83-84, 111-112, 165-166, and 219-220, plus
.github/workflows/e2e.yml lines 29-30 and 133-134.
In `@example/scripts/run-harness-android-ci.sh`:
- Around line 39-42: Update the Android readiness loop in
example/scripts/run-harness-android-ci.sh (lines 39-42) to track whether /health
succeeds with HTTP 200, then echo a clear failure message and exit 1 when all 30
attempts are exhausted. Apply the same readiness flag, success check, message,
and exit behavior to the inline iOS loop in .github/workflows/e2e.yml (lines
214-217) before yarn test:e2e:ios 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6918d29f-6dd7-4e3a-b0f9-8666c546ac3e
⛔ Files ignored due to path filters (3)
example/yarn.lockis excluded by!**/yarn.lock,!**/*.locknitrogen/generated/ios/NitroWebview-Swift-Cxx-Bridge.hppis excluded by!**/generated/**nitrogen/generated/ios/NitroWebview-Swift-Cxx-Umbrella.hppis excluded by!**/generated/**
📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/e2e.ymlREADME.mdexample/.gitignoreexample/e2e-server.mjsexample/jest.config.jsexample/metro.config.jsexample/package.jsonexample/rn-harness.config.mjsexample/scripts/run-harness-android-ci.shexample/src/__tests__/e2eServer.tsexample/src/__tests__/httperror.harness.tsxexample/src/__tests__/load.harness.tsxexample/src/__tests__/postmessage.harness.tsx
| test('mounts with a hybridRef and onMessage wired', async () => { | ||
| const ref: { current: NitroWebViewType | null } = { current: null } | ||
| const { unmount } = await render( | ||
| <NitroWebView | ||
| style={{ flex: 1 }} | ||
| source={{ uri: `${E2E_BASE}/` }} | ||
| hybridRef={callback((r) => { | ||
| ref.current = r | ||
| })} | ||
| onMessage={callback(() => {})} | ||
| /> | ||
| ) | ||
| unmount() | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files | rg 'example/src/__tests__/postmessage\.harness\.tsx|NitroWebView|waitFor|hybridRef'
printf '\n== outline of harness test ==\n'
ast-grep outline example/src/__tests__/postmessage.harness.tsx --view expanded || true
printf '\n== harness test contents ==\n'
cat -n example/src/__tests__/postmessage.harness.tsx
printf '\n== search for hybridRef usage ==\n'
rg -n "hybridRef" example/src src packages . -g '!**/node_modules/**' || true
printf '\n== search for waitFor usage in example tests ==\n'
rg -n "waitFor" example/src --glob '!**/node_modules/**' || trueRepository: l2hyunwoo/nitro-webview
Length of output: 25213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg 'example/src/__tests__/postmessage\.harness\.tsx|NitroWebView|waitFor|hybridRef' || trueRepository: l2hyunwoo/nitro-webview
Length of output: 8704
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' example/src/__tests__/postmessage.harness.tsxRepository: l2hyunwoo/nitro-webview
Length of output: 2113
Assert hybridRef is captured. ref.current is assigned but never checked, so this test still passes if the native handle is never delivered. Add a waitFor(() => expect(ref.current).not.toBeNull()) after render(), or rename the test to cover mounting only.
🤖 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/src/__tests__/postmessage.harness.tsx` around lines 9 - 22, Add an
assertion after rendering in the “mounts with a hybridRef and onMessage wired”
test that waits for ref.current to become non-null, using waitFor and expect.
Keep the existing render and unmount flow unchanged.
- avd-name: e2e_avd on both android-emulator-runner steps, matching the harness config (was defaulting to 'test', so the harness could never find the booted device: HarnessAppPathError) - keep -no-snapshot-save off the snapshot-generation step so AVD caching actually persists across runs - persist-credentials: false on every checkout step - fail fast (exit 1) when the e2e-server readiness poll times out, on both the Android script and the inline iOS step, instead of falling through to an opaque bridge timeout - E2E_PORT as a single source of truth per job instead of a bare literal duplicated across the workflow and helper script
…lute artifact path - rn-harness.config.mjs: document the e2e_avd / API_LEVEL sync requirement with e2e.yml directly on the config that was silently out of sync - e2e-server.mjs: unmatched routes now 404 instead of falling through to the 200 HTML page - run-harness-android-ci.sh: fail fast when the e2e-server never reports healthy; resolve the crash-log artifact path before the script cd's into example/, so cleanup() and the workflow's upload step agree on where it lives - e2eServer.ts: single E2E_PORT constant instead of the literal duplicated across both platform branches
…test suite jest.config.js previously switched from the RN preset to the harness preset globally, so 'yarn test' (App.test.tsx) silently stopped matching and the harness *.harness.tsx files were picked up by the wrong preset too. Split into jest.config.js (RN preset, excludes *.harness.* + the shared e2eServer.ts helper) and jest.harness.config.js (harness preset, its own file the react-native-harness CLI resolves by convention). Also drop the ref.current readiness assertion in postmessage.harness.tsx's mount smoke: hybridRef is delivered through the same Nitro callback() path as onLoadEnd/onMessage, which does not fire through harness 1.3.0's render() overlay (see load.harness.tsx) -- asserting on it here would be flaky for the same already-documented reason, not a real regression signal.
…s-15 xcodebuild failed with 'Unable to find a device matching the provided destination specifier' for iPhone 16 Pro / iOS 18.2 -- the macos-15 image now ships Xcode 16.4, whose bundled runtimes start at iOS 18.5 and don't include a plain iPhone 16 Pro model at all (only Pro Max/16e/17 series). Pinned to iPhone 17 Pro / iOS 26.1, confirmed present in this run's 'Available destinations' dump.
macOS runners don't ship GNU coreutils' timeout by default (the ubuntu Android job has it, so this only broke iOS): 'timeout: command not found' killed the step before yarn test:e2e:ios ever ran. Install coreutils via brew and call gtimeout instead.
1 participant
Summary
Adds the missing PR CI gate plus react-native-harness E2E, in two workflows:
ci.yml(Tier 1, every PR + push to main): lint / typecheck /yarn prepare/ host TS tests on ubuntu, thendorny/paths-filter-gated native jobs (Swift XCTest, Robolectric, AndroidassembleDebug, iOS compile). Docs-only PRs skip the native jobs.e2e.yml(Tier 2): harness smoke tests on an Android emulator (every PR, ubuntu + KVM) and an iOS simulator. The iOS job is label-gated (e2e-ioslabel, or push-to-main) because macOS minutes bill at 10x ubuntu and iOS E2E is the most expensive job.Wiring lives in
example/: harness deps + scripts,rn-harness.config.mjs(emulator/simulator specs,SIM_DEVICE/SIM_OSoverridable for local runs),withRnHarnessmetro wrap, a stdlib-onlye2e-server.mjsserving the pages the tests drive, andscripts/run-harness-android-ci.sh(vision-camera-style hardening).Two things surfaced during local verification and are included:
fix(ios): the committed nitrogen Swift-Cxx bridge was stale (missing thestd::optional<WebViewPoint>helpers and newer event forward-decls), so the iOS example did not compile from a clean checkout. Regenerated it; deterministic, additive only.load/postmessage/httperror) assert an on-device smoke: the realNitroWebViewmounts against the e2e-server without crashing. The strict event assertions (onLoadEnd/onMessage/onHttpError) are present buttest.skipped, because Nitro view-event callbacks do not fire through the harnessrender()overlay in harness 1.3.0 (verified: the view mounts and loads, but no callback prop is invoked, even for inline HTML with no network). Re-enable once that gap closes.Third-party actions are SHA-pinned (
dorny/paths-filterv4.0.2,reactivecircus/android-emulator-runnerv2.38.0,futureware-tech/simulator-actionv5); all three resolve to their tagged commits. Top-levelpermissions: contents: readonly.Test plan
Verified locally on an iOS simulator (iPhone 17 Pro / iOS 26.5, Xcode 26; the config defaults to the macos-15 runner's iPhone 16 Pro / 18.2 and is env-overridable):
yarn install && yarn prepareat root,pod install,xcodebuild ... -sdk iphonesimulatorbuild succeeds (after the nitrogen fix).node e2e-server.mjs+yarn test:e2e:ios:load.harness.tsxmount smoke: passpostmessage.harness.tsxmount smoke: passhttperror.harness.tsxmount smoke: pass3 passed, 3 skipped, 6 totalactionlintclean on both workflows,shellcheckclean on the Android script,node --checkclean on the server,tscclean on the test sources.Not run locally: the Android emulator path and both macOS CI jobs. The first CI run on this PR will validate the Android emulator job, the iOS simulator job (add the
e2e-ioslabel to trigger it), and the baseline gate end to end.Summary by CodeRabbit