test(query-devtools/Devtools): add tests for query list rendering, filtering, and status indicators#10684
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Vitest fake timers in setup and restores real timers in teardown; expands Devtools.test.tsx with tests for query-list rendering/filtering/reactive updates and cache clearing (including CLEAR_MUTATION_CACHE), view toggle to Mutations and mutation submission entries, disabled-query labeling, status-count badges with live Inactive updates, and status-tooltip interactions. ChangesDevtools Component Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
Comment |
|
View your CI Pipeline Execution ↗ for commit 1b7f55e
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
Contributor
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-devtools/src/__tests__/Devtools.test.tsx`:
- Around line 394-414: The test "should show the tooltip on mouse enter when
label is hidden" currently only checks the badge existence; update it to assert
the tooltip appears on hover/focus and disappears on leave/blur by querying the
tooltip DOM instead of just the badge. After fireEvent.mouseEnter(fresh) expect
a tooltip element/text to be present (e.g., use rendered.queryByText or
queryByRole to find the tooltip content shown by the tooltip component), then
after fireEvent.mouseLeave(fresh) assert that tooltip is no longer present;
repeat similar assertions for focus/blur (fireEvent.focus(fresh) -> tooltip
present, fireEvent.blur(fresh) -> tooltip absent). Use the same test helpers
(renderDevtools, fresh = rendered.getByLabelText('Fresh: 0')) to locate the
target and the rendered.queryByText/queryByRole to assert tooltip visibility so
the test actually verifies the show/hide handlers.
- Around line 316-318: The test currently asserts only on the first captured
event (listener.mock.calls[0]) which is brittle; update the assertion to verify
that at least one call contains a CustomEvent whose event.detail.type ===
'CLEAR_MUTATION_CACHE'. Locate the test using the listener mock in
Devtools.test.tsx and replace the two lines referencing listener.mock.calls[0]
and expect(...).toBe(...) with a check that iterates listener.mock.calls (or
uses Array.prototype.some) to find any call where the first argument (cast as
CustomEvent) has detail.type === 'CLEAR_MUTATION_CACHE', and assert that this is
true.
- Around line 336-350: The test is using a single microtask tick (await
Promise.resolve()) which is flaky; replace that with an async DOM assertion such
as awaiting rendered.findByLabelText(/Mutation submitted at/) (or wrap the
expect inside waitFor) after calling mutation.execute({}) so the test waits for
the mutation UI to render; update the assertion that currently uses
rendered.getByLabelText to use await rendered.findByLabelText (or waitFor(() =>
expect(rendered.getByLabelText(...)).toBeInTheDocument())) and keep references
to renderDevtools, queryClient, and mutation.execute to locate the change.
🪄 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: 403c9b71-f1da-460b-894a-8c0d9916e0c8
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/Devtools.test.tsx
Comment on lines
+336
to
+350
| it('should render mutations in the mutations view', async () => { | ||
| const rendered = renderDevtools({ initialIsOpen: true }) | ||
|
|
||
| fireEvent.click(rendered.getByText('Mutations')) | ||
|
|
||
| const mutation = queryClient.getMutationCache().build(queryClient, { | ||
| mutationKey: ['add-post'], | ||
| mutationFn: () => Promise.resolve('ok'), | ||
| }) | ||
| mutation.execute({}) | ||
| await Promise.resolve() | ||
|
|
||
| expect( | ||
| rendered.getByLabelText(/Mutation submitted at/), | ||
| ).toBeInTheDocument() |
Contributor
There was a problem hiding this comment.
Use an async DOM assertion instead of a single microtask tick
await Promise.resolve() is timing-sensitive and can make this test flaky when mutation notifications land later than one tick. Assert via findBy.../waitFor on the rendered UI state.
Proposed fix
it('should render mutations in the mutations view', async () => {
const rendered = renderDevtools({ initialIsOpen: true })
fireEvent.click(rendered.getByText('Mutations'))
const mutation = queryClient.getMutationCache().build(queryClient, {
mutationKey: ['add-post'],
mutationFn: () => Promise.resolve('ok'),
})
- mutation.execute({})
- await Promise.resolve()
-
- expect(
- rendered.getByLabelText(/Mutation submitted at/),
- ).toBeInTheDocument()
+ void mutation.execute({})
+ expect(
+ await rendered.findByLabelText(/Mutation submitted at/),
+ ).toBeInTheDocument()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('should render mutations in the mutations view', async () => { | |
| const rendered = renderDevtools({ initialIsOpen: true }) | |
| fireEvent.click(rendered.getByText('Mutations')) | |
| const mutation = queryClient.getMutationCache().build(queryClient, { | |
| mutationKey: ['add-post'], | |
| mutationFn: () => Promise.resolve('ok'), | |
| }) | |
| mutation.execute({}) | |
| await Promise.resolve() | |
| expect( | |
| rendered.getByLabelText(/Mutation submitted at/), | |
| ).toBeInTheDocument() | |
| it('should render mutations in the mutations view', async () => { | |
| const rendered = renderDevtools({ initialIsOpen: true }) | |
| fireEvent.click(rendered.getByText('Mutations')) | |
| const mutation = queryClient.getMutationCache().build(queryClient, { | |
| mutationKey: ['add-post'], | |
| mutationFn: () => Promise.resolve('ok'), | |
| }) | |
| void mutation.execute({}) | |
| expect( | |
| await rendered.findByLabelText(/Mutation submitted at/), | |
| ).toBeInTheDocument() |
🤖 Prompt for 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.
In `@packages/query-devtools/src/__tests__/Devtools.test.tsx` around lines 336 -
350, The test is using a single microtask tick (await Promise.resolve()) which
is flaky; replace that with an async DOM assertion such as awaiting
rendered.findByLabelText(/Mutation submitted at/) (or wrap the expect inside
waitFor) after calling mutation.execute({}) so the test waits for the mutation
UI to render; update the assertion that currently uses rendered.getByLabelText
to use await rendered.findByLabelText (or waitFor(() =>
expect(rendered.getByLabelText(...)).toBeInTheDocument())) and keep references
to renderDevtools, queryClient, and mutation.execute to locate the change.
sukvvon
force-pushed
the
test/query-devtools-devtools-query-list
branch
from
d4b86f6 to
7af9fbf
Compare
…ltering, and status indicators
…HE' event in dispatched calls
sukvvon
force-pushed
the
test/query-devtools-devtools-query-list
branch
from
7af9fbf to
58f3a1f
Compare
…TimersByTimeAsync' for microtask flush
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/query-devtools/src/__tests__/Devtools.test.tsx (2)
337-351:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace one-tick wait with an async DOM assertion
Line 347 (
await Promise.resolve()) is timing-sensitive and can still make this test flaky. Wait for rendered UI state instead.Proposed fix
const mutation = queryClient.getMutationCache().build(queryClient, { mutationKey: ['add-post'], mutationFn: () => Promise.resolve('ok'), }) - mutation.execute({}) - await Promise.resolve() + void mutation.execute({}) expect( - rendered.getByLabelText(/Mutation submitted at/), + await rendered.findByLabelText(/Mutation submitted at/), ).toBeInTheDocument()🤖 Prompt for 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. In `@packages/query-devtools/src/__tests__/Devtools.test.tsx` around lines 337 - 351, The test uses a fragile one-tick wait (await Promise.resolve()) after mutation.execute() in the 'should render mutations in the mutations view' test; replace that with an async DOM assertion so the test waits for the UI to update reliably—e.g. use rendered.findByLabelText(/Mutation submitted at/) or wrap the expect in waitFor. Update the test that calls renderDevtools, queryClient.getMutationCache().build(...), and mutation.execute({}) to await the DOM query (findByLabelText) or await waitFor(() => rendered.getByLabelText(/Mutation submitted at/())) instead of awaiting Promise.resolve().
395-415:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTooltip test doesn’t verify tooltip visibility transitions
Line 414 only re-checks the badge exists, which is true regardless of tooltip logic. Assert that tooltip content/element appears on enter/focus and disappears on leave/blur.
🤖 Prompt for 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. In `@packages/query-devtools/src/__tests__/Devtools.test.tsx` around lines 395 - 415, The test currently only asserts the badge exists (const fresh = rendered.getByLabelText('Fresh: 0') and expect(fresh).toBeInTheDocument()) which doesn't verify tooltip visibility transitions; update the test to assert that the tooltip element/content appears after fireEvent.mouseEnter(fresh) and fireEvent.focus(fresh) (use a selector like getByText or queryByRole for the tooltip text) and that it is removed after fireEvent.mouseLeave(fresh) and fireEvent.blur(fresh) (assert tooltip is not in the document or not visible); keep using the same element reference 'fresh' and replace the final existence-only assertion with presence/absence assertions for the tooltip element.
🤖 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.
Duplicate comments:
In `@packages/query-devtools/src/__tests__/Devtools.test.tsx`:
- Around line 337-351: The test uses a fragile one-tick wait (await
Promise.resolve()) after mutation.execute() in the 'should render mutations in
the mutations view' test; replace that with an async DOM assertion so the test
waits for the UI to update reliably—e.g. use rendered.findByLabelText(/Mutation
submitted at/) or wrap the expect in waitFor. Update the test that calls
renderDevtools, queryClient.getMutationCache().build(...), and
mutation.execute({}) to await the DOM query (findByLabelText) or await
waitFor(() => rendered.getByLabelText(/Mutation submitted at/())) instead of
awaiting Promise.resolve().
- Around line 395-415: The test currently only asserts the badge exists (const
fresh = rendered.getByLabelText('Fresh: 0') and
expect(fresh).toBeInTheDocument()) which doesn't verify tooltip visibility
transitions; update the test to assert that the tooltip element/content appears
after fireEvent.mouseEnter(fresh) and fireEvent.focus(fresh) (use a selector
like getByText or queryByRole for the tooltip text) and that it is removed after
fireEvent.mouseLeave(fresh) and fireEvent.blur(fresh) (assert tooltip is not in
the document or not visible); keep using the same element reference 'fresh' and
replace the final existence-only assertion with presence/absence assertions for
the tooltip element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db19ef3a-4fae-460f-8771-0187bc295d43
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/Devtools.test.tsx
…us instead of badge presence
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Add tests for the query/mutation list visualization on the
Devtoolsbody component.query list— query rendering, reactivity, filtering, and clear cache:queryHashvia the filter input.'CLEAR_MUTATION_CACHE'event when clear cache is clicked in mutations view.view toggle— switching between queries and mutations views:status indicators— labels and badges that summarize cache state:Fresh,Stale,Fetching,Paused,Inactive).✅ Checklist
🚀 Release Impact