Merge pull request storybookjs#29656 from storybookjs/determine-total…

…-test-count

Core + Addon Test: Refactor test API and fix total test count

.circleci/config.yml

@@ -165,17 +165,15 @@ jobs:
           root: .
           paths:
             - code/node_modules
+            - code/addons
             - scripts/node_modules
             - code/bench
             - code/examples
-            - code/node_modules
-            - code/addons
             - code/frameworks
             - code/deprecated
             - code/lib
             - code/core
             - code/builders
-            - code/ui
             - code/renderers
             - code/presets
             - .verdaccio-cache
@@ -269,16 +267,30 @@ jobs:
     steps:
       - git-shallow-clone/checkout_advanced:
           clone_options: "--depth 1 --verbose"
-      - attach_workspace:
-          at: .
       - nx/set-shas:
           main-branch-name: "next"
           workflow-name: << pipeline.parameters.workflow >>
+      - run:
+          name: install in scripts
+          command: |
+            cd scripts
+            yarn install
+      - run:
+          name: install in code
+          command: |
+            cd code
+            yarn install
+      - run:
+          name: Compile
+          command: |
+            yarn task --task compile --start-from=compile --no-link --debug
       - run:
           name: Check
           command: |
-            yarn task --task compile --start-from=auto --no-link --debug
-            yarn task --task check --start-from=auto --no-link --debug
+            yarn task --task check --start-from=check --no-link --debug
+      - run:
+          name: Ensure no changes pending
+          command: |
             git diff --exit-code
       - report-workflow-on-failure
       - cancel-workflow-on-failure
@@ -807,9 +819,7 @@ workflows:
       - bench-packages:
           requires:
             - build
-      - check:
-          requires:
-            - build
+      - check
       - unit-tests:
           requires:
             - build
@@ -885,9 +895,7 @@ workflows:
       - bench-packages:
           requires:
             - build
-      - check:
-          requires:
-            - build
+      - check
       - unit-tests:
           requires:
             - build
@@ -964,9 +972,7 @@ workflows:
       - bench-packages:
           requires:
             - build
-      - check:
-          requires:
-            - build
+      - check
       - unit-tests:
           requires:
             - build

code/addons/test/src/components/ContextMenuItem.tsx

@@ -7,7 +7,7 @@ import React, {
   useState,
 } from 'react';
 
-import { Button, type ListItem } from 'storybook/internal/components';
+import { Button, ListItem } from 'storybook/internal/components';
 import { useStorybookApi } from 'storybook/internal/manager-api';
 import { useTheme } from 'storybook/internal/theming';
 import { type API_HashEntry, type Addon_TestProviderState } from 'storybook/internal/types';
@@ -23,8 +23,7 @@ export const ContextMenuItem: FC<{
   state: Addon_TestProviderState<{
     testResults: TestResult[];
   }>;
-  ListItem: typeof ListItem;
-}> = ({ context, state, ListItem }) => {
+}> = ({ context, state }) => {
   const api = useStorybookApi();
   const [isDisabled, setDisabled] = useState(false);
 

code/addons/test/src/manager.tsx

@@ -1,7 +1,6 @@
 import React, { useState } from 'react';
 
 import { AddonPanel, Button, Link as LinkComponent } from 'storybook/internal/components';
-import { TESTING_MODULE_RUN_ALL_REQUEST } from 'storybook/internal/core-events';
 import type { Combo } from 'storybook/internal/manager-api';
 import { Consumer, addons, types } from 'storybook/internal/manager-api';
 import { styled } from 'storybook/internal/theming';
@@ -83,15 +82,15 @@ addons.register(ADDON_ID, (api) => {
       watchable: true,
       name: 'Component tests',
 
-      sidebarContextMenu: ({ context, state }, { ListItem }) => {
+      sidebarContextMenu: ({ context, state }) => {
         if (context.type === 'docs') {
           return null;
         }
         if (context.type === 'story' && !context.tags.includes('test')) {
           return null;
         }
 
-        return <ContextMenuItem context={context} state={state} ListItem={ListItem} />;
+        return <ContextMenuItem context={context} state={state} />;
       },
 
       render: (state) => {
@@ -188,9 +187,7 @@ addons.register(ADDON_ID, (api) => {
               }}
               onRerun={() => {
                 setIsModalOpen(false);
-                api
-                  .getChannel()
-                  .emit(TESTING_MODULE_RUN_ALL_REQUEST, { providerId: TEST_PROVIDER_ID });
+                api.runTestProvider(TEST_PROVIDER_ID);
               }}
             />
           </>

code/addons/test/src/node/boot-test-runner.test.ts

@@ -5,7 +5,6 @@ import { Channel, type ChannelTransport } from '@storybook/core/channels';
 import {
   TESTING_MODULE_CANCEL_TEST_RUN_REQUEST,
   TESTING_MODULE_PROGRESS_REPORT,
-  TESTING_MODULE_RUN_ALL_REQUEST,
   TESTING_MODULE_RUN_REQUEST,
   TESTING_MODULE_WATCH_MODE_REQUEST,
 } from '@storybook/core/core-events';
@@ -103,13 +102,6 @@ describe('bootTestRunner', () => {
       type: TESTING_MODULE_RUN_REQUEST,
     });
 
-    mockChannel.emit(TESTING_MODULE_RUN_ALL_REQUEST, 'bar');
-    expect(child.send).toHaveBeenCalledWith({
-      args: ['bar'],
-      from: 'server',
-      type: TESTING_MODULE_RUN_ALL_REQUEST,
-    });
-
     mockChannel.emit(TESTING_MODULE_WATCH_MODE_REQUEST, 'baz');
     expect(child.send).toHaveBeenCalledWith({
       args: ['baz'],

code/addons/test/src/node/boot-test-runner.ts

@@ -4,7 +4,6 @@ import type { Channel } from 'storybook/internal/channels';
 import {
   TESTING_MODULE_CANCEL_TEST_RUN_REQUEST,
   TESTING_MODULE_CRASH_REPORT,
-  TESTING_MODULE_RUN_ALL_REQUEST,
   TESTING_MODULE_RUN_REQUEST,
   TESTING_MODULE_WATCH_MODE_REQUEST,
   type TestingModuleCrashReportPayload,
@@ -40,16 +39,13 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
 
   const forwardRun = (...args: any[]) =>
     child?.send({ args, from: 'server', type: TESTING_MODULE_RUN_REQUEST });
-  const forwardRunAll = (...args: any[]) =>
-    child?.send({ args, from: 'server', type: TESTING_MODULE_RUN_ALL_REQUEST });
   const forwardWatchMode = (...args: any[]) =>
     child?.send({ args, from: 'server', type: TESTING_MODULE_WATCH_MODE_REQUEST });
   const forwardCancel = (...args: any[]) =>
     child?.send({ args, from: 'server', type: TESTING_MODULE_CANCEL_TEST_RUN_REQUEST });
 
   const killChild = () => {
     channel.off(TESTING_MODULE_RUN_REQUEST, forwardRun);
-    channel.off(TESTING_MODULE_RUN_ALL_REQUEST, forwardRunAll);
     channel.off(TESTING_MODULE_WATCH_MODE_REQUEST, forwardWatchMode);
     channel.off(TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, forwardCancel);
     child?.kill();
@@ -88,7 +84,6 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
 
           // Forward all events from the channel to the child process
           channel.on(TESTING_MODULE_RUN_REQUEST, forwardRun);
-          channel.on(TESTING_MODULE_RUN_ALL_REQUEST, forwardRunAll);
           channel.on(TESTING_MODULE_WATCH_MODE_REQUEST, forwardWatchMode);
           channel.on(TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, forwardCancel);
 

code/addons/test/src/node/reporter.ts

@@ -64,7 +64,7 @@ export class StorybookReporter implements Reporter {
 
   sendReport: (payload: TestingModuleProgressReportPayload) => void;
 
-  constructor(private testManager: TestManager) {
+  constructor(public testManager: TestManager) {
     this.sendReport = throttle((payload) => this.testManager.sendProgressReport(payload), 1000);
   }
 
@@ -75,18 +75,19 @@ export class StorybookReporter implements Reporter {
 
   getProgressReport(finishedAt?: number) {
     const files = this.ctx.state.getFiles();
-    const fileTests = getTests(files);
-    // The number of total tests is dynamic and can change during the run
-    const numTotalTests = fileTests.length;
+    const fileTests = getTests(files).filter((t) => t.mode === 'run' || t.mode === 'only');
+
+    // The total number of tests reported by Vitest is dynamic and can change during the run, so we
+    // use `storyCountForCurrentRun` instead, based on the list of stories provided in the run request.
+    const numTotalTests = finishedAt
+      ? fileTests.length
+      : Math.max(fileTests.length, this.testManager.vitestManager.storyCountForCurrentRun);
 
     const numFailedTests = fileTests.filter((t) => t.result?.state === 'fail').length;
     const numPassedTests = fileTests.filter((t) => t.result?.state === 'pass').length;
-    const numPendingTests = fileTests.filter(
-      (t) => t.result?.state === 'run' || t.mode === 'skip' || t.result?.state === 'skip'
-    ).length;
-    const testResults: TestResult[] = [];
+    const numPendingTests = fileTests.filter((t) => t.result?.state === 'run').length;
 
-    for (const file of files) {
+    const testResults: TestResult[] = files.map((file) => {
       const tests = getTests([file]);
       let startTime = tests.reduce(
         (prev, next) => Math.min(prev, next.result?.startTime ?? Number.POSITIVE_INFINITY),
@@ -102,7 +103,7 @@ export class StorybookReporter implements Reporter {
         startTime
       );
 
-      const assertionResults = tests.flatMap<TestResultResult>((t) => {
+      const results = tests.flatMap<TestResultResult>((t) => {
         const ancestorTitles: string[] = [];
         let iter: Suite | undefined = t.suite;
         while (iter) {
@@ -129,15 +130,14 @@ export class StorybookReporter implements Reporter {
       });
 
       const hasFailedTests = tests.some((t) => t.result?.state === 'fail');
-
-      testResults.push({
-        results: assertionResults,
+      return {
+        results,
         startTime,
         endTime,
         status: file.result?.state === 'fail' || hasFailedTests ? 'failed' : 'passed',
         message: file.result?.errors?.[0]?.stack || file.result?.errors?.[0]?.message,
-      });
-    }
+      };
+    });
 
     return {
       cancellable: !finishedAt,

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

@@ -2,12 +2,14 @@ import { describe, expect, it, vi } from 'vitest';
 import { createVitest } from 'vitest/node';
 
 import { Channel, type ChannelTransport } from '@storybook/core/channels';
+import type { StoryIndex } from '@storybook/types';
 
 import path from 'pathe';
 
 import { TEST_PROVIDER_ID } from '../constants';
 import { TestManager } from './test-manager';
 
+const setTestNamePattern = vi.hoisted(() => vi.fn());
 const vitest = vi.hoisted(() => ({
   projects: [{}],
   init: vi.fn(),
@@ -18,7 +20,14 @@ const vitest = vi.hoisted(() => ({
   globTestSpecs: vi.fn(),
   getModuleProjects: vi.fn(() => []),
   configOverride: {
-    testNamePattern: undefined,
+    actualTestNamePattern: undefined,
+    get testNamePattern() {
+      return this.actualTestNamePattern;
+    },
+    set testNamePattern(value: string) {
+      setTestNamePattern(value);
+      this.actualTestNamePattern = value;
+    },
   },
 }));
 
@@ -40,6 +49,33 @@ const tests = [
   },
 ];
 
+global.fetch = vi.fn().mockResolvedValue({
+  json: () =>
+    new Promise((resolve) =>
+      resolve({
+        v: 5,
+        entries: {
+          'story--one': {
+            type: 'story',
+            id: 'story--one',
+            name: 'One',
+            title: 'story/one',
+            importPath: 'path/to/file',
+            tags: ['test'],
+          },
+          'another--one': {
+            type: 'story',
+            id: 'another--one',
+            name: 'One',
+            title: 'another/one',
+            importPath: 'path/to/another/file',
+            tags: ['test'],
+          },
+        },
+      } as StoryIndex)
+    ),
+});
+
 const options: ConstructorParameters<typeof TestManager>[1] = {
   onError: (message, error) => {
     throw error;
@@ -83,16 +119,7 @@ describe('TestManager', () => {
 
     await testManager.handleRunRequest({
       providerId: TEST_PROVIDER_ID,
-      payload: [
-        {
-          stories: [],
-          importPath: 'path/to/file',
-        },
-        {
-          stories: [],
-          importPath: 'path/to/another/file',
-        },
-      ],
+      indexUrl: 'http://localhost:6006/index.json',
     });
     expect(createVitest).toHaveBeenCalledTimes(1);
     expect(vitest.runFiles).toHaveBeenCalledWith(tests, true);
@@ -104,33 +131,18 @@ describe('TestManager', () => {
 
     await testManager.handleRunRequest({
       providerId: TEST_PROVIDER_ID,
-      payload: [
-        {
-          stories: [],
-          importPath: 'path/to/unknown/file',
-        },
-      ],
+      indexUrl: 'http://localhost:6006/index.json',
+      storyIds: [],
     });
     expect(vitest.runFiles).toHaveBeenCalledWith([], true);
+    expect(vitest.configOverride.testNamePattern).toBeUndefined();
 
     await testManager.handleRunRequest({
       providerId: TEST_PROVIDER_ID,
-      payload: [
-        {
-          stories: [],
-          importPath: 'path/to/file',
-        },
-      ],
+      indexUrl: 'http://localhost:6006/index.json',
+      storyIds: ['story--one'],
     });
+    expect(setTestNamePattern).toHaveBeenCalledWith(/^One$/);
     expect(vitest.runFiles).toHaveBeenCalledWith(tests.slice(0, 1), true);
   });
-
-  it('should handle run all request', async () => {
-    const testManager = await TestManager.start(mockChannel, options);
-    expect(createVitest).toHaveBeenCalledTimes(1);
-
-    await testManager.handleRunAllRequest({ providerId: TEST_PROVIDER_ID });
-    expect(createVitest).toHaveBeenCalledTimes(1);
-    expect(vitest.runFiles).toHaveBeenCalledWith(tests, true);
-  });
 });