From 056997c41f8d23d1bb1199d48d405bd87a3fa2c8 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 18 Jul 2024 02:42:44 -0700 Subject: [PATCH] fix(toHaveScreenshot): attach "expected" when writing a missing expectation (#31745) Previously, only the "actual" attachment was created, pointing to the file in `test-results`. Now, the "expected" attachment pointing to the file in `__screenshots__` is also created. This will help any reporters that would like to know the "expected" path, for example to do a manual accept/decline of the baseline. Fixes #30693. --- .../src/matchers/toMatchSnapshot.ts | 8 ++--- .../to-have-screenshot.spec.ts | 29 ++++++++++++++++++- tests/playwright-test/ui-mode-trace.spec.ts | 1 + 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index a533f13b48..0923a10e82 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -222,11 +222,11 @@ class SnapshotHelper { handleMissing(actual: Buffer | string): ImageMatcherResult { const isWriteMissingMode = this.updateSnapshots === 'all' || this.updateSnapshots === 'missing'; - if (isWriteMissingMode) { + if (isWriteMissingMode) writeFileSync(this.expectedPath, actual); - writeFileSync(this.actualPath, actual); - this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); - } + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); + writeFileSync(this.actualPath, actual); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); const message = `A snapshot doesn't exist at ${this.expectedPath}${isWriteMissingMode ? ', writing actual.' : '.'}`; if (this.updateSnapshots === 'all') { /* eslint-disable no-console */ diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 39044e460a..a4eb131f07 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -263,8 +263,10 @@ test('should report toHaveScreenshot step with expectation name in title', async `end browserContext.newPage`, `end fixture: page`, `end Before Hooks`, + `end attach "foo-expected.png"`, `end attach "foo-actual.png"`, `end expect.toHaveScreenshot(foo.png)`, + `end attach "is-a-test-1-expected.png"`, `end attach "is-a-test-1-actual.png"`, `end expect.toHaveScreenshot(is-a-test-1.png)`, `end fixture: page`, @@ -654,13 +656,23 @@ test('should write missing expectations locally twice and attach them', async ({ const attachments = result.outputLines.map(l => JSON.parse(l))[0]; for (const attachment of attachments) - attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, ''); + attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, '').replace(/.*__screenshots__/, '__screenshots__'); expect(attachments).toEqual([ + { + name: 'snapshot-expected.png', + contentType: 'image/png', + path: '__screenshots__/a.spec.js/snapshot.png' + }, { name: 'snapshot-actual.png', contentType: 'image/png', path: 'a-is-a-test/snapshot-actual.png' }, + { + name: 'snapshot2-expected.png', + contentType: 'image/png', + path: '__screenshots__/a.spec.js/snapshot2.png' + }, { name: 'snapshot2-actual.png', contentType: 'image/png', @@ -1348,16 +1360,31 @@ test('should trim+sanitize attachment names and paths', async ({ runInlineTest } attachment.name = attachment.name.replace(/\\/g, '/'); } expect(attachments).toEqual([ + { + name: 'long-long-long-long-long-l-852e1-long-long-long-long-title-1-expected.png', + contentType: 'image/png', + path: '__screenshots__/a.spec.js/long-long-long-long-long-long-long-long-long-l-852e1-long-long-long-long-long-long-long-long-title-1.png', + }, { name: 'long-long-long-long-long-l-852e1-long-long-long-long-title-1-actual.png', contentType: 'image/png', path: 'test-results/a-long-long-long-long-long-abd51-g-long-long-long-long-title/long-long-long-long-long-l-852e1-long-long-long-long-title-1-actual.png', }, + { + name: 'long-long-long-long-long-l-6bf1e-ong-long-long-long-name-expected.png', + contentType: 'image/png', + path: '__screenshots__/a.spec.js/long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-name.png', + }, { name: 'long-long-long-long-long-l-6bf1e-ong-long-long-long-name-actual.png', contentType: 'image/png', path: 'test-results/a-long-long-long-long-long-abd51-g-long-long-long-long-title/long-long-long-long-long-l-6bf1e-ong-long-long-long-name-actual.png', }, + { + name: 'dir/long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long name-expected.png', + contentType: 'image/png', + path: '__screenshots__/a.spec.js/dir/long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long name.png', + }, { name: 'dir/long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long name-actual.png', contentType: 'image/png', diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts index 0f6194b25f..9f0749893e 100644 --- a/tests/playwright-test/ui-mode-trace.spec.ts +++ b/tests/playwright-test/ui-mode-trace.spec.ts @@ -94,6 +94,7 @@ test('should merge screenshot assertions', async ({ runUITest }, testInfo) => { /Before Hooks[\d.]+m?s/, /page.setContent[\d.]+m?s/, /expect.toHaveScreenshot[\d.]+m?s/, + /attach "trace-test-1-expected.png/, /attach "trace-test-1-actual.png/, /After Hooks[\d.]+m?s/, /Worker Cleanup[\d.]+m?s/,