test: replace deprecated React test renderer - #18
andrebrait wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR replaces ChangesReact test migration
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The test migration has no identified behavior or operational issue that should block readiness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 6 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments remain, and the provided assessments indicate readiness.
Pull request overview
This test-focused pull request replaces deprecated react-test-renderer usage with React Testing Library and jsdom.
Changes:
- Adds shared
jsdomsetup and Testing Library dependencies. - Migrates four test suites to DOM- and hook-based testing.
- Removes
react-test-rendererand documents updated testing conventions.
File summaries
| File | Description |
|---|---|
tests/setup-dom.mjs |
Shared jsdom environment and cleanup. |
package.json |
Testing dependency updates. |
package-lock.json |
Locked dependency graph updates. |
hooks/useTheme.test.mjs |
DOM-based theme lifecycle tests. |
hooks/useSidebarHistory.test.mjs |
Hook-based navigation tests. |
hooks/useAgentSession.rpc.test.mjs |
RTL hook lifecycle and SSE tests. |
CONTRIBUTING.md |
React testing guidance. |
components/ChatInput.navigation.test.mjs |
DOM and user-event composer tests. |
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Both bot reviews are complete on fe11b64: Copilot recommends approval with zero comments; CodeRabbit reports no actionable comments and its check passes. No code fixes were needed. CodeRabbit's default docstring-coverage advisory is acknowledged but not adopted as a new project gate: this is a test-only migration, and the test environment/conventions are documented in CONTRIBUTING.md. No boilerplate docstrings or threshold changes were added solely to satisfy that generic warning. Results are being reported to upstream kahme247#85. This remains a review-only mirror; do not merge. |
…erer, unify lockfile - MessageView.test.mjs: the grouped-tool-inputs test (added by kahme247#89 after this branch) drove Collapsible via TestRenderer internals; reworked onto Testing Library over the real DOM (row triggers, input toggles, rerender) with identical assertions. Last react-test-renderer consumer gone. - package-lock.json: regenerated for the merged dependency set (main's dbus-next plus the Testing Library/jsdom swap).
fe11b64 to
3434e75
Compare
Preserve the maintainer's migration fixups and port the new session catch-up regressions to the RTL harness. Retain the visible copy-action regression from main.
|
@coderabbitai review Upstream CI for the refreshed head 4f21694 is green on Linux and Windows: https://github.com/kahme247/ompweb/actions/runs/34812775233. The review-only base now matches upstream main at 3d11f38. Please review the current RTL migration diff, including the newly migrated session catch-up regressions. This remains a review-only companion; do not merge. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== DIFF STAT ==='
git diff --stat 3d11f389844f211533643c8ec761aa73b95a03f6...HEAD
echo '=== CHANGED FILES ==='
git diff --name-status 3d11f389844f211533643c8ec761aa73b95a03f6...HEAD
echo '=== DIFF ==='
git diff --find-renames --find-copies --unified=80 3d11f389844f211533643c8ec761aa73b95a03f6...HEAD -- \
CONTRIBUTING.md \
components/ChatInput.navigation.test.mjs \
hooks/useAgentSession.rpc.test.mjs \
hooks/useSidebarHistory.test.mjs \
hooks/useTheme.test.mjs \
package.json \
package-lock.json \
tests/setup-dom.mjs
echo '=== OUTLINES ==='
for f in components/ChatInput.navigation.test.mjs hooks/useAgentSession.rpc.test.mjs hooks/useSidebarHistory.test.mjs hooks/useTheme.test.mjs tests/setup-dom.mjs; do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all || true
fi
done
echo '=== TEST SCRIPT / DEPENDENCY CONTEXT ==='
rg -n -C 4 '"(test|`@testing-library/`(react|user-event|dom)|jsdom|react-test-renderer)"' package.json package-lock.json || true
echo '=== SETUP-DOM IMPORTS ==='
rg -n -C 3 "setup-dom|`@testing-library/react`" -g '*.mjs' . || trueLength of output: 98461 🏁 Script executed: #!/bin/bash
set -euo pipefail
base=3d11f389844f211533643c8ec761aa73b95a03f6
echo '=== SOURCE DIFF (NO LOCKFILE) ==='
git diff --unified=25 "$base...HEAD" -- \
CONTRIBUTING.md \
components/ChatInput.navigation.test.mjs \
hooks/useAgentSession.rpc.test.mjs \
hooks/useSidebarHistory.test.mjs \
hooks/useTheme.test.mjs \
package.json \
tests/setup-dom.mjs
echo '=== CURRENT SETUP AND MIGRATED SMALL TESTS ==='
cat -n tests/setup-dom.mjs
cat -n components/ChatInput.navigation.test.mjs
cat -n hooks/useSidebarHistory.test.mjs
cat -n hooks/useTheme.test.mjs
echo '=== AGENT SESSION TEST: SETUP, MOUNT, AND CATCH-UP CASES ==='
sed -n '1,340p' hooks/useAgentSession.rpc.test.mjs
sed -n '900,1160p' hooks/useAgentSession.rpc.test.mjs
echo '=== AGENT SESSION IMPLEMENTATION MAP ==='
fd -a '^useAgentSession\.(ts|tsx|js|jsx)$' hooks
ast-grep outline hooks/useAgentSession.ts --items all || true
echo '=== CATCH-UP IMPLEMENTATION REFERENCES ==='
rg -n -C 12 -i 'catch.?up|snapshot|cursor|sync|EventSource|visibility|online' hooks/useAgentSession.ts
echo '=== DOM SETUP ORDER IN ALL RTL TESTS ==='
for f in $(rg -l '`@testing-library/react`' -g '*.test.mjs'); do
echo "--- $f ---"
sed -n '1,12p' "$f"
doneLength of output: 132223 🏁 Script executed: #!/bin/bash
set -euo pipefail
base=3d11f389844f211533643c8ec761aa73b95a03f6
echo '=== FILE-SCOPED DIFFS ==='
for f in \
tests/setup-dom.mjs \
hooks/useAgentSession.rpc.test.mjs \
components/ChatInput.navigation.test.mjs \
hooks/useSidebarHistory.test.mjs \
hooks/useTheme.test.mjs \
CONTRIBUTING.md; do
echo "--- DIFF: $f ---"
git diff --unified=12 "$base...HEAD" -- "$f"
done
echo '=== AGENT SESSION TEST LOCATIONS ==='
rg -n '^(test|function)\\b|catch.?up|syncSnapshot|holdNextSync|attachNativeWrapper|afterEach|beforeEach|cleanup|restore' hooks/useAgentSession.rpc.test.mjs
echo '=== AGENT SESSION TEST: CURRENT REGRESSION SECTION ==='
sed -n '1010,1220p' hooks/useAgentSession.rpc.test.mjs
echo '=== AGENT SESSION TEST: CURRENT GLOBAL SETUP/CLEANUP ==='
sed -n '1,330p' hooks/useAgentSession.rpc.test.mjs
echo '=== SESSION CATCH-UP MODULE ==='
fd -a '^useAgentSession-sync\.(ts|tsx|js|jsx)$' hooks
ast-grep outline hooks/useAgentSession-sync.ts --items all || true
cat -n hooks/useAgentSession-sync.ts
echo '=== SESSION-CHANGE BUS ==='
fd -a '^session-change-bus\.(ts|tsx|js|jsx)$' lib
cat -n lib/session-change-bus.tsLength of output: 167986
I found no actionable issues. The JSDOM setup order, per-test cleanup, browser API restoration, real event dispatch, and I did not run repository tests in this environment. The reported upstream Linux and Windows CI results provide the execution evidence. This review does not request a merge. ✅ Action performedReview finished.
|
|
Closing this review-only companion without merging. The completed review results and fixes are recorded; ongoing implementation remains in the upstream PR. Upstream: kahme247#85. Review history remains available here. Shared feature branches are preserved while upstream work remains open. |
Review mirror of upstream kahme247#85
This PR exists to obtain Copilot and CodeRabbit reviews for kahme247#85. Do not merge this review mirror. Review results and any fixes will be reported upstream.
The base is pinned to the upstream PR base (0541712), and the head is the SAME branch/commit (fe11b64). This keeps the diff focused without updating the fork main branch.
Change
Verification
Summary by CodeRabbit
New Features
Documentation
Tests
react-test-rendererand updated development test tooling.Refreshed upstream review target
Review-only companion for kahme247#85. Do not merge this companion.
The head is now
4f21694e98a229e805c8006b7c620925458511d1; the review-only base is3d11f389844f211533643c8ec761aa73b95a03f6, matching upstream main. This refresh preserves the maintainer's previous RTL fixups and migrates the newly merged session catch-up tests to the same RTL harness. Production changes from main are excluded from the PR diff.Local verification: typecheck passes; ESLint with zero warnings passes; full suite 786 passed with one existing platform skip; final focused suites 102 passed. Single condensed review found no blockers. Updated bot reviews will be requested after CI passes.