🐛 Fix 3 test failures from React 18 setState batch timing#14009
Conversation
- useDrillDownWebSocket: openTrackedWs monkey-patches ws.close with a wrapper, making the original spy inaccessible. Store the spy on _closeSpy field and assert on that in the unmount test. - useStellar acknowledgeNotification/dismissAllNotifications: These functions relied on side effects inside setState updaters to capture snapshots (removed/snapshot variables). React 18's automatic batching may defer updater execution past the await boundary, leaving the variables at their initial values. Fix: introduce notificationsRef kept in sync via a setNotifications wrapper, and read directly from the ref before issuing state updates. Fixes #14002 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: kubestellar-bot <kubestellar-bot@kubestellar.io>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @kubestellar-hive[bot] — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/M
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Fixes #14002
Problem
3 tests failed after PR #14000 (commit 3fab259):
useDrillDownWebSocket
closes all tracked—openTrackedWs()monkey-patchesws.closewith a wrapper function, replacing the vi.fn() spy. The test then asserts.toHaveBeenCalled()on the non-spy wrapper, which always fails.useStellar
acknowledgeNotification restores— Theremovedvariable was set as a side effect inside asetNotificationsupdater callback. React 18's automatic batching may defer updater execution past theawaitboundary, leavingremovedasnullin the catch block.useStellar
dismissAllNotifications clears all— Same issue:snapshotwas captured inside a setState updater and read immediately after. With batching, it remains[], causing early return.Fix
_closeSpyfield in the mock, assert on that persistent reference.notificationsRefkept in sync via asetNotificationswrapper. BothacknowledgeNotificationanddismissAllNotificationsnow read from the ref (which reflects the last React-flushed value) instead of relying on updater side effects.Risk
Low — the hook behavior is identical; only the mechanism for reading current state in async callbacks changes from "side effect in updater" to "ref read". All other tests should continue to pass.