From 352e9e30bf3762918e9943885067584f0d373442 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:53:58 +0300 Subject: [PATCH] test_runner: call abort on test finish PR-URL: https://github.com/nodejs/node/pull/48827 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow Reviewed-By: Chemi Atlow --- lib/internal/test_runner/test.js | 6 + .../aborts/failed-test-still-call-abort.js | 25 ++++ .../successful-test-still-call-abort.js | 23 ++++ .../aborts/wait-for-abort-helper.js | 19 +++ test/parallel/test-runner-run.mjs | 111 +++++++++++++++--- 5 files changed, 166 insertions(+), 18 deletions(-) create mode 100644 test/fixtures/test-runner/aborts/failed-test-still-call-abort.js create mode 100644 test/fixtures/test-runner/aborts/successful-test-still-call-abort.js create mode 100644 test/fixtures/test-runner/aborts/wait-for-abort-helper.js diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index f5cc0fb98f6271..9a333e1457fbc0 100644 --- 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 new file mode 100644 index 00000000000000..496d2331422c00 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..4f3879c624de44 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..89eda7ed9138ca --- /dev/null +++ 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 index b5c41b8ae925de..7e4a8fbe76753a 100644 --- 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', () => {