Skip to content

Commit 046d993

Browse files
cjihrigmarco-ippolito
authored andcommitted
test_runner: make end of work check stricter
This commit updates the logic that checks for the end of the test run. Prior to this change, it was possible for root.run() to be called multiple times because of the way pending subtests were tracked. The extra calls to root.run() were harmless, but could trigger an EventEmitter leak warning due to 'abort' listeners being created. PR-URL: nodejs#52326 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent cd1415c commit 046d993

File tree

2 files changed

+20
-0
lines changed

2 files changed

+20
-0
lines changed

lib/internal/test_runner/test.js

+9
Original file line numberDiff line numberDiff line change
@@ -778,11 +778,20 @@ class Test extends AsyncResource {
778778
this.reporter.complete(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
779779

780780
this.parent.activeSubtests--;
781+
// The call to processPendingSubtests() below can change the number of
782+
// pending subtests. When detecting if we are done running tests, we want
783+
// to check if there are no pending subtests both before and after
784+
// calling processPendingSubtests(). Otherwise, it is possible to call
785+
// root.run() multiple times (which is harmless but can trigger an
786+
// EventEmitter leak warning).
787+
const pendingSiblingCount = this.parent.pendingSubtests.length;
788+
781789
this.parent.addReadySubtest(this);
782790
this.parent.processReadySubtestRange(false);
783791
this.parent.processPendingSubtests();
784792

785793
if (this.parent === this.root &&
794+
pendingSiblingCount === 0 &&
786795
this.root.activeSubtests === 0 &&
787796
this.root.pendingSubtests.length === 0 &&
788797
this.root.readySubtests.size === 0) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Flags: --test-only
2+
'use strict';
3+
const common = require('../common');
4+
const { test } = require('node:test');
5+
const { defaultMaxListeners } = require('node:events');
6+
7+
process.on('warning', common.mustNotCall());
8+
9+
for (let i = 0; i < defaultMaxListeners + 1; ++i) {
10+
test(`test ${i + 1}`);
11+
}

0 commit comments

Comments
 (0)