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
This commit is contained in:
Andrey Lushnikov 2023-08-07 04:42:35 -07:00 committed by GitHub
parent 53fd4bedb4
commit fa8f3f6454
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 14 additions and 12 deletions

View file

@ -43,7 +43,7 @@ type SnapshotNames = {
class SnapshotHelper<T extends ImageComparatorOptions> { class SnapshotHelper<T extends ImageComparatorOptions> {
readonly testInfo: TestInfoImpl; readonly testInfo: TestInfoImpl;
readonly snapshotName: string; readonly snapshotName: string;
readonly expectedPath: string; readonly legacyExpectedPath: string;
readonly previousPath: string; readonly previousPath: string;
readonly snapshotPath: string; readonly snapshotPath: string;
readonly actualPath: string; readonly actualPath: string;
@ -126,7 +126,7 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
this.snapshotPath = snapshotPathResolver(...inputPathSegments); this.snapshotPath = snapshotPathResolver(...inputPathSegments);
const inputFile = testInfo._getOutputPath(...inputPathSegments); const inputFile = testInfo._getOutputPath(...inputPathSegments);
const outputFile = testInfo._getOutputPath(...outputPathSegments); const outputFile = testInfo._getOutputPath(...outputPathSegments);
this.expectedPath = addSuffixToFilePath(inputFile, '-expected'); this.legacyExpectedPath = addSuffixToFilePath(inputFile, '-expected');
this.previousPath = addSuffixToFilePath(outputFile, '-previous'); this.previousPath = addSuffixToFilePath(outputFile, '-previous');
this.actualPath = addSuffixToFilePath(outputFile, '-actual'); this.actualPath = addSuffixToFilePath(outputFile, '-actual');
this.diffPath = addSuffixToFilePath(outputFile, '-diff'); this.diffPath = addSuffixToFilePath(outputFile, '-diff');
@ -213,9 +213,11 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
output.push(''); output.push('');
if (expected !== undefined) { if (expected !== undefined) {
writeFileSync(this.expectedPath, expected); // Copy the expectation inside the `test-results/` folder for backwards compatibility,
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); // so that one can upload `test-results/` directory and have all the data inside.
output.push(`Expected: ${colors.yellow(this.expectedPath)}`); 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) { if (previous !== undefined) {
writeFileSync(this.previousPath, previous); writeFileSync(this.previousPath, previous);

View file

@ -156,7 +156,7 @@ test('should write detailed failure result to an output folder', async ({ runInl
expect(outputText).toContain('Snapshot comparison failed:'); expect(outputText).toContain('Snapshot comparison failed:');
const expectedSnapshotArtifactPath = testInfo.outputPath('test-results', 'a-is-a-test', 'snapshot-expected.txt'); 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'); 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(outputText).toContain(`Received: ${actualSnapshotArtifactPath}`);
expect(fs.existsSync(expectedSnapshotArtifactPath)).toBe(true); expect(fs.existsSync(expectedSnapshotArtifactPath)).toBe(true);
expect(fs.existsSync(actualSnapshotArtifactPath)).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 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 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'); 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(`Received: ${actualSnapshotArtifactPath}`);
expect(outputText).toContain(`Diff: ${diffSnapshotArtifactPath}`); expect(outputText).toContain(`Diff: ${diffSnapshotArtifactPath}`);
expect(fs.existsSync(expectedSnapshotArtifactPath)).toBe(true); 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', name: 'test/path/snapshot-expected.png',
contentType: 'image/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', name: 'test/path/snapshot-actual.png',
@ -916,7 +916,7 @@ test('should attach expected/actual/diff', async ({ runInlineTest }, testInfo) =
{ {
name: 'snapshot-expected.png', name: 'snapshot-expected.png',
contentType: 'image/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', name: 'snapshot-actual.png',
@ -957,7 +957,7 @@ test('should attach expected/actual/diff for different sizes', async ({ runInlin
{ {
name: 'snapshot-expected.png', name: 'snapshot-expected.png',
contentType: 'image/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', name: 'snapshot-actual.png',

View file

@ -195,7 +195,7 @@ for (const useIntermediateMergeReport of [false, true] as const) {
await page.click('text=fails'); await page.click('text=fails');
await expect(page.locator('text=Image mismatch')).toHaveCount(2); await expect(page.locator('text=Image mismatch')).toHaveCount(2);
await expect(page.locator('text=Snapshot mismatch')).toHaveCount(0); 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) { for (let i = 0; i < 2; ++i) {
const imageDiff = page.locator('data-testid=test-result-image-mismatch').nth(i); const imageDiff = page.locator('data-testid=test-result-image-mismatch').nth(i);
const image = imageDiff.locator('img').first(); const image = imageDiff.locator('img').first();

View file

@ -1041,7 +1041,7 @@ test('should attach expected/actual/diff when sizes are different', async ({ run
{ {
name: 'snapshot-expected.png', name: 'snapshot-expected.png',
contentType: 'image/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', name: 'snapshot-actual.png',