test(query-devtools/utils): cover empty path, array nested path, and primitive cases of 'updateNestedDataByPath'#10672
Conversation
…primitive cases of 'updateNestedDataByPath'
📝 WalkthroughWalkthroughThree new test suites were added to verify ChangesTest Coverage for updateNestedDataByPath
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 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 docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit d778fc0
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/query-devtools/src/__tests__/utils.test.ts (1)
146-173: ⚡ Quick winAdd an explicit immutability assertion for the source array/object
The nested-array test checks
newDataand reference inequality, but it doesn’t verify thatoldDatacontent stays unchanged (Line 148-Line 151). Adding that assertion would better guard against accidental in-place mutation regressions.Suggested test hardening
describe('array nested path', () => { it('should update nested data inside an array correctly', () => { const oldData = [ { id: 1, title: 'first' }, { id: 2, title: 'second' }, ] const newData = updateNestedDataByPath( oldData, ['1', 'title'], 'updated', ) expect(newData).not.toBe(oldData) + expect(oldData).toMatchInlineSnapshot(` + [ + { + "id": 1, + "title": "first", + }, + { + "id": 2, + "title": "second", + }, + ] + `) expect(newData).toMatchInlineSnapshot(` [ { "id": 1, "title": "first",🤖 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__/utils.test.ts` around lines 146 - 173, The test lacks an assertion that the original source (oldData) remains unchanged after calling updateNestedDataByPath; add an explicit immutability assertion immediately after calling updateNestedDataByPath to verify oldData’s contents are intact (for example assert oldData[1].title === 'second' or use toMatchInlineSnapshot on oldData) so the test checks both reference inequality (newData !== oldData) and that updateNestedDataByPath did not mutate oldData.
🤖 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__/utils.test.ts`:
- Around line 146-173: The test lacks an assertion that the original source
(oldData) remains unchanged after calling updateNestedDataByPath; add an
explicit immutability assertion immediately after calling updateNestedDataByPath
to verify oldData’s contents are intact (for example assert oldData[1].title ===
'second' or use toMatchInlineSnapshot on oldData) so the test checks both
reference inequality (newData !== oldData) and that updateNestedDataByPath did
not mutate oldData.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3588a653-d721-4e7f-96f0-81dc09a9b29c
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/utils.test.ts
size-limit report 📦
|
sukvvon
deleted the
test/query-devtools-update-nested-data-by-path-coverage
branch
1 participant
🎯 Changes
Cover the three remaining branches of the
updateNestedDataByPathhelper inquery-devtools/utils.tsx. The existing suite already exercises the single-steparray,object,set,map, and deeply-nested cases; this PR fills in the branches that weren't hit yet.Cases:
updatePathis emptyupdatePath.length > 1Object/Array/Map/Set✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit