test_runner: replace process.kill with harness abort controller

nodejs · Jan 11, 2025 · 1d68db4 · 1d68db4
1 parent f1c269c
commit 1d68db4
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 10 deletions.
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -27,6 +27,7 @@ const {
   shouldColorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { queueMicrotask } = require('internal/process/task_queues');
+const { AbortController } = require('internal/abort_controller');
 const { bigint: hrtime } = process.hrtime;
 const resolvedPromise = PromiseResolve();
 const testResources = new SafeMap();
@@ -76,7 +77,10 @@ function createTestTree(rootTestOptions, globalOptions) {
 
       buildPhaseDeferred.resolve();
     },
-    testsProcesses: new SafeMap(),
+    abortController: new AbortController(),
+    abortTestTree() {
+      harness.abortController.abort();
+    },
   };
 
   harness.resetCounters();

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -88,6 +88,7 @@ const {
 } = require('internal/test_runner/utils');
 const { Glob } = require('internal/fs/glob');
 const { once } = require('events');
+const { AbortSignal } = require('internal/abort_controller');
 const {
   triggerUncaughtException,
   exitCodes: { kGenericUserError },
@@ -202,7 +203,12 @@ class FileTest extends Test {
   }
 
   #skipReporting() {
-    return this.#reportedChildren > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
+    return (
+      this.root.bailed ||
+      (
+        this.#reportedChildren > 0 &&
+        (!this.error || this.error.failureType === kSubtestsFailed)
+      ));
   }
   #checkNestedComment(comment) {
     const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
@@ -227,9 +233,12 @@ class FileTest extends Test {
     if (item.type === 'test:bail') {
       // <-- here we need to stop all the pending test files (aka subprocesses)
       // To be replaced, just for poc
-      this.root.harness.testsProcesses.forEach((child) => {
-        child.kill();
-      });
+      // this.root.harness.testsProcesses.forEach((child) => {
+      //   child.kill();
+      // });
+      this.root.harness.abortTestTree();
+      this.root.bailed = true;
+      this.bailed = true;
       return;
     }
     if (item.type === 'test:pass' || item.type === 'test:fail') {
@@ -378,7 +387,6 @@ function runTestFile(path, filesWatcher, opts) {
   const watchMode = filesWatcher != null;
   const testPath = path === kIsolatedProcessName ? '' : path;
   const testOpts = { __proto__: null, signal: opts.signal };
-  const subtestProcesses = opts.root.harness.testsProcesses;
   const subtest = opts.root.createSubtest(FileTest, testPath, testOpts, async (t) => {
     if (opts.root.bailed) {
       // TODO(pmarchini): this is a temporary solution to avoid running tests after bailing
@@ -399,7 +407,12 @@ function runTestFile(path, filesWatcher, opts) {
       process.execPath, args,
       {
         __proto__: null,
-        signal: t.signal,
+        signal: AbortSignal.any(
+          [
+            t.signal,
+            opts.root.harness.abortController.signal,
+          ],
+        ),
         encoding: 'utf8',
         env,
         stdio,
@@ -410,7 +423,6 @@ function runTestFile(path, filesWatcher, opts) {
       filesWatcher.runningProcesses.set(path, child);
       filesWatcher.watcher.watchChildProcessModules(child, path);
     }
-    subtestProcesses.set(path, child);
 
     let err;
 
@@ -444,7 +456,6 @@ function runTestFile(path, filesWatcher, opts) {
       finished(child.stdout, { __proto__: null, signal: t.signal }),
     ]);
 
-    subtestProcesses.delete(path);
     if (watchMode) {
       filesWatcher.runningProcesses.delete(path);
       filesWatcher.runningSubtests.delete(path);

diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs
@@ -695,7 +695,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
     }
 
     assert.strictEqual(bailedTestCount, 1);
-    assert.strictEqual(failedTestCount, 4);
+    assert.strictEqual(failedTestCount, 3);
     assert.strictEqual(passedTestCount, 0);
   });