Skip to content

chore(ci): improve visual regression test time - #486

Open
coryrylan wants to merge 1 commit into
mainfrom
topic-ci-perf-fixes
Open

coryrylan wants to merge 1 commit into
mainfrom
topic-ci-perf-fixes

Conversation

@coryrylan

@coryrylan coryrylan commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Tests
    • Expanded visual coverage for Monaco’s light and dark themes, including input, editor, diff, and problems views.
    • Improved consistency of visual test runs across projects, with controlled parallel execution and more reliable cleanup and reporting.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 06a597e and 5612784.

Files selected for processing: 13
  • .gitignore
  • projects/core/vitest.visual.ts
  • projects/internals/vite/src/configs/visual.js
  • projects/internals/vite/src/runners/playwright.js
  • projects/internals/vite/src/runners/visual-test.js
  • projects/internals/vite/src/runners/visual.js
  • projects/internals/vite/src/setup/visual.js
  • projects/media/vitest.visual.ts
  • projects/monaco/src/fixtures.test.visual.ts
  • projects/monaco/src/index.dark.test.visual.ts
  • projects/monaco/src/index.test.visual.ts
  • projects/monaco/vitest.visual.ts
  • projects/viewport/vitest.visual.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

Walkthrough

The change updates visual test runner lifecycle and execution settings across several projects. It also consolidates Monaco visual fixtures and adds a separate dark-theme visual test.

Changes: Visual test execution

Layer / File(s) Summary
Visual runner lifecycle
projects/internals/vite/src/configs/visual.js, projects/internals/vite/src/runners/*, projects/internals/vite/src/setup/visual.js, .gitignore
The visual test config selects VisualTestRunner. The runner adds worker cleanup, pool-specific instance IDs, and report-collection control. The Playwright runner recursively creates the distribution directory. The setup opens the runner in beforeAll, and .gitignore excludes .visual/port-* paths.
Visual test execution settings
projects/core/vitest.visual.ts, projects/media/vitest.visual.ts, projects/viewport/vitest.visual.ts, projects/monaco/vitest.visual.ts
The configurations enable file parallelism, set a two-worker limit and a concurrency limit of one. Monaco’s include pattern now matches src/index*.test.visual.ts.
Shared Monaco visual fixtures
projects/monaco/src/fixtures.test.visual.ts, projects/monaco/src/index*.test.visual.ts
The Monaco tests use shared fixture templates and readiness checks for light and dark themes. The dark-theme test checks that the maximum visual difference is at most 5%.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant Vitest
  participant VisualTestRunner
  participant VisualRunner
  participant VitePlaywrightRunner
  Vitest->>VisualTestRunner: run files
  VisualTestRunner->>VisualRunner: close during worker cleanup
  VisualRunner->>VitePlaywrightRunner: close with collectReports false
Loading

Possibly related PRs

  • NVIDIA/elements#479: Changes Lighthouse and Playwright runner performance and concurrency, which intersects with this PR’s runner lifecycle and port handling.
  • NVIDIA/elements#449: Changes the shared VitePlaywrightRunner lifecycle and concurrency infrastructure.
  • NVIDIA/elements#475: Changes the VitePlaywrightRunner.open() startup path that this PR also updates.

Suggested labels: scope(ci), scope(internals), scope(core), scope(media), scope(monaco)

Suggested reviewers: johnyanarella

Merge Risk: Minimal · up to 56127

No merge-blocking issue was established. Run the normal visual checks before merging.

Pre-merge checks: Passed 4
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main changes, which improve visual regression test performance through parallelism and runner updates.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

projects/core/vitest.visual.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

projects/media/vitest.visual.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

projects/monaco/src/fixtures.test.visual.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

  • 4 others

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

Copy link
Copy Markdown

Code coverage overview

Languages: TypeScript

TypeScript / code-coverage/vitest

main 06a597e topic-ci-perf-fixes 5612784 +/-
99% 99% 0%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant