From 027910d12e2b35afa5f9d8182bfc8933446e0832 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Mon, 14 Sep 2020 12:20:14 +0200 Subject: [PATCH 1/6] fix: remove inspect & inspect-brk from execArgv --- test/node.js | 104 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/test/node.js b/test/node.js index 838ab5dbe4..7176e997c4 100644 --- a/test/node.js +++ b/test/node.js @@ -3,7 +3,8 @@ import test from 'ava'; import pEvent from 'p-event'; import execa from '..'; -process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; +process.env.PATH = + path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; test('node()', async t => { const {exitCode} = await execa.node('test/fixtures/noop'); @@ -19,11 +20,14 @@ test('node pipe stdout', async t => { }); test('node correctly use nodePath', async t => { - const {stdout} = await execa.node(process.platform === 'win32' ? 'hello.cmd' : 'hello.sh', { - stdout: 'pipe', - nodePath: process.platform === 'win32' ? 'cmd.exe' : 'bash', - nodeOptions: process.platform === 'win32' ? ['/c'] : [] - }); + const {stdout} = await execa.node( + process.platform === 'win32' ? 'hello.cmd' : 'hello.sh', + { + stdout: 'pipe', + nodePath: process.platform === 'win32' ? 'cmd.exe' : 'bash', + nodeOptions: process.platform === 'win32' ? ['/c'] : [] + } + ); t.is(stdout, 'Hello World'); }); @@ -37,6 +41,94 @@ test('node pass on nodeOptions', async t => { t.is(stdout, 'foo'); }); +test.serial( + 'node removes --inspect from nodeOptions when defined by parent process', + async t => { + const originalArgv = process.execArgv; + process.execArgv = ['--inspect', '-e']; + const {stdout, stderr} = await execa.node('console.log("foo")', { + stdout: 'pipe' + }); + process.execArgv = originalArgv; + + t.is(stdout, 'foo'); + t.is(stderr, ''); + } +); + +test.serial( + 'node removes --inspect=9222 from nodeOptions when defined by parent process', + async t => { + const originalArgv = process.execArgv; + process.execArgv = ['--inspect=9222', '-e']; + const {stdout, stderr} = await execa.node('console.log("foo")', { + stdout: 'pipe' + }); + process.execArgv = originalArgv; + + t.is(stdout, 'foo'); + t.is(stderr, ''); + } +); + +test.serial( + 'node removes --inspect-brk from nodeOptions when defined by parent process', + async t => { + const originalArgv = process.execArgv; + process.execArgv = ['--inspect-brk', '-e']; + const childProc = execa.node('console.log("foo")', { + stdout: 'pipe' + }); + process.execArgv = originalArgv; + + setTimeout(() => { + childProc.cancel(); + }, 1000); + + const {stdout, stderr} = await childProc.catch(error => error); + + t.is(stdout, 'foo'); + t.is(stderr, ''); + } +); + +test.serial( + 'node removes --inspect-brk=9222 from nodeOptions when defined by parent process', + async t => { + const originalArgv = process.execArgv; + process.execArgv = ['--inspect-brk=9222', '-e']; + const childProc = execa.node('console.log("foo")', { + stdout: 'pipe' + }); + process.execArgv = originalArgv; + + setTimeout(() => { + childProc.cancel(); + }, 1000); + + const {stdout, stderr} = await childProc.catch(error => error); + + t.is(stdout, 'foo'); + t.is(stderr, ''); + } +); + +test.serial( + 'node should not remove --inspect when passed through nodeOptions', + async t => { + const {stdout, stderr} = await execa.node('console.log("foo")', { + stdout: 'pipe', + nodeOptions: ['--inspect', '-e'] + }); + + t.is(stdout, 'foo'); + t.regex( + stderr, + /^Debugger listening on ws:\/\/127.0.0.1:9229\/.*[\r\n]+For help, see: https:\/\/nodejs.org\/en\/docs\/inspector$/ + ); + } +); + test('node\'s forked script has a communication channel', async t => { const subprocess = execa.node('test/fixtures/send'); subprocess.send('ping'); From b9cc1ed1b685282be581394552421daa757d2040 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Mon, 14 Sep 2020 12:54:54 +0200 Subject: [PATCH 2/6] add inspect filters --- index.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 4835038134..c1b844d2ae 100644 --- a/index.js +++ b/index.js @@ -234,8 +234,24 @@ module.exports.node = (scriptPath, args, options = {}) => { } const stdio = normalizeStdio.node(options); + // Filter out --inspect & --inspect-brk from default execArgv + const defaultExecArgv = process.execArgv.filter(arg => { + if ( + arg === '--inspect' || + arg.startsWith('--inspect=') || + arg === '--inspect-brk' || + arg.startsWith('--inspect-brk=') + ) { + return false; + } + + return true; + }); - const {nodePath = process.execPath, nodeOptions = process.execArgv} = options; + const { + nodePath = process.execPath, + nodeOptions = defaultExecArgv + } = options; return execa( nodePath, From 9f3ac5db7b139af13c1e8fc137d517cb9d2f6aae Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Mon, 28 Sep 2020 22:50:24 +0200 Subject: [PATCH 3/6] Update index.js --- index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index c1b844d2ae..c2ceaa56b2 100644 --- a/index.js +++ b/index.js @@ -234,7 +234,8 @@ module.exports.node = (scriptPath, args, options = {}) => { } const stdio = normalizeStdio.node(options); - // Filter out --inspect & --inspect-brk from default execArgv + + // Filter out `--inspect` & `--inspect-brk` from default `execArgv`. const defaultExecArgv = process.execArgv.filter(arg => { if ( arg === '--inspect' || From 6f58dd5cd6dc793db09086f874cd87800fe5188b Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Fri, 23 Oct 2020 12:17:20 +0200 Subject: [PATCH 4/6] review changes --- index.js | 15 +------------ test/node.js | 62 +++++++++++++++++++--------------------------------- 2 files changed, 24 insertions(+), 53 deletions(-) diff --git a/index.js b/index.js index c2ceaa56b2..d94125310c 100644 --- a/index.js +++ b/index.js @@ -234,20 +234,7 @@ module.exports.node = (scriptPath, args, options = {}) => { } const stdio = normalizeStdio.node(options); - - // Filter out `--inspect` & `--inspect-brk` from default `execArgv`. - const defaultExecArgv = process.execArgv.filter(arg => { - if ( - arg === '--inspect' || - arg.startsWith('--inspect=') || - arg === '--inspect-brk' || - arg.startsWith('--inspect-brk=') - ) { - return false; - } - - return true; - }); + const defaultExecArgv = process.execArgv.filter(arg => !arg.startsWith('--inspect')); const { nodePath = process.execPath, diff --git a/test/node.js b/test/node.js index 7176e997c4..91dd2237af 100644 --- a/test/node.js +++ b/test/node.js @@ -3,9 +3,15 @@ import test from 'ava'; import pEvent from 'p-event'; import execa from '..'; -process.env.PATH = - path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; +process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; +test.beforeEach(t => { + t.context.originalArgv = process.execArgv; +}); + +test.afterEach(t => { + process.execArgv = t.context.originalArgv; +}); test('node()', async t => { const {exitCode} = await execa.node('test/fixtures/noop'); t.is(exitCode, 0); @@ -20,14 +26,11 @@ test('node pipe stdout', async t => { }); test('node correctly use nodePath', async t => { - const {stdout} = await execa.node( - process.platform === 'win32' ? 'hello.cmd' : 'hello.sh', - { - stdout: 'pipe', - nodePath: process.platform === 'win32' ? 'cmd.exe' : 'bash', - nodeOptions: process.platform === 'win32' ? ['/c'] : [] - } - ); + const {stdout} = await execa.node(process.platform === 'win32' ? 'hello.cmd' : 'hello.sh', { + stdout: 'pipe', + nodePath: process.platform === 'win32' ? 'cmd.exe' : 'bash', + nodeOptions: process.platform === 'win32' ? ['/c'] : [] + }); t.is(stdout, 'Hello World'); }); @@ -44,12 +47,10 @@ test('node pass on nodeOptions', async t => { test.serial( 'node removes --inspect from nodeOptions when defined by parent process', async t => { - const originalArgv = process.execArgv; process.execArgv = ['--inspect', '-e']; const {stdout, stderr} = await execa.node('console.log("foo")', { - stdout: 'pipe' + reject: false }); - process.execArgv = originalArgv; t.is(stdout, 'foo'); t.is(stderr, ''); @@ -59,12 +60,10 @@ test.serial( test.serial( 'node removes --inspect=9222 from nodeOptions when defined by parent process', async t => { - const originalArgv = process.execArgv; process.execArgv = ['--inspect=9222', '-e']; const {stdout, stderr} = await execa.node('console.log("foo")', { - stdout: 'pipe' + reject: false }); - process.execArgv = originalArgv; t.is(stdout, 'foo'); t.is(stderr, ''); @@ -74,18 +73,12 @@ test.serial( test.serial( 'node removes --inspect-brk from nodeOptions when defined by parent process', async t => { - const originalArgv = process.execArgv; process.execArgv = ['--inspect-brk', '-e']; - const childProc = execa.node('console.log("foo")', { - stdout: 'pipe' + const subprocess = execa.node('console.log("foo")', { + reject: false }); - process.execArgv = originalArgv; - - setTimeout(() => { - childProc.cancel(); - }, 1000); - const {stdout, stderr} = await childProc.catch(error => error); + const {stdout, stderr} = await subprocess.catch(error => error); t.is(stdout, 'foo'); t.is(stderr, ''); @@ -95,18 +88,12 @@ test.serial( test.serial( 'node removes --inspect-brk=9222 from nodeOptions when defined by parent process', async t => { - const originalArgv = process.execArgv; process.execArgv = ['--inspect-brk=9222', '-e']; - const childProc = execa.node('console.log("foo")', { - stdout: 'pipe' + const subprocess = execa.node('console.log("foo")', { + reject: false }); - process.execArgv = originalArgv; - - setTimeout(() => { - childProc.cancel(); - }, 1000); - const {stdout, stderr} = await childProc.catch(error => error); + const {stdout, stderr} = await subprocess.catch(error => error); t.is(stdout, 'foo'); t.is(stderr, ''); @@ -117,15 +104,12 @@ test.serial( 'node should not remove --inspect when passed through nodeOptions', async t => { const {stdout, stderr} = await execa.node('console.log("foo")', { - stdout: 'pipe', + reject: false, nodeOptions: ['--inspect', '-e'] }); t.is(stdout, 'foo'); - t.regex( - stderr, - /^Debugger listening on ws:\/\/127.0.0.1:9229\/.*[\r\n]+For help, see: https:\/\/nodejs.org\/en\/docs\/inspector$/ - ); + t.true(stderr.includes('Debugger listening')); } ); From b83935fadea8c899614381c17c072bd69841a010 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Mon, 26 Oct 2020 07:48:42 +0100 Subject: [PATCH 5/6] move to macro --- test/node.js | 70 ++++++++++++++++++---------------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/test/node.js b/test/node.js index 91dd2237af..5aae4247e5 100644 --- a/test/node.js +++ b/test/node.js @@ -5,13 +5,23 @@ import execa from '..'; process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; -test.beforeEach(t => { - t.context.originalArgv = process.execArgv; -}); +async function inspectMacro(t, input) { + const originalArgv = process.execArgv; + process.execArgv = [input, '-e']; + try { + const subprocess = execa.node('console.log("foo")', { + reject: false + }); + + const {stdout, stderr} = await subprocess + + t.is(stdout, 'foo'); + t.is(stderr, ''); + } finally { + process.execArgv = originalArgv; + } +} -test.afterEach(t => { - process.execArgv = t.context.originalArgv; -}); test('node()', async t => { const {exitCode} = await execa.node('test/fixtures/noop'); t.is(exitCode, 0); @@ -46,58 +56,26 @@ test('node pass on nodeOptions', async t => { test.serial( 'node removes --inspect from nodeOptions when defined by parent process', - async t => { - process.execArgv = ['--inspect', '-e']; - const {stdout, stderr} = await execa.node('console.log("foo")', { - reject: false - }); - - t.is(stdout, 'foo'); - t.is(stderr, ''); - } + inspectMacro, + `--inspect` ); test.serial( 'node removes --inspect=9222 from nodeOptions when defined by parent process', - async t => { - process.execArgv = ['--inspect=9222', '-e']; - const {stdout, stderr} = await execa.node('console.log("foo")', { - reject: false - }); - - t.is(stdout, 'foo'); - t.is(stderr, ''); - } + inspectMacro, + '--inspect=9222' ); test.serial( 'node removes --inspect-brk from nodeOptions when defined by parent process', - async t => { - process.execArgv = ['--inspect-brk', '-e']; - const subprocess = execa.node('console.log("foo")', { - reject: false - }); - - const {stdout, stderr} = await subprocess.catch(error => error); - - t.is(stdout, 'foo'); - t.is(stderr, ''); - } + inspectMacro, + '--inspect-brk' ); test.serial( 'node removes --inspect-brk=9222 from nodeOptions when defined by parent process', - async t => { - process.execArgv = ['--inspect-brk=9222', '-e']; - const subprocess = execa.node('console.log("foo")', { - reject: false - }); - - const {stdout, stderr} = await subprocess.catch(error => error); - - t.is(stdout, 'foo'); - t.is(stderr, ''); - } + inspectMacro, + '--inspect-brk=9222' ); test.serial( From 3d463cbfe0cc7edef7d4dfe21b699bb38192d025 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Mon, 26 Oct 2020 07:49:21 +0100 Subject: [PATCH 6/6] fix lint --- test/node.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/node.js b/test/node.js index 5aae4247e5..51ca5576e3 100644 --- a/test/node.js +++ b/test/node.js @@ -13,7 +13,7 @@ async function inspectMacro(t, input) { reject: false }); - const {stdout, stderr} = await subprocess + const {stdout, stderr} = await subprocess; t.is(stdout, 'foo'); t.is(stderr, ''); @@ -57,7 +57,7 @@ test('node pass on nodeOptions', async t => { test.serial( 'node removes --inspect from nodeOptions when defined by parent process', inspectMacro, - `--inspect` + '--inspect' ); test.serial(