query icon indicating copy to clipboard operation
query copied to clipboard

test(angular-query-persist-client/withPersistQueryClient): switch to fake timers, replace 'waitFor' with 'advanceTimersByTimeAsync', use 'sleep().then()' pattern, replace 'screen' with 'rendered', add 'toBeInTheDocument', and add '@testing-library/jest-dom/vitest'

Open sukvvon opened this issue 1 month ago β€’ 5 comments

🎯 Changes

βœ… Checklist

  • [x] I have followed the steps in the Contributing guide.
  • [x] I have tested this code locally with pnpm run test:pr.

πŸš€ Release Impact

  • [ ] This change affects published code, and I have generated a changeset.
  • [x] This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Tests
    • Centralized fake-timer control for deterministic timing, replacing ad-hoc waits with explicit timer advancement.
    • Restored asynchronous timing for persistence and query flows to better reflect real behavior.
    • Reworked test sequencing to advance timers, trigger DOM updates, and assert hydration/fetch transitions.
    • Expanded coverage for success and error paths through staged timer progression.
    • Added enhanced DOM matchers to the test setup for clearer UI assertions.

✏️ Tip: You can customize this high-level summary in your review settings.

sukvvon avatar Nov 23 '25 12:11 sukvvon

⚠️ No Changeset found

Latest commit: 83563740471fa78e0e67343ebc1fb3e51db21ce4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Nov 23 '25 12:11 changeset-bot[bot]

Walkthrough

Tests were refactored to use Vitest fake timers with explicit timer advancement; mock persister and prefetch query functions now resolve via sleep(...).then(...); test renders are captured once and use fixture.detectChanges() and DOM assertions; test setup imports @testing-library/jest-dom/vitest.

Changes

Cohort / File(s) Summary
Test timer refactoring
packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts
Adds vi.useFakeTimers()/vi.useRealTimers() in beforeEach/afterEach; changes restoreClient and prefetch/queryFns to return sleep(10).then(...); replaces waitFor with explicit vi.advanceTimersByTimeAsync(...); captures render(Page, ...) result, uses rendered.fixture.detectChanges() and rendered.getByText() for assertions; expands onSuccess/onError assertions.
Test environment setup
packages/angular-query-persist-client/test-setup.ts
Adds import '@testing-library/jest-dom/vitest' to enable Jest DOM matchers in Vitest tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify fake-timer setup/teardown pairing across tests.
  • Check that sleep(...) durations match vi.advanceTimersByTimeAsync(...) calls.
  • Confirm fixture.detectChanges() is applied at each expected DOM transition and onSuccess/onError assertions are correctly timed.

Possibly related PRs

  • TanStack/query#9569 β€” related changes around including @testing-library/jest-dom/vitest in test setup.
  • TanStack/query#9850 β€” similar migration of tests to fake timers and replacing waitFor with vi.advanceTimersByTimeAsync (React tests).

Suggested reviewers

  • manudeli

Poem

πŸ‡ Timers paused, I nudge the clock,
sleep().then() β€” a quiet tick-tock.
Advance the time, the DOM takes stage,
Hydrated, fetched β€” I bound the page.
Hops and cheers β€” tests pass with a twitch.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete; it fills in the checklist but leaves the 'Changes' section empty, which should explain what was changed and why. Fill in the '🎯 Changes' section with a detailed explanation of the modifications made to the tests and the motivation behind these changes.
βœ… Passed checks (2 passed)
Check name Status Explanation
Title check βœ… Passed The title comprehensively describes all major changes in the PR: switching to fake timers, replacing waitFor, using sleep().then() pattern, replacing screen with rendered, and adding testing library imports.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

πŸ“œ Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 3f92720179db10f5d52851df84263abd8a56e41e and f432d8ccb4ab7fa713e4b3eca7a20876114b1ddf.

πŸ“’ Files selected for processing (1)
  • packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts (20 hunks)
🧰 Additional context used
🧠 Learnings (3)
πŸ““ Common learnings
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
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.
πŸ“š Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.

Applied to files:

  • packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts
πŸ“š Learning: 2025-09-21T00:31:02.518Z
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.

Applied to files:

  • packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts
🧬 Code graph analysis (1)
packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts (2)
packages/query-persist-client-core/src/__tests__/utils.ts (1)
  • createMockPersister (5-20)
packages/query-persist-client-core/src/persist.ts (1)
  • persistQueryClientSave (114-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
πŸ”‡ Additional comments (8)
packages/angular-query-persist-client/src/__tests__/with-persist-query-client.test.ts (8)

21-27: LGTM! Proper fake timer setup.

The beforeEach/afterEach pattern correctly sets up and tears down fake timers for each test, ensuring deterministic timer behavior and clean test isolation.


37-37: LGTM! Follows the .then() pattern guideline.

The restoreClient implementation correctly uses the sleep().then() pattern since there are no side effects.

Based on learnings, in TanStack/query test files, when a queryFn contains side effects, prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.


74-78: LGTM! Consistent use of .then() pattern.

The queryFn implementations correctly use the sleep().then() pattern for functions without side effects.

Based on learnings, in TanStack/query test files, when a queryFn contains side effects, prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.

Also applies to: 96-99


119-126: LGTM! Correct timer-driven test flow.

The controlled sequence of timer advancement, change detection, and DOM assertions correctly verifies the hydration and fetch transitions. The 11ms advancement at line 124 ensures the fetch (which starts after the 10ms hydration and takes 10ms) completes fully.


276-280: LGTM! Correct async/await usage for side effects.

The queryFn correctly uses async/await syntax because it contains a side effect (fetched = true), which aligns with the guideline for clarity when side effects are present.

Based on learnings, in TanStack/query test files, when a queryFn contains side effects, prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.


109-109: LGTM! Consistent render result capture.

All tests correctly capture the render result and use rendered.getByText() and rendered.fixture.detectChanges() for DOM interaction and change detection, which aligns with the PR objectives.

Also applies to: 201-201, 292-292, 356-356, 406-406


1-434: LGTM! Well-executed refactoring to fake timers.

The entire refactoring successfully achieves the PR objectives:

  • Fake timers are properly set up and torn down
  • The sleep().then() pattern is correctly applied for functions without side effects
  • Timer advancement is explicit and deterministic
  • DOM assertions use the captured rendered result
  • The test logic remains sound while gaining deterministic timing control

The changes align with the project's testing patterns and learnings.


119-119: jest-dom matchers are properly configured.

The test setup file correctly imports @testing-library/jest-dom/vitest, making the toBeInTheDocument() matcher available throughout the test suite.


Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 23 '25 12:11 coderabbitai[bot]

View your CI Pipeline Execution β†— for commit 83563740471fa78e0e67343ebc1fb3e51db21ce4

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... βœ… Succeeded 57s View β†—
nx run-many --target=build --exclude=examples/*... βœ… Succeeded 1s View β†—

☁️ Nx Cloud last updated this comment at 2025-12-19 01:57:22 UTC

nx-cloud[bot] avatar Nov 23 '25 12:11 nx-cloud[bot]

More templates

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@9895
@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@9895
@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@9895
@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@9895
@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@9895
@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@9895
@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@9895
@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@9895
@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@9895
@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@9895
@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@9895
@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@9895
@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@9895
@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@9895
@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@9895
@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@9895
@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@9895
@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@9895
@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@9895
@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@9895

commit: 8356374

pkg-pr-new[bot] avatar Nov 23 '25 12:11 pkg-pr-new[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 100.00%. Comparing base (f15b7fc) to head (8356374).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             main     #9895       +/-   ##
============================================
+ Coverage   45.89%   100.00%   +54.10%     
============================================
  Files         200         1      -199     
  Lines        8437        19     -8418     
  Branches     1943         1     -1942     
============================================
- Hits         3872        19     -3853     
+ Misses       4116         0     -4116     
+ Partials      449         0      -449     
Components Coverage Ξ”
@tanstack/angular-query-experimental βˆ… <ΓΈ> (βˆ…)
@tanstack/eslint-plugin-query βˆ… <ΓΈ> (βˆ…)
@tanstack/query-async-storage-persister βˆ… <ΓΈ> (βˆ…)
@tanstack/query-broadcast-client-experimental βˆ… <ΓΈ> (βˆ…)
@tanstack/query-codemods βˆ… <ΓΈ> (βˆ…)
@tanstack/query-core βˆ… <ΓΈ> (βˆ…)
@tanstack/query-devtools βˆ… <ΓΈ> (βˆ…)
@tanstack/query-persist-client-core βˆ… <ΓΈ> (βˆ…)
@tanstack/query-sync-storage-persister βˆ… <ΓΈ> (βˆ…)
@tanstack/query-test-utils βˆ… <ΓΈ> (βˆ…)
@tanstack/react-query βˆ… <ΓΈ> (βˆ…)
@tanstack/react-query-devtools βˆ… <ΓΈ> (βˆ…)
@tanstack/react-query-next-experimental βˆ… <ΓΈ> (βˆ…)
@tanstack/react-query-persist-client βˆ… <ΓΈ> (βˆ…)
@tanstack/solid-query βˆ… <ΓΈ> (βˆ…)
@tanstack/solid-query-devtools βˆ… <ΓΈ> (βˆ…)
@tanstack/solid-query-persist-client βˆ… <ΓΈ> (βˆ…)
@tanstack/svelte-query βˆ… <ΓΈ> (βˆ…)
@tanstack/svelte-query-devtools βˆ… <ΓΈ> (βˆ…)
@tanstack/svelte-query-persist-client βˆ… <ΓΈ> (βˆ…)
@tanstack/vue-query βˆ… <ΓΈ> (βˆ…)
@tanstack/vue-query-devtools βˆ… <ΓΈ> (βˆ…)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Nov 23 '25 12:11 codecov[bot]