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.

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 (test_runner): shards not supported with watch mode #50640

Merged
merged 1 commit into from
Nov 23, 2023

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';
import path from 'node:path'

const options = {
  concurrency: 2, // Run 2 test files in parallel
  files: [path.resolve('./index.mjs')], // Specify the test file to run
  setup: (testStream) => {
    console.log("test stream ==>",testStream)
    // Setup listeners before running tests
    testStream.on('test-start', (test) => {
      console.log(`Starting test: ${test.name}`);
    });
  },
  timeout: 5000, // Set a timeout of 5 seconds for test execution
  watch: true, // Enable watch mode
  shard: {
    index: 1, // Index of the shard to run
    total: 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-bot nodejs-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
Contributor Author

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-30 pulkit-30 changed the title doc(test_runner): shards not supported with watch mode doc (test_runner): shards not supported with watch mode Nov 20, 2023
@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 23, 2023
@nodejs-github-bot nodejs-github-bot merged commit f9675e1 into nodejs:main Nov 23, 2023
16 checks passed
@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]>
@RafaelGSS RafaelGSS 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]>
@UlisesGascon UlisesGascon 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants