From 7e3d6bf562ff41e7353cefa10ed2af41721fa3c3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 May 2026 15:01:08 +0100 Subject: [PATCH] Fix playwright tests (#33592) * Fix playwright tests The screenshot reporter was using an import that no longer works as of playwright 1.60. Inline the function instead as it doesn't seem to be importable. * Our custom expect was importing itself? How did that ever work? * Nope, that doesn't seem to help and also it was aiming the in in playwright common which should be the 2nd level of extension * Provide non-overridden version of the same function as per comment --- apps/web/playwright/element-web-test.ts | 2 +- .../src/expect/screenshot.ts | 79 +++++++++++-------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/apps/web/playwright/element-web-test.ts b/apps/web/playwright/element-web-test.ts index 62e6515f62..b77a027241 100644 --- a/apps/web/playwright/element-web-test.ts +++ b/apps/web/playwright/element-web-test.ts @@ -187,7 +187,7 @@ export const expect = baseExpect.extend({ css += options.css; } - await baseExpect(receiver).toMatchScreenshot(name, { + await baseExpect(receiver).toMatchScreenshotDontOverride(name, { ...options, css, }); diff --git a/packages/playwright-common/src/expect/screenshot.ts b/packages/playwright-common/src/expect/screenshot.ts index 9936ae982b..8c2165a1cb 100644 --- a/packages/playwright-common/src/expect/screenshot.ts +++ b/packages/playwright-common/src/expect/screenshot.ts @@ -16,11 +16,15 @@ import { type PageAssertionsToHaveScreenshotOptions, type MatcherReturnType, } from "@playwright/test"; -import { sanitizeForFilePath } from "playwright-core/lib/utils"; import { extname } from "node:path"; import { ANNOTATION } from "../stale-screenshot-reporter.js"; +// Taken from playwright utils, but it's not importable +function sanitizeForFilePath(s: string): string { + return s.replace(/[\x00-\x2C\x2E-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F]+/g, "-"); +} + // Based on https://github.com/microsoft/playwright/blob/2b77ed4d7aafa85a600caa0b0d101b72c8437eeb/packages/playwright/src/util.ts#L206C8-L210C2 function sanitizeFilePathBeforeExtension(filePath: string): string { const ext = extname(filePath); @@ -39,41 +43,54 @@ export type Expectations = { name: `${string}.png`, options?: ToMatchScreenshotOptions, ) => Promise; + toMatchScreenshotDontOverride: ( + this: ExpectMatcherState, + receiver: Page | Locator, + name: `${string}.png`, + options?: ToMatchScreenshotOptions, + ) => Promise; +}; + +const toMatchScreenshot = async (receiver: Page | Locator, name: string, options?: ToMatchScreenshotOptions) => { + const testInfo = test.info(); + if (!testInfo) throw new Error(`toMatchScreenshot() must be called during the test`); + + if (!testInfo.tags.includes("@screenshot")) { + throw new Error("toMatchScreenshot() must be used in a test tagged with @screenshot"); + } + + const page = "page" in receiver ? receiver.page() : receiver; + + let style: ElementHandle | undefined; + if (options?.css) { + // We add a custom style tag before taking screenshots + style = (await page.addStyleTag({ + content: options.css, + })) as ElementHandle; + } + + const screenshotName = sanitizeFilePathBeforeExtension(name); + await baseExpect(receiver).toHaveScreenshot(screenshotName, options); + + await style?.evaluate((tag) => tag.remove()); + + testInfo.annotations.push({ + type: ANNOTATION, + description: testInfo.snapshotPath(screenshotName), + }); + + return { pass: true, message: (): string => "", name: "toMatchScreenshot" }; }; /** * Provides an upgrade to the `toHaveScreenshot` expectation. * Unfortunately, we can't just extend the existing `toHaveScreenshot` expectation + * + * Awfulness: we also provide a copy of the same function under a different name because, for reasons + * I couldn't fully say, overriding a function with one of the same name causes the base excpect function + * to actually point to the overridden one, causing infinite recursion, so there you go, everything is great. */ export const expect = baseExpect.extend({ - async toMatchScreenshot(receiver, name, options) { - const testInfo = test.info(); - if (!testInfo) throw new Error(`toMatchScreenshot() must be called during the test`); - - if (!testInfo.tags.includes("@screenshot")) { - throw new Error("toMatchScreenshot() must be used in a test tagged with @screenshot"); - } - - const page = "page" in receiver ? receiver.page() : receiver; - - let style: ElementHandle | undefined; - if (options?.css) { - // We add a custom style tag before taking screenshots - style = (await page.addStyleTag({ - content: options.css, - })) as ElementHandle; - } - - const screenshotName = sanitizeFilePathBeforeExtension(name); - await baseExpect(receiver).toHaveScreenshot(screenshotName, options); - - await style?.evaluate((tag) => tag.remove()); - - testInfo.annotations.push({ - type: ANNOTATION, - description: testInfo.snapshotPath(screenshotName), - }); - - return { pass: true, message: (): string => "", name: "toMatchScreenshot" }; - }, + toMatchScreenshot, + toMatchScreenshotDontOverride: toMatchScreenshot, });