test(charts): E2E render + interaction for Pie, Map, Graph widgets (#486) - #518
Merged
Merged
Conversation
…ph widgets (#486) Add heavy-widgets.spec.ts with 7 tests covering lazy-loaded widget types: Pie canvas render + dashboard add, Map Leaflet container + zoom controls, Graph NVL toolbar + Fit graph interaction, and incompatible data format error states for all three. Closes #486 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alfredo1996
added
enhancement
WalkthroughAdds a comprehensive end-to-end test suite for Pie, Map, and Graph widgets that verifies canvas/Leaflet/node-link rendering, user interactions (query execution, zoom controls), and error states when queries return incompatible data formats. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/e2e/heavy-widgets.spec.ts`:
- Around line 107-113: The test currently only checks that the Leaflet container
and overlay svg are visible; update the assertion to verify the actual markers
for the 5 returned points by adding an assertion against the rendered marker
primitives (e.g., count of elements matching the marker selector) from the
preview returned by getPreview(dialog): locate either ".leaflet-marker-icon"
(for default Leaflet icons) or the SVG marker primitives inside
".leaflet-overlay-pane svg" (e.g., "circle" or ".marker" elements) and assert
their count equals 5 with an appropriate timeout, keeping the existing preview
and locator usage.
- Around line 215-224: The test currently only verifies toolbar visibility by
locating the "Fit graph" button (fitBtn) and clicking it, which doesn't assert
that nodes/edges rendered or pan/zoom/styling work; update the
heavy-widgets.spec.ts test to explicitly check graph rendering and interactions:
after getPreview(dialog) and ensuring preview visible, locate the graph
canvas/SVG element (e.g., query for NVL graph container or role), assert that a
minimum number of node and edge DOM elements are present, verify styling by
checking a node/edge has the expected CSS class or computed style, and assert
pan/zoom by performing a drag or wheel event on the graph element and confirming
node positions change (or transform attribute updates); keep the existing fitBtn
click but add these explicit assertions around getPreview, the graph container,
and node/edge selectors to satisfy the PR acceptance criteria.
- Around line 147-155: The test currently exercises only zoom controls (using
zoomIn/zoomOut) but must also perform a pan; add a drag interaction against the
map surface (use the existing mapContainer locator or
preview.locator('.leaflet-container') to perform a mouse drag/locator.dragTo) to
simulate panning, then assert the map still renders (e.g. await
expect(mapContainer).toBeVisible() or another existing post-pan assertion) to
prove the map remains functional after the pan.
- Around line 50-52: The test currently only checks that the canvas mounted via
getPreview(dialog) and preview.locator("canvas"), but must also assert the
actual Pie output; update the test that uses getPreview(dialog) to add
assertions that the rendered Pie has the expected number of slices and that the
legend items are visible (use preview.locator(...) with selectors targeting the
pie slice elements and legend entries and assert .toHaveCount(expectedCount) and
.toBeVisible() accordingly). Ensure you reference preview.locator for both the
slice selector and the legend selector so the test fails if the Pie renders
wrong series or hides the legend.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8849c04d-9e78-4820-a1d1-a251fa7d7bc5
📒 Files selected for processing (1)
app/e2e/heavy-widgets.spec.ts
Comment on lines
+50
to
+52
| const preview = getPreview(dialog); | ||
| await expect(preview.locator("canvas")).toBeVisible({ timeout: 15_000 }); | ||
|
|
There was a problem hiding this comment.
Assert Pie output, not just canvas mount.
A visible canvas only proves the widget mounted. It does not cover the acceptance criteria for correct slice count and legend visibility, so this can still pass when Pie renders the wrong series data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/e2e/heavy-widgets.spec.ts` around lines 50 - 52, The test currently only
checks that the canvas mounted via getPreview(dialog) and
preview.locator("canvas"), but must also assert the actual Pie output; update
the test that uses getPreview(dialog) to add assertions that the rendered Pie
has the expected number of slices and that the legend items are visible (use
preview.locator(...) with selectors targeting the pie slice elements and legend
entries and assert .toHaveCount(expectedCount) and .toBeVisible() accordingly).
Ensure you reference preview.locator for both the slice selector and the legend
selector so the test fails if the Pie renders wrong series or hides the legend.
Comment on lines
+107
to
+113
| const preview = getPreview(dialog); | ||
| await expect(preview.locator(".leaflet-container")).toBeVisible({ | ||
| timeout: 15_000, | ||
| }); | ||
| await expect( | ||
| preview.locator(".leaflet-overlay-pane svg").first(), | ||
| ).toBeVisible({ timeout: 10_000 }); |
There was a problem hiding this comment.
Verify plotted markers, not only the Leaflet shell.
.leaflet-container and an overlay svg can exist even if the lat/lng rows were not turned into map markers correctly. Please add an assertion on rendered marker primitives/count for the 5 returned points.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/e2e/heavy-widgets.spec.ts` around lines 107 - 113, The test currently
only checks that the Leaflet container and overlay svg are visible; update the
assertion to verify the actual markers for the 5 returned points by adding an
assertion against the rendered marker primitives (e.g., count of elements
matching the marker selector) from the preview returned by getPreview(dialog):
locate either ".leaflet-marker-icon" (for default Leaflet icons) or the SVG
marker primitives inside ".leaflet-overlay-pane svg" (e.g., "circle" or
".marker" elements) and assert their count equals 5 with an appropriate timeout,
keeping the existing preview and locator usage.
Comment on lines
+147
to
+155
| // Zoom in and out via Leaflet controls — should not crash | ||
| const zoomIn = preview.locator(".leaflet-control-zoom-in"); | ||
| await expect(zoomIn).toBeVisible({ timeout: 5_000 }); | ||
| await zoomIn.click(); | ||
| await expect(mapContainer).toBeVisible(); | ||
|
|
||
| const zoomOut = preview.locator(".leaflet-control-zoom-out"); | ||
| await zoomOut.click(); | ||
| await expect(mapContainer).toBeVisible(); |
There was a problem hiding this comment.
This only tests zoom, not pan.
The test name and linked objective call out pan/zoom, but this segment only clicks zoom controls. Add a drag interaction on the map surface and assert the map remains functional after the pan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/e2e/heavy-widgets.spec.ts` around lines 147 - 155, The test currently
exercises only zoom controls (using zoomIn/zoomOut) but must also perform a pan;
add a drag interaction against the map surface (use the existing mapContainer
locator or preview.locator('.leaflet-container') to perform a mouse
drag/locator.dragTo) to simulate panning, then assert the map still renders
(e.g. await expect(mapContainer).toBeVisible() or another existing post-pan
assertion) to prove the map remains functional after the pan.
Comment on lines
+215
to
+224
| const preview = getPreview(dialog); | ||
| await expect(preview).toBeVisible({ timeout: 15_000 }); | ||
|
|
||
| // Graph toolbar proves NVL rendered successfully | ||
| const fitBtn = dialog.getByRole("button", { name: "Fit graph" }); | ||
| await expect(fitBtn).toBeVisible({ timeout: 10_000 }); | ||
|
|
||
| // Click Fit graph — should not crash | ||
| await fitBtn.click(); | ||
| await expect(dialog.getByText("Query Failed")).not.toBeVisible(); |
There was a problem hiding this comment.
Toolbar visibility is too weak for Graph coverage.
Fit graph proves the NVL UI mounted, but not that nodes and edges rendered, pan/zoom works, or styling rules apply. Those are explicit acceptance criteria for this PR, so the main Graph regression surface is still untested here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/e2e/heavy-widgets.spec.ts` around lines 215 - 224, The test currently
only verifies toolbar visibility by locating the "Fit graph" button (fitBtn) and
clicking it, which doesn't assert that nodes/edges rendered or pan/zoom/styling
work; update the heavy-widgets.spec.ts test to explicitly check graph rendering
and interactions: after getPreview(dialog) and ensuring preview visible, locate
the graph canvas/SVG element (e.g., query for NVL graph container or role),
assert that a minimum number of node and edge DOM elements are present, verify
styling by checking a node/edge has the expected CSS class or computed style,
and assert pan/zoom by performing a drag or wheel event on the graph element and
confirming node positions change (or transform attribute updates); keep the
existing fitBtn click but add these explicit assertions around getPreview, the
graph container, and node/edge selectors to satisfy the PR acceptance criteria.
|
alfredo1996
added a commit
that referenced
this pull request
May 10, 2026
…ph widgets (#486) (#518) Add heavy-widgets.spec.ts with 7 tests covering lazy-loaded widget types: Pie canvas render + dashboard add, Map Leaflet container + zoom controls, Graph NVL toolbar + Fit graph interaction, and incompatible data format error states for all three. Closes #486 Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
heavy-widgets.spec.tswith 7 E2E tests covering the three lazy-loaded widget typescreateTestDashboard()with cleanupCloses #486
Test plan
npx playwright test heavy-widgets— 7/7 passing🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes