chore: Refactors visual tests setup by pan-kot · Pull Request #393 · cloudscape-design/board-components

pan-kot · 2026-02-10T11:49:09Z

Description

The visual tests are refactored for more code reuse between different scenarios so that optimisations for less flakiness can be applied to all tests.

Rel: D383236509

Similar: cloudscape-design/chart-components#163

How has this been tested?

I tested that by running visual refresh tests locally (npm run test:visual). The first run generates screenshots - the second one generates new screenshots and compares them to ones generated previously.

Note: the Visual Regressions action is expected to fail in this PR. I expect it to work and produce less flakiness in future PRs to charts. I will experiment with that more once this PR is merged.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

Changes include appropriate documentation updates.
Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
Changes do not include unsupported browser features, see CONTRIBUTING.md.
Changes were manually tested for accessibility, see accessibility guidelines.

Security

If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

Changes are covered with new/existing unit tests?
Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2026-02-10T11:51:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.11%. Comparing base (6cae59e) to head (1ec355a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #393      +/-   ##
==========================================
- Coverage   95.54%   95.11%   -0.43%     
==========================================
  Files          67       67              
  Lines        3278     3278              
  Branches      706      702       -4     
==========================================
- Hits         3132     3118      -14     
- Misses        146      160      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pan-kot · 2026-02-10T12:34:25Z

test/visual/drag-states.test.ts

-      await page.keys(["Enter"]);
-      await page.keys(["ArrowLeft"]);
-      expect(await page.fullPageScreenshot()).toMatchImageSnapshot();
+for (let i = 1; i <= 3; i++) {


Before my changes, we could call toMatchImageSnapshot() within the same test multiple times. The snapshots would then be stored with a suffix like -2 and -3 for the subsequent calls.

However, the counter is also added when the tests are retried with vitest retry configuration - which contradicts the idea of the retries. To mitigate that, I updated the generated snapshot names and removed the counter from there. Now, there must be a single snapshot taken per test.

pan-kot · 2026-02-10T12:36:01Z

test/visual/index.test.ts

+  return `/#${page}?${urlParams.toString()}`;
+}

+const ignoredPages = new Set([


Before my change, we run tests on all pages with .screenshot-area - which actually missed some useful pages, as the screenshot area is not really a requirement for the full-page screenshots. The updated config captures all pages except those where it does not make sense.

pan-kot · 2026-02-10T12:36:21Z

test/visual/index.test.ts

+  .map((page) => "/" + path.relative("../../pages/", page))
+  .filter((page) => !ignoredPages.has(page));
+
+const rtlStaticPages = ["/dnd/engine-a2h-test", "/with-app-layout/integ"];


This is new. Previously only lrt direction was covered.

pan-kot · 2026-02-10T12:37:25Z

test/visual-test-setup.ts


 const toMatchImageSnapshot = configureToMatchImageSnapshot({
  customSnapshotsDir: snapshotDir,
+  customSnapshotIdentifier: ({ currentTestName }) => currentTestName.replace("/#/", "").replace(/[\s]/g, "-"),


this makes generated snapshots better organised (with folders) and removes the counter from the name, which is affected by retries config.

Who-is-PS

Minor suggestion: Consider extracting the timing values into named constants for better readability and easier tuning. What do you think about this?

Who-is-PS · 2026-02-11T15:57:36Z

test/visual/utils.ts

+    await this.browser.execute((target) => {
+      (document.querySelector(target) as HTMLButtonElement)!.focus();
+    }, selector);
+    await this.browser.keys(["ArrowDown"]);


Why was ArrowDown added to focus()? Is this intentional or a mistake?

I think there was a bug with that focus outline that got fixed but w/o updating the tests. The focus outline is expected to be shown only when the last user interaction was keyboard-based. Calling the ".focus()" is artificial and it does not satisfy the criteria (the focus outline would be shown if we focus the handles with Tab navigation, for instance).

Got it, makes sense, triggers :focus-visible styling. Thanks for the clarification!

Who-is-PS · 2026-02-11T16:00:09Z

package-lock.json

      "dev": true,
      "license": "MIT"
    },
+    "node_modules/react-keyed-flatten-children": {


react-keyed-flatten-children was added but doesn't appear used in these changes. Could be unrelated or needed elsewhere? Could you please explain.

This came from installing dependencies (npm install) - and is likely due to changes in the components dependency.

Ah, transitive dependency. Thanks!

Who-is-PS · 2026-02-11T16:02:37Z

test/visual/utils.ts

+    await browser.url(url);
+    const page = new DndPageObject(browser);
+    await page.waitForVisible("main");
+    await page.waitForJsTimers(500);


Did you test it with a lower number? This could cause more delay per test

I selected a rather large number in an attempt to make the flakiness minimal. It does affect the completion time of the visual regression CI action - but this time is still lower than the other actions, so it is fine to have it like that. It is also easily reversible if the visual regression tests running time will ever be a problem.

Fair enough, makes sense prioritizing stability over speed. Thanks for the explanation!

Who-is-PS · 2026-02-11T16:03:26Z

test/visual/utils.ts

+
+  async fullPageScreenshot() {
+    // Necessary for animations to complete.
+    await this.pause(200);


The original value was 100ms, is the increase to 200ms necessary? With many screenshots, this doubles the wait time per test.

It doubles the waiting time before interactions but it is still a small increase to the total execution time of each test, where the largest portion is setting up the test session. I am not sure if this particular increase makes a real difference, but it might have some effect since the most of flakiness is now observed in the dynamic tests (those where we trigger some actions before taking a screenshot).

Got it, thanks.

pan-kot · 2026-02-11T16:24:00Z

Minor suggestion: Consider extracting the timing values into named constants for better readability and easier tuning. What do you think about this?

TBH, I think this is not very practical here, as there is no need to reuse them and there is no added benefit of giving these values names: the values are based on practical experiments rather than explicit constraints (such as matching certain timeouts present in the feature implementation).

chore: Refactors visual tests setup

1ec355a

pan-kot mentioned this pull request Feb 10, 2026

chore: Refactors visual tests setup cloudscape-design/chart-components#163

Merged

pan-kot deployed to dev-pages February 10, 2026 11:51 — with GitHub Actions View deployment

pan-kot commented Feb 10, 2026

View reviewed changes

pan-kot marked this pull request as ready for review February 10, 2026 12:37

pan-kot requested a review from a team as a code owner February 10, 2026 12:37

pan-kot requested review from Who-is-PS and removed request for a team February 10, 2026 12:37

Who-is-PS reviewed Feb 11, 2026

View reviewed changes

Who-is-PS approved these changes Feb 11, 2026

View reviewed changes

pan-kot added this pull request to the merge queue Feb 11, 2026

Merged via the queue into main with commit a1466f0 Feb 11, 2026
81 of 83 checks passed

pan-kot deleted the refactor-visual-tests branch February 11, 2026 17:03

Comments

Conversation

pan-kot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Correctness

Security

Testing

Uh oh!

codecov bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Who-is-PS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan-kot commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pan-kot commented Feb 10, 2026 •

edited

Loading

codecov bot commented Feb 10, 2026 •

edited

Loading