refactor: make spawnTest async and drop dead exit-reason branch

Author: bavulapati

implementors/node/child_process.d.ts

@@ -13,4 +13,4 @@ export interface SpawnTestResult {
 export function spawnTest(
   filePath: string,
   options?: SpawnTestOptions,
-): SpawnTestResult;
+): Promise<SpawnTestResult>;

implementors/node/child_process.js

@@ -1,4 +1,4 @@
-import { spawnSync } from 'node:child_process';
+import { spawn } from 'node:child_process';
 import path from 'node:path';
 import { pathToFileURL } from 'node:url';
 
@@ -28,7 +28,7 @@ const ABORT_EXIT_CODES = [132, 133, 134, 139, 0xc0000409, 0xc000001d];
  *     `'inherit'` streams it straight to the terminal as the child runs (so the
  *     output of a slow or hanging test is visible immediately) and leaves the
  *     returned `stdout` empty. stderr is always captured for diagnostics.
- * @returns {{ status: number | null, aborted: boolean, stdout: string, stderr: string }}
+ * @returns {Promise<{ status: number | null, aborted: boolean, stdout: string, stderr: string }>}
  */
 export const spawnTest = (filePath, options = {}) => {
   // --expose-gc is mandatory: gc.js (loaded via harness.js) throws at import
@@ -42,18 +42,35 @@ export const spawnTest = (filePath, options = {}) => {
     filePath,
   ];
 
-  const result = spawnSync(process.execPath, args, {
+  // spawn (not spawnSync) so a hung child doesn't block the event loop and the
+  // test runner can still enforce its per-test timeout.
+  const child = spawn(process.execPath, args, {
     cwd: options.cwd ?? process.cwd(),
-    maxBuffer: 100 * 1024 * 1024,
     stdio: ['ignore', options.stdout ?? 'pipe', 'pipe'],
   });
-  if (result.error) throw result.error;
-  return {
-    status: result.status,
-    aborted: result.signal !== null || ABORT_EXIT_CODES.includes(result.status),
-    stderr: result.stderr?.toString() ?? '',
-    stdout: result.stdout?.toString() ?? '',
-  };
+
+  return new Promise((resolve, reject) => {
+    let stdout = '';
+    let stderr = '';
+    // child.stdout is null when stdout is 'inherit' (streamed to the terminal).
+    child.stdout?.setEncoding('utf8').on('data', (chunk) => {
+      stdout += chunk;
+    });
+    child.stderr.setEncoding('utf8').on('data', (chunk) => {
+      stderr += chunk;
+    });
+    child.on('error', reject);
+    // 'close' (not 'exit') fires once the stdio streams have drained, so stdout
+    // and stderr are complete.
+    child.on('close', (status, signal) => {
+      resolve({
+        status,
+        aborted: signal !== null || ABORT_EXIT_CODES.includes(status),
+        stderr,
+        stdout,
+      });
+    });
+  });
 };
 
 // This module is loaded in both contexts: imported by the parent test runner

implementors/node/tests.ts

@@ -31,20 +31,23 @@ export function listDirectoryEntries(dir: string) {
   return { directories, files };
 }
 
-export function runFileInSubprocess(cwd: string, filePath: string): void {
+export async function runFileInSubprocess(
+  cwd: string,
+  filePath: string,
+): Promise<void> {
   // Stream stdout live rather than buffering it, so output from a slow or
   // hanging test shows up immediately. stderr is still captured to attach to
   // the failure message below.
-  const { status, aborted, stderr } = spawnTest(filePath, {
+  const { status, aborted, stderr } = await spawnTest(filePath, {
     cwd,
     stdout: 'inherit',
   });
 
   if (status === 0) return;
 
-  const reason = aborted ?
-    'aborted' :
-    status !== null ? `exit code ${status}` : 'unknown';
+  // A null status means the child was killed by a signal, which already sets
+  // `aborted`, so the non-aborted branch always has a numeric exit code.
+  const reason = aborted ? 'aborted' : `exit code ${status}`;
   const trimmedStderr = stderr.trim();
   const stderrSuffix = trimmedStderr ?
     `\n--- stderr ---\n${trimmedStderr}\n--- end stderr ---` :

tests/harness/spawn-test.js

@@ -19,15 +19,15 @@ if (typeof spawnTest !== 'function') {
 // Successful child: exits 0, stderr empty, and harness globals are available
 // inside the child (the child checks `typeof assert === 'function'` itself).
 {
-  const result = spawnTest('spawn-test-ok-child.mjs');
+  const result = await spawnTest('spawn-test-ok-child.mjs');
   assert.strictEqual(result.status, 0, `ok child exited with status ${result.status}; stderr:\n${result.stderr}`);
   assert.strictEqual(result.aborted, false);
   assert.strictEqual(result.stderr, '');
 }
 
 // Failing child: non-zero status and stderr contains the thrown marker.
 {
-  const result = spawnTest('spawn-test-fail-child.mjs');
+  const result = await spawnTest('spawn-test-fail-child.mjs');
   assert.notStrictEqual(result.status, 0, 'fail child should exit non-zero');
   // A thrown error is a clean non-zero exit, not an abnormal termination.
   assert.strictEqual(result.aborted, false);
@@ -38,7 +38,7 @@ if (typeof spawnTest !== 'function') {
 
 // Result shape: all four fields are present.
 {
-  const result = spawnTest('spawn-test-ok-child.mjs');
+  const result = await spawnTest('spawn-test-ok-child.mjs');
   for (const key of ['status', 'aborted', 'stdout', 'stderr']) {
     if (!(key in result)) {
       throw new Error(`Expected spawnTest result to have "${key}" field`);
@@ -56,7 +56,7 @@ if (typeof spawnTest !== 'function') {
 // actually reached the terminal would need the child to write to stdout, which
 // isn't expressible in portable ECMAScript, so we only check the contract here.)
 {
-  const result = spawnTest('spawn-test-ok-child.mjs', { stdout: 'inherit' });
+  const result = await spawnTest('spawn-test-ok-child.mjs', { stdout: 'inherit' });
   assert.strictEqual(result.status, 0, `ok child exited with status ${result.status}; stderr:\n${result.stderr}`);
   assert.strictEqual(result.stdout, '', 'inherited stdout should not be captured');
 }
@@ -65,7 +65,7 @@ if (typeof spawnTest !== 'function') {
 // makes the bare child filename unresolvable. The child's stderr must name
 // the specific file Node tried to load, proving cwd actually shifted.
 {
-  const result = spawnTest('spawn-test-ok-child.mjs', { cwd: '..' });
+  const result = await spawnTest('spawn-test-ok-child.mjs', { cwd: '..' });
   assert.notStrictEqual(result.status, 0, 'expected cwd ".." to make the child filename unresolvable');
   if (!result.stderr.includes('spawn-test-ok-child.mjs')) {
     throw new Error(`Expected stderr to reference the unresolved child filename, got:\n${result.stderr}`);