From 7f513360681d2517caa24d266def9b3a7be4f469 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 21 Mar 2022 17:42:21 -0600 Subject: [PATCH] fix: allow toMatchSnapshot to use text comparator for text data (#12934) This was regressed awhile ago. In v1.17 we shipped the following code: https://github.com/microsoft/playwright/blob/30e15ad36f011c7adf5bad86beadd50ca93b01b8/packages/playwright-test/src/matchers/golden.ts#L122-L131 `toMatchSnapshot` should fallback to text comparator in case of unknown extension and string data. Fixes #12862 --- packages/playwright-core/src/server/page.ts | 4 ++-- .../playwright-core/src/utils/comparators.ts | 16 ++++++++++------ .../src/matchers/toMatchSnapshot.ts | 14 +++++--------- tests/playwright-test/golden.spec.ts | 16 ++++++++++++++++ tests/playwright-test/to-have-screenshot.spec.ts | 4 ++-- 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 06bb2cbdb3..33d8305abb 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -31,7 +31,7 @@ import { Progress, ProgressController } from './progress'; import { assert, isError } from '../utils/utils'; import { ManualPromise } from '../utils/async'; import { debugLogger } from '../utils/debugLogger'; -import { mimeTypeToComparator, ImageComparatorOptions } from '../utils/comparators'; +import { getComparator, ImageComparatorOptions } from '../utils/comparators'; import { SelectorInfo, Selectors } from './selectors'; import { CallMetadata, SdkObject } from './instrumentation'; import { Artifact } from './artifact'; @@ -456,7 +456,7 @@ export class Page extends SdkObject { return await this._screenshotter.screenshotPage(progress, options.screenshotOptions || {}); }; - const comparator = mimeTypeToComparator['image/png']; + const comparator = getComparator('image/png'); const controller = new ProgressController(metadata, this); if (!options.expected && options.isNot) return { errorMessage: '"not" matcher requires expected result' }; diff --git a/packages/playwright-core/src/utils/comparators.ts b/packages/playwright-core/src/utils/comparators.ts index 1fb342572a..aab483ccd3 100644 --- a/packages/playwright-core/src/utils/comparators.ts +++ b/packages/playwright-core/src/utils/comparators.ts @@ -26,12 +26,16 @@ const { PNG } = require(require.resolve('pngjs', { paths: [require.resolve('pixe export type ImageComparatorOptions = { threshold?: number, maxDiffPixels?: number, maxDiffPixelRatio?: number }; export type ComparatorResult = { diff?: Buffer; errorMessage: string; } | null; export type Comparator = (actualBuffer: Buffer | string, expectedBuffer: Buffer, options?: any) => ComparatorResult; -export const mimeTypeToComparator: { [key: string]: Comparator } = { - 'application/octet-string': compareBuffersOrStrings, - 'image/png': compareImages.bind(null, 'image/png'), - 'image/jpeg': compareImages.bind(null, 'image/jpeg'), - 'text/plain': compareText, -}; + +export function getComparator(mimeType: string): Comparator { + if (mimeType === 'image/png') + return compareImages.bind(null, 'image/png'); + if (mimeType === 'image/jpeg') + return compareImages.bind(null, 'image/jpeg'); + if (mimeType === 'text/plain') + return compareText; + return compareBuffersOrStrings; +} function compareBuffersOrStrings(actualBuffer: Buffer | string, expectedBuffer: Buffer): ComparatorResult { if (typeof actualBuffer === 'string') diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts index b0bf65a547..5fb1e38c43 100644 --- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts @@ -19,7 +19,7 @@ import type { Page as PageEx } from 'playwright-core/lib/client/page'; import type { Locator as LocatorEx } from 'playwright-core/lib/client/locator'; import type { Expect } from '../types'; import { currentTestInfo } from '../globals'; -import { mimeTypeToComparator, ImageComparatorOptions, Comparator } from 'playwright-core/lib/utils/comparators'; +import { getComparator, ImageComparatorOptions, Comparator } from 'playwright-core/lib/utils/comparators'; import type { PageScreenshotOptions } from 'playwright-core/types/types'; import { addSuffixToFilePath, serializeError, sanitizeForFilePath, @@ -74,6 +74,7 @@ class SnapshotHelper { readonly kind: 'Screenshot'|'Snapshot'; readonly updateSnapshots: UpdateSnapshots; readonly comparatorOptions: ImageComparatorOptions; + readonly comparator: Comparator; readonly allOptions: T; constructor( @@ -130,9 +131,7 @@ class SnapshotHelper { 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 ' + this.mimeType + ': ' + this.snapshotPath); + this.comparator = getComparator(this.mimeType); this.testInfo = testInfo; this.allOptions = options; @@ -248,14 +247,11 @@ export function toMatchSnapshot( testInfo, testInfo.snapshotPath.bind(testInfo), determineFileExtension(received), testInfo.project.expect?.toMatchSnapshot || {}, nameOrOptions, optOptions); - const comparator: Comparator = mimeTypeToComparator[helper.mimeType]; - if (!comparator) - throw new Error('Failed to find comparator with type ' + helper.mimeType + ': ' + helper.snapshotPath); if (this.isNot) { if (!fs.existsSync(helper.snapshotPath)) return helper.handleMissingNegated(); - const isDifferent = !!comparator(received, fs.readFileSync(helper.snapshotPath), helper.comparatorOptions); + const isDifferent = !!helper.comparator(received, fs.readFileSync(helper.snapshotPath), helper.comparatorOptions); return isDifferent ? helper.handleDifferentNegated() : helper.handleMatchingNegated(); } @@ -263,7 +259,7 @@ export function toMatchSnapshot( return helper.handleMissing(received); const expected = fs.readFileSync(helper.snapshotPath); - const result = comparator(received, expected, helper.comparatorOptions); + const result = helper.comparator(received, expected, helper.comparatorOptions); if (!result) return helper.handleMatching(); diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 52117bdf39..33a98144b3 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -44,6 +44,22 @@ test('should support golden', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); }); +test('should work with non-txt extensions', async ({ runInlineTest }) => { + const result = await runInlineTest({ + ...files, + 'a.spec.js-snapshots/snapshot.csv': `1,2,3`, + 'a.spec.js': ` + const { test } = require('./helper'); + test('is a test', ({}) => { + expect('1,2,4').toMatchSnapshot('snapshot.csv'); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain(`1,2,34`); +}); + + test('should generate default name', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...files, diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 2043210d61..e82a608836 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -14,14 +14,14 @@ * limitations under the License. */ -import { mimeTypeToComparator } from 'playwright-core/lib/utils/comparators'; +import { getComparator } from 'playwright-core/lib/utils/comparators'; import * as fs from 'fs'; import { PNG } from 'pngjs'; import * as path from 'path'; import { pathToFileURL } from 'url'; import { test, expect, stripAnsi, createImage, paintBlackPixels } from './playwright-test-fixtures'; -const pngComparator = mimeTypeToComparator['image/png']; +const pngComparator = getComparator('image/png'); test.describe.configure({ mode: 'parallel' });