test_runner: global after not run if handles are open

Version

v20.5.0

Platform

mac

Subsystem

test_runner

What steps will reproduce the bug?

Considder the following test:

const { before, after, test } = require('node:test')
const { createServer } = require('http')

let server

before(async () => {
  console.log('before');
  server = createServer((req, res) => {
    res.end('hello')
  })

  await new Promise((resolve, reject) => {
    server.listen(0, (err) => {
      if (err) reject(err)
      else resolve()
    })
  })
})

after(() => {
  console.log('after');
  server.close()
})

test('something', () => {
  console.log('test');
})

We are trying to dispose of a server (or a connection to a DB) inside a global after but the global after is never run.

How often does it reproduce? Is there a required condition?

all the times

What is the expected behavior? Why is that the expected behavior?

for the test to pass / the after hook to be executed

What do you see instead?

the after hook is not executed and therefore the test never terminates

Additional information

No response

I'm guessing this is because the test runner uses the beforeExit event, and the ref'ed handle prevents that from being emitted. We'll need to move to a different mechanism for detecting when all of the tests have run.

I guessed as much, however the above pattern is really good.

I looked at this really quickly. The good news is that it's pretty simple to fix this case:

diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 36c36f2de1..e9076eb378 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -158,7 +158,6 @@ function setup(root) {
 
   process.on('uncaughtException', exceptionHandler);
   process.on('unhandledRejection', rejectionHandler);
-  process.on('beforeExit', exitHandler);
   // TODO(MoLow): Make it configurable to hook when isTestRunner === false.
   if (globalOptions.isTestRunner) {
     process.on('SIGINT', terminationHandler);
@@ -180,6 +179,7 @@ function setup(root) {
       topLevel: 0,
       suites: 0,
     },
+    exitHandler,
     shouldColorizeTestFiles: false,
   };
   root.startTime = hrtime();
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index cc7c81cad8..fd22433a3d 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -687,6 +687,13 @@ 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.harness.exitHandler();
+      }
     } else if (!this.reported) {
       if (!this.passed && failed === 0 && this.error) {
         this.reporter.fail(0, kFilename, this.subtests.length + 1, kFilename, {

The bad news is that a couple tests are failing. I think it's just related to handling asynchronous activity after the tests finish running like:

test('extraneous async activity test', () => {
  setTimeout(() => { throw new Error(); }, 100);
});

This is kind of expected since the test runner finishes ASAP now instead of once the process is getting ready to exit. I'll have to keep looking into how to best handle this case.

@giltayar

CC @giltayar we might have discussed this use-case or a similar one in Node.TLV

test_runner: run global after() hook earlier

Navigation Menu

Repository navigation

test_runner: global after not run if handles are open #49056

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

test_runner: global after not run if handles are open #49056

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Activity

cjihrig commented on Aug 7, 2023

mcollina commented on Aug 7, 2023

cjihrig commented on Aug 7, 2023

MoLow commented on Aug 11, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions