diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx index 407795840e..c4e9384f3a 100644 --- a/packages/html-reporter/src/testResultView.tsx +++ b/packages/html-reporter/src/testResultView.tsx @@ -26,30 +26,68 @@ import { AttachmentLink } from './links'; import { statusIcon } from './statusIcon'; import './testResultView.css'; -const imageDiffNames = ['diff', 'expected', 'actual']; +type DiffTab = { + id: string, + title: string, + attachment: TestAttachment, +}; + +function classifyAttachments(attachments: TestAttachment[]) { + const screenshots = new Set(attachments.filter(a => a.contentType.startsWith('image/'))); + const videos = attachments.filter(a => a.name === 'video'); + const traces = attachments.filter(a => a.name === 'trace'); + + const otherAttachments = new Set(attachments); + [...screenshots, ...videos, ...traces].forEach(a => otherAttachments.delete(a)); + + const snapshotNameToDiffTabs = new Map(); + let tabId = 0; + for (const attachment of attachments) { + const match = attachment.name.match(/^(.*)-(\w+)(\.[^.]+)?$/); + if (!match) + continue; + const [, name, category, extension = ''] = match; + const snapshotName = name + extension; + let diffTabs = snapshotNameToDiffTabs.get(snapshotName); + if (!diffTabs) { + diffTabs = []; + snapshotNameToDiffTabs.set(snapshotName, diffTabs); + } + diffTabs.push({ + id: 'tab-' + (++tabId), + title: category, + attachment, + }); + } + const diffs = [...snapshotNameToDiffTabs].map(([snapshotName, diffTabs]) => { + diffTabs.sort((tab1: DiffTab, tab2: DiffTab) => { + if (tab1.title === 'diff' || tab2.title === 'diff') + return tab1.title === 'diff' ? -1 : 1; + if (tab1.title !== tab2.title) + return tab1.title < tab2.title ? -1 : 1; + return 0; + }); + const isImageDiff = diffTabs.some(tab => screenshots.has(tab.attachment)); + for (const tab of diffTabs) + screenshots.delete(tab.attachment); + return { + tabs: diffTabs, + isImageDiff, + snapshotName, + }; + }).filter(diff => diff.tabs.some(tab => ['diff', 'actual', 'expected'].includes(tab.title.toLowerCase()))); + return { diffs, screenshots: [...screenshots], videos, otherAttachments, traces }; +} export const TestResultView: React.FC<{ test: TestCase, result: TestResult, }> = ({ result }) => { - const { screenshots, videos, traces, otherAttachments, attachmentsMap } = React.useMemo(() => { - const attachmentsMap = new Map(); - const attachments = result?.attachments || []; - const otherAttachments = new Set(attachments); - const screenshots = attachments.filter(a => a.contentType.startsWith('image/') && !imageDiffNames.includes(a.name)); - const videos = attachments.filter(a => a.name === 'video'); - const traces = attachments.filter(a => a.name === 'trace'); - for (const a of attachments) - attachmentsMap.set(a.name, a); - [...screenshots, ...videos, ...traces].forEach(a => otherAttachments.delete(a)); - return { attachmentsMap, screenshots, videos, otherAttachments, traces }; + const { screenshots, videos, traces, otherAttachments, diffs } = React.useMemo(() => { + return classifyAttachments(result?.attachments || []); }, [ result ]); - const diff = attachmentsMap.get('diff'); - const expected = attachmentsMap.get('expected'); - const actual = attachmentsMap.get('actual'); - const hasImages = [actual?.contentType, expected?.contentType, diff?.contentType].some(v => v && /^image\//i.test(v)); return
{!!result.errors.length && {result.errors.map((error, index) => )} @@ -58,12 +96,12 @@ export const TestResultView: React.FC<{ {result.steps.map((step, i) => )} } - {expected && actual && - {hasImages && } - {diff && } - - - } + {diffs.map(({ tabs, snapshotName, isImageDiff }, index) => + + {isImageDiff && } + {tabs.map((tab: DiffTab) => )} + + )} {!!screenshots.length && {screenshots.map((a, i) => { @@ -117,38 +155,22 @@ const StepTreeItem: React.FC<{ }; const ImageDiff: React.FunctionComponent<{ - actual: TestAttachment, - expected: TestAttachment, - diff?: TestAttachment, -}> = ({ actual, expected, diff }) => { - const [selectedTab, setSelectedTab] = React.useState('actual'); + tabs: DiffTab[], +}> = ({ tabs }) => { + // Pre-select a tab called "actual", if any. + const preselectedTab = tabs.find(tab => tab.title.toLowerCase() === 'actual') || tabs[0]; + const [selectedTab, setSelectedTab] = React.useState(preselectedTab.id); const diffElement = React.useRef(null); - const tabs = []; - if (diff) { - tabs.push({ - id: 'diff', - title: 'Diff', - render: () => - }); - } - tabs.push({ - id: 'actual', - title: 'Actual', - render: () => { + const paneTabs = tabs.map(tab => ({ + id: tab.id, + title: tab.title, + render: () => { if (diffElement.current) diffElement.current.style.minHeight = diffElement.current.offsetHeight + 'px'; }}/> - }); - tabs.push({ - id: 'expected', - title: 'Expected', - render: () => { - if (diffElement.current) - diffElement.current.style.minHeight = diffElement.current.offsetHeight + 'px'; - }}/> - }); + })); return
- +
; }; diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts index 398f66b880..b2eb81e4fb 100644 --- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts @@ -63,6 +63,7 @@ export function getSnapshotName( class SnapshotHelper { readonly testInfo: TestInfoImpl; readonly expectedPath: string; + readonly previousPath: string; readonly snapshotPath: string; readonly actualPath: string; readonly diffPath: string; @@ -115,27 +116,22 @@ class SnapshotHelper { // sanitizes path if string const pathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, '', undefined, true)]; - const snapshotPath = snapshotPathResolver(...pathSegments); + this.snapshotPath = snapshotPathResolver(...pathSegments); const outputFile = testInfo.outputPath(...pathSegments); - const expectedPath = addSuffixToFilePath(outputFile, '-expected'); - const actualPath = addSuffixToFilePath(outputFile, '-actual'); - const diffPath = addSuffixToFilePath(outputFile, '-diff'); + this.expectedPath = addSuffixToFilePath(outputFile, '-expected'); + this.previousPath = addSuffixToFilePath(outputFile, '-previous'); + this.actualPath = addSuffixToFilePath(outputFile, '-actual'); + this.diffPath = addSuffixToFilePath(outputFile, '-diff'); - let updateSnapshots = testInfo.config.updateSnapshots; - if (updateSnapshots === 'missing' && testInfo.retry < testInfo.project.retries) - updateSnapshots = 'none'; - const mimeType = mime.getType(path.basename(snapshotPath)) ?? 'application/octet-string'; - const comparator: Comparator = mimeTypeToComparator[mimeType]; + this.updateSnapshots = testInfo.config.updateSnapshots; + if (this.updateSnapshots === 'missing' && testInfo.retry < testInfo.project.retries) + this.updateSnapshots = 'none'; + this.mimeType = mime.getType(path.basename(this.snapshotPath)) ?? 'application/octet-string'; + const comparator: Comparator = mimeTypeToComparator[this.mimeType]; if (!comparator) - throw new Error('Failed to find comparator with type ' + mimeType + ': ' + snapshotPath); + throw new Error('Failed to find comparator with type ' + this.mimeType + ': ' + this.snapshotPath); this.testInfo = testInfo; - this.mimeType = mimeType; - this.actualPath = actualPath; - this.expectedPath = expectedPath; - this.diffPath = diffPath; - this.snapshotPath = snapshotPath; - this.updateSnapshots = updateSnapshots; this.allOptions = options; this.comparatorOptions = { maxDiffPixels: options.maxDiffPixels, @@ -192,6 +188,7 @@ class SnapshotHelper { handleDifferent( actual: Buffer | string | undefined, expected: Buffer | string | undefined, + previous: Buffer | string | undefined, diff: Buffer | string | undefined, diffError: string | undefined, log: string[] | undefined, @@ -211,17 +208,22 @@ class SnapshotHelper { if (expected !== undefined) { writeFileSync(this.expectedPath, expected); - this.testInfo.attachments.push({ name: 'expected', contentType: this.mimeType, path: this.expectedPath }); + this.testInfo.attachments.push({ name: path.basename(this.expectedPath), contentType: this.mimeType, path: this.expectedPath }); output.push(`Expected: ${colors.yellow(this.expectedPath)}`); } + if (previous !== undefined) { + writeFileSync(this.previousPath, previous); + this.testInfo.attachments.push({ name: path.basename(this.previousPath), contentType: this.mimeType, path: this.previousPath }); + output.push(`Previous: ${colors.yellow(this.previousPath)}`); + } if (actual !== undefined) { writeFileSync(this.actualPath, actual); - this.testInfo.attachments.push({ name: 'actual', contentType: this.mimeType, path: this.actualPath }); + this.testInfo.attachments.push({ name: path.basename(this.actualPath), contentType: this.mimeType, path: this.actualPath }); output.push(`Received: ${colors.yellow(this.actualPath)}`); } if (diff !== undefined) { writeFileSync(this.diffPath, diff); - this.testInfo.attachments.push({ name: 'diff', contentType: this.mimeType, path: this.diffPath }); + this.testInfo.attachments.push({ name: path.basename(this.diffPath), contentType: this.mimeType, path: this.diffPath }); output.push(` Diff: ${colors.yellow(this.diffPath)}`); } return { pass: false, message: () => output.join('\n'), }; @@ -271,7 +273,7 @@ export function toMatchSnapshot( return { pass: true, message: () => helper.snapshotPath + ' running with --update-snapshots, writing actual.' }; } - return helper.handleDifferent(received, expected, result.diff, result.errorMessage, undefined); + return helper.handleDifferent(received, expected, undefined, result.diff, result.errorMessage, undefined); } type HaveScreenshotOptions = ImageComparatorOptions & Omit; @@ -344,11 +346,10 @@ 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) { - // TODO(aslushnikov): rename attachments to "actual" and "previous". They still should be somehow shown in HTML reporter. 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, previous, diff, undefined, log, title); + return helper.handleDifferent(actual, undefined, previous, diff, undefined, log, title); } // We successfully (re-)generated new screenshot. @@ -379,7 +380,7 @@ export async function toHaveScreenshot( }); return errorMessage ? - helper.handleDifferent(actual, expected, diff, errorMessage, log) : + helper.handleDifferent(actual, expected, undefined, diff, errorMessage, log) : helper.handleMatching(); } diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 9952e6163b..52117bdf39 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -758,17 +758,17 @@ test('should attach expected/actual/diff with snapshot path', async ({ runInline attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); expect(attachments).toEqual([ { - name: 'expected', + name: 'snapshot-expected.png', contentType: 'image/png', path: 'a-is-a-test/test/path/snapshot-expected.png' }, { - name: 'actual', + name: 'snapshot-actual.png', contentType: 'image/png', path: 'a-is-a-test/test/path/snapshot-actual.png' }, { - name: 'diff', + name: 'snapshot-diff.png', contentType: 'image/png', path: 'a-is-a-test/test/path/snapshot-diff.png' } @@ -797,17 +797,17 @@ test('should attach expected/actual/diff', async ({ runInlineTest }, testInfo) = attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); expect(attachments).toEqual([ { - name: 'expected', + name: 'snapshot-expected.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-expected.png' }, { - name: 'actual', + name: 'snapshot-actual.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' }, { - name: 'diff', + name: 'snapshot-diff.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-diff.png' } @@ -837,12 +837,12 @@ test('should attach expected/actual and no diff', async ({ runInlineTest }, test attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); expect(attachments).toEqual([ { - name: 'expected', + name: 'snapshot-expected.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-expected.png' }, { - name: 'actual', + name: 'snapshot-actual.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' }, diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 0a213067b0..3a562d0308 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { test as baseTest, expect } from './playwright-test-fixtures'; +import { test as baseTest, expect, createImage } from './playwright-test-fixtures'; import { HttpServer } from '../../packages/playwright-core/lib/utils/httpServer'; import { startHtmlReportServer } from '../../packages/playwright-test/lib/reporters/html'; @@ -128,6 +128,85 @@ test('should include image diff', async ({ runInlineTest, page, showReport }) => expect(set.size).toBe(3); }); +test('should include multiple image diffs', async ({ runInlineTest, page, showReport }) => { + const IMG_WIDTH = 200; + const IMG_HEIGHT = 200; + const redImage = createImage(IMG_WIDTH, IMG_HEIGHT, 255, 0, 0); + const whiteImage = createImage(IMG_WIDTH, IMG_HEIGHT, 255, 255, 255); + + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + screenshotsDir: '__screenshots__', + use: { viewport: { width: ${IMG_WIDTH}, height: ${IMG_HEIGHT} }} + }; + `, + '__screenshots__/a.test.js/fails-1.png': redImage, + '__screenshots__/a.test.js/fails-2.png': whiteImage, + '__screenshots__/a.test.js/fails-3.png': redImage, + 'a.test.js': ` + const { test } = pwt; + test('fails', async ({ page }, testInfo) => { + testInfo.snapshotSuffix = ''; + await expect.soft(page).toHaveScreenshot({ timeout: 1000 }); + await expect.soft(page).toHaveScreenshot({ timeout: 1000 }); + await expect.soft(page).toHaveScreenshot({ timeout: 1000 }); + }); + `, + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + + await showReport(); + await page.click('text=fails'); + await expect(page.locator('text=Image mismatch')).toHaveCount(2); + await expect(page.locator('text=Snapshot mismatch')).toHaveCount(0); + await expect(page.locator('text=Screenshots')).toHaveCount(0); + for (let i = 0; i < 2; ++i) { + const imageDiff = page.locator('data-testid=test-result-image-mismatch').nth(i); + const image = imageDiff.locator('img'); + await expect(image).toHaveAttribute('src', /.*png/); + } +}); + +test('should include image diff when screenshot failed to generate due to animation', async ({ runInlineTest, page, showReport }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { use: { viewport: { width: 200, height: 200 }} }; + `, + 'a.test.js': ` + const { test } = pwt; + test('fails', async ({ page }, testInfo) => { + testInfo.snapshotSuffix = ''; + await page.evaluate(() => { + setInterval(() => { + document.body.textContent = Date.now(); + }, 50); + }); + await expect.soft(page).toHaveScreenshot({ timeout: 1000 }); + }); + `, + }, { 'reporter': 'dot,html', 'update-snapshots': true }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + + await showReport(); + await page.click('text=fails'); + await expect(page.locator('text=Image mismatch')).toHaveCount(1); + await expect(page.locator('text=Snapshot mismatch')).toHaveCount(0); + await expect(page.locator('text=Screenshots')).toHaveCount(0); + const imageDiff = page.locator('data-testid=test-result-image-mismatch'); + const image = imageDiff.locator('img'); + await expect(image).toHaveAttribute('src', /.*png/); + const actualSrc = await image.getAttribute('src'); + await imageDiff.locator('text=Previous').click(); + const previousSrc = await image.getAttribute('src'); + await imageDiff.locator('text=Diff').click(); + const diffSrc = await image.getAttribute('src'); + const set = new Set([previousSrc, actualSrc, diffSrc]); + expect(set.size).toBe(3); +}); + test('should not include image diff with non-images', async ({ runInlineTest, page, showReport }) => { const expected = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAMgAAADICAYAAACtWK6eAAAAAXNSR0IArs4c6QAAAhVJREFUeJzt07ERwCAQwLCQ/Xd+FuDcQiFN4MZrZuYDjv7bAfAyg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAiEDVPZBYx6ffy+AAAAAElFTkSuQmCC', 'base64'); const result = await runInlineTest({ @@ -393,10 +472,10 @@ test('should render text attachments as text', async ({ runInlineTest, page, sho expect(result.exitCode).toBe(0); await showReport(); - await page.click('text=passing'); - await page.click('text=example.txt'); - await page.click('text=example.json'); - await page.click('text=example-utf16.txt'); + await page.locator('text=passing').click(); + await page.locator('text=example.txt').click(); + await page.locator('text=example.json').click(); + await page.locator('text=example-utf16.txt').click(); await expect(page.locator('.attachment-body')).toHaveText(['foo', '{"foo":1}', 'utf16 encoded']); }); diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index c91a31d9ac..3cb9f096c1 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -52,7 +52,8 @@ 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(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(true); - expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(true); + expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(false); + 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-diff.png'))).toBe(true); expect(fs.existsSync(testInfo.outputPath('a.spec.js-snapshots', 'is-a-test-1.png'))).toBe(false); }); @@ -286,7 +287,8 @@ 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(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(true); - expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(true); + expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(false); + 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-diff.png'))).toBe(true); expect(fs.existsSync(testInfo.outputPath('__screenshots__', 'a.spec.js', 'is-a-test-1.png'))).toBe(false); }); @@ -788,12 +790,12 @@ test('should attach expected/actual and no diff when sizes are different', async attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); expect(attachments).toEqual([ { - name: 'expected', + name: 'snapshot-expected.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-expected.png' }, { - name: 'actual', + name: 'snapshot-actual.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' },