From 7f6f17d1c343f88400c498fd1c3defbd7f420056 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Sun, 10 Mar 2024 11:11:35 -0700 Subject: [PATCH] chore: simplify toMatchSnapshot names calculation (#29862) Reference https://github.com/microsoft/playwright/issues/29719 --- .../src/matchers/toMatchSnapshot.ts | 38 +++++++++---------- packages/playwright/src/util.ts | 14 ++++--- tests/playwright-test/golden.spec.ts | 4 +- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 449c7349f0..317a0144d9 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -22,7 +22,8 @@ import { getComparator, sanitizeForFilePath, zones } from 'playwright-core/lib/u import { addSuffixToFilePath, trimLongString, callLogText, - expectTypes } from '../util'; + expectTypes, + sanitizeFilePathBeforeExtension } from '../util'; import { colors } from 'playwright-core/lib/utilsBundle'; import fs from 'fs'; import path from 'path'; @@ -101,9 +102,9 @@ class SnapshotHelper { name = nameOrOptions; this.options = { ...optOptions }; } else { - name = nameOrOptions.name; - this.options = { ...nameOrOptions }; - delete (this.options as any).name; + const { name: nameFromOptions, ...options } = nameOrOptions; + this.options = options; + name = nameFromOptions; } let snapshotNames = (testInfo as any)[snapshotNamesSymbol] as SnapshotNames; @@ -121,25 +122,34 @@ class SnapshotHelper { // // noop // expect.toMatchSnapshot('a.png') - let actualModifier = ''; + let inputPathSegments: string[]; if (!name) { const fullTitleWithoutSpec = [ ...testInfo.titlePath.slice(1), ++snapshotNames.anonymousSnapshotIndex, ].join(' '); name = sanitizeForFilePath(trimLongString(fullTitleWithoutSpec)) + '.' + anonymousSnapshotExtension; + inputPathSegments = [name]; this.snapshotName = name; } else { + // We intentionally do not sanitize user-provided array of segments, but for backwards + // compatibility we do sanitize the name if it is a single string. + // See https://github.com/microsoft/playwright/pull/9156 + inputPathSegments = Array.isArray(name) ? name : [sanitizeFilePathBeforeExtension(name)]; const joinedName = Array.isArray(name) ? name.join(path.sep) : name; snapshotNames.namedSnapshotIndex[joinedName] = (snapshotNames.namedSnapshotIndex[joinedName] || 0) + 1; const index = snapshotNames.namedSnapshotIndex[joinedName]; - if (index > 1) { - actualModifier = `-${index - 1}`; + if (index > 1) this.snapshotName = `${joinedName}-${index - 1}`; - } else { + else this.snapshotName = joinedName; - } } + this.snapshotPath = testInfo.snapshotPath(...inputPathSegments); + this.legacyExpectedPath = addSuffixToFilePath(testInfo._getOutputPath(...inputPathSegments), '-expected'); + const outputFile = testInfo._getOutputPath(sanitizeFilePathBeforeExtension(this.snapshotName)); + this.previousPath = addSuffixToFilePath(outputFile, '-previous'); + this.actualPath = addSuffixToFilePath(outputFile, '-actual'); + this.diffPath = addSuffixToFilePath(outputFile, '-diff'); const filteredConfigOptions = { ...configOptions }; for (const prop of NonConfigProperties) @@ -161,16 +171,6 @@ class SnapshotHelper { if (this.options.maxDiffPixelRatio !== undefined && (this.options.maxDiffPixelRatio < 0 || this.options.maxDiffPixelRatio > 1)) throw new Error('`maxDiffPixelRatio` option value must be between 0 and 1'); - // sanitizes path if string - const inputPathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, '', undefined, true)]; - const outputPathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, actualModifier, undefined, true)]; - this.snapshotPath = testInfo.snapshotPath(...inputPathSegments); - const inputFile = testInfo._getOutputPath(...inputPathSegments); - const outputFile = testInfo._getOutputPath(...outputPathSegments); - this.legacyExpectedPath = addSuffixToFilePath(inputFile, '-expected'); - this.previousPath = addSuffixToFilePath(outputFile, '-previous'); - this.actualPath = addSuffixToFilePath(outputFile, '-actual'); - this.diffPath = addSuffixToFilePath(outputFile, '-diff'); this.matcherName = matcherName; this.locator = locator; diff --git a/packages/playwright/src/util.ts b/packages/playwright/src/util.ts index 505a59c6b6..d67649276a 100644 --- a/packages/playwright/src/util.ts +++ b/packages/playwright/src/util.ts @@ -195,12 +195,16 @@ export function trimLongString(s: string, length = 100) { return s.substring(0, start) + middle + s.slice(-end); } -export function addSuffixToFilePath(filePath: string, suffix: string, customExtension?: string, sanitize = false): string { - const dirname = path.dirname(filePath); +export function addSuffixToFilePath(filePath: string, suffix: string): string { const ext = path.extname(filePath); - const name = path.basename(filePath, ext); - const base = path.join(dirname, name); - return (sanitize ? sanitizeForFilePath(base) : base) + suffix + (customExtension || ext); + const base = filePath.substring(0, filePath.length - ext.length); + return base + suffix + ext; +} + +export function sanitizeFilePathBeforeExtension(filePath: string): string { + const ext = path.extname(filePath); + const base = filePath.substring(0, filePath.length - ext.length); + return sanitizeForFilePath(base) + ext; } /** diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 2cba3b0623..39efa3c0b8 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -907,12 +907,12 @@ test('should attach expected/actual/diff with snapshot path', async ({ runInline { name: 'test/path/snapshot-actual.png', contentType: 'image/png', - path: 'a-is-a-test/test/path/snapshot-actual.png' + path: 'a-is-a-test/test-path-snapshot-actual.png' }, { name: 'test/path/snapshot-diff.png', contentType: 'image/png', - path: 'a-is-a-test/test/path/snapshot-diff.png' + path: 'a-is-a-test/test-path-snapshot-diff.png' } ]); });