add tests for long multibyte test/story names (#219)

* add tests for long multibyte test/story names * Fix to truncate filename based on byte size * add changeset --------- Co-authored-by: Shohei Nakayama <[email protected]>
chromaui · Dec 11, 2024 · 57ef80c · 57ef80c
1 parent d93a095
commit 57ef80c
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 30 deletions.
diff --git a/.changeset/tall-kings-happen.md b/.changeset/tall-kings-happen.md
@@ -0,0 +1,7 @@
+---
+'@chromatic-com/playwright': patch
+'@chromatic-com/cypress': patch
+'@chromatic-com/shared-e2e': patch
+---
+
+Fix to truncate filename based on byte size
diff --git a/packages/cypress/tests/cypress/e2e/long-test-names.cy.ts b/packages/cypress/tests/cypress/e2e/long-test-names.cy.ts
@@ -6,4 +6,8 @@ describe('this is a very long story name it just keeps going and going and it ca
   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('/');
   });
+
+  it('multi-byte characters test case: ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ', () => {
+    cy.visit('/');
+  });
 });
diff --git a/packages/playwright/tests/long-test-names.spec.ts b/packages/playwright/tests/long-test-names.spec.ts
@@ -1,4 +1,4 @@
-import { test, expect } from '../src';
+import { test } 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 ({
@@ -12,4 +12,10 @@ test.describe('this is a very long story name it just keeps going and going and
   }) => {
     await page.goto('/');
   });
+
+  test('multi-byte characters test case: ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ、ああだこうだ', async ({
+    page,
+  }) => {
+    await page.goto('/');
+  });
 });
diff --git a/packages/shared/src/utils/filePaths.test.ts b/packages/shared/src/utils/filePaths.test.ts
@@ -4,9 +4,9 @@ import {
   archivesDir,
   assetsDir,
   ensureDir,
-  readJSONFile,
   outputFile,
   outputJSONFile,
+  readJSONFile,
   truncateFileName,
 } from './filePaths';
 
@@ -130,13 +130,15 @@ describe('truncateFileName', () => {
   });
 
   it('ignores length of path parts before the file name', () => {
+    const encoder = new TextEncoder();
     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 filePathLength = encoder.encode(filePath).byteLength;
+    expect(filePathLength).toBeGreaterThan(255);
 
     const truncated = truncateFileName(filePath);
 
-    expect(truncated.split('/').at(-1).length).toEqual(255);
+    expect(encoder.encode(truncated.split('/').at(-1)).byteLength).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$'
@@ -145,46 +147,74 @@ describe('truncateFileName', () => {
   });
 
   it('truncates long file names without changing extension', () => {
+    const encoder = new TextEncoder();
     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);
+      'this-title-has-260-bytes-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';
+    const fileNameLength = encoder.encode(fileName).byteLength;
+    expect(fileNameLength).toBeGreaterThan(255);
 
     const truncated = truncateFileName(fileName);
+    const truncatedLength = encoder.encode(truncated).byteLength;
 
-    expect(truncated.length).toEqual(255);
+    expect(truncatedLength).toEqual(255);
     expect(truncated).toMatch(new RegExp('^this-title-.*ok-this-right-here-this-i[a-z0-9]{4}.js$'));
   });
 
+  it('correctly truncates file names with multi-byte characters', () => {
+    const encoder = new TextEncoder();
+    const fileName =
+      'このタイトルは260byteあります-私が数えたので間違いないです-そしてそれはファイルシステムにとっては大きすぎます-ああだこうだ-ああだこうだ-ああだこうだ-ああだこうだ-これで終わりです.js';
+    const fileNameLength = encoder.encode(fileName).byteLength;
+    expect(fileNameLength).toBeGreaterThan(255);
+
+    const truncated = truncateFileName(fileName);
+    const truncatedLength = encoder.encode(truncated).byteLength;
+
+    expect(truncatedLength).toEqual(255);
+    expect(truncated).toMatch(
+      new RegExp('^このタイトルは260byteあります-.*-ああだこうだ-これで終[a-z0-9]{4}.js$')
+    );
+  });
+
   it('truncates long file names without changing multiple extensions', () => {
+    const encoder = new TextEncoder();
     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);
+      'this-title-has-260-bytes-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';
+    const fileNameLength = encoder.encode(fileName).byteLength;
+    expect(fileNameLength).toBeGreaterThan(255);
 
     const truncated = truncateFileName(fileName);
+    const truncatedLength = encoder.encode(truncated).byteLength;
 
-    expect(truncated.length).toEqual(255);
+    expect(truncatedLength).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 encoder = new TextEncoder();
     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);
+      'this-title-has-260-bytes-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';
+    const fileNameLength = encoder.encode(fileName).byteLength;
+    expect(fileNameLength).toBeGreaterThan(255);
 
     const truncated = truncateFileName(fileName);
+    const truncatedLength = encoder.encode(truncated).byteLength;
 
-    expect(truncated.length).toEqual(255);
+    expect(truncatedLength).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 encoder = new TextEncoder();
     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);
+      'this-title-has-260-bytes-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';
+    const fileNameLength = encoder.encode(fileName).byteLength;
+    expect(fileNameLength).toBeGreaterThan(255);
 
     const truncated = truncateFileName(fileName, 100);
+    const truncatedLength = encoder.encode(truncated).byteLength;
 
-    expect(truncated.length).toEqual(100);
+    expect(truncatedLength).toEqual(100);
     expect(truncated).toMatch(new RegExp('^this-title-.*-a-file-system-[a-z0-9]{4}$'));
   });
 });
diff --git a/packages/shared/src/utils/filePaths.ts b/packages/shared/src/utils/filePaths.ts
@@ -56,28 +56,38 @@ function hash(data: string) {
   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;
+// 255 bytes is a good upper bound on file name size to work on most platforms
+export const MAX_FILE_NAME_BYTE_LENGTH = 255;
 
 // Ensures that the file name part on the given `filePath` is not longer
-// than the given `maxLength`.
+// than the given `maxByteLength`.
 // 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) {
+export function truncateFileName(
+  filePath: string,
+  maxByteLength: number = MAX_FILE_NAME_BYTE_LENGTH
+) {
+  const encoder = new TextEncoder();
+  const decoder = new TextDecoder();
+
   const filePathParts = filePath.split('/');
   const fileName = filePathParts.pop();
-  if (fileName.length <= maxLength) {
+
+  const fileNameByteArray = encoder.encode(fileName);
+  if (fileNameByteArray.byteLength <= maxByteLength) {
     return filePath;
   }
 
   const hashedFileName = hash(fileName);
   const [baseName, ...extensions] = fileName.split('.');
+  const baseNameByteArray = encoder.encode(baseName);
   const ext = extensions.join('.');
-  const extLength = ext.length === 0 ? 0 : ext.length + 1; // +1 for leading `.` if needed
+  const extLength = ext.length === 0 ? 0 : encoder.encode(ext).byteLength + 1; // +1 for leading `.` if needed
 
-  const lengthHashAndExt = hashedFileName.length + extLength;
-  const truncatedBaseName = baseName.slice(0, maxLength - lengthHashAndExt);
+  const lengthHashAndExt = encoder.encode(hashedFileName).byteLength + extLength;
+  const truncatedBaseNameByteArray = baseNameByteArray.slice(0, maxByteLength - lengthHashAndExt);
+  const truncatedBaseName = decoder.decode(truncatedBaseNameByteArray);
   const truncatedFileName = [`${truncatedBaseName}${hashedFileName}`, ext]
     .filter(Boolean)
     .join('.');

diff --git a/packages/shared/src/write-archive/snapshot-files.ts b/packages/shared/src/write-archive/snapshot-files.ts
@@ -1,16 +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';
+import { MAX_FILE_NAME_BYTE_LENGTH, truncateFileName } from '../utils/filePaths';
 
 const SNAPSHOT_FILE_EXT = 'snapshot.json';
 
 export function snapshotId(testTitle: string, snapshotName: string) {
   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);
+  const maxByteLength = MAX_FILE_NAME_BYTE_LENGTH - 25;
+  return truncateFileName(fullSnapshotId, maxByteLength);
 }
 
 // NOTE: This is duplicated in the shared storybook preview.ts

diff --git a/packages/shared/src/write-archive/stories-files.ts b/packages/shared/src/write-archive/stories-files.ts
@@ -3,16 +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';
+import { MAX_FILE_NAME_BYTE_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 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);
+  const maxByteLength = MAX_FILE_NAME_BYTE_LENGTH - 25;
+  return truncateFileName(fileName, maxByteLength);
 }
 
 // Converts the DOM snapshots into a JSON stories file.