Skip to content

Commit

Permalink
Revert "test_runner: run global after() hook earlier"
Browse files Browse the repository at this point in the history
This reverts commit 6346bdc.

Reason for revert: breaking CI

PR-URL: nodejs#49110
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
joyeecheung committed Aug 12, 2023
1 parent e117f16 commit bb52656
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 104 deletions.
8 changes: 4 additions & 4 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -152,8 +152,8 @@ function setup(root) {
process.removeListener('uncaughtException', exceptionHandler);
};

const terminationHandler = () => {
exitHandler();
const terminationHandler = async () => {
await exitHandler();
process.exit();
};

Expand Down
27 changes: 4 additions & 23 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ class Test extends AsyncResource {
}
}

async run() {
async run(pendingSubtestsError) {
if (this.parent !== null) {
this.parent.activeSubtests++;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 0 additions & 13 deletions test/fixtures/test-runner/output/async-test-scheduling.mjs

This file was deleted.

37 changes: 0 additions & 37 deletions test/fixtures/test-runner/output/async-test-scheduling.snapshot

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.j
*
*
*
*
...
1..1
# tests 1
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 0 additions & 26 deletions test/parallel/test-runner-root-after-with-refed-handles.js

This file was deleted.

0 comments on commit bb52656

Please sign in to comment.