Merge branch 'feature/test-syntax-in-csf' into tags-filter-exclude

Author: yannbf

code/addons/vitest/src/node/test-manager.test.ts

@@ -15,6 +15,7 @@ import path from 'pathe';
 import { STATUS_TYPE_ID_A11Y, STATUS_TYPE_ID_COMPONENT_TEST, storeOptions } from '../constants';
 import type { StoreEvent, StoreState } from '../types';
 import { TestManager, type TestManagerOptions } from './test-manager';
+import { DOUBLE_SPACES } from './vitest-manager';
 
 const setTestNamePattern = vi.hoisted(() => vi.fn());
 const vitest = vi.hoisted(() => ({
@@ -204,7 +205,7 @@ describe('TestManager', () => {
         triggeredBy: 'global',
       },
     });
-    expect(setTestNamePattern).toHaveBeenCalledWith(/^One$/);
+    expect(setTestNamePattern).toHaveBeenCalledWith(new RegExp(`^One$`));
     expect(vitest.runTestSpecifications).toHaveBeenCalledWith(tests.slice(0, 1), true);
   });
 
@@ -220,7 +221,7 @@ describe('TestManager', () => {
       },
     });
     // regex should be exact match of the story name
-    expect(setTestNamePattern).toHaveBeenCalledWith(/^One$/);
+    expect(setTestNamePattern).toHaveBeenCalledWith(new RegExp(`^One$`));
   });
 
   it('should trigger a single story test', async () => {
@@ -235,7 +236,9 @@ describe('TestManager', () => {
       },
     });
     // regex should be Parent Story Name + Test Name
-    expect(setTestNamePattern).toHaveBeenCalledWith(/^Parentstory Test name$/);
+    expect(setTestNamePattern).toHaveBeenCalledWith(
+      new RegExp(`^Parent story${DOUBLE_SPACES} Test name$`)
+    );
   });
 
   it('should trigger all tests of a story', async () => {
@@ -249,8 +252,7 @@ describe('TestManager', () => {
         triggeredBy: 'global',
       },
     });
-    // regex should be parent story name with no spaces in between plus a space at the end
-    expect(setTestNamePattern).toHaveBeenCalledWith(/^Parentstory /);
+    expect(setTestNamePattern).toHaveBeenCalledWith(new RegExp(`^Parent story${DOUBLE_SPACES}`));
   });
 
   it('should restart Vitest before a test run if coverage is enabled', async () => {

code/addons/vitest/src/node/vitest-manager.ts

@@ -31,6 +31,17 @@ const packageDir = dirname(require.resolve('@storybook/addon-vitest/package.json
 // We have to tell Vitest that it runs as part of Storybook
 process.env.VITEST_STORYBOOK = 'true';
 
+/**
+ * The Storybook vitest plugin adds double space characters so that it's possible to do a regex for
+ * all test run use cases. Otherwise, if there were two unrelated stories like "Primary Button" and
+ * "Primary Button Mobile", once you run tests for "Primary Button" and its children it would also
+ * match "Primary Button Mobile". As it turns out, this limitation is also present in the Vitest
+ * VSCode extension and the issue would occur with normal vitest tests as well, but because we use
+ * double spaces, we circumvent the issue.
+ */
+export const DOUBLE_SPACES = '  ';
+const getTestName = (name: string) => `${name}${DOUBLE_SPACES}`;
+
 export class VitestManager {
   vitest: Vitest | null = null;
 
@@ -271,10 +282,6 @@ export class VitestManager {
       ? allStories.filter((story) => runPayload.storyIds?.includes(story.id))
       : allStories;
 
-    // When we run tests that involved a describe block (parent story + tests)
-    // we need to remove spaces from the parent story name, as the describe block does not have spaces
-    const getExportName = (name: string) => name.replace(/\s+/g, '');
-
     const isSingleStoryRun = runPayload.storyIds?.length === 1;
     if (isSingleStoryRun) {
       const selectedStory = filteredStories.find((story) => story.id === runPayload.storyIds?.[0]);
@@ -291,31 +298,20 @@ export class VitestManager {
       if (isParentStory) {
         // Use case 1: "Single" story run on a story with tests
         // -> run all tests of that story, as storyName is a describe block
-        /**
-         * The vitest transformation keeps the export name as is, e.g. "PrimaryButton", while the
-         * storybook sidebar changes the name to "Primary Button". That's why we need to remove
-         * spaces from the story name, to match the test name. If we were to also beautify the test
-         * name, doing a regex wouldn't be precise because there could be two describes, for
-         * instance: "Primary Button" and "Primary Button Mobile" and both would match. The fact
-         * that there are no spaces in the test name is what makes "PrimaryButton" and
-         * "PrimaryButtonMobile" work well in the regex. As it turns out, this limitation is also
-         * present in the Vitest VSCode extension and the issue would occur with normal vitest tests
-         * as well.
-         */
-        const parentName = getExportName(selectedStory.name);
-        regex = new RegExp(`^${parentName} `); // the extra space is intentional!
+        const parentName = getTestName(selectedStory.name);
+        regex = new RegExp(`^${parentName}`);
       } else if (hasParentStory) {
         // Use case 2: Single story run on a specific story test
-        // in this case the regex pattern should be the story parentName + story.name
+        // in this case the regex pattern should be the story parentName + space + story.name
         const parentStory = allStories.find((story) => story.id === selectedStory.parent);
         if (!parentStory) {
           throw new Error(`Parent story not found for story ${selectedStory.id}`);
         }
 
-        const parentName = getExportName(parentStory.name);
+        const parentName = getTestName(parentStory.name);
         regex = new RegExp(`^${parentName} ${storyName}$`);
       } else {
-        // Use case 3: Single story run on a story without tests
+        // Use case 3: Single story run on a story without tests, should be exact match of story name
         regex = new RegExp(`^${storyName}$`);
       }
       this.vitest!.setGlobalTestNamePattern(regex);

code/addons/vitest/src/vitest-plugin/test-utils.ts

@@ -36,6 +36,7 @@ export const testStory = (
   story: ComposedStoryFn | Story<Renderer>,
   meta: ComponentAnnotations | Meta<Renderer>,
   skipTags: string[],
+  storyId: string,
   testName?: string
 ) => {
   return async (context: TestContext & { story: ComposedStoryFn }) => {
@@ -66,12 +67,9 @@ export const testStory = (
       meta: TaskMeta & { storyId: string; reports: Report[] };
     };
 
-    if (testName) {
-      // TODO: [test-syntax] isn't this done by the csf plugin somehow?
-      _task.meta.storyId = toTestId(composedStory.id, testName);
-    } else {
-      _task.meta.storyId = composedStory.id;
-    }
+    // The id will always be present, calculated by CsfFile
+    // and is needed so that we can add the test to the story in Storybook's UI for the status
+    _task.meta.storyId = storyId;
 
     await setViewport(composedStory.parameters, composedStory.globals);