Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix errors creating snapshot and stories files due to long test names #175

Merged
merged 10 commits into from
Jul 9, 2024
7 changes: 7 additions & 0 deletions .changeset/lemon-otters-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@chromatic-com/playwright': patch
'@chromatic-com/cypress': patch
'@chromatic-com/shared-e2e': patch
---

Fix ENAMETOOLONG errors by truncating snapshot and stories file names to ensure they are not too long to be written to the file system
2 changes: 2 additions & 0 deletions .codacy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ engines:
- 'packages/playwright/tests/**/*.spec.*'
- 'packages/cypress/tests/cypress/e2e/*.cy.*'
- 'packages/cypress/tests/cypress/e2e/**/*.cy.*'
- 'packages/**/*.test.*'

9 changes: 9 additions & 0 deletions packages/cypress/tests/cypress/e2e/long-test-names.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
describe('this is a very long story name it just keeps going and going and it cannot stop and it will not stop ba bada da da dum dum dum', () => {
it('and this is also an incredibly long test name because there are just a bunch of random chars at the end like this ldlk elke lekj felk felkf lkf lsf lkef lse flskef ls fls eflsj flksef', () => {
cy.visit('/');
});

it('and this is also an incredibly long test name because there are just a bunch of random chars at the end like this ldlk elke lekj felk felkf lkf lsf lkef lse flskef ls fls eflsj flksef 2', () => {
cy.visit('/');
});
});
15 changes: 15 additions & 0 deletions packages/playwright/tests/long-test-names.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { test, expect } from '../src';

test.describe('this is a very long story name it just keeps going and going and it cannot stop and it will not stop ba bada da da dum dum dum', () => {
test('and this is also an incredibly long test name because there are just a bunch of random chars at the end like this ldlk elke lekj felk felkf lkf lsf lkef lse flskef ls fls eflsj flksef', async ({
page,
}) => {
await page.goto('/');
});

test('and this is also an incredibly long test name because there are just a bunch of random chars at the end like this ldlk elke lekj felk felkf lkf lsf lkef lse flskef ls fls eflsj flksef 2', async ({
page,
}) => {
await page.goto('/');
});
});
71 changes: 71 additions & 0 deletions packages/shared/src/utils/filePaths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
readJSONFile,
outputFile,
outputJSONFile,
truncateFileName,
} from './filePaths';

jest.mock('fs');
Expand Down Expand Up @@ -117,3 +118,73 @@ describe('readJSONFile', () => {
expect(json).toEqual({ filePath: '/some/path' });
});
});

describe('truncateFileName', () => {
it('does nothing if file name is within valid length', () => {
const filePath = 'this/is/a/valid.file.length';

const truncated = truncateFileName(filePath);

expect(truncated).toEqual(filePath);
expect(truncated.split('/').at(-1)).toEqual('valid.file.length');
});

it('ignores length of path parts before the file name', () => {
const filePath =
'/a/bunch/of/paths/that/donot/affect-size/this-title-has-260-chars-exactly-i-know-because-i-counted-and-that-is-too-big-for-a-file-system-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-b-ok-this-right-here-this-is-the-end.js';
expect(filePath.length).toBeGreaterThan(255);

const truncated = truncateFileName(filePath);

expect(truncated.split('/').at(-1).length).toEqual(255);
expect(truncated).toMatch(
new RegExp(
'^/a/bunch/of/paths/that/donot/affect-size/this-title-.*ok-this-right-here-this-i[a-z0-9]{4}.js$'
)
);
});

it('truncates long file names without changing extension', () => {
const fileName =
'this-title-has-260-chars-exactly-i-know-because-i-counted-and-that-is-too-big-for-a-file-system-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-b-ok-this-right-here-this-is-the-end.js';
expect(fileName.length).toBeGreaterThan(255);

const truncated = truncateFileName(fileName);

expect(truncated.length).toEqual(255);
expect(truncated).toMatch(new RegExp('^this-title-.*ok-this-right-here-this-i[a-z0-9]{4}.js$'));
});

it('truncates long file names without changing multiple extensions', () => {
const fileName =
'this-title-has-260-chars-exactly-i-know-because-i-counted-and-that-is-too-big-for-a-file-system-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-b-ok-this-right-here-this-is-the-end.one.js';
expect(fileName.length).toBeGreaterThan(255);

const truncated = truncateFileName(fileName);

expect(truncated.length).toEqual(255);
expect(truncated).toMatch(new RegExp('^this-title-.*ok-this-right-here-th[a-z0-9]{4}.one.js$'));
});

it('truncates long names without an extension', () => {
const fileName =
'this-title-has-260-chars-exactly-i-know-because-i-counted-and-that-is-too-big-for-a-file-system-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-b-ok-this-right-here-this-is-the-end';
expect(fileName.length).toBeGreaterThan(255);

const truncated = truncateFileName(fileName);

expect(truncated.length).toEqual(255);
expect(truncated).toMatch(new RegExp('^this-title-.*ok-this-right-here-this-is-t[a-z0-9]{4}$'));
});

it('truncates long names to given size', () => {
const fileName =
'this-title-has-260-chars-exactly-i-know-because-i-counted-and-that-is-too-big-for-a-file-system-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-b-ok-this-right-here-this-is-the-end';
expect(fileName.length).toBeGreaterThan(255);

const truncated = truncateFileName(fileName, 100);

expect(truncated.length).toEqual(100);
expect(truncated).toMatch(new RegExp('^this-title-.*-a-file-system-[a-z0-9]{4}$'));
});
});
36 changes: 36 additions & 0 deletions packages/shared/src/utils/filePaths.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync, mkdirSync } from 'fs';
import { readFile, writeFile } from 'fs/promises';
import { createHash } from 'node:crypto';
import path from 'path';

function rootDir() {
Expand Down Expand Up @@ -48,3 +49,38 @@ export async function readJSONFile(filePath: string) {
const data = await readFile(filePath);
return JSON.parse(data.toString());
}

// Generates a fixed length hash for the given `data`
function hash(data: string) {
// `outputLength` of 2 bytes is 4 chars
return createHash('shake256', { outputLength: 2 }).update(data).digest('hex');
}

// 255 is a good upper bound on file name size to work on most platforms
export const MAX_FILE_NAME_LENGTH = 255;

// Ensures that the file name part on the given `filePath` is not longer
// than the given `maxLength`.
// If truncation is necessary, a hash is added to avoid collisions on the
// file system in cases where names match up until a differentiating part
// at the end that is truncated.
export function truncateFileName(filePath: string, maxLength: number = MAX_FILE_NAME_LENGTH) {
const filePathParts = filePath.split('/');
const fileName = filePathParts.pop();
if (fileName.length <= maxLength) {
return filePath;
}

const hashedFileName = hash(fileName);
const [baseName, ...extensions] = fileName.split('.');
const ext = extensions.join('.');
const extLength = ext.length === 0 ? 0 : ext.length + 1; // +1 for leading `.` if needed

const lengthHashAndExt = hashedFileName.length + extLength;
const truncatedBaseName = baseName.slice(0, maxLength - lengthHashAndExt);
const truncatedFileName = [`${truncatedBaseName}${hashedFileName}`, ext]
.filter(Boolean)
.join('.');

return [...filePathParts, truncatedFileName].join('/');
}
7 changes: 6 additions & 1 deletion packages/shared/src/write-archive/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { NodeType } from 'rrweb-snapshot';
import * as filePaths from '../utils/filePaths';
import { writeTestResult } from '.';

jest.mock('../utils/filePaths');
jest.mock('../utils/filePaths', () => ({
...jest.requireActual('../utils/filePaths'),
ensureDir: jest.fn(),
outputFile: jest.fn(),
outputJSONFile: jest.fn(),
}));

const snapshotJson = {
childNodes: [
Expand Down
22 changes: 22 additions & 0 deletions packages/shared/src/write-archive/snapshot-files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,28 @@ beforeEach(() => {
jest.resetAllMocks();
});

describe('snapshotId', () => {
it('sanitizes the snapshot ID', () => {
const snapshotId = snapshotFiles.snapshotId(
'a title *() with $%& chars',
'a snapshot name *() with $%& chars'
);

expect(snapshotId).toEqual('a-title-with-chars-a-snapshot-name-with-chars');
});

it('truncates long snashot IDs', () => {
const title =
'this title has 260 chars exactly i know because i counted and that is too big for a file system blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah ok this right here this is the end';
const snapshotName = 'and this is a snapshot name these should not be too long usually';

const snapshotId = snapshotFiles.snapshotId(title, snapshotName);

expect(snapshotId.length).toEqual(230);
expect(snapshotId).toMatch(new RegExp('^this-title-has-.*blah-blah-[a-z0-9]{4}$'));
});
});

describe('snapshotFileName', () => {
it('generates a filename with the id and viewport', () => {
const fileName = snapshotFiles.snapshotFileName('some-id', { width: 500, height: 720 });
Expand Down
7 changes: 6 additions & 1 deletion packages/shared/src/write-archive/snapshot-files.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { readdir } from 'fs/promises';
import { Viewport, parseViewport, viewportToString } from '../utils/viewport';
import { sanitize } from './storybook-sanitize';
import { MAX_FILE_NAME_LENGTH, truncateFileName } from '../utils/filePaths';

const SNAPSHOT_FILE_EXT = 'snapshot.json';

export function snapshotId(testTitle: string, snapshotName: string) {
return `${sanitize(testTitle)}-${sanitize(snapshotName)}`;
const fullSnapshotId = `${sanitize(testTitle)}-${sanitize(snapshotName)}`;
// Leave room for the viewport and extension that will be added when using this
// to create a full file path
const maxLength = MAX_FILE_NAME_LENGTH - 25;
return truncateFileName(fullSnapshotId, maxLength);
}

// NOTE: This is duplicated in the shared storybook preview.ts
Expand Down
17 changes: 17 additions & 0 deletions packages/shared/src/write-archive/stories-files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,23 @@ beforeEach(() => {
jest.resetAllMocks();
});

describe('storiesFileName', () => {
it('sanitizes the file name', () => {
const fileName = storiesFiles.storiesFileName('a title *() with $%& chars');
expect(fileName).toEqual('a-title-with-chars.stories.json');
});

it('truncates long file names', () => {
const title =
'this title has 260 chars exactly i know because i counted and that is too big for a file system blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah ok this right here this is the end';
expect(title.length).toBeGreaterThan(255);

const fileName = storiesFiles.storiesFileName(title);
expect(fileName.length).toEqual(230);
expect(fileName).toMatch(new RegExp('^this-title-has-.*blah-bl[a-z0-9]{4}.stories.json$'));
});
});

describe('createStories', () => {
it('creates stories file JSON from DOM snapshots', () => {
const title = 'some test title';
Expand Down
9 changes: 6 additions & 3 deletions packages/shared/src/write-archive/stories-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import { ChromaticStorybookParameters } from '../types';
import { snapshotId } from './snapshot-files';
import { sanitize } from './storybook-sanitize';
import { Viewport, viewportToString } from '../utils/viewport';
import { MAX_FILE_NAME_LENGTH, truncateFileName } from '../utils/filePaths';

const STORIES_FILE_EXT = 'stories.json';

// Generates a file-system-safe file name from a story title
export function storiesFileName(testTitle: string) {
const fileNameParts = [sanitize(testTitle), STORIES_FILE_EXT];

return fileNameParts.join('.');
const fileName = [sanitize(testTitle), STORIES_FILE_EXT].join('.');
// Leave room for built storybook extensions that may be added (like `-stories.iframe.bundle.js`)
const maxLength = MAX_FILE_NAME_LENGTH - 25;
return truncateFileName(fileName, maxLength);
}

// Converts the DOM snapshots into a JSON stories file.
Expand Down
2 changes: 1 addition & 1 deletion test-server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ app.use(express.static(path.join(__dirname, 'fixtures/assets')));

// Pages
app.get('/', (req, res) => {
res.send(`${htmlIntro}<body>Testing</body>${htmlOutro}`);
res.send(`${htmlIntro}<body>Testing testing just a basic page</body>${htmlOutro}`);
});

// Asset path pages
Expand Down
Loading