Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: clarify concurrency model of test runner #47642

Merged
merged 1 commit into from
Apr 24, 2023
+9 −3
Merged

doc: clarify concurrency model of test runner #47642

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions doc/api/test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,11 @@ Otherwise, the test is considered to be a failure. Test files must be
executable by Node.js, but are not required to use the `node:test` module
internally.

Each test file is executed as if it was a regular script. That is, if the test
file itself uses `node:test` to define tests, all of those tests will be
executed within a single application thread, regardless of the value of the
`concurrency` option of [`test()`][].

## Collecting code coverage

> Stability: 1 - Experimental
Expand Down Expand Up @@ -731,7 +736,8 @@ added:
* `options` {Object} Configuration options for running tests. The following
properties are supported:
* `concurrency` {number|boolean} If a number is provided,
then that many files would run in parallel.
then that many test processes would run in parallel, where each process
corresponds to one test file.
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
If `true`, it would run `os.availableParallelism() - 1` test files in
parallel.
If `false`, it would only run one test file at a time.
Expand Down Expand Up @@ -795,7 +801,7 @@ changes:
* `options` {Object} Configuration options for the test. The following
properties are supported:
* `concurrency` {number|boolean} If a number is provided,
then that many tests would run in parallel.
then that many tests would run in parallel within the application thread.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this adds clarity?

Suggested change
then that many tests would run in parallel within the application thread.
then that many tests would run in parallel within the current thread, similar to a set of promises awaited by `Promise.allSettled`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think using current vs application does a great job at clarifying things. Why do you think it's important to call out Promise.allSettled vs anything else? I don't think it's helpful, it might even be a bit confusing given that the code doesn't use Promise.allSettled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m trying to get across that it’s “in parallel” only in the sense that there are several async resources running within the current thread and process, as opposed to “true” parallelism from using multiple threads/processes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #47642 (review) is a better suggestion, I agree that "parallel" is simply not a great word to describe what happens here, but let's take that discussion to another PR/issue.

If `true`, it would run `os.availableParallelism() - 1` tests in parallel.
For subtests, it will be `Infinity` tests in parallel.
If `false`, it would only run one test at a time.
Expand Down Expand Up @@ -1766,7 +1772,7 @@ changes:
* `options` {Object} Configuration options for the subtest. The following
properties are supported:
* `concurrency` {number|boolean|null} If a number is provided,
then that many tests would run in parallel.
then that many tests would run in parallel within the application thread.
If `true`, it would run all subtests in parallel.
If `false`, it would only run one test at a time.
If unspecified, subtests inherit this value from their parent.
Expand Down