Synced from GitHub
test(web): migrate browser tests to the disposable render helper (8/8)#22267
The last slice of apps/web browser tests still managed React roots, act and lifecycle hooks by hand, so t3code/require-disposable-render flagged them. This moves them onto the disposable render helper from #22187.
Each test now acquires its own render (and any fake timers, spies, storage cleanup or atom registry) with using/await using. Mock state read by vi.mock factories is reset by a setup() call at the top of each test. Assertions are unchanged, and the slice runs the same 69 tests as before.
Files (chunk 8 of 8)
src/hooks/useHtmlRenderTheme.browser.test.tsxsrc/hooks/useLiveRefresh.browser.test.tsxsrc/hooks/useLocalStorage.hook.browser.test.tsxsrc/hooks/usePullRequestChecksRefresh.browser.test.tsxsrc/hooks/useResizableWidth.browser.test.tsxsrc/hooks/useSettings.favorites.browser.test.tsxsrc/hooks/useSettings.selector.browser.test.tsxsrc/hooks/useSettings.sync.browser.test.tsxsrc/hooks/useStableCallback.browser.test.tsxsrc/panelAnimations.browser.test.tsxsrc/state/queries.threadSearch.browser.test.tsxsrc/state/terminalSessionAvailability.browser.test.tsxsrc/state/usage.browser.test.tsx
Left unmigrated: none.
router: false: none. Every file runs with the default test router, including the fake-timer and hand-stepped requestAnimationFrame tests.
One small structural change: panelAnimations now passes its observed array as a prop, so the probe's effect lists it as a dependency to satisfy exhaustive-effect-dependencies. The array is stable, so the effect still fires only when suppressed changes.
Verification
vp test run --project browser --reporter=verbose on the 13 files, three consecutive runs:
| Run | Result | act warnings | act-environment warnings | router warnings |
|---|---|---|---|---|
| 1 | 69/69 passed | 0 | 0 | 0 |
| 2 | 69/69 passed | 0 | 0 | 0 |
| 3 | 69/69 passed | 0 | 0 | 0 |
vp linton the 13 files: 0 warnings, includingt3code/require-disposable-rendertsc-rs --noEmitfor apps/web: no errors- No touched file imports jsdom, happy-dom or
react-dom/client
Done by Claude Opus 5.5 in Claude Code.
🤖 Generated with Claude Code
Checks
- SuccessfulCodeRabbitGitHub
Review completed
- Successful
Skipped - Not affected
- Skipped
- Skipped
- Skipped
- SuccessfulRemove preview downloadGitHub
- SuccessfulCheckGitHub
- SkippedRelease SmokeGitHub
- SkippedRustGitHub
- Skipped
- SuccessfulTestGitHub
- SuccessfulTest WebGitHub
- SuccessfulBuild and TypecheckGitHub
- SkippedEAS PreviewGitHub
- Skipped
- SuccessfulLabel PR sizeGitHub
- SuccessfulLabel PR 22267GitHub
- SkippedDeploy web previewGitHub
- SuccessfulNative fingerprint diffGitHub
- SuccessfulTest Server 2GitHub
- SuccessfulTest Server 7GitHub
- SuccessfulTest Server 8GitHub
- SuccessfulTest Server 6GitHub
- SuccessfulTest Server 3GitHub
- SuccessfulTest Server 4GitHub
- SuccessfulTest Server 1GitHub
- SuccessfulTest Server 5GitHub
- SuccessfulLintGitHub
- SuccessfulChangesGitHub
- SuccessfulPrepare PR size configGitHub
- SuccessfulCollect PR targetsGitHub
- Successful
- Skipped
Added 1 commit
- test(web): migrate browser tests to the disposable render helper (8/8)a4ffbda
Synced from GitHub
Approvability
Verdict: Approved at 4eb6aab
Macroscope's review found this PR approvable — This PR is a browser-test-only migration to disposable rendering and scoped cleanup; product code, shipped defaults, and production behavior remain unchanged. Its runtime impact is limited to test execution and isolation.
Notes:
- No code objects were reviewed. Approvability was decided on eligibility alone.
You can add or adjust custom eligibility rules. Learn more.
Added 1 commit
- test(web): keep probe bookkeeping and fixture stacks per test4eb6aab
Synced from GitHub
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
393b089d-c024-4cbc-a389-c7b8727815ea
📥 Commits
Reviewing files that changed from the base of the PR and between ab423b24a8b52ca24e59975f15e917f35d37a68f and 4eb6aabc23d5760f2c10f40041157c8c908fb8eb.
📒 Files selected for processing: 13
apps/web/src/hooks/useHtmlRenderTheme.browser.test.tsxapps/web/src/hooks/useLiveRefresh.browser.test.tsxapps/web/src/hooks/useLocalStorage.hook.browser.test.tsxapps/web/src/hooks/usePullRequestChecksRefresh.browser.test.tsxapps/web/src/hooks/useResizableWidth.browser.test.tsxapps/web/src/hooks/useSettings.favorites.browser.test.tsxapps/web/src/hooks/useSettings.selector.browser.test.tsxapps/web/src/hooks/useSettings.sync.browser.test.tsxapps/web/src/hooks/useStableCallback.browser.test.tsxapps/web/src/panelAnimations.browser.test.tsxapps/web/src/state/queries.threadSearch.browser.test.tsxapps/web/src/state/terminalSessionAvailability.browser.test.tsxapps/web/src/state/usage.browser.test.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📝 Walkthrough
The browser test suites now use shared rendering and timer utilities, per-test fixtures, and asynchronous cleanup instead of manually managed React roots and global lifecycle setup. Existing behavioral coverage remains, with additional pointer-capture assertions in the resizing tests.
Changes: Browser test harness migration
Layer / File(s) Summary Async rendering and timer-driven tests
apps/web/src/hooks/useHtmlRenderTheme.browser.test.tsx,apps/web/src/hooks/useLiveRefresh.browser.test.tsx,apps/web/src/hooks/usePullRequestChecksRefresh.browser.test.tsx,apps/web/src/state/queries.threadSearch.browser.test.tsx,apps/web/src/panelAnimations.browser.test.tsxThese tests use shared rendering and timer utilities with awaited render, rerender, timer, or event operations. Their existing theme, polling, search, and animation assertions remain. Disposable hook and settings fixtures
apps/web/src/hooks/useLocalStorage.hook.browser.test.tsx,apps/web/src/hooks/useResizableWidth.browser.test.tsx,apps/web/src/hooks/useSettings.favorites.browser.test.tsx,apps/web/src/hooks/useSettings.sync.browser.test.tsxThese tests replace manual root and lifecycle management with scoped fixtures and asynchronous disposal. The resizing tests also assert that pointer capture clears in additional cases. Isolated selector and state observations
apps/web/src/hooks/useSettings.selector.browser.test.tsx,apps/web/src/hooks/useStableCallback.browser.test.tsx,apps/web/src/state/terminalSessionAvailability.browser.test.tsx,apps/web/src/state/usage.browser.test.tsxThese tests keep render counters, callbacks, and observed state within per-test helpers while preserving their existing assertions. Priority: ⬇️ Low
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Other
Merge Risk: ⚪ Minimal · up to 4eb6a
The migrated browser tests have no established merge-blocking issue; merge after normal checks.
Pre-merge checks: Passed 3 · Failed 1
Failed Failed checks: 1 warning
Check name Status Explanation Resolution Description check Warning The description explains the problem, change, affected files, and verification results in detail. It does not provide the required Scope and approval information, such as a triaged issue or explicit m… Add a Scope and approval section. Link the relevant triaged issue or discussion with explicit maintainer approval, or explain why this focused test-harness migration is an obvious small fix that does not require prior approval. Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly identifies the web test migration and the move to the disposable render helper. The chunk indicator is useful context and does not obscure the main change. 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.
Full details: Description check
Explanation
The description explains the problem, change, affected files, and verification results in detail. It does not provide the required Scope and approval information, such as a triaged issue or explicit maintainer approval, or explain why this focused migration qualifies for an exemption.
Resolution
Add a Scope and approval section. Link the relevant triaged issue or discussion with explicit maintainer approval, or explain why this focused test-harness migration is an obvious small fix that does not require prior approval.
- Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.