Skip to content

Commit

Permalink
test_runner: fix timeout in *Each hook failing further tests
Browse files Browse the repository at this point in the history
PR-URL: nodejs#48925
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
  • Loading branch information
rluvaton authored and Ceres6 committed Aug 14, 2023
1 parent b8a4981 commit f248697
Show file tree
Hide file tree
Showing 7 changed files with 385 additions and 15 deletions.
30 changes: 19 additions & 11 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ class SuiteContext {
}

class Test extends AsyncResource {
#abortController;
#outerSignal;
abortController;
outerSignal;
#reportedSubtest;

constructor(options) {
Expand Down Expand Up @@ -292,16 +292,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 },
Expand Down Expand Up @@ -441,7 +441,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);
};
Expand All @@ -459,7 +459,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) {
Expand Down Expand Up @@ -527,7 +527,7 @@ class Test extends AsyncResource {
if (this.signal.aborted) {
return true;
}
if (this.#outerSignal?.aborted) {
if (this.outerSignal?.aborted) {
this.#abortHandler();
return true;
}
Expand Down Expand Up @@ -639,7 +639,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();
}
}

Expand Down Expand Up @@ -679,7 +679,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) {
Expand Down Expand Up @@ -795,6 +795,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();
}
Expand Down
63 changes: 63 additions & 0 deletions test/fixtures/test-runner/output/abort_hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// 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');
});
});
188 changes: 188 additions & 0 deletions test/fixtures/test-runner/output/abort_hooks.snapshot
Original file line number Diff line number Diff line change
@@ -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: 'This operation was aborted'
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: 'This operation was aborted'
code: 20
name: 'AbortError'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: 3 beforeEach describe
# Subtest: test 1
not ok 1 - test 1
---
duration_ms: *
failureType: 'hookFailed'
error: 'This operation was aborted'
code: 20
name: 'AbortError'
stack: |-
*
*
*
*
*
*
*
*
*
async Promise.all (index 0)
...
# Subtest: test 2
not ok 2 - test 2
---
duration_ms: *
failureType: 'hookFailed'
error: 'This operation was aborted'
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: 'This operation was aborted'
code: 20
name: 'AbortError'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: test 2
not ok 2 - test 2
---
duration_ms: *
failureType: 'hookFailed'
error: 'This operation was aborted'
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 *
4 changes: 0 additions & 4 deletions test/fixtures/test-runner/output/hooks.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ not ok 3 - after throws
*
*
*
async Promise.all (index 0)
*
*
...
1..2
Expand Down Expand Up @@ -183,7 +181,6 @@ not ok 4 - beforeEach throws
*
*
*
async Promise.all (index 0)
*
...
1..2
Expand Down Expand Up @@ -265,7 +262,6 @@ not ok 6 - afterEach when test fails
*
*
*
async Promise.all (index 0)
*
...
1..2
Expand Down
Loading

0 comments on commit f248697

Please sign in to comment.