diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index f94e044914..5c20264261 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -21,7 +21,7 @@ import * as input from './input'; import * as js from './javascript'; import * as network from './network'; import type { ScreenshotOptions } from './screenshotter'; -import { Screenshotter } from './screenshotter'; +import { Screenshotter, validateScreenshotOptions } from './screenshotter'; import { TimeoutSettings } from '../common/timeoutSettings'; import type * as types from './types'; import { BrowserContext } from './browserContext'; @@ -477,6 +477,13 @@ export class Page extends SdkObject { const controller = new ProgressController(metadata, this); if (!options.expected && options.isNot) return { errorMessage: '"not" matcher requires expected result' }; + try { + const format = validateScreenshotOptions(options.screenshotOptions || {}); + if (format !== 'png') + throw new Error('Only PNG screenshots are supported'); + } catch (error) { + return { errorMessage: error.message }; + } let intermediateResult: { actual?: Buffer, previous?: Buffer, @@ -549,7 +556,7 @@ export class Page extends SdkObject { return { log: e.message ? [...metadata.log, e.message] : metadata.log, ...intermediateResult, - errorMessage: intermediateResult?.errorMessage ?? e.message, + errorMessage: e.message, }; }); } diff --git a/packages/playwright-core/src/server/screenshotter.ts b/packages/playwright-core/src/server/screenshotter.ts index 869e06b191..9e117dee16 100644 --- a/packages/playwright-core/src/server/screenshotter.ts +++ b/packages/playwright-core/src/server/screenshotter.ts @@ -321,7 +321,7 @@ function trimClipToSize(clip: types.Rect, size: types.Size): types.Rect { return result; } -function validateScreenshotOptions(options: ScreenshotOptions): 'png' | 'jpeg' { +export function validateScreenshotOptions(options: ScreenshotOptions): 'png' | 'jpeg' { if (options.fonts && !experimentalFeaturesEnabled()) throw new Error(`To use the experimental option "fonts", set PLAYWRIGHT_EXPERIMENTAL_FEATURES=1 enviroment variable.`); diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts index 6aec9147d6..ddf602f8e4 100644 --- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts @@ -357,12 +357,8 @@ export async function toHaveScreenshot( }); // We tried re-generating new snapshot but failed. // This can be due to e.g. spinning animation, so we want to show it as a diff. - if (errorMessage) { - const title = actual && previous ? - `Timeout ${timeout}ms exceeded while generating screenshot because ${locator ? 'element' : 'page'} kept changing` : - `Timeout ${timeout}ms exceeded while generating screenshot`; - return helper.handleDifferent(actual, undefined, previous, diff, undefined, log, title); - } + if (errorMessage) + return helper.handleDifferent(actual, undefined, previous, diff, undefined, log, errorMessage); // We successfully (re-)generated new screenshot. if (!hasSnapshot) diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 317601ff6e..5951329a06 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -50,7 +50,7 @@ test('should fail to screenshot a page with infinite animation', async ({ runInl ` }); expect(result.exitCode).toBe(1); - expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded while generating screenshot because page kept changing`); + expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded`); expect(stripAnsi(result.output)).toContain(`expect.toHaveScreenshot with timeout 2000ms`); expect(stripAnsi(result.output)).toContain(`generating new stable screenshot expectation`); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(true); @@ -74,6 +74,29 @@ test('should disable animations by default', async ({ runInlineTest }, testInfo) expect(result.exitCode).toBe(0); }); +test('should fail with proper error when unsupported argument is given', async ({ runInlineTest }, testInfo) => { + const cssTransitionURL = pathToFileURL(path.join(__dirname, '../assets/css-transition.html')); + const result = await runInlineTest({ + ...playwrightConfig({}), + 'a.spec.js': ` + pwt.test('is a test', async ({ page }) => { + await page.goto('${cssTransitionURL}'); + await expect(page).toHaveScreenshot({ + clip: { + x: 0, + y: 0, + width: 0, + height: 0, + }, + timeout: 2000, + }); + }); + ` + }, { 'update-snapshots': true }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain(`Expected options.clip.width not to be 0`); +}); + test('should have scale:css by default', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...playwrightConfig({ screenshotsDir: '__screenshots__' }), @@ -284,7 +307,7 @@ test('should fail to screenshot an element with infinite animation', async ({ ru ` }); expect(result.exitCode).toBe(1); - expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded while generating screenshot because element kept changing`); + expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded`); expect(stripAnsi(result.output)).toContain(`expect.toHaveScreenshot with timeout 2000ms`); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-previous.png'))).toBe(true); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(true);