Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Docker-backed Playwright component testing for ChangesWebview visual testing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PlaywrightContainer
participant PlaywrightCT
participant Codecov
GitHubActions->>PlaywrightContainer: Run filtered visual workflow
PlaywrightContainer->>PlaywrightCT: Execute RooHero screenshot tests
PlaywrightCT->>Codecov: Upload coverage-ct/lcov.info
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
da45467 to
32b1842
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/visual-regression.yml:
- Around line 27-28: Update the actions/checkout step in the visual regression
workflow to set persist-credentials to false, preventing the checkout action
from retaining the GITHUB_TOKEN while PR-controlled test results are present.
Keep the existing pinned checkout action unchanged.
In `@knip.json`:
- Line 7: Update the Knip configuration entry for the playwright-ct plugin from
disabled to enabled so Knip discovers webview-ui/playwright/index.tsx and the
custom Playwright CT test files, while preserving the existing CT dependency
handling.
In `@webview-ui/package.json`:
- Line 12: Remove the test:visual:update script from package.json so committed
visual snapshots cannot be generated through host rendering. Preserve
test:visual:docker:update as the supported baseline-update command, consistent
with the Docker-only source-of-truth workflow.
In `@webview-ui/playwright-ct.config.ts`:
- Around line 16-18: The visual regression workflow uploads CT coverage that is
never generated. Update the Playwright CT reporter configuration in
webview-ui/playwright-ct.config.ts and the test:visual script in
webview-ui/package.json to collect and write coverage to
webview-ui/coverage-ct/lcov.info before upload, or remove the coverage upload if
CT coverage is not intended.
In `@webview-ui/playwright/run-docker.mjs`:
- Line 18: Update webview-ui/playwright/run-docker.mjs at line 18 to pass an
environment object to spawnSync that sets UID and GID from process.getuid() and
process.getgid(), while preserving any explicit environment overrides. Update
webview-ui/docker-compose.visual.yml at line 5 to replace the 1000 UID/GID
fallbacks with required interpolation or a clear missing-variable error,
preventing direct Compose usage from silently selecting another identity.
In `@webview-ui/playwright/vscode-theme-dark.css`:
- Around line 3-6: Update the font-family declarations in the theme stylesheet
to lowercase the value keywords BlinkMacSystemFont, Consolas, and Menlo,
preserving the existing font order and fallback values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d17b0be-d4c4-4795-b8db-5677e1af1c4c
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlwebview-ui/src/components/welcome/__tests__/__screenshots__/zoo-hero-dark.pngis excluded by!**/*.png
📒 Files selected for processing (16)
.github/workflows/code-qa.yml.github/workflows/visual-regression.ymlcodecov.ymlknip.jsonwebview-ui/.gitignorewebview-ui/AGENTS.mdwebview-ui/docker-compose.visual.ymlwebview-ui/package.jsonwebview-ui/playwright-ct.config.tswebview-ui/playwright/index.htmlwebview-ui/playwright/index.tsxwebview-ui/playwright/run-docker.mjswebview-ui/playwright/vscode-theme-dark.csswebview-ui/src/components/welcome/__tests__/RooHero.visual.tsxwebview-ui/tsconfig.jsonwebview-ui/vitest.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
webview-ui/playwright/monocart-reporter.d.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the local declaration with the package’s coverage API.
monocart-reporter@2.9.20typestestInfoas PlaywrightTestInfo, butcoverageDataasany[] | any. Replace this localany[]and add a local/external coverage-data type if invalid coverage payloads should not be allowed at this integration boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/playwright/monocart-reporter.d.ts` around lines 1 - 3, Update the addCoverageReport declaration in the monocart-reporter module to match monocart-reporter@2.9.20: type testInfo as Playwright TestInfo and coverageData as any[] | any, or introduce a suitable local/external coverage-data type that preserves the package’s accepted shapes while rejecting invalid payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webview-ui/src/components/welcome/__tests__/RooHero.visual.tsx`:
- Around line 31-40: Generate the missing hovered-state screenshot baseline for
the RooHero visual test using the Docker-backed visual test workflow, then
commit zoo-hero-dark-hover.png. Use pnpm test:visual:docker for comparison and
pnpm test:visual:docker:update only to intentionally create or update the
baseline.
---
Nitpick comments:
In `@webview-ui/playwright/monocart-reporter.d.ts`:
- Around line 1-3: Update the addCoverageReport declaration in the
monocart-reporter module to match monocart-reporter@2.9.20: type testInfo as
Playwright TestInfo and coverageData as any[] | any, or introduce a suitable
local/external coverage-data type that preserves the package’s accepted shapes
while rejecting invalid payloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58e3683d-b3b3-4758-aa3e-8611fdc47672
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/visual-regression.ymlknip.jsonwebview-ui/.gitignorewebview-ui/AGENTS.mdwebview-ui/docker-compose.visual.ymlwebview-ui/package.jsonwebview-ui/playwright-ct.config.tswebview-ui/playwright/coverage-fixture.tswebview-ui/playwright/monocart-reporter.d.tswebview-ui/playwright/run-docker.mjswebview-ui/src/components/welcome/__tests__/RooHero.visual.tsxwebview-ui/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/visual-regression.yml
- webview-ui/docker-compose.visual.yml
- webview-ui/vitest.config.ts
- webview-ui/package.json
- webview-ui/playwright/run-docker.mjs
- knip.json
- webview-ui/AGENTS.md
- webview-ui/playwright-ct.config.ts
| test("renders the welcome hero hovered state", async ({ mount }) => { | ||
| const component = await mount(<RooHero />) | ||
|
|
||
| await waitForAssetsAndRender(component) | ||
| await component.hover() | ||
|
|
||
| await expect(component).toHaveScreenshot("zoo-hero-dark-hover.png", { | ||
| animations: "disabled", | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Commit the hovered-state baseline before merging.
The CI check reports that zoo-hero-dark-hover.png is missing, so the assertion at Line 37 fails. Generate and commit the expected snapshot using the Docker-backed visual test workflow.
As per coding guidelines, use pnpm test:visual:docker for comparison and pnpm test:visual:docker:update only for intentional baseline updates.
🧰 Tools
🪛 GitHub Actions: Webview Visual Regression / 0_webview-visual.txt
[error] 31-31: Playwright visual test failed: "renders the welcome hero hovered state" (chromium).
🪛 GitHub Actions: Webview Visual Regression / webview-visual
[error] 31-31: Playwright visual test failed for hovered state. Step: "playwright test -c playwright-ct.config.ts". Test: "renders the welcome hero hovered state" (chromium).
🪛 GitHub Check: webview-visual
[failure] 37-37: [chromium] › src/components/welcome/tests/RooHero.visual.tsx:31:1 › renders the welcome hero hovered state
-
[chromium] › src/components/welcome/tests/RooHero.visual.tsx:31:1 › renders the welcome hero hovered state
Error: A snapshot doesn't exist at /__w/Zoo-Code/Zoo-Code/webview-ui/src/components/welcome/tests/screenshots/zoo-hero-dark-hover.png, writing actual.35 | await component.hover()
36 |
> 37 | await expect(component).toHaveScreenshot("zoo-hero-dark-hover.png", {
| ^
38 | animations: "disabled",
39 | })
40 | })
at /__w/Zoo-Code/Zoo-Code/webview-ui/src/components/welcome/tests/RooHero.visual.tsx:37:2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/welcome/__tests__/RooHero.visual.tsx` around lines
31 - 40, Generate the missing hovered-state screenshot baseline for the RooHero
visual test using the Docker-backed visual test workflow, then commit
zoo-hero-dark-hover.png. Use pnpm test:visual:docker for comparison and pnpm
test:visual:docker:update only to intentionally create or update the baseline.
Sources: Coding guidelines, Linters/SAST tools
Related Issue
Closes: #515
Description
Adds a Playwright component-testing (CT) visual regression layer to
webview-ui/for components where layout, VS Code theme variables, or web-component rendering are under test—scenarios that Vitest/jsdom cannot verify. Also configures Playwright CT code coverage collection and Codecov ratcheting.What was added:
Visual Test Harness & Runner:
webview-ui/playwright-ct.config.ts— Component test config with Vite path aliases,vscodemock alias, absolute CI output paths, andpublicDirpointing tosrc/assets/images.webview-ui/playwright/— Harness setup:index.htmlapplies thevscode-darkbody class;index.tsxloads VS Code CSS variables, codicons, app stylesheet, and seedswindow.IMAGES_BASE_URI;vscode-theme-dark.cssprovides a complete set of--vscode-*tokens for Default Dark Modern theme.webview-ui/playwright/run-docker.mjs&docker-compose.visual.yml— Reproducible baseline Docker execution environment using a pinned Playwright Noble container, detecting Compose v2/v1 automatically..github/workflows/visual-regression.yml— CI workflow triggered onwebview-ui/**andsrc/shared/**changes; runs inside the pinned Playwright container and uploads test artifacts on failure.Reference Visual Test:
src/components/welcome/__tests__/RooHero.visual.tsx— Reference visual test rendering the welcome branding hero (RooHero) with font readiness andrequestAnimationFrameflushes before screenshotting.Code Coverage & Ratcheting Integration:
webview-ui/playwright/coverage-fixture.ts— Custom Playwright fixture extracting Istanbul coverage objects from CT page contexts and emitting LCOV reports.codecov.yml&.github/workflows/code-qa.yml— Configured webview JSDOM and Playwright CT coverage targets and ratcheting rules.Docs & Tooling Configuration:
webview-ui/AGENTS.md— Authoring guidance, multi-browser constraints, Docker baseline requirement, and coverage notes.knip.json,vitest.config.ts,tsconfig.json,.gitignore— Ignore/exclude visual test artifacts, coverage reports, and Playwright files from Vitest/build.Key Technical Decisions:
outputDirand HTML reporter output paths are resolved viapath.resolve(__dirname, ...)so artifacts land predictably inwebview-ui/regardless of working directory.RooHero(a toolkit-free component). Components rendering@vscode/webview-ui-toolkit/reactprimitives currently trigger module-cycle scope-hoisting issues in Vite 8 / Rolldown (vitejs/vite#22583). Toolkit-based visual tests will be supported once the app migrates off the archived toolkit or Rolldown fixes the issue.Test Procedure
Run visual tests locally (via Docker):
cd webview-ui pnpm test:visual:dockerRegenerate baselines after intentional UI changes:
CI Verification:
Webview Visual Regressionworkflow runs automatically on PRs touchingwebview-ui/**orsrc/shared/**.Code Qualityworkflow collects Playwright CT coverage and enforces Codecov ratcheting.Pre-Submission Checklist
pnpm check-types,pnpm lint, and all Vitest unit tests passwebview-ui/AGENTS.mdupdated with authoring guidance, Docker requirements, and coverage detailsSummary by CodeRabbit
Tests
Documentation
Chores