kyfx
/
playwright
mirror of https://github.com/microsoft/playwright.git
4
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

chore: unify toHaveScreenshot error formatting (#33300)

This commit is contained in:
Yury Semikhatsky 2024-10-25 12:36:39 -07:00 committed by GitHub
parent f98531baee
commit 1e8884621a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 41 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/html-reporter/src/testResultView.tsx
View File

@ -152,7 +152,8 @@ export const TestResultView: React.FC<{
function classifyErrors(testErrors: string[], diffs: ImageDiff[]) {
return testErrors.map(error => {
if (error.includes('Screenshot comparison failed:')) {
const firstLine = error.split('\n')[0];
if (firstLine.includes('toHaveScreenshot') || firstLine.includes('toMatchSnapshot')) {
const matchingDiff = diffs.find(diff => {
const attachmentName = diff.actual?.attachment.name;
return attachmentName && error.includes(attachmentName);

3
packages/playwright-core/src/client/page.ts
View File

@ -63,6 +63,7 @@ type PDFOptions = Omit<channels.PagePdfParams, 'width' | 'height' | 'margin'> &
export type ExpectScreenshotOptions = Omit<channels.PageExpectScreenshotOptions, 'locator' | 'expected' | 'mask'> & {
expected?: Buffer,
locator?: api.Locator,
timeout: number,
isNot: boolean,
mask?: api.Locator[],
};
@ -589,7 +590,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
return result.binary;
}
async _expectScreenshot(options: ExpectScreenshotOptions): Promise<{ actual?: Buffer, previous?: Buffer, diff?: Buffer, errorMessage?: string, log?: string[]}> {
async _expectScreenshot(options: ExpectScreenshotOptions): Promise<{ actual?: Buffer, previous?: Buffer, diff?: Buffer, errorMessage?: string, log?: string[], timedOut?: boolean}> {
const mask = options?.mask ? options?.mask.map(locator => ({
frame: (locator as Locator)._frame._channel,
selector: (locator as Locator)._selector,

3
packages/playwright-core/src/protocol/validator.ts
View File

@ -1165,7 +1165,7 @@ scheme.PageReloadResult = tObject({
});
scheme.PageExpectScreenshotParams = tObject({
expected: tOptional(tBinary),
timeout: tOptional(tNumber),
timeout: tNumber,
isNot: tBoolean,
locator: tOptional(tObject({
frame: tChannel(['Frame']),
@ -1193,6 +1193,7 @@ scheme.PageExpectScreenshotResult = tObject({
errorMessage: tOptional(tString),
actual: tOptional(tBinary),
previous: tOptional(tBinary),
timedOut: tOptional(tBoolean),
log: tOptional(tArray(tString)),
});
scheme.PageScreenshotParams = tObject({

3
packages/playwright-core/src/server/page.ts
View File

@ -674,11 +674,12 @@ export class Page extends SdkObject {
throw e;
let errorMessage = e.message;
if (e instanceof TimeoutError && intermediateResult?.previous)
errorMessage = `Failed to take two consecutive stable screenshots. ${e.message}`;
errorMessage = `Failed to take two consecutive stable screenshots.`;
return {
log: e.message ? [...metadata.log, e.message] : metadata.log,
...intermediateResult,
errorMessage,
timedOut: (e instanceof TimeoutError),
};
});
}

39
packages/playwright/src/matchers/toMatchSnapshot.ts
View File

@ -18,7 +18,7 @@ import type { Locator, Page } from 'playwright-core';
import type { ExpectScreenshotOptions, Page as PageEx } from 'playwright-core/lib/client/page';
import { currentTestInfo } from '../common/globals';
import type { ImageComparatorOptions, Comparator } from 'playwright-core/lib/utils';
import { getComparator, sanitizeForFilePath } from 'playwright-core/lib/utils';
import { getComparator, isString, sanitizeForFilePath } from 'playwright-core/lib/utils';
import {
addSuffixToFilePath,
trimLongString, callLogText,
@ -31,7 +31,7 @@ import path from 'path';
import { mime } from 'playwright-core/lib/utilsBundle';
import type { TestInfoImpl } from '../worker/testInfo';
import type { ExpectMatcherState } from '../../types/test';
import type { MatcherResult } from './matcherHint';
import { matcherHint, type MatcherResult } from './matcherHint';
import type { FullProjectInternal } from '../common/config';
type NameOrSegments = string | string[];
@ -250,16 +250,10 @@ class SnapshotHelper {
expected: Buffer | string | undefined,
previous: Buffer | string | undefined,
diff: Buffer | string | undefined,
diffError: string | undefined,
log: string[] | undefined,
title = `${this.kind} comparison failed:`): ImageMatcherResult {
const output = [
colors.red(title),
'',
];
if (diffError)
output.push(indent(diffError, ' '));
header: string,
diffError: string,
log: string[] | undefined): ImageMatcherResult {
const output = [`${header}${indent(diffError, ' ')}`];
if (expected !== undefined) {
// Copy the expectation inside the `test-results/` folder for backwards compatibility,
// so that one can upload `test-results/` directory and have all the data inside.
@ -338,7 +332,9 @@ export function toMatchSnapshot(
return helper.createMatcherResult(helper.expectedPath + ' running with --update-snapshots, writing actual.', true);
}
return helper.handleDifferent(received, expected, undefined, result.diff, result.errorMessage, undefined);
const receiver = isString(received) ? 'string' : 'Buffer';
const header = matcherHint(this, undefined, 'toMatchSnapshot', receiver, undefined, undefined);
return helper.handleDifferent(received, expected, undefined, result.diff, header, result.errorMessage, undefined);
}
export function toHaveScreenshotStepTitle(
@ -374,6 +370,7 @@ export async function toHaveScreenshot(
throw new Error(`Screenshot name "${path.basename(helper.expectedPath)}" must have '.png' extension`);
expectTypes(pageOrLocator, ['Page', 'Locator'], 'toHaveScreenshot');
const style = await loadScreenshotStyles(helper.options.stylePath);
const timeout = helper.options.timeout ?? this.timeout;
const expectScreenshotOptions: ExpectScreenshotOptions = {
locator,
animations: helper.options.animations ?? 'disabled',
@ -386,7 +383,7 @@ export async function toHaveScreenshot(
scale: helper.options.scale ?? 'css',
style,
isNot: !!this.isNot,
timeout: helper.options.timeout ?? this.timeout,
timeout,
comparator: helper.options.comparator,
maxDiffPixels: helper.options.maxDiffPixels,
maxDiffPixelRatio: helper.options.maxDiffPixelRatio,
@ -410,13 +407,16 @@ export async function toHaveScreenshot(
if (helper.updateSnapshots === 'none' && !hasSnapshot)
return helper.createMatcherResult(`A snapshot doesn't exist at ${helper.expectedPath}.`, false);
const receiver = locator ? 'locator' : 'page';
if (!hasSnapshot) {
// Regenerate a new screenshot by waiting until two screenshots are the same.
const { actual, previous, diff, errorMessage, log } = await page._expectScreenshot(expectScreenshotOptions);
const { actual, previous, diff, errorMessage, log, timedOut } = await page._expectScreenshot(expectScreenshotOptions);
// 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)
return helper.handleDifferent(actual, undefined, previous, diff, undefined, log, errorMessage);
if (errorMessage) {
const header = matcherHint(this, locator, 'toHaveScreenshot', receiver, undefined, undefined, timedOut ? timeout : undefined);
return helper.handleDifferent(actual, undefined, previous, diff, header, errorMessage, log);
}
// We successfully generated new screenshot.
return helper.handleMissing(actual!);
@ -427,7 +427,7 @@ export async function toHaveScreenshot(
// - regular matcher (i.e. not a `.not`)
// - perhaps an 'all' flag to update non-matching screenshots
expectScreenshotOptions.expected = await fs.promises.readFile(helper.expectedPath);
const { actual, previous, diff, errorMessage, log } = await page._expectScreenshot(expectScreenshotOptions);
const { actual, previous, diff, errorMessage, log, timedOut } = await page._expectScreenshot(expectScreenshotOptions);
if (!errorMessage)
return helper.handleMatching();
@ -440,7 +440,8 @@ export async function toHaveScreenshot(
return helper.createMatcherResult(helper.expectedPath + ' running with --update-snapshots, writing actual.', true);
}
return helper.handleDifferent(actual, expectScreenshotOptions.expected, previous, diff, errorMessage, log);
const header = matcherHint(this, undefined, 'toHaveScreenshot', receiver, undefined, undefined, timedOut ? timeout : undefined);
return helper.handleDifferent(actual, expectScreenshotOptions.expected, previous, diff, header, errorMessage, log);
}
function writeFileSync(aPath: string, content: Buffer | string) {

4
packages/protocol/src/channels.ts
View File

@ -2141,7 +2141,7 @@ export type PageReloadResult = {
};
export type PageExpectScreenshotParams = {
expected?: Binary,
timeout?: number,
timeout: number,
isNot: boolean,
locator?: {
frame: FrameChannel,
@ -2166,7 +2166,6 @@ export type PageExpectScreenshotParams = {
};
export type PageExpectScreenshotOptions = {
expected?: Binary,
timeout?: number,
locator?: {
frame: FrameChannel,
selector: string,
@ -2193,6 +2192,7 @@ export type PageExpectScreenshotResult = {
errorMessage?: string,
actual?: Binary,
previous?: Binary,
timedOut?: boolean,
log?: string[],
};
export type PageScreenshotParams = {

3
packages/protocol/src/protocol.yml
View File

@ -1482,7 +1482,7 @@ Page:
expectScreenshot:
parameters:
expected: binary?
timeout: number?
timeout: number
isNot: boolean
locator:
type: object?
@ -1501,6 +1501,7 @@ Page:
errorMessage: string?
actual: binary?
previous: binary?
timedOut: boolean?
log:
type: array?
items: string

4
tests/page/expect-matcher-result.spec.ts
View File

@ -295,7 +295,7 @@ test('toHaveScreenshot should populate matcherResult', async ({ page, server, is
actual: expect.stringContaining('screenshot-sanity-actual'),
expected: expect.stringContaining('screenshot-sanity-'),
diff: expect.stringContaining('screenshot-sanity-diff'),
message: expect.stringContaining(`Screenshot comparison failed`),
message: expect.stringContaining(`expect(page).toHaveScreenshot(expected)`),
name: 'toHaveScreenshot',
pass: false,
log: expect.any(Array),
@ -303,7 +303,7 @@ test('toHaveScreenshot should populate matcherResult', async ({ page, server, is
printedReceived: expect.stringContaining('screenshot-sanity-actual'),
});
expect.soft(stripAnsi(e.toString())).toContain(`Error: Screenshot comparison failed:
expect.soft(stripAnsi(e.toString())).toContain(`Error: expect(page).toHaveScreenshot(expected)
23362 pixels (ratio 0.10 of all image pixels) are different.

5
tests/playwright-test/golden.spec.ts
View File

@ -223,7 +223,7 @@ test('should write detailed failure result to an output folder', async ({ runInl
expect(result.exitCode).toBe(1);
const outputText = result.output;
expect(outputText).toContain('Snapshot comparison failed:');
expect(outputText).toContain('Error: expect(string).toMatchSnapshot(expected)');
const expectedSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-expected.txt');
const actualSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-actual.txt');
expect(outputText).toMatch(/Expected:.*a\.spec\.js-snapshots.snapshot\.txt/);
@ -635,7 +635,8 @@ test('should compare different PNG images', async ({ runInlineTest }, testInfo)
const outputText = result.output;
expect(result.exitCode).toBe(1);
expect(outputText).toContain('Screenshot comparison failed:');
expect(outputText).toContain('Error: expect(Buffer).toMatchSnapshot(expected)');
expect(outputText).toContain('1 pixels (ratio 1.00 of all image pixels) are different.');
const expectedSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-expected.png');
const actualSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-actual.png');
const diffSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-diff.png');

4
tests/playwright-test/reporter-html.spec.ts
View File

@ -330,7 +330,9 @@ for (const useIntermediateMergeReport of [true, false] as const) {
await expect(page.locator('text=Image mismatch')).toHaveCount(1);
await expect(page.locator('text=Snapshot mismatch')).toHaveCount(0);
await expect(page.locator('.chip-header', { hasText: 'Screenshots' })).toHaveCount(0);
await expect(page.getByTestId('test-result-image-mismatch-tabs').locator('div')).toHaveText([
const errorChip = page.getByTestId('test-screenshot-error-view');
await expect(errorChip).toContainText('Failed to take two consecutive stable screenshots.');
await expect(errorChip.getByTestId('test-result-image-mismatch-tabs').locator('div')).toHaveText([
'Diff',
'Actual',
'Previous',

2
tests/playwright-test/to-have-screenshot.spec.ts
View File

@ -551,7 +551,7 @@ test('should fail when screenshot is different pixels', async ({ runInlineTest }
`
});
expect(result.exitCode).toBe(1);
expect(result.output).toContain('Screenshot comparison failed');
expect(result.output).toContain('Error: expect(page).toHaveScreenshot(expected)');
expect(result.output).toContain('12345 pixels');
expect(result.output).toContain('Call log');
expect(result.output).toContain('ratio 0.02');