Skip to content

Stabilize and Standardize VRT Infrastructure - #9651

Merged
arii merged 4 commits into
leaderfrom
vrt-infrastructure-stabilization-3688477558357611503
Mar 23, 2026
Merged

Stabilize and Standardize VRT Infrastructure#9651
arii merged 4 commits into
leaderfrom
vrt-infrastructure-stabilization-3688477558357611503

Conversation

@arii

@arii arii commented Mar 20, 2026

Copy link
Copy Markdown
Owner

This PR stabilizes the Visual Regression Testing (VRT) infrastructure by centralizing state management and UI stabilization into Playwright fixtures. It addresses flakiness caused by animations, scrollbars, and state leakage across tests.

Key changes:

  • Standardized Fixtures: dashboardPage, controlPage, mockPage, and connectPage now automatically handle authentication mocking, server state resets, and UI freezing.
  • UI Freezing: A new freezeUIForVRT helper aggressively disables all CSS animations, transitions, and scrollbars before the React tree mounts.
  • Global Masking: The takeScreenshot utility now automatically masks .variable-text-container elements.
  • Reliable Mobile Viewports: Added explicit awaiting for layout reflow and bounding box clipping for mobile screenshots.
  • Code Cleanup: Refactored vrt-dashboard.spec.ts, vrt-timer-controls.spec.ts, and vrt-components.spec.ts to use the new fixtures, reducing boilerplate and increasing maintainability.

Fixes #9649


PR created automatically by Jules for task 3688477558357611503 started by @arii

- Centralize setup (auth mock, server reset, UI freeze) and teardown (timer stop) in Playwright fixtures.
- Aggressively disable animations, transitions, and scrollbars via `freezeUIForVRT` (CSS injection).
- Automatically mask `.variable-text-container` in `takeScreenshot` globally.
- Improve mobile viewport VRT with layout awaiting and explicit clipping.
- Clean up spec files by removing redundant boilerplate.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions

Copy link
Copy Markdown
Contributor

👋 Welcome to HRM!

Thanks for your contribution. This repository uses Gemini AI for automated triage, code review, and generation.

🤖 Gemini Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-enrich Run PR Enrichment (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message
@pr-squash Squash PR commits (PR only)
@conflict-resolve Resolve merge conflicts (PR only)

For more details and GitHub CLI examples, see the Manual Trigger Guide.

@github-actions

Copy link
Copy Markdown
Contributor

ℹ️ Gemini Review Disabled

Automatic review is currently disabled via GEMINI_ENABLE_PR_REVIEW. To run review manually:

Via Comment:

@gemini-bot

Via GitHub Actions:

  1. Go to Actions → Reusable Gemini Review
  2. Click "Run workflow"
  3. Enter PR number: 9651

Via GitHub CLI:

gh workflow run reusable-gemini-review.yml -f pr_number=9651

🤖 Gemini Manual Trigger Guide

To re-enable: Set repository variable GEMINI_ENABLE_PR_REVIEW=true

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added the not reviewed The review could not be performed or completed successfully. label Mar 20, 2026
- Centralize setup (auth mock, server reset, UI freeze) and teardown (timer stop) in Playwright fixtures.
- Aggressively disable animations, transitions, and scrollbars via `freezeUIForVRT` (CSS injection).
- Automatically mask `.variable-text-container` in `takeScreenshot` globally.
- Improve mobile viewport VRT with layout awaiting and explicit clipping.
- Clean up spec files by removing redundant boilerplate.
- Fixed linting errors (unused imports and prettier formatting).

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 20, 2026
@arii

arii commented Mar 20, 2026

Copy link
Copy Markdown
Owner Author

🤖 AI Technical Audit

PR #9651: VRT Infrastructure Stabilization Review

🏗️ Architectural Impact

This PR significantly shifts the burden of test environment consistency from individual test files to Playwright fixtures. By leveraging base.extend, we ensure that every test page starts with a clean server state, mocked auth, and a "frozen" UI. This is a robust pattern that will reduce VRT flakiness across the suite.

🛡️ ANTI-AI-SLOP DIRECTIVES

  1. OVERLY VERBOSE COMMENTS:
    • tests/playwright/fixtures.ts: The numbered comments (1, 2, 3) and "Teardown" label state what is immediately obvious from the code. Recommend removal.
    • tests/playwright/lib/setup.ts: The JSDoc for freezeUIForVRT and mockLoggedInSession repeats the function name in prose. Simplified JSDocs are preferred.
  2. OVER-ENGINEERING:
    • freezeUIForVRT uses both addInitScript and addStyleTag with identical CSS content. This is redundant. Using addInitScript alone is sufficient to inject the style before the DOM parses, covering both initial load and late injections.
  3. DUPLICATE HOOKS/TYPES:
    • The PR moves mockLoggedInSession and mockSpotifyAccessToken from mocks.ts to setup.ts. While the move is fine, ensure no other packages (like external unit tests) were importing from the old path.
  4. CODE RATIO:
    • Found 14 lines for deletion in tests/playwright/fixtures.ts and vrt-dashboard.spec.ts. Specifically, manual context management and browser.newContext() calls inside fixtures are redundant when Playwright provides context as a fixture.
  5. STALE FEATURES:
    • Verified: The old beforeAll/afterAll logic in vrt-dashboard.spec.ts and vrt-timer-controls.spec.ts has been successfully deleted in favor of fixtures.

📁 File-by-File Analysis

tests/playwright/fixtures.ts

Problem: Re-creating a new context manually using browser.newContext() inside the fixture is unnecessary and prevents Playwright from managing the context lifecycle/tracing effectively.

Implementation Sample:

// Change this:
dashboardPage: async ({ browser, request }, use) => {
  const context = await browser.newContext()
  const page = await context.newPage()
// To this:
dashboardPage: async ({ context, request }, use) => {
  const page = await context.newPage()

tests/playwright/lib/setup.ts

Problem: Redundant injection of CSS. addInitScript runs before the page script, so addStyleTag is a double-execution of the same logic.

Implementation Sample:

export async function freezeUIForVRT(page: Page): Promise<void> {
  await page.addInitScript(() => {
    const style = document.createElement('style')
    style.id = 'vrt-freeze-styles'
    style.textContent = `...` // contents
    document.head.appendChild(style)
  })
  // DELETE the page.addStyleTag call below
}

tests/playwright/lib/visual.ts

Problem: The masking logic for .variable-text-container assumes the element always exists. If it doesn't, page.locator() won't throw, but the array manipulation logic is a bit wordy.

Implementation Sample:

const existingMasks = [screenshotOptions.mask].flat().filter(Boolean);
const finalOptions = {
  ...SCREENSHOT_OPTIONS,
  ...screenshotOptions,
  mask: [...existingMasks, page.locator('.variable-text-container')],
};

Review automatically published via RepoAuditor.

- Refactor Playwright fixtures to use provided `context`, improving lifecycle management.
- Remove redundant boilerplate and verbose comments in fixtures and spec files.
- Simplify `freezeUIForVRT` by removing redundant `addStyleTag` and keeping only `addInitScript`.
- Clean up JSDocs in `setup.ts`.
- Refine masking logic in `takeScreenshot` for better readability and robustness.
- Fix linting errors in `mocks.ts`.
- Maintain all previous stabilization improvements (auth mock, server reset, mobile viewport clipping).

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 20, 2026
@arii

arii commented Mar 21, 2026

Copy link
Copy Markdown
Owner Author

@Gemini-bot

@github-actions

github-actions Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

🤖 Command Operation Status

Review: Success


Triggered by: @arii via comment

@arii

arii commented Mar 21, 2026

Copy link
Copy Markdown
Owner Author

Reviewed commit: 6bb9d108c2d0b7ef7f3d8d834da4462b73d9e886

This PR significantly stabilizes and standardizes the Visual Regression Testing (VRT) infrastructure, which is a critical improvement for the project's quality assurance. The changes demonstrate a strong adherence to the VRT stabilization standards outlined in .github/copilot-instructions.md.

Key improvements include:

  1. Centralized and Isolated Test Setup: Moving common setup logic (like mockLoggedInSession, resetServerState, freezeUIForVRT) into Playwright fixtures (tests/playwright/fixtures.ts) and applying setupMinimalVisualRegressionTest on a per-test basis in vrt-components.spec.ts, vrt-dashboard.spec.ts, and vrt-timer-controls.spec.ts drastically improves test isolation. This addresses pre-existing technical debt related to tightly coupled tests and flakiness caused by shared state across test runs.
  2. Enhanced VRT Stability: The freezeUIForVRT function, now injected via page.addInitScript in tests/playwright/lib/setup.ts, ensures animations and transitions are disabled before page load. This is a robust approach to prevent visual inconsistencies and flaky snapshots.
  3. Global Dynamic Content Masking: The addition of global masking for .variable-text-container in tests/playwright/lib/visual.ts is an excellent step to prevent VRT failures due to dynamic text content, further enhancing test reliability.
  4. Improved Test Teardown: The inclusion of stopTimer in the controlPage fixture's teardown ensures a clean state after each test, preventing resource leaks and state contamination.
  5. Code Organization: Refactoring MOCK_IMAGE, mockLoggedInSession, and mockSpotifyAccessToken into tests/playwright/lib/setup.ts from mocks.ts improves the logical grouping of VRT setup utilities, promoting the DRY principle.
  6. New Mobile Viewport Test: The addition of a mobile viewport test for the dashboard is valuable for ensuring responsive design consistency.

Overall, this PR is a well-executed refactor that significantly enhances the reliability and maintainability of our VRT suite. The changes align perfectly with our guiding principles for code quality and conciseness.

Reviewed at commit: 6bb9d108c2d0b7ef7f3d8d834da4462b73d9e886


🤖 Gemini Manual Trigger Guide

@arii arii added approved tests and removed not reviewed The review could not be performed or completed successfully. labels Mar 21, 2026
- Refactor Playwright fixtures to use provided `context`, improving lifecycle management.
- Remove redundant boilerplate and verbose comments in fixtures and spec files.
- Simplify `freezeUIForVRT` by removing redundant `addStyleTag` and keeping only `addInitScript`.
- Clean up JSDocs in `setup.ts`.
- Refine masking logic in `takeScreenshot` for better readability and robustness.
- Fix linting errors in `mocks.ts`.
- Maintain all previous stabilization improvements (auth mock, server reset, mobile viewport clipping).

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed approved labels Mar 21, 2026
@arii
arii marked this pull request as ready for review March 23, 2026 05:37
@arii
arii merged commit 44e9477 into leader Mar 23, 2026
29 checks passed
@arii
arii deleted the vrt-infrastructure-stabilization-3688477558357611503 branch March 23, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure not reviewed The review could not be performed or completed successfully. refactor testing tests vrt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Report] VRT Infrastructure Stabilization and Standardization

1 participant