node:test run() w/ isolation=none waits for beforeExit

[aecepoglu]

Version

23.4

Platform

Linux minitower 6.12.9-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 10 Jan 2025 00:39:41 +0000 x86_64 GNU/Linux

Subsystem

node:test

What steps will reproduce the bug?

const {run} = require('node:test');

const t0 = Date.now()
const s = run({
  isolation: 'none',
  files: [__dirname + '/code.test.js'],
  // timeout: 2000, // no effect
  // forceExit: true, // no effect
});
[ 'close', 'error', 'data', 'end', 'finish' ].forEach(e => {
  s.on(e, x => console.log(Date.now() - t0, e, x?.type))
});

setTimeout(() => {console.log('************* t=3000')}, 3000)
/*
5 data test:enqueue
5 data test:dequeue
5 data test:enqueue
6 data test:complete
6 data test:start
6 data test:pass
6 data test:dequeue
6 data test:complete
6 data test:start
6 data test:fail
************* t=3000
3006 data test:plan
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:diagnostic
3006 data test:summary
3007 end undefined
3007 close undefined
*/

And if we were to emit a "beforeExit" event:

setTimeout(() => {
  console.log('************* beforeExit')
  process.emit('beforeExit')
}, 1500)

/*
4 data test:enqueue
5 data test:dequeue
5 data test:enqueue
6 data test:complete
6 data test:start
6 data test:pass
6 data test:dequeue
6 data test:complete
6 data test:start
6 data test:fail
************* beforeExit
1504 data test:plan
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:diagnostic
1504 data test:summary
1504 end undefined
1505 close undefined
************* t=3000
*/

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

100% reproduceable.

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

test runner should end after all planned tests are complete.

OR

it should allow me to explicitly declare my test plan so it knows after the Nth test that no more tests are gonig to be enqueued.

What do you see instead?

test runner waits for the "beforeExit" event before it terminates.

Additional information

No response

[pmarchini]

Hey @aecepoglu, thanks for opening this issue 🚀
Could I please ask you to clarify what your use case is and what exactly you feel is missing?

[aecepoglu]

Hey @aecepoglu, thanks for opening this issue 🚀 Could I please ask you to clarify what your use case is ...

From within a long running process, I want to be able to run a specific test (in the same process (isolation=none)).

... and what exactly you feel is missing?

What's missing is that the test run does not finish.

A run({isolation: "none"}) call waits for the process in which tests are ran to near its end of life before the test run finishes.

A test:summary event should be emitted. A close and end event should be emitted.
But they're not emitted until the process receives a beforeExit event.
The test runner shouldn't be dependent on a beforeExit event.

[pmarchini]

Thanks for the clarification!😁
Would you like to try opening a PR to address this issue?

[aecepoglu]

If we can agree on how it should work, then I may be able to solve it.

Let me know which of the below sound agreeable:

1. add plan :number to RunOptions. Assume the run is complete if the planned number of tests are complete.
2. change dependency from "beforeExit" event to something else (lets say "i-am-done-with-tests-thanks"). It can be echoed by the "beforeExit" event.
3. If we give up on dynamic test discovery, we'll always know when the last "test:complete" event is emitted.

[cjihrig]

There is already logic that detects when all previously known tests have finished. That could probably be leveraged. I don't think we should introduce a plan option to run() for this or any custom events.

[pmarchini]

There is already logic that detects when all previously known tests have finished. That could probably be leveraged. I don't think we should introduce a plan option to run() for this or any custom events.

+1

I'm planning to take a look at this in these days!