Add missing regression tests for CRT-1083, CRT-1078, CRT-705#6505
Merged
alantreadway merged 14 commits intolatestfrom Mar 25, 2026
Merged
Add missing regression tests for CRT-1083, CRT-1078, CRT-705#6505alantreadway merged 14 commits intolatestfrom
alantreadway merged 14 commits intolatestfrom
Conversation
Each test creates a real chart instance, triggers the exact failure scenario, and asserts observed internal state. All four tests were verified to fail with the corresponding fix reverted and pass with the fix in place.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
|
❌ Codex review complete; 1 issue found (P0: 0 | P1: 0 | P2: 1 | P3: 0) View full reviewAdd missing regression tests for CRT-1083, CRT-1064, CRT-1078, CRT-705PR: #6505 SummaryThis PR adds four targeted regression tests across community and enterprise suites for CRT-1083, CRT-705, CRT-1078, and CRT-1064. The tests mostly exercise the intended internal edge paths, but one case does not precisely match the documented null-datum scenario. FindingsP0: 0 | P1: 0 | P2: 1 | P3: 0 1 inline comments posted. VerdictAssessment: incorrect The PR is close, but one regression test does not faithfully reproduce the stated failure mode, leaving a coverage gap for CRT-1078. Required Actions:
|
CRT-705: Use dropdown.visible:'always' option instead of directly setting rangesModule.isDropdown = true. CRT-1064: Override contentGroup.pickNodes to inject the line path node (which naturally carries a boolean datum) into pick results, instead of appending a synthetic Rect to the scene graph.
…ghlightManager call Replaces highlightManager.updateHighlight() + getHighlightedDatum() with hoverAction on the legend item coordinates. The hover naturally triggers the null-datum highlight path, and without the fix the render cycle crashes accessing point.x on the undefined datum.
…JSDOM The typeof guard in pickNodesExactShape protects a canvas hit-testing path that cannot be exercised through user-facing options in JSDOM.
alantreadway
changed the title
alantreadway
force-pushed
the
merge/b13.2.0-to-latest
branch
from
30d0384 to
71fb6fb
Compare
…sophy - Document preference hierarchy: user-facing options > test harness utilities > internal state reads > internal state writes - Document that isPointInPath is stubbed in JSDOM and when to escalate to Playwright E2E tests
Contributor
|
Snapshots automatically updated, please review before merge: |
2 similar comments
Contributor
|
Snapshots automatically updated, please review before merge: |
Contributor
|
Snapshots automatically updated, please review before merge: |
The CDP tracing-based test intermittently fails in CI due to timing sensitivity, then cascades via serial mode to skip the remaining suite.
This reverts commit 91446f0.
lsjroberts
reviewed
Mar 25, 2026
Google Font downloads intermittently timeout in CI, causing flaky failures.
…test Address PR feedback: isDropdown is an implementation detail. Replace with an e2e test that verifies range buttons disappear when disabled and reappear when re-enabled, testing the user-visible behaviour.
lsjroberts
approved these changes
Mar 25, 2026
Member
lsjroberts
left a comment
lsjroberts
left a comment
There was a problem hiding this comment.
LGTM pending snapshot updates
alantreadway
commented
Mar 25, 2026
Use AgCharts.getInstance() + updateDelta() from the e2e test instead of exposing chart, AgCharts, and options on window.
Add Disable/Enable Ranges buttons so human operators can reproduce the issue manually. Simplify the e2e test to click the buttons directly.
Add @ag-skip-fws since the example uses direct DOM event listeners. Use AgCartesianChartOptions with number axes and valid button values.
Document the convention of adding interactive buttons to e2e examples so they serve as self-contained reproducers for human operators.
Direct DOM manipulation in e2e examples requires the @ag-skip-fws directive to prevent framework generation failures in CI.
alantreadway
added a commit
that referenced
this pull request
Mar 25, 2026
The enable/disable toggle test from PR #6505 uses waitForAllChartUpdates but the import was missing after rebase.
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
Adds regression tests for four b13.2.0 fixes that shipped without test coverage. Each test creates a real chart, triggers the exact failure scenario, and asserts internal state. All four were verified with the fix-revert protocol (fails without fix, passes with fix).
lineSeries.test.ts): Stale_contextNodeData.visiblewhen series hidden during animation skip — uses line series (not bar) because bar hasanimationAlwaysUpdateSelections: truewhich bypasses the affected pathmapMarkerSeries.test.ts): Legend hover crash whenhighlightedDatum.datum == null— exercisesgetHighlightedDatum()with an undefined datum highlightranges.test.ts):isDropdownstate not reset when ranges module disabled — verifies state resets to undefined on disableTest plan