From d3702088d5340f528984043a62e34acc1050497f Mon Sep 17 00:00:00 2001 From: wenshao Date: Thu, 16 Jul 2026 21:57:53 +0800 Subject: [PATCH 1/3] test(web-shell): replace flaky split-view-restored shot with a workspace-sidebar scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The split-view "restored" screenshot was byte-nondeterministic between identical renders (the reappearing pane re-renders its content just after the restore click), so it periodically diffed above the before/after threshold and surfaced a false-positive "changed view" unrelated to the PR under review. It is also visually identical to the tiled `split view` shot. Drop the capture but keep the restore click + "both panes back" assertion, so the restore path still has behavioral coverage. Add a `workspace sidebar` scenario with two workspaces so the sidebar groups sessions per workspace and tags the primary one. This is the only scenario that renders the primary-workspace label/badge (it is gated on more than one displayed workspace), so changes to those labels — which no single-workspace scenario can surface — now show up in the visual preview. --- .../client/e2e/visuals/screenshots.spec.ts | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts index 99324562036..76a635e4fb5 100644 --- a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts +++ b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts @@ -266,12 +266,16 @@ for (const theme of THEMES) { // Restore the tiled layout (#6951): the solo pane returns to the split and // the hidden pane reappears — so the maximize control is back on both - // panes. Captures the restore path so a regression there is caught too. + // panes. Assert the restore path (behavioral coverage) but do NOT capture + // a screenshot: the restored layout is visually identical to the tiled + // `split view` shot above, and the reappearing pane re-renders its content + // just after this click, so the capture is byte-nondeterministic between + // identical runs — a flaky, redundant view that surfaces false-positive + // "changed" previews unrelated to the PR under review. await page.getByRole('button', { name: 'Restore pane' }).click(); await expect( page.getByRole('button', { name: 'Maximize pane' }).first(), ).toBeVisible(); - await captureScreenshot(page, `split-view-restored-${theme}`); }); test(`sidebar attention`, async ({ page }, testInfo) => { @@ -337,6 +341,57 @@ for (const theme of THEMES) { await captureScreenshot(page, `sidebar-attention-${theme}`); }); + test(`workspace sidebar`, async ({ page }, testInfo) => { + // Two workspaces make the sidebar group sessions per workspace and tag the + // primary one — the surface the "primary workspace" label/badge lives on. + // Every other scenario here is single-workspace, where that tag never + // renders (it is gated on more than one displayed workspace), so this is + // the only scenario that can surface a change to the workspace labels. + const scenario = createWebShellDaemonScenario({ + capabilities: { + workspaces: [ + { + id: 'ws-primary', + cwd: '/tmp/qwen-web-shell-e2e', + primary: true, + trusted: true, + }, + { + id: 'ws-api', + cwd: '/tmp/qwen-api-service', + primary: false, + trusted: true, + }, + ], + }, + }); + const daemon = await installScenario( + page, + scenario, + resolveBaseURL(testInfo), + ); + await gotoSession(page, scenario, daemon, theme); + // Each workspace renders a section headed by its basename; the primary one + // also carries a "Primary" tag. Assert both workspace names and the tag so + // a regression in the grouping or the (removable) primary label fails an + // assertion, not only the visually-reviewed screenshot. + const sidebar = page.getByRole('complementary'); + await expect( + sidebar.getByText('qwen-web-shell-e2e', { exact: true }), + ).toBeVisible(); + await expect( + sidebar.getByText('qwen-api-service', { exact: true }), + ).toBeVisible(); + await expect(sidebar.getByText('Primary', { exact: true })).toBeVisible(); + // The primary workspace auto-expands and streams its session rows in via a + // per-workspace fetch. Wait for a row before capturing so the async load + // has settled — otherwise the row list races the screenshot and the + // capture is byte-nondeterministic between runs (the flake this scenario + // was added to replace). + await expect(sidebar.getByText('E2E Harness Session')).toBeVisible(); + await captureScreenshot(page, `workspace-sidebar-${theme}`); + }); + test(`slash menu`, async ({ page }, testInfo) => { const scenario = createWebShellDaemonScenario(); const daemon = await installScenario( From 3b6862edecbe3d31edab42cb2735235cbeefbd64 Mon Sep 17 00:00:00 2001 From: wenshao Date: Thu, 16 Jul 2026 23:01:51 +0800 Subject: [PATCH 2/3] test(web-shell): freeze looping animations so captures are deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar's activity spinner is a GPU-composited transform loop that Playwright's `animations: 'disabled'` captures mid-rotation at a random angle, so `sidebar attention` differed in ~0.12% of pixels between two identical renders — above the 0.02% before/after threshold, i.e. a false-positive "changed view" on any PR that renders it. Before each capture, pause every infinite Web Animation and rewind it to time 0 (a two-frame wait lets the compositor commit the frozen frame); finite animations are still left to `animations: 'disabled'`. Verified with a pixel diff: the whole suite now renders pixel-identical across two runs (worst 0.0001% of pixels, vs the 0.02% threshold). --- .../web-shell/client/e2e/visuals/harness.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/web-shell/client/e2e/visuals/harness.ts b/packages/web-shell/client/e2e/visuals/harness.ts index 21a5dcca635..dc7d8de88de 100644 --- a/packages/web-shell/client/e2e/visuals/harness.ts +++ b/packages/web-shell/client/e2e/visuals/harness.ts @@ -140,12 +140,42 @@ export async function captureScreenshot( name: string, ): Promise { mkdirSync(SCREENSHOTS_DIR, { recursive: true }); + await freezeLoopingAnimations(page); await page.screenshot({ path: join(SCREENSHOTS_DIR, `${name}.png`), animations: 'disabled', }); } +/** + * Pin looping animations to their first frame before a capture. Playwright's + * `animations: 'disabled'` settles finite animations and is meant to reset + * infinite ones, but a GPU-composited transform loop — e.g. the sidebar's + * rotating activity spinner — is still captured mid-rotation at a random angle. + * That angle differs between the base and head render passes, so the view reads + * as "changed" against the 0.02% before/after threshold even when nothing did. + * Pausing the infinite Web Animations and rewinding them to time 0 pins them to + * a deterministic frame (verified: sidebar-attention drops from ~0.12% of pixels + * differing between identical renders to 0); a two-frame wait lets the compositor + * commit that frame before the capture reads it. + */ +async function freezeLoopingAnimations(page: Page): Promise { + await page.evaluate( + /* global document, requestAnimationFrame */ + async () => { + for (const animation of document.getAnimations()) { + if (animation.effect?.getTiming().iterations === Infinity) { + animation.pause(); + animation.currentTime = 0; + } + } + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())), + ); + }, + ); +} + /** * Record a continuous flow to `/video/.webm`. A dedicated * browser context owns the video lifecycle so the file can be saved under a From f6a535d5ff781f68f0b60907ad64d325d6a0f894 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 17 Jul 2026 00:51:13 +0800 Subject: [PATCH 3/3] test(web-shell): document freeze scope, pin scenario deps, test the freeze Address review on the visual-capture determinism changes: - Note freezeLoopingAnimations' coverage scope in its docstring (WAAPI + CSS @keyframes via document.getAnimations(), not a hand-rolled requestAnimationFrame loop), so a future spinner rewrite that reintroduces the flake leads a debugger back to this function. - Pin the workspace-sidebar scenario's primary workspace cwd and loaded session name explicitly rather than leaning on createWebShellDaemonScenario defaults, so renaming those defaults in mockDaemon.ts can't turn the settle-wait into a cryptic "not visible" failure. - Add harness.spec.ts pinning the freeze contract: an infinite animation is paused and rewound to time 0, while a finite one is left running. --- .../client/e2e/visuals/harness.spec.ts | 55 +++++++++++++++++++ .../web-shell/client/e2e/visuals/harness.ts | 8 ++- .../client/e2e/visuals/screenshots.spec.ts | 21 +++++-- 3 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 packages/web-shell/client/e2e/visuals/harness.spec.ts diff --git a/packages/web-shell/client/e2e/visuals/harness.spec.ts b/packages/web-shell/client/e2e/visuals/harness.spec.ts new file mode 100644 index 00000000000..0e9e596cb4d --- /dev/null +++ b/packages/web-shell/client/e2e/visuals/harness.spec.ts @@ -0,0 +1,55 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { expect, test } from '@playwright/test'; +import { freezeLoopingAnimations } from './harness'; + +// `freezeLoopingAnimations` is the load-bearing step that keeps spinner-bearing +// captures deterministic (see its docstring). It runs only implicitly via +// `captureScreenshot`, so pin its contract explicitly here: an infinite +// animation must be paused and rewound to time 0, while a finite one must be +// left alone for Playwright's own `animations: 'disabled'` to settle. +test('freezeLoopingAnimations pins infinite animations to frame 0 and leaves finite ones', async ({ + page, +}) => { + await page.setContent(` + +
+
+ `); + // Advance both animations past frame 0 first, so a freeze that did nothing + // would leave a non-zero currentTime and fail the assertion below. + await page.waitForTimeout(100); + + await freezeLoopingAnimations(page); + + const state = await page.evaluate( + /* global document */ + () => { + const animOf = (id: string) => { + const el = document.getElementById(id); + if (!el) throw new Error(`element #${id} not found`); + return el.getAnimations()[0]; + }; + const loop = animOf('loop'); + return { + loopPlayState: loop.playState, + loopCurrentTime: Number(loop.currentTime), + oncePlayState: animOf('once').playState, + }; + }, + ); + + // The infinite loop is paused at its first frame… + expect(state.loopPlayState).toBe('paused'); + expect(state.loopCurrentTime).toBe(0); + // …while the finite animation is untouched, still running toward completion. + expect(state.oncePlayState).toBe('running'); +}); diff --git a/packages/web-shell/client/e2e/visuals/harness.ts b/packages/web-shell/client/e2e/visuals/harness.ts index dc7d8de88de..a7bf5273d20 100644 --- a/packages/web-shell/client/e2e/visuals/harness.ts +++ b/packages/web-shell/client/e2e/visuals/harness.ts @@ -158,8 +158,14 @@ export async function captureScreenshot( * a deterministic frame (verified: sidebar-attention drops from ~0.12% of pixels * differing between identical renders to 0); a two-frame wait lets the compositor * commit that frame before the capture reads it. + * + * Scope: this covers WAAPI and CSS `@keyframes` animations — everything + * `document.getAnimations()` reports. A spinner hand-rolled on a + * `requestAnimationFrame` loop instead would NOT be caught, and the flake would + * silently return; if a spinner reimplementation ever reintroduces it, this is + * the function to extend. `harness.spec.ts` pins the pause/rewind contract. */ -async function freezeLoopingAnimations(page: Page): Promise { +export async function freezeLoopingAnimations(page: Page): Promise { await page.evaluate( /* global document, requestAnimationFrame */ async () => { diff --git a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts index 76a635e4fb5..29abe49c4b8 100644 --- a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts +++ b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts @@ -347,12 +347,22 @@ for (const theme of THEMES) { // Every other scenario here is single-workspace, where that tag never // renders (it is gated on more than one displayed workspace), so this is // the only scenario that can surface a change to the workspace labels. + // + // Pin the primary workspace cwd and its loaded session name explicitly, + // rather than leaning on createWebShellDaemonScenario's defaults: the + // basename ("qwen-web-shell-e2e") and the settle-wait below both depend on + // them, so a rename of those defaults in mockDaemon.ts would otherwise + // turn this into a cryptic "not visible" failure. + const primaryCwd = '/tmp/qwen-web-shell-e2e'; + const primarySessionName = 'Run auth migration'; const scenario = createWebShellDaemonScenario({ + workspaceCwd: primaryCwd, + displayName: primarySessionName, capabilities: { workspaces: [ { id: 'ws-primary', - cwd: '/tmp/qwen-web-shell-e2e', + cwd: primaryCwd, primary: true, trusted: true, }, @@ -384,11 +394,10 @@ for (const theme of THEMES) { ).toBeVisible(); await expect(sidebar.getByText('Primary', { exact: true })).toBeVisible(); // The primary workspace auto-expands and streams its session rows in via a - // per-workspace fetch. Wait for a row before capturing so the async load - // has settled — otherwise the row list races the screenshot and the - // capture is byte-nondeterministic between runs (the flake this scenario - // was added to replace). - await expect(sidebar.getByText('E2E Harness Session')).toBeVisible(); + // per-workspace fetch. Wait for the loaded session's row before capturing + // so the async load has settled — otherwise the row list races the + // screenshot and the capture differs between runs. + await expect(sidebar.getByText(primarySessionName)).toBeVisible(); await captureScreenshot(page, `workspace-sidebar-${theme}`); });