watch: fix watch path with equals

[MoLow]

Fixes: #47296

prior to this fix node --watch-path path.js worked but node --watch-path=path.js did not

[debadree25]

Is there was no general test for —watch-path both the tests for checking for non-existing files, if there isn't i think we could add a test for —watch-path watching over a directory and executing a file like it is for the —watch tests

[MoLow]

Is there was no general test for —watch-path both the tests for checking for non-existing files, if there isn't i think we could add a test for —watch-path watching over a directory and executing a file like it is for the —watch tests

There is such a test. see test/sequential/test-watch-mode.mjs

[debadree25]

Is there was no general test for —watch-path both the tests for checking for non-existing files, if there isn't i think we could add a test for —watch-path watching over a directory and executing a file like it is for the —watch tests

There is such a test. see test/sequential/test-watch-mode.mjs

could you point out which ones 😅😅 on test/sequential/test-watch-mode.mjs was able to find two tests:

node/test/sequential/test-watch-mode.mjs

Line 158 in 5c04888

it('should watch when running an non-existing file - when specified under --watch-path with equals', {

node/test/sequential/test-watch-mode.mjs

Line 140 in 5c04888

it('should watch when running an non-existing file - when specified under --watch-path', {

both of which mention watching non-existing case

[debadree25]

Also, I tried running to see if the console log not appearing issue is fixed for example I have a file test.mjs and test_dir/test.js, test.mjs has a statement console.log('hello world'); and running out/Release/node --watch-path=./test_dir test.mjs gave the following output, hello world not printed

[debadree25]

I was basically talking of a test something like this

  it('should watch changes to a file with watch-path', {
    skip: !supportsRecursive,
  }, async () => {
    const file = createTmpFile();
    const watchedFile = fixtures.path('watch-mode/subdir/file.js');
    const { stderr, stdout } = await spawnWithRestarts({
      file,
      watchedFile,
      args: ['--watch-path', fixtures.path('./watch-mode/subdir'), file],
    });
    assert.strictEqual(stderr, '');
    assertRestartedCorrectly({
      stdout,
      messages: { inner: 'running', completed: `Completed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },
    });
  });

[debadree25]

ok was able make the test case pass with the following changes:

in lib/internal/main/watch_mode.js passing the 'inherit' option when using watch mode

--- a/lib/internal/main/watch_mode.js
+++ b/lib/internal/main/watch_mode.js
@@ -53,7 +53,7 @@ let exited;
 
 function start() {
   exited = false;
-  const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : undefined;
+  const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit';
   child = spawn(process.execPath, args, { stdio, env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1' } });
   watcher.watchChildProcessModules(child);
   child.once('exit', (code) => {

and updated the tests as follows:

--- a/test/sequential/test-watch-mode.mjs
+++ b/test/sequential/test-watch-mode.mjs
@@ -148,7 +165,8 @@ describe('watch mode', { concurrency: false, timeout: 60_000 }, () => {
       args: ['--watch-path', fixtures.path('./watch-mode/subdir/'), file],
     });
 
-    assert.strictEqual(stderr, '');
+    assert.match(stderr, /Error: Cannot find module/);
+    assert(stderr.match(/Error: Cannot find module/g).length >= 2);
     assertRestartedCorrectly({
       stdout,
       messages: { completed: `Failed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },
@@ -166,7 +184,8 @@ describe('watch mode', { concurrency: false, timeout: 60_000 }, () => {
       args: [`--watch-path=${fixtures.path('./watch-mode/subdir/')}`, file],
     });
 
-    assert.strictEqual(stderr, '');
+    assert.match(stderr, /Error: Cannot find module/);
+    assert(stderr.match(/Error: Cannot find module/g).length >= 2);
     assertRestartedCorrectly({
       stdout,
       messages: { completed: `Failed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },

does this look ok? if yes could commit it to this branch @MoLow

[MoLow]

thanks @debadree25!
feel free to commit to this branch if you find anything else

[debadree25]

just added the test for watch-path in the general case @MoLow

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/50823/

[tniessen]

FWIW it seems odd to implement this in JS, separate from the usual C++ argument parsing logic. I had similar concerns in the original PR.

[debadree25]

Is there a similar example in present cpp where multiple options are parsed like this?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/50827/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/50832/

[nodejs-github-bot]

Landed in 25858e3