Revert "test_runner: run global after() hook earlier"

This reverts commit 6346bdc.

Reason for revert: breaking CI

PR-URL: nodejs#49110
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

lib/internal/test_runner/harness.js

@@ -142,8 +142,8 @@ function setup(root) {
   const rejectionHandler =
     createProcessEventHandler('unhandledRejection', root);
   const coverage = configureCoverage(root, globalOptions);
-  const exitHandler = () => {
-    root.postRun(new ERR_TEST_FAILURE(
+  const exitHandler = async () => {
+    await root.run(new ERR_TEST_FAILURE(
       'Promise resolution is still pending but the event loop has already resolved',
       kCancelledByParent));
 
@@ -152,8 +152,8 @@ function setup(root) {
     process.removeListener('uncaughtException', exceptionHandler);
   };
 
-  const terminationHandler = () => {
-    exitHandler();
+  const terminationHandler = async () => {
+    await exitHandler();
     process.exit();
   };
 

lib/internal/test_runner/test.js

@@ -574,7 +574,7 @@ class Test extends AsyncResource {
     }
   }
 
-  async run() {
+  async run(pendingSubtestsError) {
     if (this.parent !== null) {
       this.parent.activeSubtests++;
     }
@@ -662,16 +662,9 @@ class Test extends AsyncResource {
       }
     }
 
-    if (this.parent !== null) {
-      // Clean up the test. Then, try to report the results and execute any
-      // tests that were pending due to available concurrency.
-      //
-      // The root test is skipped here because it is a special case. Its
-      // postRun() method is called when the process is getting ready to exit.
-      // This helps catch any asynchronous activity that occurs after the tests
-      // have finished executing.
-      this.postRun();
-    }
+    // Clean up the test. Then, try to report the results and execute any
+    // tests that were pending due to available concurrency.
+    this.postRun(pendingSubtestsError);
   }
 
   postRun(pendingSubtestsError) {
@@ -713,18 +706,6 @@ class Test extends AsyncResource {
       this.parent.addReadySubtest(this);
       this.parent.processReadySubtestRange(false);
       this.parent.processPendingSubtests();
-
-      if (this.parent === this.root &&
-          this.root.activeSubtests === 0 &&
-          this.root.pendingSubtests.length === 0 &&
-          this.root.readySubtests.size === 0 &&
-          this.root.hooks.after.length > 0) {
-        // This is done so that any global after() hooks are run. 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.
-        this.root.run();
-      }
     } else if (!this.reported) {
       const {
         diagnostics,

test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot

@@ -22,6 +22,7 @@ not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.j
     *
     *
     *
+    *
   ...
 1..1
 # tests 1

test/parallel/test-runner-output.mjs

@@ -74,7 +74,6 @@ const tests = [
   { name: 'test-runner/output/unresolved_promise.js' },
   { name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
   { name: 'test-runner/output/arbitrary-output.js' },
-  { name: 'test-runner/output/async-test-scheduling.mjs' },
   !skipForceColors ? {
     name: 'test-runner/output/arbitrary-output-colored.js',
     transform: snapshot.transform(specTransform, replaceTestDuration), tty: true