From b4ccb6c626256c0abc29b67427460a51c8b68595 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 11 Apr 2024 20:52:20 -0400 Subject: [PATCH] test_runner: move end of work check to finalize() This commit moves the end of work check from postRun() to finalize(). The reasoning is that finalize() is guaranteed to run in the order that the tests are defined, while postRun() is not. This makes the check a little simpler. PR-URL: https://github.com/nodejs/node/pull/52488 Reviewed-By: Yagiz Nizipli Reviewed-By: Chemi Atlow Reviewed-By: Benjamin Gruenbaum Reviewed-By: Marco Ippolito Reviewed-By: Moshe Atlow --- lib/internal/test_runner/test.js | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9c7d95f9420de5..8bf63654bf1f21 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -829,29 +829,9 @@ class Test extends AsyncResource { this.parent.activeSubtests--; } - // The call to processPendingSubtests() below can change the number of - // pending subtests. When detecting if we are done running tests, we want - // to check if there are no pending subtests both before and after - // calling processPendingSubtests(). Otherwise, it is possible to call - // root.run() multiple times (which is harmless but can trigger an - // EventEmitter leak warning). - const pendingSiblingCount = this.parent.pendingSubtests.length; - this.parent.addReadySubtest(this); this.parent.processReadySubtestRange(false); this.parent.processPendingSubtests(); - - if (this.parent === this.root && - pendingSiblingCount === 0 && - this.root.activeSubtests === 0 && - this.root.pendingSubtests.length === 0 && - this.root.readySubtests.size === 0) { - // At this point all of the tests have finished running. However, there - // might be ref'ed handles keeping the event loop alive. This gives the - // global after() hook a chance to clean them up. The user may also - // want to force the test runner to exit despite ref'ed handles. - this.root.run(); - } } else if (!this.reported) { const { diagnostics, @@ -914,6 +894,15 @@ class Test extends AsyncResource { this.report(); this.parent.waitingOn++; this.finished = true; + + if (this.parent === this.root && + this.root.waitingOn > this.root.subtests.length) { + // At this point all of the tests have finished running. However, there + // might be ref'ed handles keeping the event loop alive. This gives the + // global after() hook a chance to clean them up. The user may also + // want to force the test runner to exit despite ref'ed handles. + this.root.run(); + } } duration() {