From fa8f3f6454ec795a47aa67a54c70599baff5a5c6 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 7 Aug 2023 04:42:35 -0700 Subject: [PATCH] fix: use `snapshotPath` instead of `expectedPath` for image diffs (#24567) This opens a road to the "accept new screenshot" button in Playwright tools. References https://github.com/microsoft/playwright/issues/24310 --- .../playwright-test/src/matchers/toMatchSnapshot.ts | 12 +++++++----- tests/playwright-test/golden.spec.ts | 10 +++++----- tests/playwright-test/reporter-html.spec.ts | 2 +- tests/playwright-test/to-have-screenshot.spec.ts | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts index 1390a03457..95840c8ba7 100644 --- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts @@ -43,7 +43,7 @@ type SnapshotNames = { class SnapshotHelper { readonly testInfo: TestInfoImpl; readonly snapshotName: string; - readonly expectedPath: string; + readonly legacyExpectedPath: string; readonly previousPath: string; readonly snapshotPath: string; readonly actualPath: string; @@ -126,7 +126,7 @@ class SnapshotHelper { this.snapshotPath = snapshotPathResolver(...inputPathSegments); const inputFile = testInfo._getOutputPath(...inputPathSegments); const outputFile = testInfo._getOutputPath(...outputPathSegments); - this.expectedPath = addSuffixToFilePath(inputFile, '-expected'); + this.legacyExpectedPath = addSuffixToFilePath(inputFile, '-expected'); this.previousPath = addSuffixToFilePath(outputFile, '-previous'); this.actualPath = addSuffixToFilePath(outputFile, '-actual'); this.diffPath = addSuffixToFilePath(outputFile, '-diff'); @@ -213,9 +213,11 @@ class SnapshotHelper { output.push(''); if (expected !== undefined) { - writeFileSync(this.expectedPath, expected); - this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); - output.push(`Expected: ${colors.yellow(this.expectedPath)}`); + // 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. + writeFileSync(this.legacyExpectedPath, expected); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-expected'), contentType: this.mimeType, path: this.snapshotPath }); + output.push(`Expected: ${colors.yellow(this.snapshotPath)}`); } if (previous !== undefined) { writeFileSync(this.previousPath, previous); diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 89ea675bbc..643fcaa738 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -156,7 +156,7 @@ test('should write detailed failure result to an output folder', async ({ runInl expect(outputText).toContain('Snapshot comparison failed:'); 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).toContain(`Expected: ${expectedSnapshotArtifactPath}`); + expect(outputText).toMatch(/Expected:.*a\.spec\.js-snapshots.snapshot\.txt/); expect(outputText).toContain(`Received: ${actualSnapshotArtifactPath}`); expect(fs.existsSync(expectedSnapshotArtifactPath)).toBe(true); expect(fs.existsSync(actualSnapshotArtifactPath)).toBe(true); @@ -569,7 +569,7 @@ test('should compare different PNG images', async ({ runInlineTest }, testInfo) 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'); - expect(outputText).toContain(`Expected: ${expectedSnapshotArtifactPath}`); + expect(outputText).toMatch(/Expected:.*a\.spec\.js-snapshots.snapshot\.png/); expect(outputText).toContain(`Received: ${actualSnapshotArtifactPath}`); expect(outputText).toContain(`Diff: ${diffSnapshotArtifactPath}`); expect(fs.existsSync(expectedSnapshotArtifactPath)).toBe(true); @@ -877,7 +877,7 @@ test('should attach expected/actual/diff with snapshot path', async ({ runInline { name: 'test/path/snapshot-expected.png', contentType: 'image/png', - path: 'a-is-a-test/test/path/snapshot-expected.png' + path: 'golden-should-attach-expected-actual-diff-with-snapshot-path-playwright-test/a.spec.js-snapshots/test/path/snapshot.png' }, { name: 'test/path/snapshot-actual.png', @@ -916,7 +916,7 @@ test('should attach expected/actual/diff', async ({ runInlineTest }, testInfo) = { name: 'snapshot-expected.png', contentType: 'image/png', - path: 'a-is-a-test/snapshot-expected.png' + path: 'golden-should-attach-expected-actual-diff-playwright-test/a.spec.js-snapshots/snapshot.png' }, { name: 'snapshot-actual.png', @@ -957,7 +957,7 @@ test('should attach expected/actual/diff for different sizes', async ({ runInlin { name: 'snapshot-expected.png', contentType: 'image/png', - path: 'a-is-a-test/snapshot-expected.png' + path: 'golden-should-attach-expected-actual-diff-for-different-sizes-playwright-test/a.spec.js-snapshots/snapshot.png' }, { name: 'snapshot-actual.png', diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 74c6f5758b..ac7cbf2fe0 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -195,7 +195,7 @@ for (const useIntermediateMergeReport of [false, true] as const) { 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); + 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').first(); diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index a0c05f5a9a..ca7b20f9a0 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -1041,7 +1041,7 @@ test('should attach expected/actual/diff when sizes are different', async ({ run { name: 'snapshot-expected.png', contentType: 'image/png', - path: 'a-is-a-test/snapshot-expected.png' + path: 'to-have-screenshot-should-attach-expected-actual-diff-when-sizes-are-different-playwright-test/__screenshots__/a.spec.js/snapshot.png', }, { name: 'snapshot-actual.png',