doc: update test concurrency description / default value by richiemccoll · Pull Request #46457 · nodejs/node · GitHub

Fixes: #45643.

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.

Review requested:

@nodejs/test_runner

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.

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

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

cc @JakobJingleheimer

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.

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.

@richiemccoll linter is failing

@richiemccoll linter is failing

@MoLow Thanks. Pushed a fix.

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>

Landed in 7d68b7b

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

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 Issues and PRs related to the test runner subsystem. labels Feb 1, 2023

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

richiemccoll force-pushed the doc/test-runner-context-concurrency branch from 85910bb to 0210c43 Compare February 1, 2023 09:37

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

Comment thread doc/api/test.md Outdated

Comment thread doc/api/test.md Outdated

Comment thread doc/api/test.md Outdated

richiemccoll force-pushed the doc/test-runner-context-concurrency branch 4 times, most recently from 2937cfc to ac76ec2 Compare February 1, 2023 10:10

richiemccoll force-pushed the doc/test-runner-context-concurrency branch from ac76ec2 to f23581b Compare February 1, 2023 15:07

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

Comment thread doc/api/test.md Outdated

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

aduh95 reviewed Feb 1, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

richiemccoll force-pushed the doc/test-runner-context-concurrency branch 2 times, most recently from ed6de70 to 75f89a6 Compare February 1, 2023 16:15

aduh95 mentioned this pull request Feb 2, 2023

Pull latest commits from node core nodejs/node-core-test#43

Merged

richiemccoll force-pushed the doc/test-runner-context-concurrency branch 2 times, most recently from d73bf28 to 5156131 Compare February 2, 2023 15:26

aduh95 requested a review from MoLow February 2, 2023 15:40

MoLow approved these changes Feb 2, 2023

View reviewed changes

lpinca approved these changes Feb 2, 2023

View reviewed changes

richiemccoll force-pushed the doc/test-runner-context-concurrency branch from 5156131 to 388b813 Compare February 3, 2023 10:17

JakobJingleheimer approved these changes Feb 3, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

MoLow reviewed Feb 4, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

richiemccoll force-pushed the doc/test-runner-context-concurrency branch 3 times, most recently from 4d9eae5 to 1f61d56 Compare February 5, 2023 06:37

MoLow approved these changes Feb 5, 2023

View reviewed changes

doc: update test concurrency description / default values

2109a3d

richiemccoll force-pushed the doc/test-runner-context-concurrency branch from 1f61d56 to 2109a3d Compare February 6, 2023 07:56

MoLow approved these changes Feb 6, 2023

View reviewed changes

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 6, 2023

MoLow pushed a commit that referenced this pull request Feb 6, 2023

doc: update test concurrency description / default values

7d68b7b

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 closed this Feb 6, 2023

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023

doc: update test concurrency description / default values

375bb22

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>

MylesBorins mentioned this pull request Feb 19, 2023

v19.7.0 proposal #46725

Merged

danielleadams pushed a commit that referenced this pull request Apr 11, 2023

doc: update test concurrency description / default values

2b702c9

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>

danielleadams mentioned this pull request Apr 11, 2023

v18.16.0 release proposal #47502

Merged

doc: update test concurrency description / default value by richiemccoll · Pull Request #46457 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: update test concurrency description / default value - #46457

doc: update test concurrency description / default value#46457richiemccoll wants to merge 1 commit intonodejs:mainnodejs/node:mainfrom richiemccoll:doc/test-runner-context-concurrencyrichiemccoll/node:doc/test-runner-context-concurrencyCopy head branch name to clipboard

Conversation

richiemccoll commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Feb 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduh95 commented Feb 1, 2023

Uh oh!

richiemccoll commented Feb 1, 2023

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GeoffreyBooth commented Feb 2, 2023

Uh oh!

JakobJingleheimer commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richiemccoll commented Feb 3, 2023

Uh oh!

Uh oh!

Uh oh!

MoLow commented Feb 5, 2023

Uh oh!

richiemccoll commented Feb 6, 2023

Uh oh!

nodejs-github-bot commented Feb 6, 2023

Uh oh!

MoLow commented Feb 6, 2023

Uh oh!

aduh95 commented Feb 6, 2023

Uh oh!

richiemccoll commented Feb 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

doc: update test concurrency description / default value#46457
richiemccoll wants to merge 1 commit into
nodejs:mainfrom
richiemccoll:doc/test-runner-context-concurrency

richiemccoll commented Feb 1, 2023 •
edited
Loading

JakobJingleheimer commented Feb 2, 2023 •
edited
Loading

richiemccoll commented Feb 6, 2023 •
edited
Loading