fix(webview): address code review feedback for Playwright CT harness

Author: edelauna

.github/workflows/visual-regression.yml

@@ -13,6 +13,10 @@ on:
     merge_group:
         types: [checks_requested]
 
+concurrency:
+    group: ${{ github.workflow }}-${{ github.ref }}
+    cancel-in-progress: true
+
 permissions:
     contents: read
 

webview-ui/docker-compose.visual.yml

@@ -5,7 +5,7 @@ services:
         user: "${UID:-1000}:${GID:-1000}"
         working_dir: /work
         environment:
-            COREPACK_HOME: /tmp/corepack
+            COREPACK_HOME: /work/node_modules/.cache/corepack
             HOME: /tmp/playwright
         volumes:
             - ..:/work

webview-ui/playwright-ct.config.ts

@@ -1,7 +1,8 @@
 import path from "path"
 import { fileURLToPath } from "url"
 
-import { defineConfig, type ReporterDescription } from "@playwright/experimental-ct-react"
+import { defineConfig } from "@playwright/experimental-ct-react"
+import type { ReporterDescription } from "@playwright/test"
 import react from "@vitejs/plugin-react"
 import tailwindcss from "@tailwindcss/vite"
 
@@ -15,18 +16,21 @@ const monocartReporter: ReporterDescription = [
 		coverage: {
 			outputDir: path.resolve(dirname, "coverage-ct"),
 			reports: ["lcovonly", "v8"],
-			// Entry URLs are the served asset chunks (`/assets/*.js`); filter kept
-			// permissive so V8 coverage can then source-map back to originals.
-			entryFilter: (entry: { url: string }) => entry.url.includes("/assets/"),
-			sourceFilter: (sourcePath: string) => sourcePath.includes("src/"),
+			// Entry URLs from Vite CT dev server or built bundle.
+			entryFilter: (entry: { url: string }) => entry.url.includes("/src/") || entry.url.includes("/assets/"),
+			sourceFilter: (sourcePath: string) =>
+				sourcePath.includes("src/") &&
+				!sourcePath.includes(".visual.") &&
+				!sourcePath.includes(".spec.") &&
+				!sourcePath.includes(".test."),
 		},
 	},
 ]
 
 export default defineConfig({
 	testDir: "./src",
 	testMatch: "**/*.visual.tsx",
-	outputDir: process.env.CI ? path.resolve(dirname, "test-results") : "/tmp/webview-ui-playwright-test-results",
+	outputDir: path.resolve(dirname, "test-results"),
 	snapshotPathTemplate: "{testDir}/{testFileDir}/__screenshots__/{arg}{ext}",
 	fullyParallel: true,
 	reporter: process.env.CI
@@ -36,7 +40,11 @@ export default defineConfig({
 				["list"],
 				monocartReporter,
 			]
-		: [["html", { open: "never", outputFolder: "/tmp/webview-ui-playwright-report" }], ["list"], monocartReporter],
+		: [
+				["html", { open: "never", outputFolder: path.resolve(dirname, "playwright-report") }],
+				["list"],
+				monocartReporter,
+			],
 	use: {
 		ctTemplateDir: "./playwright",
 		ctViteConfig: {

webview-ui/playwright/monocart-reporter.d.ts

@@ -0,0 +1,3 @@
+declare module "monocart-reporter" {
+	export function addCoverageReport(coverageData: any[], testInfo: any): Promise<void>
+}

webview-ui/playwright/run-docker.mjs

@@ -1,4 +1,4 @@
-import { spawnSync } from "node:child_process"
+import { spawn, spawnSync } from "node:child_process"
 import path from "node:path"
 import { fileURLToPath } from "node:url"
 
@@ -27,11 +27,27 @@ const spawnEnv = {
 const hasComposePlugin = spawnSync("docker", ["compose", "version"], { stdio: "ignore" }).status === 0
 const command = hasComposePlugin ? "docker" : "docker-compose"
 const args = hasComposePlugin ? ["compose", ...composeArgs] : composeArgs
-const result = spawnSync(command, args, { stdio: "inherit", env: spawnEnv })
 
-if (result.error) {
-	console.error(`Unable to run ${command}: ${result.error.message}`)
-	process.exit(1)
+const child = spawn(command, args, { stdio: "inherit", env: spawnEnv })
+
+const forwardSignal = (signal) => {
+	if (child.pid && !child.killed) {
+		child.kill(signal)
+	}
 }
 
-process.exit(result.status ?? 1)
+process.on("SIGINT", () => forwardSignal("SIGINT"))
+process.on("SIGTERM", () => forwardSignal("SIGTERM"))
+
+child.on("error", (err) => {
+	console.error(`Unable to run ${command}: ${err.message}`)
+	process.exit(1)
+})
+
+child.on("close", (code, signal) => {
+	if (signal) {
+		process.kill(process.pid, signal)
+	} else {
+		process.exit(code ?? 1)
+	}
+})

webview-ui/src/components/welcome/__tests__/RooHero.visual.tsx

@@ -3,13 +3,38 @@ import React from "react"
 import { expect, test } from "../../../../playwright/coverage-fixture"
 import RooHero from "../RooHero"
 
-test("renders the welcome hero in the VS Code dark theme", async ({ mount }) => {
-	const component = await mount(<RooHero />)
-
+async function waitForAssetsAndRender(component: any) {
 	await component.evaluate(async () => {
 		await document.fonts.ready
+		const images = Array.from(document.querySelectorAll("img"))
+		await Promise.all(
+			images.map((img) => {
+				if (img.complete) return Promise.resolve()
+				return new Promise((resolve) => {
+					img.onload = resolve
+					img.onerror = resolve
+				})
+			}),
+		)
 		await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()))
 	})
+}
+
+test("renders the welcome hero in the VS Code dark theme", async ({ mount }) => {
+	const component = await mount(<RooHero />)
+
+	await waitForAssetsAndRender(component)
 
 	await expect(component).toHaveScreenshot("zoo-hero-dark.png")
 })
+
+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",
+	})
+})

webview-ui/vitest.config.ts

@@ -15,7 +15,7 @@ export default defineConfig({
 		reporters,
 		silent,
 		environment: "jsdom",
-		include: ["src/**/*.spec.ts", "src/**/*.spec.tsx"],
+		include: ["src/**/*.spec.{ts,tsx}", "src/**/*.test.{ts,tsx}"],
 		onConsoleLog,
 		maxWorkers: isCI ? 1 : undefined,
 		testTimeout: isCI ? 15000 : 5000,