chore: trim file names longer than 60 chars (was 100) (#29725)

Test-specific output dir and snapshot names are trimmed to 60 chars
instead of just 100. The snapshot names are still trimmed at 100 chars
for backwards compatibility.


Reference https://github.com/microsoft/playwright/issues/29719
This commit is contained in:
Yury Semikhatsky 2024-03-18 12:53:15 -07:00 committed by GitHub
parent ef4438ee99
commit 35db70ea1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 20 additions and 17 deletions

View file

@ -23,7 +23,8 @@ import {
addSuffixToFilePath, addSuffixToFilePath,
trimLongString, callLogText, trimLongString, callLogText,
expectTypes, expectTypes,
sanitizeFilePathBeforeExtension } from '../util'; sanitizeFilePathBeforeExtension,
windowsFilesystemFriendlyLength } from '../util';
import { colors } from 'playwright-core/lib/utilsBundle'; import { colors } from 'playwright-core/lib/utilsBundle';
import fs from 'fs'; import fs from 'fs';
import path from 'path'; import path from 'path';
@ -74,7 +75,7 @@ const NonConfigProperties: (keyof ToHaveScreenshotOptions)[] = [
class SnapshotHelper { class SnapshotHelper {
readonly testInfo: TestInfoImpl; readonly testInfo: TestInfoImpl;
readonly snapshotName: string; readonly outputBaseName: string;
readonly legacyExpectedPath: string; readonly legacyExpectedPath: string;
readonly previousPath: string; readonly previousPath: string;
readonly snapshotPath: string; readonly snapshotPath: string;
@ -128,9 +129,9 @@ class SnapshotHelper {
...testInfo.titlePath.slice(1), ...testInfo.titlePath.slice(1),
++snapshotNames.anonymousSnapshotIndex, ++snapshotNames.anonymousSnapshotIndex,
].join(' '); ].join(' ');
name = sanitizeForFilePath(trimLongString(fullTitleWithoutSpec)) + '.' + anonymousSnapshotExtension; inputPathSegments = [sanitizeForFilePath(trimLongString(fullTitleWithoutSpec)) + '.' + anonymousSnapshotExtension];
inputPathSegments = [name]; // Trim the output file paths more aggresively to avoid hitting Windows filesystem limits.
this.snapshotName = name; this.outputBaseName = sanitizeForFilePath(trimLongString(fullTitleWithoutSpec, windowsFilesystemFriendlyLength)) + '.' + anonymousSnapshotExtension;
} else { } else {
// We intentionally do not sanitize user-provided array of segments, but for backwards // 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. // compatibility we do sanitize the name if it is a single string.
@ -140,13 +141,13 @@ class SnapshotHelper {
snapshotNames.namedSnapshotIndex[joinedName] = (snapshotNames.namedSnapshotIndex[joinedName] || 0) + 1; snapshotNames.namedSnapshotIndex[joinedName] = (snapshotNames.namedSnapshotIndex[joinedName] || 0) + 1;
const index = snapshotNames.namedSnapshotIndex[joinedName]; const index = snapshotNames.namedSnapshotIndex[joinedName];
if (index > 1) if (index > 1)
this.snapshotName = addSuffixToFilePath(joinedName, `-${index - 1}`); this.outputBaseName = addSuffixToFilePath(joinedName, `-${index - 1}`);
else else
this.snapshotName = joinedName; this.outputBaseName = joinedName;
} }
this.snapshotPath = testInfo.snapshotPath(...inputPathSegments); this.snapshotPath = testInfo.snapshotPath(...inputPathSegments);
this.legacyExpectedPath = addSuffixToFilePath(testInfo._getOutputPath(...inputPathSegments), '-expected'); const outputFile = testInfo._getOutputPath(sanitizeFilePathBeforeExtension(this.outputBaseName));
const outputFile = testInfo._getOutputPath(sanitizeFilePathBeforeExtension(this.snapshotName)); this.legacyExpectedPath = addSuffixToFilePath(outputFile, '-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');
@ -222,7 +223,7 @@ class SnapshotHelper {
if (isWriteMissingMode) { if (isWriteMissingMode) {
writeFileSync(this.snapshotPath, actual); writeFileSync(this.snapshotPath, actual);
writeFileSync(this.actualPath, actual); writeFileSync(this.actualPath, actual);
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-actual'), contentType: this.mimeType, path: this.actualPath }); this.testInfo.attachments.push({ name: addSuffixToFilePath(this.outputBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath });
} }
const message = `A snapshot doesn't exist at ${this.snapshotPath}${isWriteMissingMode ? ', writing actual.' : '.'}`; const message = `A snapshot doesn't exist at ${this.snapshotPath}${isWriteMissingMode ? ', writing actual.' : '.'}`;
if (this.updateSnapshots === 'all') { if (this.updateSnapshots === 'all') {
@ -256,22 +257,22 @@ class SnapshotHelper {
// Copy the expectation inside the `test-results/` folder for backwards compatibility, // Copy the expectation inside the `test-results/` folder for backwards compatibility,
// so that one can upload `test-results/` directory and have all the data inside. // so that one can upload `test-results/` directory and have all the data inside.
writeFileSync(this.legacyExpectedPath, expected); writeFileSync(this.legacyExpectedPath, expected);
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-expected'), contentType: this.mimeType, path: this.snapshotPath }); this.testInfo.attachments.push({ name: addSuffixToFilePath(this.outputBaseName, '-expected'), contentType: this.mimeType, path: this.snapshotPath });
output.push(`\nExpected: ${colors.yellow(this.snapshotPath)}`); output.push(`\nExpected: ${colors.yellow(this.snapshotPath)}`);
} }
if (previous !== undefined) { if (previous !== undefined) {
writeFileSync(this.previousPath, previous); writeFileSync(this.previousPath, previous);
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-previous'), contentType: this.mimeType, path: this.previousPath }); this.testInfo.attachments.push({ name: addSuffixToFilePath(this.outputBaseName, '-previous'), contentType: this.mimeType, path: this.previousPath });
output.push(`Previous: ${colors.yellow(this.previousPath)}`); output.push(`Previous: ${colors.yellow(this.previousPath)}`);
} }
if (actual !== undefined) { if (actual !== undefined) {
writeFileSync(this.actualPath, actual); writeFileSync(this.actualPath, actual);
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-actual'), contentType: this.mimeType, path: this.actualPath }); this.testInfo.attachments.push({ name: addSuffixToFilePath(this.outputBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath });
output.push(`Received: ${colors.yellow(this.actualPath)}`); output.push(`Received: ${colors.yellow(this.actualPath)}`);
} }
if (diff !== undefined) { if (diff !== undefined) {
writeFileSync(this.diffPath, diff); writeFileSync(this.diffPath, diff);
this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-diff'), contentType: this.mimeType, path: this.diffPath }); this.testInfo.attachments.push({ name: addSuffixToFilePath(this.outputBaseName, '-diff'), contentType: this.mimeType, path: this.diffPath });
output.push(` Diff: ${colors.yellow(this.diffPath)}`); output.push(` Diff: ${colors.yellow(this.diffPath)}`);
} }

View file

@ -185,6 +185,8 @@ export function expectTypes(receiver: any, types: string[], matcherName: string)
} }
} }
export const windowsFilesystemFriendlyLength = 60;
export function trimLongString(s: string, length = 100) { export function trimLongString(s: string, length = 100) {
if (s.length <= length) if (s.length <= length)
return s; return s;

View file

@ -24,7 +24,7 @@ import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutMana
import type { RunnableDescription } from './timeoutManager'; import type { RunnableDescription } from './timeoutManager';
import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config'; import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
import type { Location } from '../../types/testReporter'; import type { Location } from '../../types/testReporter';
import { debugTest, filteredStackTrace, formatLocation, getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString } from '../util'; import { debugTest, filteredStackTrace, formatLocation, getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString, windowsFilesystemFriendlyLength } from '../util';
import { TestTracing } from './testTracing'; import { TestTracing } from './testTracing';
import type { Attachment } from './testTracing'; import type { Attachment } from './testTracing';
import type { StackFrame } from '@protocol/channels'; import type { StackFrame } from '@protocol/channels';
@ -175,7 +175,7 @@ export class TestInfoImpl implements TestInfo {
const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-'); const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-');
const fullTitleWithoutSpec = this.titlePath.slice(1).join(' '); const fullTitleWithoutSpec = this.titlePath.slice(1).join(' ');
let testOutputDir = trimLongString(sanitizedRelativePath + '-' + sanitizeForFilePath(fullTitleWithoutSpec)); let testOutputDir = trimLongString(sanitizedRelativePath + '-' + sanitizeForFilePath(fullTitleWithoutSpec), windowsFilesystemFriendlyLength);
if (projectInternal.id) if (projectInternal.id)
testOutputDir += '-' + sanitizeForFilePath(projectInternal.id); testOutputDir += '-' + sanitizeForFilePath(projectInternal.id);
if (this.retry) if (this.retry)

View file

@ -413,7 +413,7 @@ test('should allow shorten long output dirs characters in the output dir', async
`, `,
}); });
const outputDir = result.outputLines[0]; const outputDir = result.outputLines[0];
expect(outputDir).toBe(path.join(testInfo.outputDir, 'test-results', 'very-deep-and-long-file-name-that-i-want-to-be-99202--keeps-going-and-going-and-we-should-shorten-it')); expect(outputDir).toBe(path.join(testInfo.outputDir, 'test-results', 'very-deep-and-long-file-na-99202-ng-and-we-should-shorten-it'));
}); });
test('should not mangle double dashes', async ({ runInlineTest }, testInfo) => { test('should not mangle double dashes', async ({ runInlineTest }, testInfo) => {