diff --git a/.gitignore b/.gitignore index aadc481067..1f3b9a7a72 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ test-results .cache/ .eslintcache playwright.env +firefox diff --git a/packages/html-reporter/src/testErrorView.css b/packages/html-reporter/src/testErrorView.css index afb543a0c2..d5a4534e7e 100644 --- a/packages/html-reporter/src/testErrorView.css +++ b/packages/html-reporter/src/testErrorView.css @@ -14,9 +14,8 @@ limitations under the License. */ -.test-error-message { +.test-error-view { white-space: pre; - font-family: monospace; overflow: auto; flex: none; padding: 0; @@ -26,3 +25,7 @@ line-height: initial; margin-bottom: 6px; } + +.test-error-text { + font-family: monospace; +} diff --git a/packages/html-reporter/src/testErrorView.tsx b/packages/html-reporter/src/testErrorView.tsx index 5208158b1c..520da1fc19 100644 --- a/packages/html-reporter/src/testErrorView.tsx +++ b/packages/html-reporter/src/testErrorView.tsx @@ -17,21 +17,39 @@ import ansi2html from 'ansi-to-html'; import * as React from 'react'; import './testErrorView.css'; +import type { ImageDiff } from '@web/shared/imageDiffView'; +import { ImageDiffView } from '@web/shared/imageDiffView'; export const TestErrorView: React.FC<{ error: string; }> = ({ error }) => { - const html = React.useMemo(() => { - const config: any = { - bg: 'var(--color-canvas-subtle)', - fg: 'var(--color-fg-default)', - }; - config.colors = ansiColors; - return new ansi2html(config).toHtml(escapeHTML(error)); - }, [error]); - return
; + const html = React.useMemo(() => ansiErrorToHtml(error), [error]); + return
; }; +export const TestScreenshotErrorView: React.FC<{ + errorPrefix?: string, + diff: ImageDiff, + errorSuffix?: string, +}> = ({ errorPrefix, diff, errorSuffix }) => { + const prefixHtml = React.useMemo(() => ansiErrorToHtml(errorPrefix), [errorPrefix]); + const suffixHtml = React.useMemo(() => ansiErrorToHtml(errorSuffix), [errorSuffix]); + return
+
+ +
+
; +}; + +function ansiErrorToHtml(text?: string): string { + const config: any = { + bg: 'var(--color-canvas-subtle)', + fg: 'var(--color-fg-default)', + }; + config.colors = ansiColors; + return new ansi2html(config).toHtml(escapeHTML(text || '')); +} + const ansiColors = { 0: '#000', 1: '#C00', diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx index 8ee36d0cda..273703a0c7 100644 --- a/packages/html-reporter/src/testResultView.tsx +++ b/packages/html-reporter/src/testResultView.tsx @@ -24,7 +24,7 @@ import { AttachmentLink, generateTraceUrl } from './links'; import { statusIcon } from './statusIcon'; import type { ImageDiff } from '@web/shared/imageDiffView'; import { ImageDiffView } from '@web/shared/imageDiffView'; -import { TestErrorView } from './testErrorView'; +import { TestErrorView, TestScreenshotErrorView } from './testErrorView'; import './testResultView.css'; function groupImageDiffs(screenshots: Set): ImageDiff[] { @@ -67,7 +67,7 @@ export const TestResultView: React.FC<{ anchor: 'video' | 'diff' | '', }> = ({ result, anchor }) => { - const { screenshots, videos, traces, otherAttachments, diffs, htmls } = React.useMemo(() => { + const { screenshots, videos, traces, otherAttachments, diffs, errors, htmls } = React.useMemo(() => { const attachments = result?.attachments || []; const screenshots = new Set(attachments.filter(a => a.contentType.startsWith('image/'))); const videos = attachments.filter(a => a.name === 'video'); @@ -76,7 +76,8 @@ export const TestResultView: React.FC<{ const otherAttachments = new Set(attachments); [...screenshots, ...videos, ...traces, ...htmls].forEach(a => otherAttachments.delete(a)); const diffs = groupImageDiffs(screenshots); - return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs, htmls }; + const errors = classifyErrors(result.errors, diffs); + return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs, errors, htmls }; }, [result]); const videoRef = React.useRef(null); @@ -94,15 +95,19 @@ export const TestResultView: React.FC<{ }, [scrolled, anchor, setScrolled, videoRef]); return
- {!!result.errors.length && - {result.errors.map((error, index) => )} + {!!errors.length && + {errors.map((error, index) => { + if (error.type === 'screenshot') + return ; + return ; + })} } {!!result.steps.length && {result.steps.map((step, i) => )} } {diffs.map((diff, index) => - + )} @@ -145,6 +150,29 @@ export const TestResultView: React.FC<{
; }; +function classifyErrors(testErrors: string[], diffs: ImageDiff[]) { + return testErrors.map(error => { + if (error.includes('Screenshot comparison failed:')) { + const matchingDiff = diffs.find(diff => { + const attachmentName = diff.actual?.attachment.name; + return attachmentName && error.includes(attachmentName); + }); + + if (matchingDiff) { + const lines = error.split('\n'); + const index = lines.findIndex(line => /Expected:|Previous:|Received:/.test(line)); + const errorPrefix = index !== -1 ? lines.slice(0, index).join('\n') : lines[0]; + + const diffIndex = lines.findIndex(line => / +Diff:/.test(line)); + const errorSuffix = diffIndex !== -1 ? lines.slice(diffIndex + 2).join('\n') : lines.slice(1).join('\n'); + + return { type: 'screenshot', diff: matchingDiff, errorPrefix, errorSuffix }; + } + } + return { type: 'regular', error }; + }); +} + const StepTreeItem: React.FC<{ step: TestStep; depth: number, diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index f7f1ef67e8..b78bd91ee1 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -43,7 +43,7 @@ import type { TimeoutOptions } from '../common/types'; import { isInvalidSelectorError } from '../utils/isomorphic/selectorParser'; import { parseEvaluationResultValue, source } from './isomorphic/utilityScriptSerializers'; import type { SerializedValue } from './isomorphic/utilityScriptSerializers'; -import { TargetClosedError } from './errors'; +import { TargetClosedError, TimeoutError } from './errors'; import { asLocator } from '../utils'; import { helper } from './helper'; @@ -662,7 +662,7 @@ export class Page extends SdkObject { return {}; } - if (areEqualScreenshots(actual, options.expected, previous)) { + if (areEqualScreenshots(actual, options.expected, undefined)) { progress.log(`screenshot matched expectation`); return {}; } @@ -672,10 +672,13 @@ export class Page extends SdkObject { // A: We want user to receive a friendly diff between actual and expected/previous. if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e)) throw e; + let errorMessage = e.message; + if (e instanceof TimeoutError && intermediateResult?.previous) + errorMessage = `Failed to take two consecutive stable screenshots. ${e.message}`; return { log: e.message ? [...metadata.log, e.message] : metadata.log, ...intermediateResult, - errorMessage: e.message, + errorMessage, }; }); } diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 0923a10e82..b3fa3f556e 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -423,7 +423,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, diff, errorMessage, log } = await page._expectScreenshot(expectScreenshotOptions); + const { actual, previous, diff, errorMessage, log } = await page._expectScreenshot(expectScreenshotOptions); if (!errorMessage) return helper.handleMatching(); @@ -436,7 +436,7 @@ export async function toHaveScreenshot( return helper.createMatcherResult(helper.expectedPath + ' running with --update-snapshots, writing actual.', true); } - return helper.handleDifferent(actual, expectScreenshotOptions.expected, undefined, diff, errorMessage, log); + return helper.handleDifferent(actual, expectScreenshotOptions.expected, previous, diff, errorMessage, log); } function writeFileSync(aPath: string, content: Buffer | string) { diff --git a/packages/web/src/shared/imageDiffView.tsx b/packages/web/src/shared/imageDiffView.tsx index ea0f1e0042..6f0028bca9 100644 --- a/packages/web/src/shared/imageDiffView.tsx +++ b/packages/web/src/shared/imageDiffView.tsx @@ -61,11 +61,13 @@ const checkerboardStyle: React.CSSProperties = { export const ImageDiffView: React.FC<{ diff: ImageDiff, noTargetBlank?: boolean, -}> = ({ diff, noTargetBlank }) => { + hideDetails?: boolean, +}> = ({ diff, noTargetBlank, hideDetails }) => { const [mode, setMode] = React.useState<'diff' | 'actual' | 'expected' | 'slider' | 'sxs'>(diff.diff ? 'diff' : 'actual'); const [showSxsDiff, setShowSxsDiff] = React.useState(false); const [expectedImage, setExpectedImage] = React.useState(null); + const [expectedImageTitle, setExpectedImageTitle] = React.useState('Expected'); const [actualImage, setActualImage] = React.useState(null); const [diffImage, setDiffImage] = React.useState(null); const [measure, ref] = useMeasure(); @@ -73,6 +75,7 @@ export const ImageDiffView: React.FC<{ React.useEffect(() => { (async () => { setExpectedImage(await loadImage(diff.expected?.attachment.path)); + setExpectedImageTitle(diff.expected?.title || 'Expected'); setActualImage(await loadImage(diff.actual?.attachment.path)); setDiffImage(await loadImage(diff.diff?.attachment.path)); })(); @@ -98,31 +101,31 @@ export const ImageDiffView: React.FC<{
{diff.diff &&
setMode('diff')}>Diff
}
setMode('actual')}>Actual
-
setMode('expected')}>Expected
+
setMode('expected')}>{expectedImageTitle}
setMode('sxs')}>Side by side
setMode('slider')}>Slider
- {diff.diff && mode === 'diff' && } - {diff.diff && mode === 'actual' && } - {diff.diff && mode === 'expected' && } - {diff.diff && mode === 'slider' && } + {diff.diff && mode === 'diff' && } + {diff.diff && mode === 'actual' && } + {diff.diff && mode === 'expected' && } + {diff.diff && mode === 'slider' && } {diff.diff && mode === 'sxs' &&
- - setShowSxsDiff(!showSxsDiff)} canvasWidth={sxsScale * imageWidth} canvasHeight={sxsScale * imageHeight} scale={sxsScale} /> + + setShowSxsDiff(!showSxsDiff)} hideSize={hideDetails} canvasWidth={sxsScale * imageWidth} canvasHeight={sxsScale * imageHeight} scale={sxsScale} />
} - {!diff.diff && mode === 'actual' && } - {!diff.diff && mode === 'expected' && } + {!diff.diff && mode === 'actual' && } + {!diff.diff && mode === 'expected' && } {!diff.diff && mode === 'sxs' &&
- +
}
- } } ; }; @@ -133,7 +136,9 @@ export const ImageDiffSlider: React.FC<{ canvasWidth: number, canvasHeight: number, scale: number, -}> = ({ expectedImage, actualImage, canvasWidth, canvasHeight, scale }) => { + expectedTitle: string, + hideSize?: boolean, +}> = ({ expectedImage, actualImage, canvasWidth, canvasHeight, scale, expectedTitle, hideSize }) => { const absoluteStyle: React.CSSProperties = { position: 'absolute', top: 0, @@ -144,7 +149,7 @@ export const ImageDiffSlider: React.FC<{ const sameSize = expectedImage.naturalWidth === actualImage.naturalWidth && expectedImage.naturalHeight === actualImage.naturalHeight; return
-
+ {!hideSize &&
{!sameSize && Expected } {expectedImage.naturalWidth} x @@ -153,7 +158,7 @@ export const ImageDiffSlider: React.FC<{ {!sameSize && {actualImage.naturalWidth}} {!sameSize && x} {!sameSize && {actualImage.naturalHeight}} -
+
}
setSlider(offsets[0])} resizerColor={'#57606a80'} resizerWidth={6}> - Expected @@ -179,18 +184,19 @@ const ImageWithSize: React.FunctionComponent<{ image: HTMLImageElement, title?: string, alt?: string, + hideSize?: boolean, canvasWidth: number, canvasHeight: number, scale: number, onClick?: () => void; -}> = ({ image, title, alt, canvasWidth, canvasHeight, scale, onClick }) => { +}> = ({ image, title, alt, hideSize, canvasWidth, canvasHeight, scale, onClick }) => { return
-
+ {!hideSize &&
{title && {title}} {image.naturalWidth} x {image.naturalHeight} -
+
}
{ - await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Diff'); - }); + for (const testId of ['test-results-image-diff', 'test-screenshot-error-view']) { + await test.step(testId, async () => { + const imageDiff = page.getByTestId(testId).getByTestId('test-result-image-mismatch'); + await test.step('Diff', async () => { + await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Diff'); + }); - await test.step('Actual', async () => { - await imageDiff.getByText('Actual', { exact: true }).click(); - await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Actual'); - }); + await test.step('Actual', async () => { + await imageDiff.getByText('Actual', { exact: true }).click(); + await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Actual'); + }); - await test.step('Expected', async () => { - await imageDiff.getByText('Expected', { exact: true }).click(); - await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Expected'); - }); + await test.step('Expected', async () => { + await imageDiff.getByText('Expected', { exact: true }).click(); + await expect(imageDiff.locator('img')).toHaveAttribute('alt', 'Expected'); + }); - await test.step('Side by side', async () => { - await imageDiff.getByText('Side by side').click(); - await expect(imageDiff.locator('img')).toHaveCount(2); - await expect(imageDiff.locator('img').first()).toHaveAttribute('alt', 'Expected'); - await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Actual'); - await imageDiff.locator('img').last().click(); - await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Diff'); - }); + await test.step('Side by side', async () => { + await imageDiff.getByText('Side by side').click(); + await expect(imageDiff.locator('img')).toHaveCount(2); + await expect(imageDiff.locator('img').first()).toHaveAttribute('alt', 'Expected'); + await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Actual'); + await imageDiff.locator('img').last().click(); + await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Diff'); + }); - await test.step('Slider', async () => { - await imageDiff.getByText('Slider', { exact: true }).click(); - await expect(imageDiff.locator('img')).toHaveCount(2); - await expect(imageDiff.locator('img').first()).toHaveAttribute('alt', 'Expected'); - await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Actual'); - }); + await test.step('Slider', async () => { + await imageDiff.getByText('Slider', { exact: true }).click(); + await expect(imageDiff.locator('img')).toHaveCount(2); + await expect(imageDiff.locator('img').first()).toHaveAttribute('alt', 'Expected'); + await expect(imageDiff.locator('img').last()).toHaveAttribute('alt', 'Actual'); + }); + }); + } }); test('should include multiple image diffs', async ({ runInlineTest, page, showReport }) => { @@ -285,8 +289,14 @@ for (const useIntermediateMergeReport of [false] as const) { await showReport(); await page.click('text=fails'); - await expect(page.locator('data-testid=test-result-image-mismatch')).toHaveCount(3); - await expect(page.locator('text=Image mismatch:')).toHaveText([ + await expect(page.getByTestId('test-screenshot-error-view').getByTestId('error-suffix')).toContainText([ + `> 6 | await expect.soft(screenshot).toMatchSnapshot('expected.png');`, + `> 7 | await expect.soft(screenshot).toMatchSnapshot('expected.png');`, + `> 8 | await expect.soft(screenshot).toMatchSnapshot('expected.png');`, + ]); + const imageDiffs = page.getByTestId('test-results-image-diff'); + await expect(imageDiffs.getByTestId('test-result-image-mismatch')).toHaveCount(3); + await expect(imageDiffs.getByText('Image mismatch:')).toHaveText([ 'Image mismatch: expected.png', 'Image mismatch: expected-1.png', 'Image mismatch: expected-2.png', @@ -323,7 +333,7 @@ for (const useIntermediateMergeReport of [false] as const) { await expect(page.getByTestId('test-result-image-mismatch-tabs').locator('div')).toHaveText([ 'Diff', 'Actual', - 'Expected', + 'Previous', 'Side by side', 'Slider', ]); @@ -460,7 +470,7 @@ for (const useIntermediateMergeReport of [false] as const) { await showReport(); await page.click('text=fails'); - await expect(page.locator('.test-error-message span:has-text("received")').nth(1)).toHaveCSS('color', 'rgb(204, 0, 0)'); + await expect(page.locator('.test-error-view span:has-text("received")').nth(1)).toHaveCSS('color', 'rgb(204, 0, 0)'); }); test('should show trace source', async ({ runInlineTest, page, showReport }) => {