test(query-core/query): add test for refetch after a previous fetch errored#10529
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 due to trivial changes (1)
📝 WalkthroughWalkthroughAdded a test that prefetches a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query-core/src/__tests__/query.test.tsx`:
- Around line 863-877: The test must exercise the Query.fetch early-return guard
path where an existing retryer is present and its status() === 'rejected' while
fetchStatus !== 'idle'; modify the setup so the second call to query.fetch
happens while the rejected retryer is still attached (e.g., start the failing
prefetch with queryClient.prefetchQuery({ queryKey: key, queryFn, retry: false
}), advance timers only enough to make the retryer reach rejected but do not let
the Query reset fetchStatus to 'idle', then call query.fetch({ queryKey: key,
queryFn, retry: false }) and assert the guard path is taken), or if that is
brittle, change the test title and behavior to a plain “refetch after error”
scenario that does not attempt to cover the rejected-retryer guard; target
symbols: queryClient.prefetchQuery, query.fetch, Query.fetch, this.#retryer,
fetchStatus.
🪄 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: 3880683d-2327-4cd7-9041-14637cf79e7b
📒 Files selected for processing (1)
packages/query-core/src/__tests__/query.test.tsx
The previous title ('fetch should start a new fetch when the previous
retryer has rejected') claimed coverage of the
`this.#retryer?.status() !== 'rejected'` branch in Query.fetch, but that
guard sits behind `state.fetchStatus !== 'idle'`. Once the prior fetch has
rejected and state.status becomes 'error', fetchStatus is already back to
'idle', so the first predicate short-circuits and the retryer.status()
check is never evaluated. Removing the rejected-retryer exception would
not flip this assertion.
Rename and trim to what the test actually exercises: a rejected fetch
followed by a successful refetch via Query.fetch. Per CodeRabbit review
on PR TanStack#10529.
… the previous retryer has rejected
The previous title ('fetch should start a new fetch when the previous
retryer has rejected') claimed coverage of the
`this.#retryer?.status() !== 'rejected'` branch in Query.fetch, but that
guard sits behind `state.fetchStatus !== 'idle'`. Once the prior fetch has
rejected and state.status becomes 'error', fetchStatus is already back to
'idle', so the first predicate short-circuits and the retryer.status()
check is never evaluated. Removing the rejected-retryer exception would
not flip this assertion.
Rename and trim to what the test actually exercises: a rejected fetch
followed by a successful refetch via Query.fetch. Per CodeRabbit review
on PR TanStack#10529.
47d908c to
762c163
Compare
|
I think the claim in the description doesn't actually hold. After So when That rejected-retryer guard was added in #9565 to fix a StrictMode race where Could you either adjust the scenario to cover that race, or reword the description so it doesn't claim to cover the |
|
You're right — without the retryer-rejected angle, this duplicates existing refetch-after-error coverage. Closing this one. |
🎯 Changes
Adds a behavioral test for
Query.fetchthat covers the case where the previous retryer has ended in arejectedstate. After an initial fetch fails withretry: false, callingquery.fetch(...)directly must fall through the early-return guard inquery.ts:401-417(specifically thethis.#retryer?.status() !== 'rejected'condition) and create a fresh retryer. The test also asserts the resulting status transition and that the newqueryFnruns.This complements the existing "fetch should not dispatch 'fetch' query is already fetching" test (query.test.tsx:819), which only validates the in-flight concurrent-fetch path, not the post-rejection recovery path.
✅ Checklist
npx nx run @tanstack/query-core:test:libandnpx nx run @tanstack/query-core:test:types.🚀 Release Impact
Summary by CodeRabbit