Skip to content

chore: 🤖 support local visual regression testing via docker#931

Open
punkbit wants to merge 19 commits intochore/init-css-modules-setupfrom
chore/vis-regression-docker-setup-consistent-screenshots
Open

chore: 🤖 support local visual regression testing via docker#931
punkbit wants to merge 19 commits intochore/init-css-modules-setupfrom
chore/vis-regression-docker-setup-consistent-screenshots

Conversation

@punkbit
Copy link
Collaborator

@punkbit punkbit commented Mar 17, 2026

Why?

Ensures consistent visual regression snapshots by enabling local testing within a Dockerized environment. This prevents false positive diffs caused by operating system variations, e.g., macOS vs. linux.

⚠️ Depends on #810 (should be merged first)

How?

  • Create Docker setup
  • Create Docker compose to ease orchestration
  • Create script to handle initialisation and update
  • Create visual regression testing for all button states
  • Add documentation

Preview?

👀 Playwright highlights elements it's targeting/clicking with blue styles to help you see what's happening during test execution. It does NOT affect the component's actual styles or screenshots

Screenshot 2026-03-17 at 10 13 20

@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2026

⚠️ No Changeset found

Latest commit: f98c9bc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@punkbit punkbit changed the base branch from main to chore/init-css-modules-setup March 17, 2026 10:27
@ClickHouse ClickHouse deleted a comment from github-actions bot Mar 17, 2026
@ClickHouse ClickHouse deleted a comment from github-actions bot Mar 17, 2026
punkbit added 5 commits March 17, 2026 12:56
…ar. The docker rm -f playwright-ui command handles both stopping and removing the container
…rrect for a group of related buttons) and updated the test to use page.getByRole('group')
@ClickHouse ClickHouse deleted a comment from github-actions bot Mar 17, 2026
@github-actions
Copy link

github-actions bot commented Mar 17, 2026

Claude finished @punkbit's task in 1m 45s —— View job


PR Review

  • Gather context
  • Read key files
  • Analyze changes
  • Post review

Solid PR — the Dockerized visual regression approach is the right call for cross-OS consistency, and the test coverage is thorough. All previous feedback has been addressed. Here's the full picture:

✅ Previously addressed

  • animations: 'disabled' on all loading-state snapshots ✓
  • overview.spec.ts uses page.getByRole('group')
  • ButtonGroup.tsx has role="group" on the wrapper ✓
  • if (box) guard replaced with explicit throw new Error(...)
  • Typo separatilyseparately fixed ✓
  • Docker cleanup simplified ✓

💡 Suggestions (non-blocking)

  • tests/buttons/button.spec.ts — Uses waitUntil: 'networkidle' inline, while overview.spec.ts uses the two-step domcontentloaded + waitForLoadState('networkidle') pattern. The two-step approach is more explicit and better aligns with Playwright's recommended practices. Minor inconsistency worth aligning eventually. Fix this →

  • docker-compose.playwright.yml:11CI=true triggers the Storybook dev server inside the container on every test run. Serving a pre-built static Storybook via http-server (which already has a storybook:serve script in package.json) would be faster and more cache-friendly in CI.


✅ Everything looks good — clean implementation, well-structured tests covering all button variants, interactive states, and a11y checks (disabled + loading aria attributes, keyboard navigation). Nice work! 🙌

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

Claude finished @punkbit's task in 1m 52s —— View job


PR Review

  • Gather context
  • Read key files
  • Analyze changes
  • Post review

Solid Docker + visual regression setup. The approach is well-structured and the test coverage is comprehensive. One carry-over from the previous review plus a new observation:

💡 Suggestions (non-blocking)

  • tests/buttons/button.spec.tswaitUntil: 'networkidle' is still used inline on every page.goto() call, while overview.spec.ts uses the two-step domcontentloaded + waitForLoadState('networkidle') pattern. Minor inconsistency worth aligning when convenient. Fix this →

  • playwright.config.ts:6 + docker-compose.playwright.yml:13CI=true inside Docker causes reuseExistingServer: false, so every test run spins up yarn storybook (the dev server) fresh from scratch. For a faster, more deterministic Docker run, consider switching to the pre-built static output: replace command: "yarn storybook" with command: "yarn storybook:build && yarn storybook:serve" (or just pre-build into the image), and update the port accordingly. The storybook:serve script already exists in package.json. Fix this →

  • tests/utils/index.ts — The new getStoryUrl utility and overview.spec.ts's local getStory helper serve slightly different purposes (one returns a string, the other an object with id + pathname). Fine as-is, but if button.spec.ts-style tests expand, consider unifying to reduce drift.


✅ Everything looks good — Docker setup is clean, tests cover all button variants (types, loading, disabled, icons, layout, interactive states, a11y), and snapshots are correctly scoped to linux/chromium. Nice work! 🙌

@workflow-authentication-public
Copy link
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-g45xwr8sa-clickhouse.vercel.app

Built from commit: 26c91489e0245a8a6e8c113eea685383b36e9ec1

@punkbit punkbit requested a review from Firefds March 17, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants