test(query-devtools/Explorer): add tests for 'Map' and iterable rendering branches#10721
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a new test suite to ChangesExplorer Map and iterable rendering tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
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 34d7877
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview2 package(s) bumped directly, 23 bumped as dependents. 🟩 Patch bumps
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/query-devtools/src/__tests__/Explorer.test.tsx (2)
151-161: ⚡ Quick winConsider verifying the rendered values for completeness.
Similar to the array test (lines 95-107), which verifies both index labels (0:, 1:) AND values ("a", "b"), this test could also assert that the Set values ("x", "y") are rendered after expansion. This would improve test completeness and maintain consistency with the established pattern.
🧪 Suggested enhancement for test completeness
fireEvent.click(rendered.getByRole('button', { expanded: false })) expect(rendered.getByText('0:')).toBeInTheDocument() +expect(rendered.getByText('"x"')).toBeInTheDocument() expect(rendered.getByText('1:')).toBeInTheDocument() +expect(rendered.getByText('"y"')).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__/Explorer.test.tsx` around lines 151 - 161, The test "should render iterable children under their numeric index when expanded" currently asserts the index labels only; update it to also verify the Set's values are rendered after expansion by adding assertions that rendered.getByText('x') and rendered.getByText('y') are in the document (locate the test using the it(...) block and the renderExplorer call with label: 's' and value: new Set(['x','y']) and the fireEvent.click on the expansion button).
125-138: ⚡ Quick winConsider verifying the rendered values for completeness.
The test correctly verifies that Map keys are preserved as labels, but doesn't assert that the corresponding values (1 and 2) are rendered. The existing array test (lines 95-107) and object test (lines 109-121) both verify labels/indices AND values. Adding value assertions would improve test completeness and maintain consistency with the established pattern.
🧪 Suggested enhancement for test completeness
fireEvent.click(rendered.getByRole('button', { expanded: false })) expect(rendered.getByText('first:')).toBeInTheDocument() +expect(rendered.getByText('1')).toBeInTheDocument() expect(rendered.getByText('second:')).toBeInTheDocument() +expect(rendered.getByText('2')).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__/Explorer.test.tsx` around lines 125 - 138, Update the test "should preserve \"Map\" keys as labels when expanded" to also assert that Map values are rendered: after using renderExplorer(...) and firing the click on the expansion button (fireEvent.click(rendered.getByRole('button', { expanded: false }))), add expectations that the numeric values "1" and "2" appear (e.g., expect(rendered.getByText('1')).toBeInTheDocument() and expect(rendered.getByText('2')).toBeInTheDocument()). This keeps the Map test consistent with the array and object tests and uses the existing renderExplorer helper and rendered query methods.
🤖 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.
Nitpick comments:
In `@packages/query-devtools/src/__tests__/Explorer.test.tsx`:
- Around line 151-161: The test "should render iterable children under their
numeric index when expanded" currently asserts the index labels only; update it
to also verify the Set's values are rendered after expansion by adding
assertions that rendered.getByText('x') and rendered.getByText('y') are in the
document (locate the test using the it(...) block and the renderExplorer call
with label: 's' and value: new Set(['x','y']) and the fireEvent.click on the
expansion button).
- Around line 125-138: Update the test "should preserve \"Map\" keys as labels
when expanded" to also assert that Map values are rendered: after using
renderExplorer(...) and firing the click on the expansion button
(fireEvent.click(rendered.getByRole('button', { expanded: false }))), add
expectations that the numeric values "1" and "2" appear (e.g.,
expect(rendered.getByText('1')).toBeInTheDocument() and
expect(rendered.getByText('2')).toBeInTheDocument()). This keeps the Map test
consistent with the array and object tests and uses the existing renderExplorer
helper and rendered query methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a811cc94-01ce-48d4-98c1-55d1da40e293
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/Explorer.test.tsx
Contributor
size-limit report 📦
|
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
Extend
Explorer.test.tsxwith tests for theMap/ iterable branches insubEntriesandtypememos. These branches were previously uncovered by theDevtools.test.tsxintegration tests because user query data rarely usesMap/Setinstances.Added cases under a new
Map and iterable valuesdescribe:should preserve "Map" keys as labels when expandedshould mark an iterable value with an "(Iterable)" prefix on the expandershould render iterable children under their numeric index when expanded✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit