Replace Node's process.fork with Execa and refactor/cleanup a bunch o…

…f things
storybookjs · Sep 24, 2024 · 72bd862 · 72bd862
1 parent 123c6d2
commit 72bd862
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 112 deletions.
diff --git a/code/addons/test/package.json b/code/addons/test/package.json
@@ -79,6 +79,7 @@
     "@vitest/browser": "^2.1.1",
     "@vitest/runner": "^2.1.1",
     "boxen": "^8.0.1",
+    "execa": "^8.0.1",
     "find-up": "^7.0.0",
     "lodash": "^4.17.21",
     "semver": "^7.6.3",

diff --git a/code/addons/test/src/node/boot-test-runner.ts b/code/addons/test/src/node/boot-test-runner.ts
@@ -1,4 +1,4 @@
-import { type ChildProcess, fork } from 'node:child_process';
+import { type ChildProcess } from 'node:child_process';
 import { join } from 'node:path';
 
 import type { Channel } from 'storybook/internal/channels';
@@ -9,109 +9,91 @@ import {
   TESTING_MODULE_WATCH_MODE_REQUEST,
 } from 'storybook/internal/core-events';
 
+import { execaNode } from 'execa';
+
 import { log } from '../logger';
 
-const MAX_RESTART_ATTEMPTS = 2;
+const MAX_START_ATTEMPTS = 3;
+const MAX_START_TIME = 8000;
 
 // This path is a bit confusing, but essentially `boot-test-runner` gets bundled into the preset bundle
 // which is at the root. Then, from the root, we want to load `node/vitest.js`
 const vitestModulePath = join(__dirname, 'node', 'vitest.js');
 
-export const bootTestRunner = (channel: Channel, initEvent?: string, initArgs?: any[]) =>
-  new Promise((resolve, reject) => {
-    let attempts = 0;
-    let child: null | ChildProcess;
-
-    const forwardRun = (...args: any[]): void => {
-      child?.send({ type: TESTING_MODULE_RUN_REQUEST, args, from: 'server' });
-    };
-    const forwardRunAll = (...args: any[]): void => {
-      child?.send({ type: TESTING_MODULE_RUN_ALL_REQUEST, args, from: 'server' });
-    };
-    const forwardWatchMode = (...args: any[]): void => {
-      child?.send({ type: TESTING_MODULE_WATCH_MODE_REQUEST, args, from: 'server' });
-    };
-    const forwardCancel = (...args: any[]): void => {
-      child?.send({ type: TESTING_MODULE_CANCEL_TEST_RUN_REQUEST, args, from: 'server' });
-    };
-
-    const startChildProcess = () => {
-      child = fork(vitestModulePath, [], {
-        // We want to pipe output and error
-        // so that we can prefix the logs in the terminal
-        // with a clear identifier
-        stdio: ['inherit', 'pipe', 'pipe', 'ipc'],
-        silent: true,
-      });
+export const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: any[]) => {
+  const now = Date.now();
+  let child: null | ChildProcess;
+
+  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();
+    child = null;
+  };
+
+  const exit = (code = 0) => {
+    killChild();
+    process.exit(code);
+  };
+
+  process.on('exit', exit);
+  process.on('SIGINT', () => exit(0));
+  process.on('SIGTERM', () => exit(0));
+
+  const startChildProcess = (attempt = 1) =>
+    new Promise<void>((resolve, reject) => {
+      child = execaNode(vitestModulePath);
+      child.stdout?.on('data', log);
+      child.stderr?.on('data', log);
 
-      child.stdout?.on('data', (data) => {
-        log(data);
-      });
+      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
+          child?.send({ type: initEvent, args: initArgs, from: 'server' });
 
-      child.stderr?.on('data', (data) => {
-        log(data);
-      });
+          // 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);
 
-      child.on('message', (result: any) => {
-        switch (result.type) {
-          case 'ready': {
-            attempts = 0;
-            child?.send({ type: initEvent, args: initArgs, from: 'server' });
-            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);
-            channel.emit(result.type, ...(result.args || []));
-            resolve(result);
-            return;
+          resolve();
+        }
+
+        if (result.type === 'error') {
+          killChild();
+
+          if (result.message) {
+            log(result.message);
+          }
+          if (result.error) {
+            log(result.error);
           }
 
-          case 'error': {
-            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();
-            child = null;
-
-            if (result.message) {
-              log(result.message);
-            }
-            if (result.error) {
-              log(result.error);
-            }
-
-            if (attempts >= MAX_RESTART_ATTEMPTS) {
-              log(`Aborting test runner process after ${MAX_RESTART_ATTEMPTS} restart attempts`);
-              channel.emit(
-                'error',
-                `Aborting test runner process after ${MAX_RESTART_ATTEMPTS} restart attempts`
-              );
-              reject(new Error('Test runner process failed to start'));
-            } else {
-              attempts += 1;
-              log(`Restarting test runner process (attempt ${attempts}/${MAX_RESTART_ATTEMPTS})`);
-              setTimeout(startChildProcess, 500);
-            }
-            return;
+          if (attempt >= MAX_START_ATTEMPTS) {
+            log(`Aborting test runner process after ${attempt} restart attempts`);
+            reject();
+          } else if (Date.now() - now > MAX_START_TIME) {
+            log(`Aborting test runner process after ${MAX_START_TIME / 1000} seconds`);
+            reject();
+          } else {
+            log(`Restarting test runner process (attempt ${attempt}/${MAX_START_ATTEMPTS})`);
+            setTimeout(() => startChildProcess(attempt + 1).then(resolve, reject), 1000);
           }
         }
       });
-    };
-
-    startChildProcess();
-
-    process.on('exit', () => {
-      child?.kill();
-      process.exit(0);
     });
-    process.on('SIGINT', () => {
-      child?.kill();
-      process.exit(0);
-    });
-    process.on('SIGTERM', () => {
-      child?.kill();
-      process.exit(0);
-    });
-  });
+
+  await startChildProcess();
+};
diff --git a/code/addons/test/src/node/test-manager.ts b/code/addons/test/src/node/test-manager.ts
@@ -33,7 +33,6 @@ export class TestManager {
   async restartVitest(watchMode = false) {
     await this.vitestManager.closeVitest();
     await this.vitestManager.startVitest(watchMode);
-    process.send?.({ type: 'ready', watchMode });
   }
 
   async handleWatchModeRequest(request: TestingModuleWatchModeRequestPayload) {

diff --git a/code/addons/test/src/node/vitest.ts b/code/addons/test/src/node/vitest.ts
@@ -12,9 +12,7 @@ const channel: Channel = new Channel({
   async: true,
   transport: {
     send: (event) => {
-      if (process.send) {
-        process.send(event);
-      }
+      process.send?.(event);
     },
     setHandler: (handler) => {
       process.on('message', handler);
@@ -23,26 +21,23 @@ const channel: Channel = new Channel({
 });
 
 const testManager = new TestManager(channel);
-testManager.restartVitest();
+testManager.restartVitest().then(() => process.send?.({ type: 'ready' }));
+
+const exit = (code = 0) => {
+  channel?.removeAllListeners();
+  process.exit(code);
+};
+
+process.on('exit', exit);
+process.on('SIGINT', () => exit(0));
+process.on('SIGTERM', () => exit(0));
 
 process.on('uncaughtException', (err) => {
-  process.send?.({ type: 'error', message: 'Uncaught Exception', error: err.stack });
-  process.exit(1);
+  process.send?.({ type: 'error', message: 'Uncaught exception', error: err.stack });
+  exit(1);
 });
 
 process.on('unhandledRejection', (reason) => {
-  throw reason;
-});
-
-process.on('exit', () => {
-  channel?.removeAllListeners();
-  process.exit(0);
-});
-process.on('SIGINT', () => {
-  channel?.removeAllListeners();
-  process.exit(0);
-});
-process.on('SIGTERM', () => {
-  channel?.removeAllListeners();
-  process.exit(0);
+  process.send?.({ type: 'error', message: 'Unhandled rejection', error: reason });
+  exit(1);
 });
diff --git a/code/yarn.lock b/code/yarn.lock
@@ -6245,6 +6245,7 @@ __metadata:
     "@vitest/runner": "npm:^2.1.1"
     boxen: "npm:^8.0.1"
     chalk: "npm:^5.3.0"
+    execa: "npm:^8.0.1"
     find-up: "npm:^7.0.0"
     lodash: "npm:^4.17.21"
     semver: "npm:^7.6.3"