test(react-query-persist-client/use-queries-with-persist): switch to fake timers, replace 'waitFor' with 'vi.advanceTimersByTimeAsync', and replace 'setTimeout' with 'sleep' #9850
Conversation
|
WalkthroughA test file is refactored to use controlled fake timers and explicit timer advances instead of relying on real-time delays and polling. A sleep utility is introduced, replacing direct setTimeout calls and waitFor patterns with deterministic vi.advanceTimersByTimeAsync invocations. Changes
Estimated code review effortπ― 2 (Simple) | β±οΈ ~10β15 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touchesβ Failed checks (1 warning)
β Passed checks (2 passed)
β¨ Finishing touches
π§ͺ Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution β for commit 56b9380
βοΈ Nx Cloud last updated this comment at |
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9850 +/- ##
============================================
+ Coverage 45.76% 100.00% +54.23%
============================================
Files 200 1 -199
Lines 8410 18 -8392
Branches 1930 2 -1928
============================================
- Hits 3849 18 -3831
+ Misses 4113 0 -4113
+ Partials 448 0 -448 π New features to boost your workflow:
|
β¦fake timers, replace 'waitFor' with 'vi.advanceTimersByTimeAsync', and replace 'setTimeout' with 'sleep'
sukvvon
force-pushed
the
test/react-query-persist-client-use-queries-with-persist-migrate-fake-timers
branch
from
847baa2 to
39fe259
Compare
β¦with-persist-migrate-fake-timers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
π§Ή Nitpick comments (1)
packages/react-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx (1)
131-134: Consider adding a clarifying comment for the two-step timer advance.The logic is correct: the first advance completes the
sleep(10)restoration, and the second flushes microtasks to ensure all React state updates complete. However, this pattern could benefit from a brief inline comment explaining the two-step approach for future maintainers.Consider adding a comment:
+ // Advance 10ms to complete restoreClient's sleep(10), then flush microtasks await act(() => vi.advanceTimersByTimeAsync(10)) await act(() => vi.advanceTimersByTimeAsync(0)) expect(getByTestId('pending').textContent).toBe('false') expect(getByTestId('data').textContent).toBe('1,2,3')
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
packages/react-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx(6 hunks)
π§° Additional context used
π§ Learnings (1)
π Common learnings
Learnt from: justjake
Repo: TanStack/query PR: 9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.575Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
Learnt from: arnoud-dv
Repo: TanStack/query PR: 9669
File: docs/framework/angular/guides/testing.md:49-56
Timestamp: 2025-09-21T00:31:02.518Z
Learning: TestBed.tick() is a valid API introduced in Angular 20 for triggering effects in unit tests, similar to ApplicationRef.tick() but for testing contexts.
𧬠Code graph analysis (1)
packages/react-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx (1)
packages/query-persist-client-core/src/persist.ts (1)
PersistedClient(18-22)
π Additional comments (5)
packages/react-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx (5)
1-2: LGTM: Imports correctly updated for fake timer migration.The addition of
vifor timer control andactfor wrapping async state updates aligns with the migration to fake timers.
5-5: LGTM: Correct utility for fake timer testing.Switching from
waitFor(polling-based) tosleep(timer-based) is the right choice for deterministic fake timer control.
17-17: LGTM: Proper timer lifecycle management.Fake timers are correctly initialized before each test and restored afterward, ensuring proper isolation.
Also applies to: 36-36
57-59: LGTM: Controlled async restoration with fake timers.The
sleep(10)introduces a deterministic 10ms delay before restoration, which is properly controlled by the test's timer advances.
100-100: LGTM: Controlled async query function.The
sleep(100)delay works correctly with fake timers. Note that this query function shouldn't execute in the test since the persisted data is still fresh (staleTime: 30,000ms, data age: 1,000ms), which is why the test correctly doesn't advance the full 100ms.
2 participants
π― Changes
β Checklist
pnpm run test:pr.π Release Impact
Summary by CodeRabbit