Skip to content

Conversation

@pulkit-30
Copy link
Contributor

Affected URL: https://nodejs.org/docs/latest-v20.x/api/test.html#runoptions

It's not mention that we can use shard with watch mode.

Code:

import{tap}from'node:test/reporters'import{run}from'node:test';importpathfrom'node:path'constoptions={concurrency: 2,// Run 2 test files in parallelfiles: [path.resolve('./index.mjs')],// Specify the test file to runsetup: (testStream)=>{console.log("test stream ==>",testStream)// Setup listeners before running teststestStream.on('test-start',(test)=>{console.log(`Starting test: ${test.name}`);});},timeout: 5000,// Set a timeout of 5 seconds for test executionwatch: true,// Enable watch modeshard: {index: 1,// Index of the shard to runtotal: 2,// Total number of shards},};run(options).compose(tap).pipe(process.stdout);

error:

node:internal/test_runner/runner:466 throw new ERR_INVALID_ARG_VALUE('options.shard', watch, 'shards not supported with watch mode'); ^ TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.shard' shards not supported with watch mode. Received true at run (node:internal/test_runner/runner:466:13) at file:///Users/pulkitgupta/Desktop/node/index.test.mjs:23:1 at ModuleJob.run (node:internal/modules/esm/module_job:218:25) at async ModuleLoader.import (node:internal/modules/esm/loader:329:24) at async loadESM (node:internal/process/esm_loader:28:7) at async handleMainPromise (node:internal/modules/run_main:113:12){code: 'ERR_INVALID_ARG_VALUE' } Node.js v22.0.0-pre

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem. labels Nov 9, 2023
@benjamingr
Copy link
Member

Why would you use sharding (A feature designed to horizontally parallelize test running across machines) with watch mode (a feature designed for development time to iterate on code quickly)?

Not judging, genuinely curious.

@benjamingr
Copy link
Member

@pulkit-30
Copy link
ContributorAuthor

Why would you use sharding (A feature designed to horizontally parallelize test running across machines) with watch mode (a feature designed for development time to iterate on code quickly)?

Not judging, genuinely curious.

Not any particular reason, I was learning about node:test module from node-api docs where I found this missing.

I wasn't expecting any error with the above code, If sharding is not supported with watch mode then it is better to show a warning or log instead and mark watch false internally, and run the test.

Better to not judge me 😅.

@rluvaton
Copy link
Member

rluvaton commented Nov 11, 2023

Should we document why sharding is not supported?

@pulkit-30pulkit-30 changed the title doc(test_runner): shards not supported with watch modedoc (test_runner): shards not supported with watch modeNov 20, 2023
@MoLowMoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 23, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 23, 2023
@nodejs-github-botnodejs-github-bot merged commit f9675e1 into nodejs:mainNov 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in f9675e1

martenrichter pushed a commit to martenrichter/node that referenced this pull request Nov 26, 2023
PR-URL: nodejs#50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
lucshi pushed a commit to lucshi/node that referenced this pull request Nov 27, 2023
PR-URL: nodejs#50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 27, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request Nov 28, 2023
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 13, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
PR-URL: #50640 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@pulkit-30@nodejs-github-bot@benjamingr@rluvaton@MoLow