Add CI test job and global error boundary for webview-app#1957
Add CI test job and global error boundary for webview-app#1957Tranquil-Flow wants to merge 6 commits into
Conversation
…lures The CI workflow ran build, lint, format, and type-check but never executed tests. Add a test job that runs both webview-app (147 tests) and webview-bridge (68 tests) on every PR and push. Fix two pre-existing test file failures: vi.mock hoisting issue in settingsScreens and missing euclid mock exports in recoverySupportScreens.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an ErrorBoundary component and wraps the app to capture render-time errors; updates multiple tests and mocks to reflect changed behavior; modifies tunnel flow tests to assert async lifecycle/analytics side effects on close; and adds a CI Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Wrap the app in a React error boundary that catches unhandled render errors and shows a recovery UI instead of a white screen. The fallback screen offers "Try again" (resets error state) and "Close" (fires lifecycle dismiss to the host app). Placed between BridgeProvider and App so it has bridge access for host notification. Includes 4 tests covering render, catch, retry, and dismiss flows.
Tranquil-Flow
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 94d4884d-5481-4829-ba9f-efb05f8dadba
📒 Files selected for processing (6)
.github/workflows/webview-app-ci.ymlpackages/webview-app/src/components/ErrorBoundary.tsxpackages/webview-app/src/main.tsxpackages/webview-app/tests/components/ErrorBoundary.test.tsxpackages/webview-app/tests/screens/account/settingsScreens.test.tsxpackages/webview-app/tests/screens/recovery/recoverySupportScreens.test.tsx
| test: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - name: Install dependencies | ||
| uses: ./.github/actions/yarn-install | ||
| - name: Build common | ||
| run: yarn workspace @selfxyz/common build | ||
| - name: Build mobile-sdk-alpha | ||
| run: yarn workspace @selfxyz/mobile-sdk-alpha build | ||
| - name: Build webview-bridge | ||
| run: yarn workspace @selfxyz/webview-bridge build | ||
| - name: Run tests | ||
| run: yarn workspace @selfxyz/webview-app test | ||
| - name: Run webview-bridge tests | ||
| run: yarn workspace @selfxyz/webview-bridge test | ||
|
|
There was a problem hiding this comment.
Missing required CI fast-fail guard before test execution
Line 68 onward adds the new test job, but it does not include the required pre-test validation step to detect nested require() patterns. This can reintroduce avoidable OOM-style CI failures later in the job.
As per coding guidelines: ".github/workflows/*.yml: Add a CI fast-fail check step that runs the validation script to detect nested require() patterns before test execution to prevent out-of-memory pipeline failures."
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.spyOn(console, 'error').mockImplementation(() => {}); | ||
| }); | ||
|
|
||
| afterEach(cleanup); |
There was a problem hiding this comment.
Restore console.error spy after each test to avoid cross-test leakage
Line 35 installs a global spy, but Line 38 does not restore it. This can leak mocked console behavior into subsequent tests.
Suggested fix
- afterEach(cleanup);
+ afterEach(() => {
+ cleanup();
+ vi.restoreAllMocks();
+ });📝 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.
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| vi.spyOn(console, 'error').mockImplementation(() => {}); | |
| }); | |
| afterEach(cleanup); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| vi.spyOn(console, 'error').mockImplementation(() => {}); | |
| }); | |
| afterEach(() => { | |
| cleanup(); | |
| vi.restoreAllMocks(); | |
| }); |
Wrap componentDidCatch logging in a DEV check so production builds only emit a generic message. Prevents raw error objects and component stacks from surfacing in host app logs where they could expose internal state.
|
@greptileai review |
Greptile SummaryThis PR adds CI test coverage for the webview-app and webview-bridge packages, introduces a global React error boundary that surfaces a recovery UI instead of a white screen, and fixes three pre-existing test failures that were blocking CI.
Confidence Score: 4/5Safe to merge — one non-blocking style rule violation remains (raw hex in ErrorBoundary styles). The CI job, error boundary logic, bridge integration, and all three test fixes are correct. The only finding is raw hex color values in the ErrorBoundary styles object instead of euclid design tokens, which violates project style rules but has no runtime impact. packages/webview-app/src/components/ErrorBoundary.tsx — styles object should use euclid tokens instead of raw hex values. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const styles = { | ||
| container: { | ||
| display: 'flex', | ||
| flexDirection: 'column' as const, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| height: '100vh', | ||
| width: '100%', | ||
| padding: 24, | ||
| backgroundColor: '#fff', | ||
| fontFamily: '-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif', | ||
| }, | ||
| content: { | ||
| display: 'flex', | ||
| flexDirection: 'column' as const, | ||
| alignItems: 'center', | ||
| maxWidth: 320, | ||
| textAlign: 'center' as const, | ||
| }, | ||
| icon: { | ||
| width: 48, | ||
| height: 48, | ||
| borderRadius: 24, | ||
| backgroundColor: '#FEE2E2', | ||
| color: '#DC2626', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| fontSize: 24, | ||
| fontWeight: 700, | ||
| marginBottom: 16, | ||
| }, | ||
| title: { | ||
| fontSize: 20, | ||
| fontWeight: 600, | ||
| color: '#111', | ||
| margin: '0 0 8px', | ||
| }, | ||
| message: { | ||
| fontSize: 14, | ||
| color: '#666', | ||
| lineHeight: 1.5, | ||
| margin: '0 0 24px', | ||
| }, | ||
| errorDetail: { | ||
| fontSize: 11, | ||
| color: '#999', | ||
| backgroundColor: '#f5f5f5', | ||
| borderRadius: 8, | ||
| padding: 12, | ||
| width: '100%', | ||
| overflow: 'auto' as const, | ||
| maxHeight: 80, | ||
| marginBottom: 24, | ||
| textAlign: 'left' as const, | ||
| }, | ||
| primaryButton: { | ||
| width: '100%', | ||
| padding: '14px 24px', | ||
| fontSize: 16, | ||
| fontWeight: 600, | ||
| color: '#fff', | ||
| backgroundColor: '#111', | ||
| border: 'none', | ||
| borderRadius: 12, | ||
| cursor: 'pointer', | ||
| marginBottom: 12, | ||
| }, | ||
| secondaryButton: { | ||
| width: '100%', | ||
| padding: '14px 24px', | ||
| fontSize: 16, | ||
| fontWeight: 600, | ||
| color: '#666', | ||
| backgroundColor: 'transparent', | ||
| border: '1px solid #ddd', | ||
| borderRadius: 12, | ||
| cursor: 'pointer', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The styles object uses raw hex literals throughout (#fff, #FEE2E2, #DC2626, #111, #666, #f5f5f5, #999, #ddd) instead of tokens from @selfxyz/euclid. Per the project rules, shared color/font/spacing tokens should be used instead of raw hex values in UI code. For example, colors.red600, colors.black, colors.grey600, etc. should replace the raw values where equivalents exist in the euclid token set.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
TunnelResultScreen no longer navigates on close. It sends a failure VerificationResult via lifecycle.setResult and then dismisses. The four close-action tests that previously asserted on post-close navigation now assert on the lifecycle.setResult payload, analytics trackEvent for tunnel_result_cancelled with the correct source, and lifecycle.dismiss being called. Update the receipt-from-success-context test to assert close-only controls, matching TunnelProofReceiptScreen which does not currently pass an onConfirm prop. Pairs with the existing tests that hide the confirm button when backState is missing or indicates failure.
1 participant
Summary
testjob towebview-app-ci.ymlthat runs webview-app (151 tests) and webview-bridge (68 tests) on every PR and pushvi.mockhoisting in settingsScreens, missing euclid mock exports in recoverySupportScreenslifecycle.dismissto host)Closes SELF-2436, SELF-2440
Test plan
yarn workspace @selfxyz/webview-app test— 151/151 passyarn workspace @selfxyz/webview-bridge test— 68/68 passyarn fmtandyarn lintcleanyarn buildsucceedstestjob runs successfully on this PRlifecycle.dismissto host (verified via test)Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores