Skip to content

Commit

Permalink
cherry-pick(#24213): Revert "fix: do not collide with other tests whe…
Browse files Browse the repository at this point in the history
…n test names have special chars (#23414)"
  • Loading branch information
github-actions[bot] committed Jul 13, 2023
1 parent 74ec8c2 commit b4215e3
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 88 deletions.
13 changes: 1 addition & 12 deletions packages/playwright-test/src/matchers/toMatchSnapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
22 changes: 10 additions & 12 deletions packages/playwright-test/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/playwright-test/playwright.artifacts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion tests/playwright-test/reporter-attachment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
6 changes: 3 additions & 3 deletions tests/playwright-test/reporter-raw.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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!');
}
Expand Down Expand Up @@ -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']));
});
66 changes: 7 additions & 59 deletions tests/playwright-test/test-output-dir.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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 }) => {
Expand Down Expand Up @@ -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': `
Expand All @@ -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) => {
Expand Down

0 comments on commit b4215e3

Please sign in to comment.