From b6ab200451a275f493fc9e943d9449548fae079d Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 19 Sep 2026 13:44:55 +0200 Subject: [PATCH 1/2] fix(video): don't leave a stale test annotation overlay in the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit installScreencastTitleUpdater() updates the caption from two places: the test.step callbacks and the 'page' event. The page handler is fire-and-forget, and an update reads overlays.get(page) before awaiting showOverlay(), so a step that begins while a page-event update is in flight finds nothing to remove and adds a second overlay. The reference to the first one is lost and it stays in the page for the rest of the recording. Both overlays are anchored to the same edge and differ by one line, so the previous caption line is drawn over the current one — a ghost that shows wherever it is longer than the text on top of it. With the default top-left position the two line up and the duplicate goes unnoticed; with a bottom position it is plainly visible in the video. Serialize the updates into a promise chain so removing the previous overlay and adding the next one cannot interleave. A failed update no longer breaks the chain, but is still reported to whoever awaited it. --- packages/playwright/src/index.ts | 18 ++++- tests/playwright-test/video-annotate.spec.ts | 71 ++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 tests/playwright-test/video-annotate.spec.ts diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index b1e14b7810c68..7af7eae3b8e0e 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -877,7 +877,7 @@ async function installScreencastTitleUpdater(testInfo: TestInfoImpl, context: Br const fontSize = testAnnotate.fontSize ?? 14; const level = testAnnotate.level ?? 'step'; - const updateOverlay = async () => { + const doUpdateOverlay = async () => { const parts = level === 'step' ? [...testTitle, ...stepStack] : testTitle; const html = createTestOverlay(parts, position, fontSize); for (const page of context.pages()) { @@ -887,6 +887,18 @@ async function installScreencastTitleUpdater(testInfo: TestInfoImpl, context: Br overlays.set(page, disposable); } }; + + // Updates come from step callbacks and from the 'page' event, and each one removes the + // previous overlay before adding a new one. Serialize them: an update that starts while + // another is mid-flight finds nothing to remove and leaves a stale overlay in the page + // for the rest of the recording. + let pendingUpdate = Promise.resolve(); + const updateOverlay = () => { + const update = pendingUpdate.then(doUpdateOverlay); + // Keep the chain going when an update fails, but still report the failure to the caller. + pendingUpdate = update.catch(() => {}); + return update; + }; testInfo._onUserStepBegin = async title => { stepStack.push(title); await updateOverlay(); @@ -896,8 +908,8 @@ async function installScreencastTitleUpdater(testInfo: TestInfoImpl, context: Br await updateOverlay(); }; - context.on('page', async () => { - void updateOverlay(); + context.on('page', () => { + void updateOverlay().catch(() => {}); }); await updateOverlay(); } diff --git a/tests/playwright-test/video-annotate.spec.ts b/tests/playwright-test/video-annotate.spec.ts new file mode 100644 index 0000000000000..90ff9c0cc4936 --- /dev/null +++ b/tests/playwright-test/video-annotate.spec.ts @@ -0,0 +1,71 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test, expect } from './playwright-test-fixtures'; + +const configWithTestAnnotation = ` + module.exports = { use: { video: { mode: 'on', show: { test: { level: 'step' } } } }, name: 'chromium' }; +`; + +test('should show file, test title and step in the annotation overlay', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': configWithTestAnnotation, + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + + async function overlays(page) { + return await page.locator('.x-pw-user-overlay').evaluateAll( + els => els.map(el => el.innerText.split('\\n'))); + } + + test('my test', async ({ page }) => { + await test.step('first step', async () => { + expect(await overlays(page)).toEqual([['a.test.ts', 'my test', 'first step']]); + }); + expect(await overlays(page)).toEqual([['a.test.ts', 'my test']]); + await test.step('second step', async () => { + expect(await overlays(page)).toEqual([['a.test.ts', 'my test', 'second step']]); + await test.step('nested step', async () => { + expect(await overlays(page)).toEqual([['a.test.ts', 'my test', 'second step', 'nested step']]); + }); + }); + }); + `, + }, { workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); +}); + +test('should not stack annotation overlays when a page opens during a step', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': configWithTestAnnotation, + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + + test('my test', async ({ page, context }) => { + await test.step('first step', async () => { + const second = await context.newPage(); + await expect(page.locator('.x-pw-user-overlay')).toHaveCount(1); + await expect(second.locator('.x-pw-user-overlay')).toHaveCount(1); + }); + }); + `, + }, { workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); +}); From 50752b373e536216d70932ac67e02cc4b82f43f3 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 19 Sep 2026 14:17:53 +0200 Subject: [PATCH 2/2] fix(video): never fail a test because of the annotation overlay --- packages/playwright/src/index.ts | 14 +++++++---- ...c.ts => playwright.video-annotate.spec.ts} | 24 ++++++++++++++++++- 2 files changed, 33 insertions(+), 5 deletions(-) rename tests/playwright-test/{video-annotate.spec.ts => playwright.video-annotate.spec.ts} (77%) diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 7af7eae3b8e0e..7be86bc7b87e9 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -881,10 +881,16 @@ async function installScreencastTitleUpdater(testInfo: TestInfoImpl, context: Br const parts = level === 'step' ? [...testTitle, ...stepStack] : testTitle; const html = createTestOverlay(parts, position, fontSize); for (const page of context.pages()) { - await overlays.get(page)?.dispose(); - overlays.delete(page); - const disposable = await page.screencast.showOverlay(html); - overlays.set(page, disposable); + // The annotation is cosmetic, and the page list is a snapshot: a page that closes + // while we are updating it must not fail the test sitting at a step boundary. + try { + await overlays.get(page)?.dispose(); + overlays.delete(page); + const disposable = await page.screencast.showOverlay(html); + overlays.set(page, disposable); + } catch (error) { + debugLogger.log('error', `failed to update the video annotation overlay: ${error}`); + } } }; diff --git a/tests/playwright-test/video-annotate.spec.ts b/tests/playwright-test/playwright.video-annotate.spec.ts similarity index 77% rename from tests/playwright-test/video-annotate.spec.ts rename to tests/playwright-test/playwright.video-annotate.spec.ts index 90ff9c0cc4936..275142deca52a 100644 --- a/tests/playwright-test/video-annotate.spec.ts +++ b/tests/playwright-test/playwright.video-annotate.spec.ts @@ -17,7 +17,7 @@ import { test, expect } from './playwright-test-fixtures'; const configWithTestAnnotation = ` - module.exports = { use: { video: { mode: 'on', show: { test: { level: 'step' } } } }, name: 'chromium' }; + module.exports = { use: { video: { mode: 'on', show: { test: { level: 'step' } } } } }; `; test('should show file, test title and step in the annotation overlay', async ({ runInlineTest }) => { @@ -69,3 +69,25 @@ test('should not stack annotation overlays when a page opens during a step', asy expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); }); + +test('should not fail a step when a page closes while the overlay is updated', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': configWithTestAnnotation, + 'a.test.ts': ` + import { test } from '@playwright/test'; + + test('my test', async ({ context, page }) => { + const pages = []; + for (let i = 0; i < 5; i++) + pages.push(await context.newPage()); + const closed = Promise.all(pages.map(p => p.close())); + await test.step('first step', async () => {}); + await test.step('second step', async () => {}); + await closed; + }); + `, + }, { workers: 1 }); + + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); +});