Skip to content

Commit b4ccb6c

Browse files
cjihrigaduh95
authored andcommitted
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: #52488 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
1 parent b00766d commit b4ccb6c

File tree

1 file changed

+9
-20
lines changed

1 file changed

+9
-20
lines changed

lib/internal/test_runner/test.js

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -829,29 +829,9 @@ class Test extends AsyncResource {
829829
this.parent.activeSubtests--;
830830
}
831831

832-
// The call to processPendingSubtests() below can change the number of
833-
// pending subtests. When detecting if we are done running tests, we want
834-
// to check if there are no pending subtests both before and after
835-
// calling processPendingSubtests(). Otherwise, it is possible to call
836-
// root.run() multiple times (which is harmless but can trigger an
837-
// EventEmitter leak warning).
838-
const pendingSiblingCount = this.parent.pendingSubtests.length;
839-
840832
this.parent.addReadySubtest(this);
841833
this.parent.processReadySubtestRange(false);
842834
this.parent.processPendingSubtests();
843-
844-
if (this.parent === this.root &&
845-
pendingSiblingCount === 0 &&
846-
this.root.activeSubtests === 0 &&
847-
this.root.pendingSubtests.length === 0 &&
848-
this.root.readySubtests.size === 0) {
849-
// At this point all of the tests have finished running. However, there
850-
// might be ref'ed handles keeping the event loop alive. This gives the
851-
// global after() hook a chance to clean them up. The user may also
852-
// want to force the test runner to exit despite ref'ed handles.
853-
this.root.run();
854-
}
855835
} else if (!this.reported) {
856836
const {
857837
diagnostics,
@@ -914,6 +894,15 @@ class Test extends AsyncResource {
914894
this.report();
915895
this.parent.waitingOn++;
916896
this.finished = true;
897+
898+
if (this.parent === this.root &&
899+
this.root.waitingOn > this.root.subtests.length) {
900+
// At this point all of the tests have finished running. However, there
901+
// might be ref'ed handles keeping the event loop alive. This gives the
902+
// global after() hook a chance to clean them up. The user may also
903+
// want to force the test runner to exit despite ref'ed handles.
904+
this.root.run();
905+
}
917906
}
918907

919908
duration() {

0 commit comments

Comments
 (0)