From ce61105cb6dabf09c8e579dba23942229159c997 Mon Sep 17 00:00:00 2001 From: jvelezpo Date: Thu, 25 Jan 2018 14:47:52 -0500 Subject: [PATCH 1/3] test: Verify the shell option works properly on execFile Useful for executing in a shell because it accepts arguments as an array instead of a string as exec does. Depending on the circumstances, that can prove to be useful if the arguments are already prepared. --- test/parallel/test-child-process-execfile.js | 8 ++++++++ test/sequential/test-child-process-execsync.js | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js index 415489e9db8f65..7dc5e7e67c4c41 100644 --- a/test/parallel/test-child-process-execfile.js +++ b/test/parallel/test-child-process-execfile.js @@ -7,6 +7,7 @@ const { getSystemErrorName } = require('util'); const fixtures = require('../common/fixtures'); const fixture = fixtures.path('exit.js'); +const execOpts = { encoding: 'utf8', shell: process.env.SHELL }; { execFile( @@ -39,3 +40,10 @@ const fixture = fixtures.path('exit.js'); child.kill(); child.emit('close', code, null); } + +{ + // Verify the shell option works properly + execFile('ls', [process.execPath, '*'], execOpts, common.mustCall((err) => { + assert.strictEqual(err, null); + })); +} diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index da80e80bce01d2..35cc98c5a62881 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -29,6 +29,7 @@ const TIMER = 200; const SLEEP = 2000; const start = Date.now(); +const execOpts = { encoding: 'utf8', shell: process.env.SHELL }; let err; let caught = false; @@ -141,3 +142,8 @@ assert.strictEqual(ret, `${msg}\n`); return true; }); } + +// Verify the shell option works properly +assert.doesNotThrow(() => { + execFileSync('ls', [process.execPath, '*'], execOpts); +}); From c1aa51557a0851e9fe35fa81f98ebf694cd0c107 Mon Sep 17 00:00:00 2001 From: jvelezpo Date: Fri, 26 Jan 2018 09:19:36 -0500 Subject: [PATCH 2/3] test: Verify the shell option works properly on execFile --- test/parallel/test-child-process-execfile.js | 2 +- test/sequential/test-child-process-execsync.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js index 7dc5e7e67c4c41..93250b96c53e6e 100644 --- a/test/parallel/test-child-process-execfile.js +++ b/test/parallel/test-child-process-execfile.js @@ -43,7 +43,7 @@ const execOpts = { encoding: 'utf8', shell: process.env.SHELL }; { // Verify the shell option works properly - execFile('ls', [process.execPath, '*'], execOpts, common.mustCall((err) => { + execFile(process.execPath, [fixture, 0], execOpts, common.mustCall((err) => { assert.strictEqual(err, null); })); } diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index 35cc98c5a62881..d2b9a6f343ee61 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -145,5 +145,5 @@ assert.strictEqual(ret, `${msg}\n`); // Verify the shell option works properly assert.doesNotThrow(() => { - execFileSync('ls', [process.execPath, '*'], execOpts); + execFileSync(process.execPath, [], execOpts); }); From c85545b07e7d37da997b4610c8293731e90a404a Mon Sep 17 00:00:00 2001 From: jvelezpo Date: Fri, 26 Jan 2018 15:06:38 -0500 Subject: [PATCH 3/3] test: Verify the shell option works properly on execFile from @richardlau `process.env.SHELL is likely to not be set on Windows so this will end up being undefined.` This commit prevents this from happening. --- test/parallel/test-child-process-execfile.js | 2 +- test/sequential/test-child-process-execsync.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js index 93250b96c53e6e..67bb5924ea73af 100644 --- a/test/parallel/test-child-process-execfile.js +++ b/test/parallel/test-child-process-execfile.js @@ -7,7 +7,7 @@ const { getSystemErrorName } = require('util'); const fixtures = require('../common/fixtures'); const fixture = fixtures.path('exit.js'); -const execOpts = { encoding: 'utf8', shell: process.env.SHELL }; +const execOpts = { encoding: 'utf8', shell: true }; { execFile( diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index d2b9a6f343ee61..133217dcdf7c3f 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -29,7 +29,7 @@ const TIMER = 200; const SLEEP = 2000; const start = Date.now(); -const execOpts = { encoding: 'utf8', shell: process.env.SHELL }; +const execOpts = { encoding: 'utf8', shell: true }; let err; let caught = false;