From de2dc623927a9001a35de90c940d33681000f7ee Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 21:02:54 +0300 Subject: [PATCH 1/9] test_runner: fix timeout in *Each hook failing further tests --- lib/internal/test_runner/test.js | 12 ++++ .../test-runner/output/hooks.snapshot | 4 -- ...re-each-should-not-affect-further-tests.js | 46 +++++++++++++ ...h-should-not-affect-further-tests.snapshot | 67 +++++++++++++++++++ test/parallel/test-runner-output.mjs | 1 + 5 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js create mode 100644 test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 0d8e3dbd6e8d3c..8089fd9f06c3ee 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -681,6 +681,12 @@ class Test extends AsyncResource { } } + recreateAbortController() { + this.#abortController.abort(); + this.#abortController = new AbortController(); + this.signal = this.#abortController.signal; + } + isClearToSend() { return this.parent === null || ( @@ -762,6 +768,12 @@ class TestHook extends Test { super({ __proto__: null, fn, timeout, signal }); } run(args) { + if (this.error) { + this.passed = false; + this.error = null; + super.recreateAbortController(); + } + this.#args = args; return super.run(); } diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index b9fd23640373de..676e1c7a3287e3 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -134,8 +134,6 @@ not ok 3 - after throws * * * - async Promise.all (index 0) - * * ... 1..2 @@ -183,7 +181,6 @@ not ok 4 - beforeEach throws * * * - async Promise.all (index 0) * ... 1..2 @@ -265,7 +262,6 @@ not ok 6 - afterEach when test fails * * * - async Promise.all (index 0) * ... 1..2 diff --git a/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js b/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js new file mode 100644 index 00000000000000..4f46154f0ab05f --- /dev/null +++ b/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js @@ -0,0 +1,46 @@ +const {describe, test, beforeEach, afterEach} = require("node:test"); +const {setTimeout} = require("timers/promises"); + + +describe('before each timeout', () => { + let i = 0; + + beforeEach(async () => { + if (i++ === 0) { + console.log('gonna timeout') + await setTimeout(50); + return; + } + console.log('not gonna timeout'); + }, {timeout: 20}); + + test('first describe first test', () => { + console.log('before each test first'); + }); + + test('first describe second test', () => { + console.log('before each test second'); + }); +}); + + +describe('after each timeout', () => { + let i = 0; + + afterEach(async () => { + if (i++ === 0) { + console.log('gonna timeout') + await setTimeout(50); + return; + } + console.log('not gonna timeout'); + }, {timeout: 20}); + + test('second describe first test', () => { + console.log('after each test first'); + }); + + test('second describe second test', () => { + console.log('after each test second'); + }); +}); diff --git a/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot b/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot new file mode 100644 index 00000000000000..1518b5ca13108b --- /dev/null +++ b/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot @@ -0,0 +1,67 @@ +gonna timeout +TAP version 13 +not gonna timeout +before each test second +after each test first +gonna timeout +# Subtest: before each timeout + # Subtest: first describe first test + not ok 1 - first describe first test + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running beforeEach hook' + code: 'ERR_TEST_FAILURE' + stack: |- + async Promise.all (index 0) + ... + # Subtest: first describe second test + ok 2 - first describe second test + --- + duration_ms: * + ... + 1..2 +not ok 1 - before each timeout + --- + duration_ms: * + type: 'suite' + failureType: 'subtestsFailed' + error: '1 subtest failed' + code: 'ERR_TEST_FAILURE' + ... +after each test second +not gonna timeout +# Subtest: after each timeout + # Subtest: second describe first test + not ok 1 - second describe first test + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running afterEach hook' + code: 'ERR_TEST_FAILURE' + stack: |- + async Promise.all (index 0) + ... + # Subtest: second describe second test + ok 2 - second describe second test + --- + duration_ms: * + ... + 1..2 +not ok 2 - after each timeout + --- + duration_ms: * + type: 'suite' + failureType: 'subtestsFailed' + error: '1 subtest failed' + code: 'ERR_TEST_FAILURE' + ... +1..2 +# tests 4 +# suites 2 +# pass 2 +# fail 2 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms * diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 76c511117ea091..98d5d5ab649739 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -36,6 +36,7 @@ const tests = [ { name: 'test-runner/output/describe_it.js' }, { name: 'test-runner/output/describe_nested.js' }, { name: 'test-runner/output/hooks.js' }, + { name: 'test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js' }, { name: 'test-runner/output/hooks-with-no-global-test.js' }, { name: 'test-runner/output/no_refs.js' }, { name: 'test-runner/output/no_tests.js' }, From ac25a43f2c1789b652f51958a0b5d09de00d2ad9 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 22:16:00 +0300 Subject: [PATCH 2/9] fix when hook was aborted by user --- lib/internal/test_runner/test.js | 6 +- .../test-runner/output/abort_hooks.js | 68 +++++++++++++++++++ test/parallel/test-runner-output.mjs | 1 + 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/test-runner/output/abort_hooks.js diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 8089fd9f06c3ee..058e5c14cbca85 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -681,6 +681,10 @@ class Test extends AsyncResource { } } + wasAbortedByUser() { + return this.#outerSignal?.aborted; + } + recreateAbortController() { this.#abortController.abort(); this.#abortController = new AbortController(); @@ -768,7 +772,7 @@ class TestHook extends Test { super({ __proto__: null, fn, timeout, signal }); } run(args) { - if (this.error) { + if (this.error && !this.wasAbortedByUser()) { this.passed = false; this.error = null; super.recreateAbortController(); diff --git a/test/fixtures/test-runner/output/abort_hooks.js b/test/fixtures/test-runner/output/abort_hooks.js new file mode 100644 index 00000000000000..3076316775916c --- /dev/null +++ b/test/fixtures/test-runner/output/abort_hooks.js @@ -0,0 +1,68 @@ +// Flags: --no-warnings +'use strict'; +const { before, beforeEach, describe, it, after, afterEach } = require('node:test'); + +describe('1 before describe', () => { + const ac = new AbortController(); + before(() => { + console.log('before'); + ac.abort() + }, {signal: ac.signal}); + + it('test 1', () => { + console.log('1.1'); + }); + it('test 2', () => { + console.log('1.2'); + }); +}); + +describe('2 after describe', () => { + const ac = new AbortController(); + after(() => { + console.log('after'); + ac.abort() + }, {signal: ac.signal}); + + it('test 1', () => { + console.log('2.1'); + }); + it('test 2', () => { + console.log('2.2'); + }); +}); + +describe('3 beforeEach describe', () => { + const ac = new AbortController(); + beforeEach(() => { + console.log('beforeEach'); + ac.abort() + }, {signal: ac.signal}); + + it('test 1', () => { + console.log('3.1'); + }); + it('test 2', () => { + console.log('3.2'); + }); +}); + +describe('4 afterEach describe', () => { + const ac = new AbortController(); + afterEach(() => { + console.log('afterEach'); + ac.abort() + }, {signal: ac.signal}); + + it('test 1', () => { + console.log('4.1'); + }); + it('test 2', () => { + console.log('4.2'); + }); +}); + +// AbortSignal.timeout(1) doesn't prevent process from closing +// thus we have to keep the process open to prevent cancelation +// of the entire test tree +setTimeout(() => {}, 1000); diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 98d5d5ab649739..c36e43e5c0cc35 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -33,6 +33,7 @@ const specTransform = snapshot const tests = [ { name: 'test-runner/output/abort.js' }, { name: 'test-runner/output/abort_suite.js' }, + { name: 'test-runner/output/abort_hooks.js' }, { name: 'test-runner/output/describe_it.js' }, { name: 'test-runner/output/describe_nested.js' }, { name: 'test-runner/output/hooks.js' }, From 0caf49c6af96175ea88c7add3c466019edce0af8 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 22:19:24 +0300 Subject: [PATCH 3/9] rename --- .../test-runner/output/abort_hooks.js | 5 - .../test-runner/output/abort_hooks.snapshot | 188 ++++++++++++++++++ ...e_each_should_not_affect_further_tests.js} | 0 ..._should_not_affect_further_tests.snapshot} | 0 test/parallel/test-runner-output.mjs | 2 +- 5 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/test-runner/output/abort_hooks.snapshot rename test/fixtures/test-runner/output/{timeout-in-before-each-should-not-affect-further-tests.js => timeout_in_before_each_should_not_affect_further_tests.js} (100%) rename test/fixtures/test-runner/output/{timeout-in-before-each-should-not-affect-further-tests.snapshot => timeout_in_before_each_should_not_affect_further_tests.snapshot} (100%) diff --git a/test/fixtures/test-runner/output/abort_hooks.js b/test/fixtures/test-runner/output/abort_hooks.js index 3076316775916c..b0f1da80d62719 100644 --- a/test/fixtures/test-runner/output/abort_hooks.js +++ b/test/fixtures/test-runner/output/abort_hooks.js @@ -61,8 +61,3 @@ describe('4 afterEach describe', () => { console.log('4.2'); }); }); - -// AbortSignal.timeout(1) doesn't prevent process from closing -// thus we have to keep the process open to prevent cancelation -// of the entire test tree -setTimeout(() => {}, 1000); diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot new file mode 100644 index 00000000000000..0c74f2c5b7967e --- /dev/null +++ b/test/fixtures/test-runner/output/abort_hooks.snapshot @@ -0,0 +1,188 @@ +before +2.1 +2.2 +after +beforeEach +4.1 +afterEach +4.2 +TAP version 13 +# Subtest: 1 before describe + # Subtest: test 1 + not ok 1 - test 1 + --- + duration_ms: ZERO + failureType: 'cancelledByParent' + error: 'test did not finish before its parent and was cancelled' + code: 'ERR_TEST_FAILURE' + ... + # Subtest: test 2 + not ok 2 - test 2 + --- + duration_ms: ZERO + failureType: 'cancelledByParent' + error: 'test did not finish before its parent and was cancelled' + code: 'ERR_TEST_FAILURE' + ... + 1..2 +not ok 1 - 1 before describe + --- + duration_ms: * + type: 'suite' + failureType: 'hookFailed' + error: 'failed running before hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... +# Subtest: 2 after describe + # Subtest: test 1 + ok 1 - test 1 + --- + duration_ms: * + ... + # Subtest: test 2 + ok 2 - test 2 + --- + duration_ms: * + ... + 1..2 +not ok 2 - 2 after describe + --- + duration_ms: * + type: 'suite' + failureType: 'hookFailed' + error: 'failed running after hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... +# Subtest: 3 beforeEach describe + # Subtest: test 1 + not ok 1 - test 1 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running beforeEach hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + async Promise.all (index 0) + ... + # Subtest: test 2 + not ok 2 - test 2 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running beforeEach hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + async Promise.all (index 0) + ... + 1..2 +not ok 3 - 3 beforeEach describe + --- + duration_ms: * + type: 'suite' + failureType: 'subtestsFailed' + error: '2 subtests failed' + code: 'ERR_TEST_FAILURE' + ... +# Subtest: 4 afterEach describe + # Subtest: test 1 + not ok 1 - test 1 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running afterEach hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + # Subtest: test 2 + not ok 2 - test 2 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running afterEach hook' + code: 20 + name: 'AbortError' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + 1..2 +not ok 4 - 4 afterEach describe + --- + duration_ms: * + type: 'suite' + failureType: 'subtestsFailed' + error: '2 subtests failed' + code: 'ERR_TEST_FAILURE' + ... +1..4 +# tests 8 +# suites 4 +# pass 2 +# fail 4 +# cancelled 2 +# skipped 0 +# todo 0 +# duration_ms * diff --git a/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js similarity index 100% rename from test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js rename to test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js diff --git a/test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot similarity index 100% rename from test/fixtures/test-runner/output/timeout-in-before-each-should-not-affect-further-tests.snapshot rename to test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index c36e43e5c0cc35..cd26c805f7f238 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -37,7 +37,7 @@ const tests = [ { name: 'test-runner/output/describe_it.js' }, { name: 'test-runner/output/describe_nested.js' }, { name: 'test-runner/output/hooks.js' }, - { name: 'test-runner/output/timeout-in-before-each-should-not-affect-further-tests.js' }, + { name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' }, { name: 'test-runner/output/hooks-with-no-global-test.js' }, { name: 'test-runner/output/no_refs.js' }, { name: 'test-runner/output/no_tests.js' }, From 32d225a992f9bc664889e54e865d99f197a66850 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 22:51:54 +0300 Subject: [PATCH 4/9] change to public --- lib/internal/test_runner/test.js | 38 +++++++++++++------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 058e5c14cbca85..99d405ae60153b 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -164,8 +164,8 @@ class SuiteContext { } class Test extends AsyncResource { - #abortController; - #outerSignal; + abortController; + outerSignal; #reportedSubtest; constructor(options) { @@ -263,16 +263,16 @@ class Test extends AsyncResource { fn = noop; } - this.#abortController = new AbortController(); - this.#outerSignal = signal; - this.signal = this.#abortController.signal; + this.abortController = new AbortController(); + this.outerSignal = signal; + this.signal = this.abortController.signal; validateAbortSignal(signal, 'options.signal'); if (signal) { kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation; } - this.#outerSignal?.addEventListener( + this.outerSignal?.addEventListener( 'abort', this.#abortHandler, { __proto__: null, [kResistStopPropagation]: true }, @@ -412,7 +412,7 @@ class Test extends AsyncResource { } #abortHandler = () => { - const error = this.#outerSignal?.reason || new AbortError('The test was aborted'); + const error = this.outerSignal?.reason || new AbortError('The test was aborted'); error.failureType = kAborted; this.#cancel(error); }; @@ -430,7 +430,7 @@ class Test extends AsyncResource { ); this.startTime = this.startTime || this.endTime; // If a test was canceled before it was started, e.g inside a hook this.cancelled = true; - this.#abortController.abort(); + this.abortController.abort(); } createHook(name, fn, options) { @@ -498,7 +498,7 @@ class Test extends AsyncResource { if (this.signal.aborted) { return true; } - if (this.#outerSignal?.aborted) { + if (this.outerSignal?.aborted) { this.#abortHandler(); return true; } @@ -606,7 +606,7 @@ class Test extends AsyncResource { // 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(); + this.abortController.abort(); } } @@ -646,7 +646,7 @@ class Test extends AsyncResource { this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed)); } - this.#outerSignal?.removeEventListener('abort', this.#abortHandler); + this.outerSignal?.removeEventListener('abort', this.#abortHandler); this.mock?.reset(); if (this.parent !== null) { @@ -681,16 +681,6 @@ class Test extends AsyncResource { } } - wasAbortedByUser() { - return this.#outerSignal?.aborted; - } - - recreateAbortController() { - this.#abortController.abort(); - this.#abortController = new AbortController(); - this.signal = this.#abortController.signal; - } - isClearToSend() { return this.parent === null || ( @@ -772,10 +762,12 @@ class TestHook extends Test { super({ __proto__: null, fn, timeout, signal }); } run(args) { - if (this.error && !this.wasAbortedByUser()) { + if (this.error && !this.outerSignal?.aborted) { this.passed = false; this.error = null; - super.recreateAbortController(); + this.abortController.abort(); + this.abortController = new AbortController(); + this.signal = this.abortController.signal; } this.#args = args; From 69853c95be881199c632068ad9fe6a607e819a73 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 28 Jul 2023 11:06:51 +0300 Subject: [PATCH 5/9] try fixing the failed test --- ...eout_in_before_each_should_not_affect_further_tests.js | 8 ++++---- ...n_before_each_should_not_affect_further_tests.snapshot | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js index 4f46154f0ab05f..19b32308f80cb5 100644 --- a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js +++ b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js @@ -15,11 +15,11 @@ describe('before each timeout', () => { }, {timeout: 20}); test('first describe first test', () => { - console.log('before each test first'); + console.log('before each test first ' + i); }); test('first describe second test', () => { - console.log('before each test second'); + console.log('before each test second ' + i); }); }); @@ -37,10 +37,10 @@ describe('after each timeout', () => { }, {timeout: 20}); test('second describe first test', () => { - console.log('after each test first'); + console.log('after each test first ' + i); }); test('second describe second test', () => { - console.log('after each test second'); + console.log('after each test second ' + i); }); }); diff --git a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot index 1518b5ca13108b..cac7facf893309 100644 --- a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot +++ b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.snapshot @@ -1,8 +1,8 @@ gonna timeout TAP version 13 not gonna timeout -before each test second -after each test first +before each test second 2 +after each test first 0 gonna timeout # Subtest: before each timeout # Subtest: first describe first test @@ -29,7 +29,7 @@ not ok 1 - before each timeout error: '1 subtest failed' code: 'ERR_TEST_FAILURE' ... -after each test second +after each test second 1 not gonna timeout # Subtest: after each timeout # Subtest: second describe first test From 0a558cd0d8397e3d6088a20437a6679d601947bb Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 28 Jul 2023 19:07:18 +0300 Subject: [PATCH 6/9] try something --- lib/internal/test_runner/test.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 99d405ae60153b..a46c7b064fca06 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -511,17 +511,26 @@ class Test extends AsyncResource { async runHook(hook, args) { validateOneOf(hook, 'hook name', kHookNames); + let failedHook; try { await ArrayPrototypeReduce(this.hooks[hook], async (prev, hook) => { await prev; await hook.run(args); if (hook.error) { + failedHook = hook; throw hook.error; } }, PromiseResolve()); } catch (err) { const error = new ERR_TEST_FAILURE(`failed running ${hook} hook`, kHookFailure); error.cause = isTestFailureError(err) ? err.cause : err; + if (failedHook && !failedHook.outerSignal?.aborted) { + failedHook.passed = false; + failedHook.error = null; + failedHook.abortController.abort(); + failedHook.abortController = new AbortController(); + failedHook.signal = this.abortController.signal; + } throw error; } } @@ -762,14 +771,6 @@ class TestHook extends Test { super({ __proto__: null, fn, timeout, signal }); } run(args) { - if (this.error && !this.outerSignal?.aborted) { - this.passed = false; - this.error = null; - this.abortController.abort(); - this.abortController = new AbortController(); - this.signal = this.abortController.signal; - } - this.#args = args; return super.run(); } From f879b4004a8ee6950db4bc5f041b98a77d80e0cc Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sat, 29 Jul 2023 22:52:49 +0300 Subject: [PATCH 7/9] revert --- lib/internal/test_runner/test.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index a46c7b064fca06..99d405ae60153b 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -511,26 +511,17 @@ class Test extends AsyncResource { async runHook(hook, args) { validateOneOf(hook, 'hook name', kHookNames); - let failedHook; try { await ArrayPrototypeReduce(this.hooks[hook], async (prev, hook) => { await prev; await hook.run(args); if (hook.error) { - failedHook = hook; throw hook.error; } }, PromiseResolve()); } catch (err) { const error = new ERR_TEST_FAILURE(`failed running ${hook} hook`, kHookFailure); error.cause = isTestFailureError(err) ? err.cause : err; - if (failedHook && !failedHook.outerSignal?.aborted) { - failedHook.passed = false; - failedHook.error = null; - failedHook.abortController.abort(); - failedHook.abortController = new AbortController(); - failedHook.signal = this.abortController.signal; - } throw error; } } @@ -771,6 +762,14 @@ class TestHook extends Test { super({ __proto__: null, fn, timeout, signal }); } run(args) { + if (this.error && !this.outerSignal?.aborted) { + this.passed = false; + this.error = null; + this.abortController.abort(); + this.abortController = new AbortController(); + this.signal = this.abortController.signal; + } + this.#args = args; return super.run(); } From 438831e9a8a6200521c8fa4a90c6aeef8e29c88c Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 30 Jul 2023 07:14:25 +0300 Subject: [PATCH 8/9] update snapshot after main update --- .../fixtures/test-runner/output/abort_hooks.snapshot | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot index 0c74f2c5b7967e..a1b5ddcd5f1908 100644 --- a/test/fixtures/test-runner/output/abort_hooks.snapshot +++ b/test/fixtures/test-runner/output/abort_hooks.snapshot @@ -30,7 +30,7 @@ not ok 1 - 1 before describe duration_ms: * type: 'suite' failureType: 'hookFailed' - error: 'failed running before hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- @@ -62,7 +62,7 @@ not ok 2 - 2 after describe duration_ms: * type: 'suite' failureType: 'hookFailed' - error: 'failed running after hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- @@ -83,7 +83,7 @@ not ok 2 - 2 after describe --- duration_ms: * failureType: 'hookFailed' - error: 'failed running beforeEach hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- @@ -103,7 +103,7 @@ not ok 2 - 2 after describe --- duration_ms: * failureType: 'hookFailed' - error: 'failed running beforeEach hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- @@ -133,7 +133,7 @@ not ok 3 - 3 beforeEach describe --- duration_ms: * failureType: 'hookFailed' - error: 'failed running afterEach hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- @@ -153,7 +153,7 @@ not ok 3 - 3 beforeEach describe --- duration_ms: * failureType: 'hookFailed' - error: 'failed running afterEach hook' + error: 'This operation was aborted' code: 20 name: 'AbortError' stack: |- From da8e5fd2dd40ab39b6703e9061ffbb9f1256d65e Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 30 Jul 2023 18:51:12 +0300 Subject: [PATCH 9/9] fix failure in linux (low ms have problem) --- ..._before_each_should_not_affect_further_tests.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js index 19b32308f80cb5..6205e2c403fc86 100644 --- a/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js +++ b/test/fixtures/test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js @@ -7,12 +7,12 @@ describe('before each timeout', () => { beforeEach(async () => { if (i++ === 0) { - console.log('gonna timeout') - await setTimeout(50); + console.log('gonna timeout'); + await setTimeout(700); return; } console.log('not gonna timeout'); - }, {timeout: 20}); + }, {timeout: 500}); test('first describe first test', () => { console.log('before each test first ' + i); @@ -27,14 +27,14 @@ describe('before each timeout', () => { describe('after each timeout', () => { let i = 0; - afterEach(async () => { + afterEach(async function afterEach1() { if (i++ === 0) { - console.log('gonna timeout') - await setTimeout(50); + console.log('gonna timeout'); + await setTimeout(700); return; } console.log('not gonna timeout'); - }, {timeout: 20}); + }, {timeout: 500}); test('second describe first test', () => { console.log('after each test first ' + i);