Skip to content

feat(vrt): stabilize date-dependent Calendar and DatePicker stories#36437

Open
PaulGMardling wants to merge 14 commits into
microsoft:masterfrom
PaulGMardling:feat/35414-vr-drift-cleanup
Open

feat(vrt): stabilize date-dependent Calendar and DatePicker stories#36437
PaulGMardling wants to merge 14 commits into
microsoft:masterfrom
PaulGMardling:feat/35414-vr-drift-cleanup

Conversation

@PaulGMardling

@PaulGMardling PaulGMardling commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Previous Behavior

Calendar and DatePicker VRT stories relied on the current date when today and value were omitted, causing screenshots to change over time.

New Behavior

These stories now use a fixed reference date for deterministic screenshots.

This is the first targeted fix from #35414. Follow-up work will investigate:

  • Recurring Callout and Positioning pixel drift.
  • Asset readiness using native StoryWright/Playwright wait APIs.
  • Nx-native VRT scoping through package-level visual-regression projects.

@github-actions github-actions Bot added the CI label Jul 22, 2026
@PaulGMardling PaulGMardling changed the title Feat/35414 vr drift cleanup feat(vrt): optimize VRT tests produce diffs on most PRs Jul 22, 2026
@PaulGMardling PaulGMardling changed the title feat(vrt): optimize VRT tests produce diffs on most PRs feat(vrt): reduce PR VRT noise with selective story execution Jul 22, 2026
@github-actions

Copy link
Copy Markdown

📊 Bundle size report

✅ No changes found

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to reduce PR-time Visual Regression Test (VRT) noise by (1) selectively skipping unaffected stories per suite and (2) improving capture determinism by adding an explicit “render ready” signal and syncing the StoryWright patch metadata so immutable installs pass.

Changes:

  • Add a CI selector script and workflow steps to compute suite-specific StoryWright --excludePatterns based on affected packages, with a safe fallback to full runs.
  • Wire the computed exclude patterns into each VR suite’s test-vr script via env vars.
  • Harden v9 VRT capture timing via a Storybook readiness marker and a StoryWright patch that waits for it before screenshots.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
yarn.lock Sync StoryWright patch hash/checksum so yarn install --immutable succeeds.
apps/vr-tests/package.json Add optional --excludePatterns injection for the v8 VR suite.
apps/vr-tests-web-components/package.json Add optional --excludePatterns injection for the web-components VR suite.
apps/vr-tests-react-components/package.json Add optional --excludePatterns injection for the v9 VR suite.
apps/vr-tests-react-components/.storybook/preview.js Add a VRT readiness signal and asset-wait logic for improved capture stability.
.yarn/patches/storywright-npm-0.0.27-storybook7.14-bfcc4eb382.patch Patch StoryWright to wait for __fluentVrtReady__ before screenshot timing logic.
.github/workflows/pr-vrt.yml Run the selector per suite and export env vars before nx affected -t test-vr.
.github/scripts/build-vr-storywright-excludes.js New script that maps changed packages to story titles and emits StoryWright exclude regexes.

Comment thread .github/scripts/build-vr-storywright-excludes.js Outdated
Comment thread apps/vr-tests-react-components/.storybook/preview.js Outdated
@github-actions

Copy link
Copy Markdown

Pull request demo site: URL

…lure

Adds global.d.ts in .storybook/ to augment the Window interface with __fluentVrtReady__. This fixes TS2339 surfaced by vr-tests-react-components:type-check — preview.js uses // @ts-check and the tsconfig has checkJs: true, so the untyped window property assignment was a compile error.
Comment thread .github/scripts/build-vr-storywright-excludes.js Outdated
Convert preview.js to use CommonJS require and add proper JSDoc type annotations for all parameters and exports. This resolves all TS errors:
- TS1259: React module import (use require instead of import)
- TS7006/TS7031: Implicit any type errors on function parameters

The file uses @ts-check with checkJs: true in the tsconfig, so JSDoc annotations are necessary instead of TypeScript syntax.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 6 comments.

Comment thread apps/vr-tests-react-components/.storybook/preview.js Outdated
Comment thread .yarn/patches/storywright-npm-0.0.27-storybook7.14-bfcc4eb382.patch Outdated
Comment thread .github/scripts/build-vr-storywright-excludes.js Outdated
Comment thread apps/vr-tests-react-components/package.json Outdated
Comment thread apps/vr-tests/package.json Outdated
Comment thread apps/vr-tests-web-components/package.json Outdated
PaulGMardling and others added 2 commits July 23, 2026 10:10
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@PaulGMardling
PaulGMardling marked this pull request as ready for review July 23, 2026 11:58
@PaulGMardling
PaulGMardling requested review from a team as code owners July 23, 2026 11:58
@Hotell
Hotell requested review from Hotell and mainframev July 23, 2026 12:07
@PaulGMardling
PaulGMardling requested a review from Copilot July 23, 2026 14:49
Comment thread yarn.lock

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

apps/vr-tests-react-components/.storybook/preview.js:27

  • The PR description says V9 readiness hardening waits for "fonts/images/frame readiness" before capture, but waitForVrtAssets() currently only waits for fonts (best-effort) + two animation frames. Either add an image readiness check (best-effort + short timeout) or update the PR description to avoid overstating the behavior.
  // Keep readiness checks lightweight to avoid inflating total VRT runtime.
  const timeout = new Promise(resolve => setTimeout(resolve, 300));

  try {
    const fontsReady = document.fonts?.ready;

Comment thread .github/scripts/build-vr-storywright-excludes.js Outdated
Comment thread .github/scripts/run-storywright.js Outdated
Avoid shell interpolation when reading changed files, preserve StoryWright arguments without a separator, and include rendered images in the bounded VRT readiness check.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.

@Hotell Hotell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General feedback: this adds too much plumbing for uncertain gains.

  • We're not addressing the non-deterministic UI called out in the origin issue (Date.now(), Math.random(), missing value/today props). That was the primary root cause, and it's untouched here — so the noise it produces remains.

  • The timing mechanism (StoryWright patching + manual timers) feels brittle. SW already ships wait APIs that use Playwright under the hood; we should lean on those rather than a global timed heuristic in a vendored patch. If those genuinely don't cover the case, let's experiment with it as a follow-up rather than bundling it here.

  • The affected chain is managed imperatively (regex matchers + alias tables + title token-matching), which is fragile and risks silently skipping stories that should run. The direction we agreed some time ago was collocating VRT stories under the package layer (<package-name>/visual-regression) so nx affected scopes them natively — verbose, but it does exactly what we need. We can also lean on our workspace plugin to construct these sub-projects dynamically, without touching the file system.

@PaulGMardling

PaulGMardling commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

General feedback: this adds too much plumbing for uncertain gains.

  • We're not addressing the non-deterministic UI called out in the origin issue (Date.now(), Math.random(), missing value/today props). That was the primary root cause, and it's untouched here — so the noise it produces remains.
  • The timing mechanism (StoryWright patching + manual timers) feels brittle. SW already ships wait APIs that use Playwright under the hood; we should lean on those rather than a global timed heuristic in a vendored patch. If those genuinely don't cover the case, let's experiment with it as a follow-up rather than bundling it here.
  • The affected chain is managed imperatively (regex matchers + alias tables + title token-matching), which is fragile and risks silently skipping stories that should run. The direction we agreed some time ago was collocating VRT stories under the package layer (<package-name>/visual-regression) so nx affected scopes them natively — verbose, but it does exactly what we need. We can also lean on our workspace plugin to construct these sub-projects dynamically, without touching the file system.

Very fair feedback.
I’ll simplify the PR to address the known nondeterministic stories directly and move the broader VRT selection/timing work out, and maybe something to explore later, in another ticket if we can't get more out of the suggestions you mentioned.

Pin CalendarCompat and DatePickerCompat dates while removing the selective StoryWright execution and global readiness plumbing.
@github-actions github-actions Bot removed the CI label Jul 23, 2026
@PaulGMardling
PaulGMardling requested a review from Hotell July 23, 2026 18:21
@PaulGMardling PaulGMardling changed the title feat(vrt): reduce PR VRT noise with selective story execution feat(vrt): stabilize date-dependent Calendar and DatePicker stories Jul 24, 2026
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.

3 participants