test_runner: call abort on test finish

PR-URL: nodejs#48827 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
pluris · Aug 7, 2023 · 22606ae · 22606ae
1 parent 0480818
commit 22606ae
Show file tree

Hide file tree

Showing 5 changed files with 166 additions and 18 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -601,6 +601,12 @@ class Test extends AsyncResource {
       } else {
         this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
       }
+    } finally {
+      // Do not abort hooks and the root test as hooks instance are shared between tests suite so aborting them will
+      // cause them to not run for further tests.
+      if (this.parent !== null) {
+        this.#abortController.abort();
+      }
     }
 
     // Clean up the test. Then, try to report the results and execute any

diff --git a/test/fixtures/test-runner/aborts/failed-test-still-call-abort.js b/test/fixtures/test-runner/aborts/failed-test-still-call-abort.js
@@ -0,0 +1,25 @@
+const {test, afterEach} = require('node:test');
+const assert = require('node:assert');
+const { waitForAbort } = require('./wait-for-abort-helper');
+
+let testCount = 0;
+let signal;
+
+afterEach(() => {
+  assert.equal(signal.aborted, false);
+
+  waitForAbort({ testNumber: ++testCount, signal });
+});
+
+test("sync", (t) => {
+  signal = t.signal;
+  assert.equal(signal.aborted, false);
+  throw new Error('failing the sync test');
+});
+
+test("async", async (t) => {
+  await null;
+  signal = t.signal;
+  assert.equal(signal.aborted, false);
+  throw new Error('failing the async test');
+});
diff --git a/test/fixtures/test-runner/aborts/successful-test-still-call-abort.js b/test/fixtures/test-runner/aborts/successful-test-still-call-abort.js
@@ -0,0 +1,23 @@
+const {test, afterEach} = require('node:test');
+const assert = require('node:assert');
+const {waitForAbort} = require("./wait-for-abort-helper");
+
+let testCount = 0;
+let signal;
+
+afterEach(() => {
+  assert.equal(signal.aborted, false);
+
+  waitForAbort({ testNumber: ++testCount, signal });
+});
+
+test("sync", (t) => {
+  signal = t.signal;
+  assert.equal(signal.aborted, false);
+});
+
+test("async", async (t) => {
+  await null;
+  signal = t.signal;
+  assert.equal(signal.aborted, false);
+});
diff --git a/test/fixtures/test-runner/aborts/wait-for-abort-helper.js b/test/fixtures/test-runner/aborts/wait-for-abort-helper.js
@@ -0,0 +1,19 @@
+module.exports = {
+  waitForAbort: function ({ testNumber, signal }) {
+    let retries = 0;
+
+    const interval = setInterval(() => {
+      retries++;
+      if(signal.aborted) {
+        console.log(`abort called for test ${testNumber}`);
+        clearInterval(interval);
+        return;
+      }
+
+      if(retries > 100) {
+        clearInterval(interval);
+        throw new Error(`abort was not called for test ${testNumber}`);
+      }
+    }, 10);
+  }
+}
diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs
@@ -118,29 +118,104 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
     assert.strictEqual(result[5], 'ok 2 - this should be executed\n');
   });
 
-  it('should stop watch mode when abortSignal aborts', async () => {
+  it('should emit "test:watch:drained" event on watch mode', async () => {
     const controller = new AbortController();
-    const result = await run({ files: [join(testFixtures, 'test/random.cjs')], watch: true, signal: controller.signal })
-      .compose(async function* (source) {
-        for await (const chunk of source) {
-          if (chunk.type === 'test:pass') {
-            controller.abort();
-            yield chunk.data.name;
-          }
-        }
-      })
-      .toArray();
-    assert.deepStrictEqual(result, ['this should pass']);
+    await run({
+      files: [join(testFixtures, 'test/random.cjs')],
+      watch: true,
+      signal: controller.signal,
+    }).on('data', function({ type }) {
+      if (type === 'test:watch:drained') {
+        controller.abort();
+      }
+    });
   });
 
-  it('should emit "test:watch:drained" event on watch mode', async () => {
-    const controller = new AbortController();
-    await run({ files: [join(testFixtures, 'test/random.cjs')], watch: true, signal: controller.signal })
-      .on('data', function({ type }) {
-        if (type === 'test:watch:drained') {
-          controller.abort();
+  describe('AbortSignal', () => {
+    it('should stop watch mode when abortSignal aborts', async () => {
+      const controller = new AbortController();
+      const result = await run({
+        files: [join(testFixtures, 'test/random.cjs')],
+        watch: true,
+        signal: controller.signal,
+      })
+        .compose(async function* (source) {
+          for await (const chunk of source) {
+            if (chunk.type === 'test:pass') {
+              controller.abort();
+              yield chunk.data.name;
+            }
+          }
+        })
+        .toArray();
+      assert.deepStrictEqual(result, ['this should pass']);
+    });
+
+    it('should abort when test succeeded', async () => {
+      const stream = run({
+        files: [
+          fixtures.path(
+            'test-runner',
+            'aborts',
+            'successful-test-still-call-abort.js'
+          ),
+        ],
+      });
+
+      let passedTestCount = 0;
+      let failedTestCount = 0;
+
+      let output = '';
+      for await (const data of stream) {
+        if (data.type === 'test:stdout') {
+          output += data.data.message.toString();
+        }
+        if (data.type === 'test:fail') {
+          failedTestCount++;
         }
+        if (data.type === 'test:pass') {
+          passedTestCount++;
+        }
+      }
+
+      assert.match(output, /abort called for test 1/);
+      assert.match(output, /abort called for test 2/);
+      assert.strictEqual(failedTestCount, 0, new Error('no tests should fail'));
+      assert.strictEqual(passedTestCount, 2);
+    });
+
+    it('should abort when test failed', async () => {
+      const stream = run({
+        files: [
+          fixtures.path(
+            'test-runner',
+            'aborts',
+            'failed-test-still-call-abort.js'
+          ),
+        ],
       });
+
+      let passedTestCount = 0;
+      let failedTestCount = 0;
+
+      let output = '';
+      for await (const data of stream) {
+        if (data.type === 'test:stdout') {
+          output += data.data.message.toString();
+        }
+        if (data.type === 'test:fail') {
+          failedTestCount++;
+        }
+        if (data.type === 'test:pass') {
+          passedTestCount++;
+        }
+      }
+
+      assert.match(output, /abort called for test 1/);
+      assert.match(output, /abort called for test 2/);
+      assert.strictEqual(passedTestCount, 0, new Error('no tests should pass'));
+      assert.strictEqual(failedTestCount, 2);
+    });
   });
 
   describe('sharding', () => {