test(query-core/onlineManager): add test for online status updates from window events#10886
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a unit test that subscribes to OnlineManager, dispatches window 'offline' and 'online' events, asserts isOnline() updates, and verifies unsubscribe() removes the listener. ChangesOnlineManager Event Handling Test
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
|
View your CI Pipeline Execution ↗ for commit 48db8fb
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/query-core/src/__tests__/onlineManager.test.tsx`:
- Around line 160-167: The test currently asserts offline state transition using
onlineManager.isOnline() which can be true even if the manager started offline;
make the assertion transition-based by first forcing a known online precondition
(dispatch a 'online' event and assert onlineManager.isOnline() === true or
assert the subscription listener was called), then subscribe via
onlineManager.subscribe(...) and dispatch 'offline', and finally assert that the
listener was invoked or that onlineManager.isOnline() === false; reference
onlineManager.subscribe and onlineManager.isOnline() to locate and update the
test accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5009e2fe-6ffc-4e9a-916c-0910c8a1d9e2
📒 Files selected for processing (1)
packages/query-core/src/__tests__/onlineManager.test.tsx
size-limit report 📦
|
48db8fb to
e2df09b
Compare
|
Tip For best results, initiate chat on the files or code changes.
|
🎯 Changes
Adds a unit test for
OnlineManagercovering the default window event listeners: dispatching the browserofflineandonlineevents updates the online status accordingly.The existing tests only assert that the listeners are registered/removed; this covers the listener callbacks actually reacting to the events.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit