test_runner: allow running test files in single process

[MoLow]

Fixes: https://github.com/nodejs/node/issues/51548

this PR is still missing tests & documentation

[nodejs-github-bot]

Review requested:

• [ ] @nodejs/test_runner

[github-actions[bot]]

The https://github.com/nodejs/node/labels/notable-change label has been added by @MoLow.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[rluvaton]

Could you please add a test for before/after all hooks and make sure they are not executed multiple times for each file?

Couple of thoughts:

I think in case of no isolation we should set the --test-concurrency default value to be 1

If there is concurrency, should it run in different process or same one?

[rluvaton]

Can you add a test that we still capture the stdout and sterr correctly?

[MoLow]

I'v ended up adding a single process as an entry point for importing all files - this way we support features such as coverage, watch mode, timeout etc. @nodejs/test_runner please take a look. @rluvaton are there still tests you think should be added?

[rluvaton]

@rluvaton are there still tests you think should be added?

The tests that I wrote in my prev comment, the implementation detail should not affect the test cases

Also, there are some thoughts regarding concurrency that seem like you missed

[MoLow]

Also, there are some thoughts regarding concurrency that seem like you missed

can you elaborate?

[rluvaton]

I think in case of no isolation we should set the --test-concurrency default value to be 1

Having concurrent test can lead to flaky tests when mutating some global state

If there is concurrency, should it run in different process or same one?

Because it can cause flakiness, I would assume concurrency will run in different processes each process won't isolation

[cjihrig]

I think it makes sense to document that --test-concurrency will be 1 when running everything in the same process.

@MoLow once this lands, do you think it will be feasible to make --test-only only necessary when running multiple processes? It seems like it to me, but I haven't reviewed this code yet (there are conflicts). If we could manage that, I think it would be a huge win.

[MoLow]

@MoLow once this lands, do you think it will be feasible to make --test-only only necessary when running multiple processes? It seems like it to me, but I haven't reviewed this code yet (there are conflicts). If we could manage that, I think it would be a huge win.

Yes, I think that will be possible. Il try getting back to this PR this week

[adrian-85]

Having concurrent test can lead to flaky tests when mutating some global state

I strongly feel this is the responsibility of the test author to ensure they do not create dependencies between tests, and properly manage variables in the global context. I don't think node should have to baby-sit anyone.

Jest allows for certain globals, and they make the same very clear in their docs.