Merge pull request #29829 from storybookjs/fix-run-during-boot-or-res…

…tart

Addon Test: Fix run request while booting or restarting Vitest

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

@@ -23,10 +23,13 @@ const MAX_START_TIME = 30000;
 // which is at the root. Then, from the root, we want to load `node/vitest.mjs`
 const vitestModulePath = join(__dirname, 'node', 'vitest.mjs');
 
+// Events that were triggered before Vitest was ready are queued up and resent once it's ready
+const eventQueue: { type: string; args: any[] }[] = [];
+
 let child: null | ChildProcess;
 let ready = false;
 
-const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: any[]) => {
+const bootTestRunner = async (channel: Channel) => {
   let stderr: string[] = [];
 
   function reportFatalError(e: any) {
@@ -58,6 +61,7 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
 
   const exit = (code = 0) => {
     killChild();
+    eventQueue.length = 0;
     process.exit(code);
   };
 
@@ -81,9 +85,10 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
 
       child.on('message', (result: any) => {
         if (result.type === 'ready') {
-          // Resend the event that triggered the boot sequence, now that the child is ready to handle it
-          if (initEvent && initArgs) {
-            child?.send({ type: initEvent, args: initArgs, from: 'server' });
+          // Resend events that triggered (during) the boot sequence, now that Vitest is ready
+          while (eventQueue.length) {
+            const { type, args } = eventQueue.shift();
+            child?.send({ type, args, from: 'server' });
           }
 
           // Forward all events from the channel to the child process
@@ -124,14 +129,18 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
 
   await Promise.race([startChildProcess(), timeout]).catch((e) => {
     reportFatalError(e);
+    eventQueue.length = 0;
     throw e;
   });
 };
 
 export const runTestRunner = async (channel: Channel, initEvent?: string, initArgs?: any[]) => {
+  if (!ready && initEvent) {
+    eventQueue.push({ type: initEvent, args: initArgs });
+  }
   if (!child) {
     ready = false;
-    await bootTestRunner(channel, initEvent, initArgs);
+    await bootTestRunner(channel);
     ready = true;
   }
 };
@@ -141,4 +150,6 @@ export const killTestRunner = () => {
     child.kill();
     child = null;
   }
+  ready = false;
+  eventQueue.length = 0;
 };

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

@@ -39,12 +39,6 @@ export class TestManager {
     this.vitestManager.startVitest().then(() => options.onReady?.());
   }
 
-  async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) {
-    await this.vitestManager.vitest?.runningPromise;
-    await this.vitestManager.closeVitest();
-    await this.vitestManager.startVitest({ watchMode, coverage });
-  }
-
   async handleConfigChange(
     payload: TestingModuleConfigChangePayload<{ coverage: boolean; a11y: boolean }>
   ) {
@@ -54,7 +48,10 @@ export class TestManager {
     if (this.coverage !== payload.config.coverage) {
       try {
         this.coverage = payload.config.coverage;
-        await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
+        await this.vitestManager.restartVitest({
+          watchMode: this.watchMode,
+          coverage: this.coverage,
+        });
       } catch (e) {
         const isV8 = e.message?.includes('@vitest/coverage-v8');
         const isIstanbul = e.message?.includes('@vitest/coverage-istanbul');
@@ -76,7 +73,7 @@ export class TestManager {
 
       if (this.watchMode !== payload.watchMode) {
         this.watchMode = payload.watchMode;
-        await this.restartVitest({ watchMode: this.watchMode, coverage: false });
+        await this.vitestManager.restartVitest({ watchMode: this.watchMode, coverage: false });
       }
     } catch (e) {
       this.reportFatalError('Failed to change watch mode', e);
@@ -88,32 +85,39 @@ export class TestManager {
       if (payload.providerId !== TEST_PROVIDER_ID) {
         return;
       }
+
+      const allTestsRun = (payload.storyIds ?? []).length === 0;
+
       if (payload.config && this.coverage !== payload.config.coverage) {
         this.coverage = payload.config.coverage;
       }
 
-      const allTestsRun = (payload.storyIds ?? []).length === 0;
       if (this.coverage) {
         /*
            If we have coverage enabled and we're running all stories,
            we have to restart Vitest AND disable watch mode otherwise the coverage report will be incorrect,
            Vitest behaves wonky when re-using the same Vitest instance but with watch mode disabled,
            among other things it causes the coverage report to be incorrect and stale.
-           
+
            If we're only running a subset of stories, we have to temporarily disable coverage,
            as a coverage report for a subset of stories is not useful.
          */
-        await this.restartVitest({
+        await this.vitestManager.restartVitest({
           watchMode: allTestsRun ? false : this.watchMode,
           coverage: allTestsRun,
         });
+      } else {
+        await this.vitestManager.vitestRestartPromise;
       }
 
       await this.vitestManager.runTests(payload);
 
       if (this.coverage && !allTestsRun) {
         // Re-enable coverage if it was temporarily disabled because of a subset of stories was run
-        await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
+        await this.vitestManager.restartVitest({
+          watchMode: this.watchMode,
+          coverage: this.coverage,
+        });
       }
     } catch (e) {
       this.reportFatalError('Failed to run tests', e);

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

@@ -34,6 +34,8 @@ export class VitestManager {
 
   vitestStartupCounter = 0;
 
+  vitestRestartPromise: Promise<void> | null = null;
+
   storyCountForCurrentRun: number = 0;
 
   constructor(private testManager: TestManager) {}
@@ -99,12 +101,30 @@ export class VitestManager {
     }
   }
 
+  async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) {
+    await this.vitestRestartPromise;
+    this.vitestRestartPromise = new Promise(async (resolve, reject) => {
+      try {
+        await this.vitest?.runningPromise;
+        await this.closeVitest();
+        await this.startVitest({ watchMode, coverage });
+        resolve();
+      } catch (e) {
+        reject(e);
+      } finally {
+        this.vitestRestartPromise = null;
+      }
+    });
+    return this.vitestRestartPromise;
+  }
+
   private updateLastChanged(filepath: string) {
     const projects = this.vitest!.getModuleProjects(filepath);
     projects.forEach(({ server, browser }) => {
-      const serverMods = server.moduleGraph.getModulesByFile(filepath);
-      serverMods?.forEach((mod) => server.moduleGraph.invalidateModule(mod));
-
+      if (server) {
+        const serverMods = server.moduleGraph.getModulesByFile(filepath);
+        serverMods?.forEach((mod) => server.moduleGraph.invalidateModule(mod));
+      }
       if (browser) {
         const browserMods = browser.vite.moduleGraph.getModulesByFile(filepath);
         browserMods?.forEach((mod) => browser.vite.moduleGraph.invalidateModule(mod));
@@ -148,6 +168,8 @@ export class VitestManager {
   async runTests(requestPayload: TestingModuleRunRequestPayload<Config>) {
     if (!this.vitest) {
       await this.startVitest();
+    } else {
+      await this.vitestRestartPromise;
     }
 
     this.resetTestNamePattern();

code/core/src/manager-api/modules/experimental_testmodule.ts

@@ -70,7 +70,7 @@ export const init: ModuleFn<SubAPI, SubState> = ({ store, fullAPI }) => {
     clearTestProviderState(id) {
       const update = {
         cancelling: false,
-        running: true,
+        running: false,
         failed: false,
         crashed: false,
         progress: undefined,
@@ -85,7 +85,13 @@ export const init: ModuleFn<SubAPI, SubState> = ({ store, fullAPI }) => {
     runTestProvider(id, options) {
       const index = store.getState().index;
       invariant(index, 'The index is currently unavailable');
-      api.updateTestProviderState(id, { running: true, failed: false, crashed: false });
+
+      api.updateTestProviderState(id, {
+        running: true,
+        failed: false,
+        crashed: false,
+        progress: undefined,
+      });
 
       const provider = store.getState().testProviders[id];