From b700c08dc5e6bed3dc23fa6f210d6f9d7a7c6500 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 20 Jan 2023 19:41:43 -0800 Subject: [PATCH] feat(screenshots): when actual and expected have different sizes, pad and produce the diff image (#20208) Also show sizes in the html report to easier spot the size mismatch issue. diff Fixes #15802. --- packages/html-reporter/src/imageDiffView.css | 21 +++++++- .../html-reporter/src/imageDiffView.spec.tsx | 6 +-- packages/html-reporter/src/imageDiffView.tsx | 37 ++++++++++--- .../playwright-core/src/utils/comparators.ts | 52 ++++++++++++++----- tests/playwright-test/golden.spec.ts | 8 ++- tests/playwright-test/reporter-html.spec.ts | 1 + .../to-have-screenshot.spec.ts | 8 ++- 7 files changed, 107 insertions(+), 26 deletions(-) diff --git a/packages/html-reporter/src/imageDiffView.css b/packages/html-reporter/src/imageDiffView.css index ab1f467c29..021fe082aa 100644 --- a/packages/html-reporter/src/imageDiffView.css +++ b/packages/html-reporter/src/imageDiffView.css @@ -21,10 +21,27 @@ position: relative; } -.image-diff-view img { - flex: none; +.image-diff-view .image-wrapper img { + flex: auto; box-shadow: var(--box-shadow-thick); margin: 24px auto; min-width: 200px; max-width: 80%; } + +.image-diff-view .image-wrapper { + flex: auto; + display: flex; + flex-direction: column; + align-items: center; +} + +.image-diff-view .image-wrapper div { + flex: none; + align-self: stretch; + height: 2em; + font-weight: 500; + padding-top: 1em; + display: flex; + flex-direction: row; +} diff --git a/packages/html-reporter/src/imageDiffView.spec.tsx b/packages/html-reporter/src/imageDiffView.spec.tsx index d07e8dd40a..5f7cd71770 100644 --- a/packages/html-reporter/src/imageDiffView.spec.tsx +++ b/packages/html-reporter/src/imageDiffView.spec.tsx @@ -54,7 +54,7 @@ test('should show actual by default', async ({ mount }) => { for (let i = 0; i < imageCount; ++i) { const image = images.nth(i); const box = await image.boundingBox(); - expect(box).toEqual({ x: 400, y: 80, width: 200, height: 200 }); + expect(box).toEqual({ x: 400, y: 108, width: 200, height: 200 }); } }); @@ -69,7 +69,7 @@ test('should switch to expected', async ({ mount }) => { for (let i = 0; i < imageCount; ++i) { const image = images.nth(i); const box = await image.boundingBox(); - expect(box).toEqual({ x: 400, y: 80, width: 200, height: 200 }); + expect(box).toEqual({ x: 400, y: 108, width: 200, height: 200 }); } }); @@ -79,5 +79,5 @@ test('should switch to diff', async ({ mount }) => { const image = component.locator('img'); const box = await image.boundingBox(); - expect(box).toEqual({ x: 400, y: 80, width: 200, height: 200 }); + expect(box).toEqual({ x: 400, y: 108, width: 200, height: 200 }); }); diff --git a/packages/html-reporter/src/imageDiffView.tsx b/packages/html-reporter/src/imageDiffView.tsx index 21146e31de..2c7492caec 100644 --- a/packages/html-reporter/src/imageDiffView.tsx +++ b/packages/html-reporter/src/imageDiffView.tsx @@ -54,35 +54,35 @@ export const ImageDiffView: React.FunctionComponent<{ id: 'actual', title: 'Actual', render: () => - onImageLoaded('right')} ref={imageElement} /> - + onImageLoaded('right')} imageRef={imageElement} style={{ boxShadow: 'none' }} /> + , }); tabs.push({ id: 'expected', title: diff.expected!.title, render: () => - onImageLoaded('left')} ref={imageElement} /> - + onImageLoaded('left')} imageRef={imageElement} /> + , }); } else { tabs.push({ id: 'actual', title: 'Actual', - render: () => onImageLoaded()} /> + render: () => onImageLoaded()} /> }); tabs.push({ id: 'expected', title: diff.expected!.title, - render: () => onImageLoaded()} /> + render: () => onImageLoaded()} /> }); } if (diff.diff) { tabs.push({ id: 'diff', title: 'Diff', - render: () => onImageLoaded()} /> + render: () => onImageLoaded()} /> }); } return
@@ -167,6 +167,29 @@ export const ImageDiffSlider: React.FC; }; +const ImageWithSize: React.FunctionComponent<{ + src: string, + onLoad?: () => void, + imageRef?: React.RefObject, + style?: React.CSSProperties, +}> = ({ src, onLoad, imageRef, style }) => { + const newRef = React.useRef(null); + const ref = imageRef ?? newRef; + const [size, setSize] = React.useState<{ width: number, height: number } | null>(null); + return
+
+ { size ? size.width : ''} + x + { size ? size.height : ''} +
+ { + onLoad?.(); + if (ref.current) + setSize({ width: ref.current.naturalWidth, height: ref.current.naturalHeight }); + }} ref={ref} style={style} /> +
; +}; + const absolute: React.CSSProperties = { position: 'absolute', top: 0, diff --git a/packages/playwright-core/src/utils/comparators.ts b/packages/playwright-core/src/utils/comparators.ts index 6704ada9ff..790f58056a 100644 --- a/packages/playwright-core/src/utils/comparators.ts +++ b/packages/playwright-core/src/utils/comparators.ts @@ -47,27 +47,31 @@ function compareBuffersOrStrings(actualBuffer: Buffer | string, expectedBuffer: return null; } +type ImageData = { width: number, height: number, data: Buffer }; + function compareImages(mimeType: string, actualBuffer: Buffer | string, expectedBuffer: Buffer, options: ImageComparatorOptions = {}): ComparatorResult { if (!actualBuffer || !(actualBuffer instanceof Buffer)) return { errorMessage: 'Actual result should be a Buffer.' }; - const actual = mimeType === 'image/png' ? PNG.sync.read(actualBuffer) : jpegjs.decode(actualBuffer, { maxMemoryUsageInMB: JPEG_JS_MAX_BUFFER_SIZE_IN_MB }); - const expected = mimeType === 'image/png' ? PNG.sync.read(expectedBuffer) : jpegjs.decode(expectedBuffer, { maxMemoryUsageInMB: JPEG_JS_MAX_BUFFER_SIZE_IN_MB }); + let actual: ImageData = mimeType === 'image/png' ? PNG.sync.read(actualBuffer) : jpegjs.decode(actualBuffer, { maxMemoryUsageInMB: JPEG_JS_MAX_BUFFER_SIZE_IN_MB }); + let expected: ImageData = mimeType === 'image/png' ? PNG.sync.read(expectedBuffer) : jpegjs.decode(expectedBuffer, { maxMemoryUsageInMB: JPEG_JS_MAX_BUFFER_SIZE_IN_MB }); + const size = { width: Math.max(expected.width, actual.width), height: Math.max(expected.height, actual.height) }; + let sizesMismatchError = ''; if (expected.width !== actual.width || expected.height !== actual.height) { - return { - errorMessage: `Expected an image ${expected.width}px by ${expected.height}px, received ${actual.width}px by ${actual.height}px. ` - }; + sizesMismatchError = `Expected an image ${expected.width}px by ${expected.height}px, received ${actual.width}px by ${actual.height}px. `; + actual = resizeImage(actual, size); + expected = resizeImage(expected, size); } - const diff = new PNG({ width: expected.width, height: expected.height }); + const diff = new PNG({ width: size.width, height: size.height }); let count; if (options._comparator === 'ssim-cie94') { - count = compare(expected.data, actual.data, diff.data, expected.width, expected.height, { + count = compare(expected.data, actual.data, diff.data, size.width, size.height, { // All ΔE* formulae are originally designed to have the difference of 1.0 stand for a "just noticeable difference" (JND). // See https://en.wikipedia.org/wiki/Color_difference#CIELAB_%CE%94E* maxColorDeltaE94: 1.0, }); } else if ((options._comparator ?? 'pixelmatch') === 'pixelmatch') { - count = pixelmatch(expected.data, actual.data, diff.data, expected.width, expected.height, { + count = pixelmatch(expected.data, actual.data, diff.data, size.width, size.height, { threshold: options.threshold ?? 0.2, }); } else { @@ -82,10 +86,10 @@ function compareImages(mimeType: string, actualBuffer: Buffer | string, expected else maxDiffPixels = maxDiffPixels1 ?? maxDiffPixels2 ?? 0; const ratio = Math.ceil(count / (expected.width * expected.height) * 100) / 100; - return count > maxDiffPixels ? { - errorMessage: `${count} pixels (ratio ${ratio.toFixed(2)} of all image pixels) are different`, - diff: PNG.sync.write(diff), - } : null; + const pixelsMismatchError = count > maxDiffPixels ? `${count} pixels (ratio ${ratio.toFixed(2)} of all image pixels) are different.` : ''; + if (pixelsMismatchError || sizesMismatchError) + return { errorMessage: sizesMismatchError + pixelsMismatchError, diff: PNG.sync.write(diff) }; + return null; } function compareText(actual: Buffer | string, expectedBuffer: Buffer): ComparatorResult { @@ -122,3 +126,27 @@ function diff_prettyTerminal(diffs: [number, string][]) { } return html.join(''); } + +function resizeImage(image: ImageData, size: { width: number, height: number }): ImageData { + if (image.width === size.width && image.height === size.height) + return image; + const buffer = new Uint8Array(size.width * size.height * 4); + for (let y = 0; y < size.height; y++) { + for (let x = 0; x < size.width; x++) { + const to = (y * size.width + x) * 4; + if (y < image.height && x < image.width) { + const from = (y * image.width + x) * 4; + buffer[to] = image.data[from]; + buffer[to + 1] = image.data[from + 1]; + buffer[to + 2] = image.data[from + 2]; + buffer[to + 3] = image.data[from + 3]; + } else { + buffer[to] = 0; + buffer[to + 1] = 0; + buffer[to + 2] = 0; + buffer[to + 3] = 0; + } + } + } + return { data: Buffer.from(buffer), width: size.width, height: size.height }; +} diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 6c19a39f51..33e82d9f34 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -927,7 +927,7 @@ test('should attach expected/actual/diff', async ({ runInlineTest }, testInfo) = ]); }); -test('should attach expected/actual and no diff', async ({ runInlineTest }, testInfo) => { +test('should attach expected/actual/diff for different sizes', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...files, 'a.spec.js-snapshots/snapshot.png': @@ -945,6 +945,7 @@ test('should attach expected/actual and no diff', async ({ runInlineTest }, test const outputText = stripAnsi(result.output); expect(outputText).toContain('Expected an image 2px by 2px, received 1px by 1px.'); + expect(outputText).toContain('4 pixels (ratio 1.00 of all image pixels) are different.'); const attachments = outputText.split('\n').filter(l => l.startsWith('## ')).map(l => l.substring(3)).map(l => JSON.parse(l))[0]; for (const attachment of attachments) attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); @@ -959,6 +960,11 @@ test('should attach expected/actual and no diff', async ({ runInlineTest }, test contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' }, + { + name: 'snapshot-diff.png', + contentType: 'image/png', + path: 'a-is-a-test/snapshot-diff.png' + }, ]); }); diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 89d2e30009..ab68166437 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -137,6 +137,7 @@ test('should include image diff', async ({ runInlineTest, page, showReport }) => set.add(await expectedImage.getAttribute('src')); set.add(await actualImage.getAttribute('src')); expect(set.size, 'Should be two images overlaid').toBe(2); + await expect(imageDiff).toContainText('200x200'); const sliderElement = imageDiff.locator('data-testid=test-result-image-mismatch-grip'); await expect.poll(() => sliderElement.evaluate(e => e.style.left), 'Actual slider is on the right').toBe('590px'); diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index d6df3d423f..07b63d78e1 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -943,7 +943,7 @@ test('should throw for invalid maxDiffPixelRatio values', async ({ runInlineTest }); -test('should attach expected/actual and no diff when sizes are different', async ({ runInlineTest }, testInfo) => { +test('should attach expected/actual/diff when sizes are different', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...playwrightConfig({ snapshotPathTemplate: '__screenshots__/{testFilePath}/{arg}{ext}', @@ -962,6 +962,7 @@ test('should attach expected/actual and no diff when sizes are different', async expect(result.exitCode).toBe(1); const outputText = stripAnsi(result.output); expect(outputText).toContain('Expected an image 2px by 2px, received 1280px by 720px.'); + expect(outputText).toContain('4 pixels (ratio 0.01 of all image pixels) are different.'); const attachments = outputText.split('\n').filter(l => l.startsWith('## ')).map(l => l.substring(3)).map(l => JSON.parse(l))[0]; for (const attachment of attachments) attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); @@ -976,6 +977,11 @@ test('should attach expected/actual and no diff when sizes are different', async contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' }, + { + name: 'snapshot-diff.png', + contentType: 'image/png', + path: 'a-is-a-test/snapshot-diff.png' + }, ]); });