Skip to content

Commit

Permalink
test_runner: call abort on test finish
Browse files Browse the repository at this point in the history
PR-URL: #48827
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
  • Loading branch information
rluvaton authored and targos committed Oct 28, 2023
1 parent fe41d22 commit 2d4511b
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 18 deletions.
6 changes: 6 additions & 0 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions test/fixtures/test-runner/aborts/failed-test-still-call-abort.js
Original file line number Diff line number Diff line change
@@ -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');
});
Original file line number Diff line number Diff line change
@@ -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);
});
19 changes: 19 additions & 0 deletions test/fixtures/test-runner/aborts/wait-for-abort-helper.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
111 changes: 93 additions & 18 deletions test/parallel/test-runner-run.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -118,28 +118,103 @@ 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);
});
});
});

0 comments on commit 2d4511b

Please sign in to comment.