From cf5900f14281fc80f7b578f93345833b1615d17f Mon Sep 17 00:00:00 2001 From: Playwright Service <89237858+playwrightmachine@users.noreply.github.com> Date: Fri, 14 Jul 2023 06:33:48 -0700 Subject: [PATCH] cherry-pick(#24213): Revert "fix: do not collide with other tests when test names have special chars (#23414)" (#24217) This PR cherry-picks the following commits: - 57cca1d96e34ab9242570abc2464302d80cde504 Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- .../src/matchers/toMatchSnapshot.ts | 13 +--- packages/playwright-test/src/util.ts | 22 +++---- .../playwright.artifacts.spec.ts | 2 +- .../reporter-attachment.spec.ts | 2 +- tests/playwright-test/reporter-raw.spec.ts | 6 +- tests/playwright-test/test-output-dir.spec.ts | 66 ++----------------- 6 files changed, 23 insertions(+), 88 deletions(-) diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts index 6c25e04c8e7e2..0cbe9b03c89a5 100644 --- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts @@ -22,7 +22,7 @@ import type { ImageComparatorOptions, Comparator } from 'playwright-core/lib/uti import { getComparator } from 'playwright-core/lib/utils'; import type { PageScreenshotOptions } from 'playwright-core/types/types'; import { - serializeError, sanitizeForFilePath, + addSuffixToFilePath, serializeError, sanitizeForFilePath, trimLongString, callLogText, expectTypes } from '../util'; import { colors } from 'playwright-core/lib/utilsBundle'; @@ -440,14 +440,3 @@ function determineFileExtension(file: string | Buffer): string { function compareMagicBytes(file: Buffer, magicBytes: number[]): boolean { return Buffer.compare(Buffer.from(magicBytes), file.slice(0, magicBytes.length)) === 0; } - -function addSuffixToFilePath(filePath: string, suffix: string, customExtension?: string, sanitize = false): string { - const dirname = path.dirname(filePath); - const ext = path.extname(filePath); - const name = path.basename(filePath, ext); - const base = path.join(dirname, name); - const sanitizeForSnapshotFilePath = (s: string) => { - return s.replace(/[\x00-\x2C\x2E-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F]+/g, '-'); - }; - return (sanitize ? sanitizeForSnapshotFilePath(base) : base) + suffix + (customExtension || ext); -} diff --git a/packages/playwright-test/src/util.ts b/packages/playwright-test/src/util.ts index 30f7158c80783..e67ca4719db89 100644 --- a/packages/playwright-test/src/util.ts +++ b/packages/playwright-test/src/util.ts @@ -195,18 +195,8 @@ export function expectTypes(receiver: any, types: string[], matcherName: string) } } -export function sanitizeForFilePath(input: string) { - let nonTrivialSubstitute = false; - let sanitized = input.replace(/[\x00-\x2C\x2E-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F\x2A\-\*]+/g, substring => { - if (substring !== ' ') - nonTrivialSubstitute = true; - return '-'; - }); - if (!nonTrivialSubstitute) - return sanitized; - // If we sanitized the beginning or end, remove it for cosmetic reasons. - sanitized = sanitized.replace(/^-/, '').replace(/-$/, ''); - return sanitized + '-' + calculateSha1(input).substring(0, 6); +export function sanitizeForFilePath(s: string) { + return s.replace(/[\x00-\x2C\x2E-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F]+/g, '-'); } export function trimLongString(s: string, length = 100) { @@ -219,6 +209,14 @@ 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); + 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); +} + /** * Returns absolute path contained within parent directory. */ diff --git a/tests/playwright-test/playwright.artifacts.spec.ts b/tests/playwright-test/playwright.artifacts.spec.ts index f952b35a837b2..62a2dcf0d2f9e 100644 --- a/tests/playwright-test/playwright.artifacts.spec.ts +++ b/tests/playwright-test/playwright.artifacts.spec.ts @@ -54,7 +54,7 @@ const testFiles = { await page.click('text=Click me'); }); - test('shared failing', async ({ }) => { + test('shared failing', async ({ }) => { await page.click('text=And me'); expect(1).toBe(2); }); diff --git a/tests/playwright-test/reporter-attachment.spec.ts b/tests/playwright-test/reporter-attachment.spec.ts index 37fffd33d67fb..d761c0a4da975 100644 --- a/tests/playwright-test/reporter-attachment.spec.ts +++ b/tests/playwright-test/reporter-attachment.spec.ts @@ -235,7 +235,7 @@ test(`testInfo.attach name should be sanitized`, async ({ runInlineTest }) => { expect(result.failed).toBe(1); expect(result.output).toContain('attachment #1: ../../../test (text/plain)'); - expect(result.output).toContain(`attachments${path.sep}test-8d909b-`); + expect(result.output).toContain(`attachments${path.sep}-test`); }); test(`testInfo.attach name can be empty string`, async ({ runInlineTest }) => { diff --git a/tests/playwright-test/reporter-raw.spec.ts b/tests/playwright-test/reporter-raw.spec.ts index 194fcfbc1caa1..9b38d96db746a 100644 --- a/tests/playwright-test/reporter-raw.spec.ts +++ b/tests/playwright-test/reporter-raw.spec.ts @@ -48,7 +48,7 @@ test('should use project name', async ({ runInlineTest }, testInfo) => { test('passes', async ({ page }, testInfo) => {}); `, }, { reporter: 'dot,' + kRawReporterPath }); - const json = JSON.parse(fs.readFileSync(testInfo.outputPath('output', 'report', 'project-name-656a9b.report'), 'utf-8')); + const json = JSON.parse(fs.readFileSync(testInfo.outputPath('output', 'report', 'project-name.report'), 'utf-8')); expect(json.project.name).toBe('project-name'); expect(result.exitCode).toBe(0); }); @@ -173,7 +173,7 @@ test(`testInfo.attach should save attachments via path`, async ({ runInlineTest expect(result.attachments[0].name).toBe('example.png'); expect(result.attachments[0].contentType).toBe('x-playwright/custom'); const p = result.attachments[0].path; - expect(p).toMatch(/[/\\]attachments[/\\]example-png-[0-9a-f]{6}-[0-9a-f]+\.json$/); + expect(p).toMatch(/[/\\]attachments[/\\]example-png-[0-9a-f]+\.json$/); const contents = fs.readFileSync(p); expect(contents.toString()).toBe('We <3 Playwright!'); } @@ -255,5 +255,5 @@ test('dupe project names', async ({ runInlineTest }, testInfo) => { `, }, { reporter: 'dot,' + kRawReporterPath }); const files = fs.readdirSync(testInfo.outputPath('test-results', 'report')); - expect(new Set(files)).toEqual(new Set(['project-name-656a9b.report', 'project-name-656a9b-1.report', 'project-name-656a9b-2.report'])); + expect(new Set(files)).toEqual(new Set(['project-name.report', 'project-name-1.report', 'project-name-2.report'])); }); diff --git a/tests/playwright-test/test-output-dir.spec.ts b/tests/playwright-test/test-output-dir.spec.ts index 203542559bd04..07a5f5be15923 100644 --- a/tests/playwright-test/test-output-dir.spec.ts +++ b/tests/playwright-test/test-output-dir.spec.ts @@ -194,10 +194,10 @@ test('should include the project name', async ({ runInlineTest }) => { expect(result.output).toContain('my-test.spec.js-snapshots/bar-foo.txt'); // test1, run with bar - expect(result.output).toContain('test-results/my-test-test-1-Bar-space-dc1461/bar.txt'); - expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space-dc1461.txt'); - expect(result.output).toContain('test-results/my-test-test-1-Bar-space-dc1461-retry1/bar.txt'); - expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space-dc1461.txt'); + expect(result.output).toContain('test-results/my-test-test-1-Bar-space-/bar.txt'); + expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space-.txt'); + expect(result.output).toContain('test-results/my-test-test-1-Bar-space--retry1/bar.txt'); + expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space-.txt'); // test2, run with empty expect(result.output).toContain('test-results/my-test-test-2/bar.txt'); @@ -212,8 +212,8 @@ test('should include the project name', async ({ runInlineTest }) => { expect(result.output).toContain('my-test.spec.js-snapshots/bar-foo-suffix.txt'); // test2, run with bar - expect(result.output).toContain('test-results/my-test-test-2-Bar-space-dc1461/bar.txt'); - expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space-dc1461-suffix.txt'); + expect(result.output).toContain('test-results/my-test-test-2-Bar-space-/bar.txt'); + expect(result.output).toContain('my-test.spec.js-snapshots/bar-Bar-space--suffix.txt'); }); test('should include path option in snapshot', async ({ runInlineTest }) => { @@ -401,58 +401,6 @@ test('should allow nonAscii characters in the output dir', async ({ runInlineTes expect(outputDir).toBe(path.join(testInfo.outputDir, 'test-results', 'my-test-こんにちは世界')); }); -test('should not collide with other tests when nonAscii characters are replaced', async ({ runInlineTest }) => { - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23386' }); - const result = await runInlineTest({ - 'my-test.spec.js': ` - import { test, expect } from '@playwright/test'; - const specialChars = ['>', '=', '<', '+', '#', '-', '.', '!', '$', '%', '&', '\\'', '*', '/', '?', '^', '_', '\`', '{', '|', '}', '~', '(', ')', '[', ']', '@']; - for (const char of specialChars) { - test('test' + char, async ({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); - }); - } - `, - }); - expect(result.exitCode).toBe(0); - const outputDirs = result.outputLines; - expect(outputDirs.length).toBe(27); - expect(new Set(outputDirs).size).toBe(outputDirs.length); - const forbiddenCharacters = ['\\', '/', ':', '*', '?', '"', '<', '>', '|', '%']; - for (const outputDir of outputDirs) { - const relativePath = path.relative(path.join(test.info().outputDir, 'test-results'), outputDir); - for (const forbiddenCharacter of forbiddenCharacters) - expect(relativePath).not.toContain(forbiddenCharacter); - } -}); - -test('should generate expected output dir names', async ({ runInlineTest }, testInfo) => { - const runTests = async (fileName: string, testNames: string[]) => { - const result = await runInlineTest({ - [fileName]: ` - import { test, expect } from '@playwright/test'; - ${testNames.map(name => `test('${name}', async ({}, testInfo) => console.log('\\n%%' + testInfo.outputDir));`).join('\n')} - `, - }); - expect(result.exitCode).toBe(0); - const outputDirs = result.outputLines.map(line => path.relative(path.join(testInfo.outputDir, 'test-results'), line)); - return outputDirs; - }; - expect(await runTests('filename.spec.js', [ - 'testing > foo', - 'testing multiple spaces', - 'testing multiple spaces', - '!!!hello!!!', - 'dashes-are-used', - ])).toEqual([ - 'filename-testing-foo-574286', - 'filename-testing-multiple-spaces', - 'filename-testing-multiple-spaces-f5d359', - 'filename-hello-8eb257', - 'filename-dashes-are-used-c67e31', - ]); -}); - test('should allow shorten long output dirs characters in the output dir', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'very/deep/and/long/file/name/that/i/want/to/be/trimmed/my-test.spec.js': ` @@ -478,7 +426,7 @@ test('should not mangle double dashes', async ({ runInlineTest }, testInfo) => { `, }); const outputDir = result.outputLines[0]; - expect(outputDir).toBe(path.join(testInfo.outputDir, 'test-results', 'my--file-my-test-68b7dd')); + expect(outputDir).toBe(path.join(testInfo.outputDir, 'test-results', 'my--file-my--test')); }); test('should allow include the describe name the output dir', async ({ runInlineTest }, testInfo) => {