doc: update test concurrency description / default value #46457

richiemccoll · 2023-02-01T08:24:23Z

This PR aligns the documentation for the test runner concurrency options. The description for context.test and run are now consistent with the test section as they both allow number or boolean for concurrency.

I've also updated:

the default value for context.test from 1 to false so that it's consistent with test concurrency default.
the default value for run from true to false so that it's consistent with the test/context.test defaults.

nodejs-github-bot · 2023-02-01T08:24:29Z

Review requested:

@nodejs/test_runner

doc/api/test.md

aduh95 · 2023-02-01T13:41:40Z

Other occurrences of concurrency default being documented:

node/doc/api/test.md

Lines 679 to 685 in 1ecc03e

    
           * `concurrency` {number|boolean} If a number is provided, 
        
             then that many files would run in parallel. 
        
             If truthy, it would run (number of cpu cores - 1) 
        
             files in parallel. 
        
             If falsy, it would only run one file at a time. 
        
             If unspecified, subtests inherit this value from their parent. 
        
             **Default:** `true`.

node/doc/api/test.md

Lines 1648 to 1650 in 1ecc03e

    
           * `concurrency` {number} The number of tests that can be run at the same time. 
        
             If unspecified, subtests inherit this value from their parent. 
        
             **Default:** `1`.

It'd be nice to make all of them be consistent w.r.t to truthy/true etc.

richiemccoll · 2023-02-01T15:13:01Z

@aduh95 Thanks. The options.concurrency description and default values for test, context.text and run should be consistent now.

aduh95

The default is actually neither 1 nor false for t.test, the default is to inherit from the parent, let's fix that.

doc/api/test.md

GeoffreyBooth · 2023-02-02T18:53:03Z

cc @JakobJingleheimer

JakobJingleheimer · 2023-02-02T20:57:20Z

This updates only the documentation, but no implementation. Was the documentation merely incorrect? (I'm pretty sure at least 1 of the changes does correct the doc to the actual behaviour).

If so, happy to switch to approve.

richiemccoll · 2023-02-03T10:30:06Z

This updates only the documentation, but no implementation. Was the documentation merely incorrect? (I'm pretty sure at > least 1 of the changes does correct the doc to the actual behaviour).

@JakobJingleheimer Yes, the documentation was incorrect regarding subtest options.concurrency. The other changes are small clarifications regarding os. availableParallelism - 1 and subtest default value.

doc/api/test.md

MoLow · 2023-02-05T08:27:19Z

@richiemccoll linter is failing

richiemccoll · 2023-02-06T07:57:23Z

@richiemccoll linter is failing

@MoLow Thanks. Pushed a fix.

nodejs-github-bot · 2023-02-06T10:38:04Z

Commit Queue failed

https://github.com/nodejs/node/actions/runs/4102747738

PR-URL: #46457 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>

MoLow · 2023-02-06T10:52:52Z

Landed in 7d68b7b

aduh95 · 2023-02-06T11:04:29Z

Thanks a lot for the contribution @richiemccoll and congrats on your first commit landed on nodejs/node 🎉

richiemccoll · 2023-02-06T12:13:39Z

Thanks a lot for the contribution @richiemccoll and congrats on your first commit landed on nodejs/node

Thanks for the reviews and feedback! @aduh95 @MoLow

PR-URL: nodejs/node#46457 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> (cherry picked from commit 7d68b7bbfc9ffbd2ad0913972ac0b1a315679b06)

PR-URL: #46457 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>

nodejs-github-bot added doc Issues and PRs related to the documentations. dont-land-on-v14.x test_runner labels Feb 1, 2023