feat: add chart ui e2e-test #82
Conversation
WalkthroughThis pull request introduces a comprehensive suite of visual regression tests for various chart components using the Playwright testing framework. The changes span multiple chart types including bar, boxplot, candle, funnel, gauge, graph, heatmap, histogram, line, map, pie, process, radar, ring, sankey, scatter, sunburst, tree, waterfall, and wordcloud charts. Each test navigates to specific demo pages, locates chart elements, and captures screenshots to ensure visual consistency across different chart configurations. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (39)
tests/chart/map/map.spec.ts (1)
3-7: Use of baseline screenshot approach is good, but consider an event-driven wait.
Relying solely on time-based waits from the shared config might lead to flakiness if charts take longer to render in some cases. Consider adding an explicit wait for a known element or event to ensure that the chart is fully rendered before taking a screenshot.tests/chart/graph/graph.spec.ts (1)
4-4: Consider using an event-based wait for dynamic charts.
The explicit 4000ms wait may be excessive or too short in certain environments. Using a condition-based wait (e.g., waiting for a specific DOM event or a data load completion) can reduce flakiness and ensure consistency.tests/chart/fixtures.js (2)
3-5: Configuring a global waitTime is convenient but may be prone to flakiness.
If chart rendering time varies, consider providing a more robust waiting mechanism or an optional override for longer or shorter waits in certain tests.
7-18: Custom test fixture is a good approach for shared wait logic.
The override ofpage.gotoensures consistent waiting across all tests. However, you might also support an event-based or condition-based wait to avoid blindly waiting the entire duration.tests/chart/sankey/sankey.spec.ts (2)
3-8: Suggest Using a Smaller Static WaitThe static wait time of 5000ms in line 4 might slow down CI/CD pipelines. Consider using a targeted approach like
page.waitForSelectorif there's a need to ensure content is fully loaded before taking the screenshot.
10-15: Screenshot Naming ConsistencyYour screenshot naming is helpful, but ensure that it remains consistent across all test files for clarity and easier maintenance. For example, consider prefixing with "sankey" for quick identification (e.g., "sankey_demo2.png").
tests/chart/radar/radar.spec.ts (1)
9-13: Improve Test DescriptionTo enhance clarity, consider adding more descriptive test titles, such as
'radar chart - demo2 configuration', making it easier to understand which scenario is being tested.tests/chart/heatmap/heatmap.spec.ts (2)
9-13: Use of Constants for SelectorsRepeated selector patterns (
'#heatmap-demo2 .hui-chart') can be prone to typos. Consider storing them in constants or test config for improved maintainability.
15-19: Naming Test IDsFor better readability, add purposeful naming to your test IDs (e.g.,
'heatmap - large dataset demo'), especially if you have more internal variations.tests/chart/scatter/scatter.spec.ts (3)
3-7: Clear Test TitleWhile "base" is short, it may be unclear to new contributors. A more descriptive name could be
"scatter chart - default view".
9-13: Avoid Potential Duplication
demo2documentation is minimal. If the differences from the base test are significant, consider adding short comments or docstrings indicating what’s being tested.
15-19: Screenshot Verification StrategyConsider storing baseline screenshots in a dedicated folder structure (e.g.,
tests/__screenshots__/scatter). This helps keep reference images organized and reduces clutter in the main directory.tests/chart/process/process.spec.ts (1)
3-7: Consider a parameterized testing approach.
These lines effectively test the screenshot for thedemo2scenario. Since the structure is duplicated across multiple demos in this file, you could abstract out the navigation and screenshot validation into a helper function or parameterized test to reduce repetition.tests/chart/sunburst/sunburst.spec.ts (1)
9-13: Refactor repeated patterns.
Like the previous tests,demo2also uses the same pattern of navigation and screenshot verification. Consider consolidating common code into a utility to reduce duplication across demos.tests/chart/wordcloud/wordcloud.spec.ts (1)
9-13: Unused or commented code.
Thedemo2test is commented out. If it’s no longer valid, consider removing it to keep the test file clean, or re-enable it if required for coverage.tests/chart/tree/tree.spec.ts (1)
1-31: Consider refactoring repetitive test patterns into a single parameterized test function.All added test cases follow the same pattern: navigate to a URL, locate the chart, and compare screenshots. While this is perfectly valid, you could streamline these tests with a parameterized approach, reducing duplicated code:
- Create an array of objects representing each test scenario (e.g.,
{ url: 'chart-tree#tree-base', screenshot: 'base.png' }).- Iterate through the array within a single
testor multipletest.describeblocks to handle all scenarios.tests/chart/candle/candle.spec.ts (1)
1-37: Enhance maintainability by parameterizing repetitive blocks.Similar to
tree.spec.ts, each test block duplicates the core logic of navigating to a URL and verifying a chart screenshot. Consider centralizing this logic in a helper or utilizing parameterized tests to:
- Reduce boilerplate.
- Simplify maintenance when future demos are added or existing demos change.
tests/chart/gauge/gauge.spec.ts (2)
1-3: Use explicit waiting instead of increasing a global wait time.You set
config.waitTime = 4000, which might unnecessarily lengthen all tests. Instead, consider using targeted waits (e.g.,page.waitForSelector(...)) or verifying that elements have fully rendered before taking screenshots. This approach ensures reliability, especially in faster or slower environments, without globally extending test duration.
4-39: Parameterize repeated chart tests.Just like in your other chart tests, the code repeatedly performs the same steps for different demos. A parameterized or data-driven approach would help minimize duplication and simplify adding new demo coverage. You can store test configurations in an array and optionally combine them with the explicit wait approach to ensure each scenario is thoroughly validated.
tests/chart/pie/pie.spec.ts (1)
1-43: Centralize test logic for repeated pie chart demos.Each test repeats the same tasks: navigation and screenshot comparison. For better maintainability and avoid repeated code blocks, consider one of these:
- Create a helper function that accepts the identifier (e.g.,
pie-demo3) and expected screenshot filename (e.g.,demo3.png).- Use a parameterized test setup or a data-driven approach, reducing code duplication and offering a single location for potential modifications later.
tests/chart/bar/bar.spec.ts (5)
1-2: Consider adding a wait or visibility check for consistency.
The first few tests usetoBeInViewport()to confirm the chart is visible before capturing a screenshot, which is a good practice. However, the subsequent tests (e.g.,demo3onward) skip this step and directly capture screenshots. Consider aligning all tests to consistently verify the chart visibility to reduce potential flakiness.
3-7: Ensure stable chart rendering before screenshot.
Although capturing a screenshot here works, using a dedicated wait strategy (likepage.waitForSelector) can prevent issues if the chart takes time to load.
15-19: Add a visibility check for consistency.
Unlike previous tests,demo3omitstoBeInViewport(). Consider using it ortoBeVisible()for uniformity across tests.
21-25: Maintain consistent test style.
In prior tests, you explicitly fetched the locator and performed a visibility or viewport check. For clarity, consider using the same pattern here.
33-37: Ensure uniform checks for loading.
Since the chart might take time to render, consider a uniform waiting approach for all tests, eitherpage.waitForSelectoror an increased default wait time.tests/chart/funnel/funnel.spec.ts (3)
3-3: Validate necessity of a fixed wait time.
Usingconfig.waitTime = 4000might mask potential loading or performance issues. A more targeted wait likewaitForSelectorcan offer granularity, though this approach is acceptable for quick reliability.
17-21: No viewport check.
Consider adding an explicit visibility or viewport check prior totoHaveScreenshot. This can help detect partial render issues, especially as funnel charts can vary in size.
29-33: Minor suggestion: unify step naming.
When naming screenshots for more advanced tests, consider more descriptive names (e.g.,funnel-demo5-loaded.png) if it clarifies what's being tested.tests/chart/ring/ring.spec.ts (5)
3-4: Potential duplication of waitTime configuration.
You setconfig.waitTime = 5000here and again at line 30. Consider setting it once at the top or adjusting it case-by-case in a shorter scope.
10-14: Use consistent checks.
Like before, consider verifying the chart is in the viewport or visible as well. Maintaining the same approach across tests promotes clarity.
29-34: Duplicate waitTime configuration.
You resetconfig.waitTime = 5000at line 30, but it was already set at line 3. Unless you intend to reduce it for other tests, consider removing redundancy.
36-40: Maintain uniform approach.
Just like the prior demos, verifying the chart is loaded or in viewport can further safeguard this test against timing issues.
42-47: Test naming improvement.
The test namering-titleis descriptive, but consider using the same naming convention (e.g.,demo7) for a consistent pattern.tests/chart/histogram/histogram.spec.ts (3)
3-7: Add explicit chart visibility checks.
WhiletoHaveScreenshotpartially confirms the chart is present, an explicittoBeVisibleortoBeInViewportcheck helps ensure no hidden rendering or partial load.
27-31: Potential for common test logic extraction.
If each chart fosters the same pattern, consider extracting a helper function to minimize repetition.
39-43: Encourage verifying element is fully rendered.
For large or dynamic data sets, verifying partial rendering might be insufficient for screenshot comparison. Consider an approach to confirm no placeholders remain.tests/chart/line/line.spec.ts (3)
1-2: Explicitly declare dependencies.The import statement references
../fixtures, presumably containing shared test setup. Ensure this file is well-documented or self-documenting, so future maintainers can easily locate and understand the shared fixtures.
9-19: Avoid duplication with a reusable test function or loop.Almost each test block repeats similar steps: navigating to a URL, locating a chart, and taking a screenshot. Consider abstracting these steps into a shared helper function or parameterized test to reduce code repetition. This approach will simplify maintenance and make it straightforward to add additional demos in the future.
- test('demo2', async ({ page }) => { - await page.goto('chart-line#line-demo2') - const chart = page.locator('#line-demo2 .hui-chart') - await expect(chart).toHaveScreenshot('demo2.png') - }) ... + const verifyScreenshot = async (page, id, screenshotName) => { + await page.goto(`chart-line#${id}`) + const chart = page.locator(`#${id} .hui-chart`) + await expect(chart).toHaveScreenshot(`${screenshotName}.png`) + } + test('demo2', async ({ page }) => { + await verifyScreenshot(page, 'line-demo2', 'demo2') + }) ...Also applies to: 27-55
21-25: Consider clarifying the reason for commented-out code.Leaving tests commented without context may cause confusion. If
demo4is intentionally skipped, consider using a.skipflag or adding a comment explaining why it’s disabled and under what conditions it should be re-enabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (91)
tests/chart/bar/bar.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/bar/bar.spec.ts-snapshots/demo7-chromium-win32.pngis excluded by!**/*.pngtests/chart/boxplot/boxplot.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/boxplot/boxplot.spec.ts-snapshots/multiple-chromium-win32.pngis excluded by!**/*.pngtests/chart/boxplot/boxplot.spec.ts-snapshots/vertical-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/candle/candle.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/funnel/funnel.spec.ts-snapshots/demo7-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/gauge/gauge.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/graph/graph.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/graph/graph.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/heatmap/heatmap.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/heatmap/heatmap.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/heatmap/heatmap.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo7-chromium-win32.pngis excluded by!**/*.pngtests/chart/histogram/histogram.spec.ts-snapshots/demo8-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo7-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo8-chromium-win32.pngis excluded by!**/*.pngtests/chart/line/line.spec.ts-snapshots/demo9-chromium-win32.pngis excluded by!**/*.pngtests/chart/map/map.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/pie/pie.spec.ts-snapshots/demo7-chromium-win32.pngis excluded by!**/*.pngtests/chart/process/process.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/process/process.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/process/process.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/radar/radar.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/radar/radar.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/radar/radar.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/demo6-chromium-win32.pngis excluded by!**/*.pngtests/chart/ring/ring.spec.ts-snapshots/ring-title-chromium-win32.pngis excluded by!**/*.pngtests/chart/sankey/sankey.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/sankey/sankey.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/scatter/scatter.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/scatter/scatter.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/scatter/scatter.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/sunburst/sunburst.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/sunburst/sunburst.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/sunburst/sunburst.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/tree/tree.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/tree/tree.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/tree/tree.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/tree/tree.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/tree/tree.spec.ts-snapshots/demo5-chromium-win32.pngis excluded by!**/*.pngtests/chart/waterfall/waterfall.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/waterfall/waterfall.spec.ts-snapshots/demo2-chromium-win32.pngis excluded by!**/*.pngtests/chart/waterfall/waterfall.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/waterfall/waterfall.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.pngtests/chart/wordcloud/wordcloud.spec.ts-snapshots/base-chromium-win32.pngis excluded by!**/*.pngtests/chart/wordcloud/wordcloud.spec.ts-snapshots/demo3-chromium-win32.pngis excluded by!**/*.pngtests/chart/wordcloud/wordcloud.spec.ts-snapshots/demo4-chromium-win32.pngis excluded by!**/*.png
📒 Files selected for processing (21)
tests/chart/bar/bar.spec.ts(1 hunks)tests/chart/boxplot/boxplot.spec.ts(1 hunks)tests/chart/candle/candle.spec.ts(1 hunks)tests/chart/fixtures.js(1 hunks)tests/chart/funnel/funnel.spec.ts(1 hunks)tests/chart/gauge/gauge.spec.ts(1 hunks)tests/chart/graph/graph.spec.ts(1 hunks)tests/chart/heatmap/heatmap.spec.ts(1 hunks)tests/chart/histogram/histogram.spec.ts(1 hunks)tests/chart/line/line.spec.ts(1 hunks)tests/chart/map/map.spec.ts(1 hunks)tests/chart/pie/pie.spec.ts(1 hunks)tests/chart/process/process.spec.ts(1 hunks)tests/chart/radar/radar.spec.ts(1 hunks)tests/chart/ring/ring.spec.ts(1 hunks)tests/chart/sankey/sankey.spec.ts(1 hunks)tests/chart/scatter/scatter.spec.ts(1 hunks)tests/chart/sunburst/sunburst.spec.ts(1 hunks)tests/chart/tree/tree.spec.ts(1 hunks)tests/chart/waterfall/waterfall.spec.ts(1 hunks)tests/chart/wordcloud/wordcloud.spec.ts(1 hunks)
🔇 Additional comments (42)
tests/chart/boxplot/boxplot.spec.ts (2)
3-7: Well-structured test case.
The test name is concise, and the screenshot assertion is clear.
9-13: Multiple scenario coverage looks good.
Tests for multiple boxplots provide better test coverage.tests/chart/graph/graph.spec.ts (2)
3-8: Base test logic is clear.
Navigating to the base demo and visually verifying the chart screenshot is useful for regression checks.
10-14: Additional coverage with 'demo2' is appreciated.
Testing distinct chart states or demos helps safeguard different configurations.tests/chart/radar/radar.spec.ts (2)
3-7: Verifying Page NavigationGood use of direct navigation to specific chart demos. Consider verifying any console errors on the page for extra reliability in your tests (e.g.,
page.on('pageerror', ...)).
15-19: Consistent Test PatternsIt's great to see consistent test patterns across different demos. Continue this pattern for simpler maintenance and uniform coverage.
tests/chart/heatmap/heatmap.spec.ts (1)
3-7: Ensure Adequate CoverageThe base test navigates to
#heatmap-base, but ensure all relevant heatmap configurations are covered if multiple visual variations or data sets exist beyond “base,” “demo2,” and “demo3.”tests/chart/process/process.spec.ts (3)
1-2: Good import practice.
The import from@playwright/testis the recommended approach for creating browser automation tests with Playwright.
9-13: Consistent naming for the screenshot.
The test namedemo3aligns well with the screenshot filenamedemo3.png, maintaining clarity and consistency.
15-19: Ensure coverage for multiple process demos.
The third test verifiesdemo6; if there are other demos (likedemo4,demo5) that need coverage, consider adding them. Otherwise, this test structure is consistent with the preceding ones.tests/chart/sunburst/sunburst.spec.ts (3)
1-2: Importing from shared fixtures.
Importingtestandexpectfrom a shared fixtures file simplifies test configuration. Ensure it’s updated if additional test utilities are needed.
3-7: Clear and focused test naming.
Thebaselabel succinctly describes the default or initial configuration being tested.
15-19: Consistent screenshot naming.
Testingdemo3aligns with the filenamedemo3.png.This clarity helps correlate test results with their references.tests/chart/waterfall/waterfall.spec.ts (5)
1-2: Shared fixtures usage is good.
Using a shared fixture for test setup is standard, ensuring consistent configuration across the suite.
3-7: Descriptive test naming.
The test namebaseindicates the baseline waterfall chart, aiding maintainability.
9-13: Screenshot-based regression checks.
Comparing the rendered chart against a reference image (demo2.png) is an effective visual regression approach.
15-19: Expand or clarify coverage.
demo3might represent a distinct variation of the waterfall chart. If more advanced scenarios exist, ensure they are added over time for comprehensive coverage.
21-25: Enhanced test coverage.
demo4coverage is consistent with the existing pattern. Continuing to add multiple demos ensures the waterfall chart behaves correctly across different configurations.tests/chart/wordcloud/wordcloud.spec.ts (4)
1-2: Shared fixtures import.
Importing from'../fixtures'promotes a consistent testing framework.
3-7: Baseline test clarity.
Thebasetest is a good starting point to validate the default appearance of the wordcloud chart.
15-19: Consistent test pattern.
Thedemo3test follows the same pattern, ensuring easy maintainability.
21-25: Verify full coverage.
Thedemo4test extends coverage. If there are additional demos or states for the wordcloud, consider adding them as well.tests/chart/bar/bar.spec.ts (3)
8-13: Kudos on verifying the chart is in the viewport and matching the baseline.
This approach helps ensure that the bar chart UI is rendered as expected. The usage oftoHaveScreenshotis both straightforward and effective.
27-31: Acknowledge well-structured test.
The flow—navigate, locate chart, capture screenshot—remains straightforward and consistent with other test demos.
39-43: Check potential loading times for final test.
If the chart is heavier or more complex, waiting for the UI to load might be beneficial to prevent flakiness in CI environments.tests/chart/funnel/funnel.spec.ts (6)
1-2: Good import structure and config usage.
Specifically importingconfigto overridewaitTimeis clear and keeps test-specific logic in one place.
5-9: Checks are straightforward and robust.
Ensuring the chart locator is in viewport before capturing screenshot is a solid practice to reduce test flakiness and verify the correct element is displayed.
11-15: Maintain consistent approach across tests.
Here, the chart is saved to a local variable before the screenshot step. This pattern is consistent; keep it uniform for clarity.
23-27: Continuous improvement on clarity.
Tests that contain the same step pattern—goto -> locate -> viewport check -> screenshot—are easier to maintain. Great consistency so far!
35-39: Ensure unstyled content does not appear.
Setting a wait time typically helps, but confirm that any data or style fetches are complete before capturing screenshots.
41-45: All-around thorough coverage.
Kudos on providing a wide range of funnel demos. These tests help ensure visual regressions are caught early.tests/chart/ring/ring.spec.ts (4)
1-2: Great use of config overrides.
Usingconfig.waitTime = 5000can help with more complex charts, but carefully confirm this is enough time if the chart data is fetched asynchronously.
4-8: Straightforward flow for the base chart test.
Navigating, checking the chart locator, and capturing the screenshot is a minimal yet effective approach.
16-20: Methodical structure recognized.
The repeated pattern—goto → locate → screenshot—makes these specs concise and easy to follow.
22-27: Visibility check is helpful.
Indemo4, you explicitly verify visibility. This consistency is beneficial.tests/chart/histogram/histogram.spec.ts (6)
1-2: Direct usage oftestandexpectis neat.
Centralizing test utilities in../fixturesfosters reuse.
9-13: Clear pattern repeated.
The minimal pattern—goto → locate → haveScreenshot—keeps tests concise and direct.
15-19: Consider wait strategies if needed.
If the chart is heavy or requires data fetching, a short wait orwaitForSelectorcan ensure the chart renders before comparing screenshots.
21-25: Keep naming consistent.
Different test demos are labeleddemo2,demo3, etc., which aligns well with typical incremental naming. Good job.
33-37: Test coverage is thorough.
You are systematically covering multiple histogram scenarios. This strongly supports the PR's stated objective of robust e2e coverage.
45-49: Well-defined incremental test coverage.
demo8extends coverage to additional use-cases. Great job systematically stepping through.tests/chart/line/line.spec.ts (1)
3-7: Good readability and clarity.The test steps are clear and concise, and the naming convention in
'base'effectively conveys the test’s purpose. No issues found here.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit