Skip to content

Commit

Permalink
refactor: create common.execPath (#636)
Browse files Browse the repository at this point in the history 
Switch to using common.execPath instead of process.execPath directly and warn
electron users if we were unable to find the correct path to NodeJS.
  • Loading branch information
@nfischer @freitagbr
nfischer authored and freitagbr committed Jan 8, 2017
1 parent ac0ff87 commit 4c48631
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 23 deletions.
13 changes: 11 additions & 2 deletions src/common.js
View file
Expand Up @@ -26,24 +26,33 @@ var objectAssign = typeof Object.assign === 'function' ?
};
exports.extend = objectAssign;

// Module globals
// Check if we're running under electron
var isElectron = Boolean(process.versions.electron);

// Module globals (assume no execPath by default)
var DEFAULT_CONFIG = {
fatal: false,
globOptions: {},
maxdepth: 255,
noglob: false,
silent: false,
verbose: false,
execPath: null,
};

var config = {
reset: function () {
objectAssign(this, DEFAULT_CONFIG);
if (!isElectron) {
this.execPath = process.execPath;
}
},
resetForTesting: function () {
objectAssign(this, DEFAULT_CONFIG);
this.reset();
this.silent = true;
},
};

config.reset();
exports.config = config;

Expand Down
6 changes: 5 additions & 1 deletion src/exec.js
View file
Expand Up @@ -19,6 +19,10 @@ common.register('exec', _exec, {
// Node is single-threaded; callbacks and other internal state changes are done in the
// event loop).
function execSync(cmd, opts, pipe) {
if (!common.config.execPath) {
common.error('Unable to find a path to the node binary. Please manually set config.execPath');
}

var tempDir = _tempDir();
var stdoutFile = path.resolve(tempDir + '/' + common.randomFileName());
var stderrFile = path.resolve(tempDir + '/' + common.randomFileName());
Expand Down Expand Up @@ -66,7 +70,7 @@ function execSync(cmd, opts, pipe) {
if (fs.existsSync(stderrFile)) common.unlinkSync(stderrFile);
if (fs.existsSync(codeFile)) common.unlinkSync(codeFile);

var execCommand = JSON.stringify(process.execPath) + ' ' + JSON.stringify(scriptFile);
var execCommand = JSON.stringify(common.config.execPath) + ' ' + JSON.stringify(scriptFile);
var script;

opts.cwd = path.resolve(opts.cwd);
Expand Down
10 changes: 10 additions & 0 deletions test/common.js
View file
Expand Up @@ -135,3 +135,13 @@ test.cb('Commands that fail will still output error messages to stderr', t => {
});
});

test('execPath value makes sense', t => {
// TODO(nate): change this test if we add electron support in the unit tests
t.is(common.config.execPath, process.execPath);
t.is(typeof common.config.execPath, 'string');
});

test('Changing common.config.execPath does not modify process', t => {
common.config.execPath = 'foo';
t.not(common.config.execPath, process.execPath);
});
36 changes: 24 additions & 12 deletions test/exec.js
View file
Expand Up @@ -5,12 +5,15 @@ import util from 'util';
import test from 'ava';

import shell from '..';
import common from '../src/common';

const CWD = process.cwd();
const ORIG_EXEC_PATH = common.config.execPath;
shell.config.silent = true;

test.afterEach.always(() => {
process.chdir(CWD);
common.config.execPath = ORIG_EXEC_PATH;
});

//
Expand All @@ -36,6 +39,15 @@ test('config.fatal and unknown command', t => {
shell.config.fatal = oldFatal;
});

test('exec exits gracefully if we cannot find the execPath', t => {
common.config.execPath = null;
shell.exec('echo foo');
t.regex(
shell.error(),
/Unable to find a path to the node binary\. Please manually set config\.execPath/
);
});

//
// Valids
//
Expand All @@ -45,44 +57,44 @@ test('config.fatal and unknown command', t => {
//

test('check if stdout goes to output', t => {
const result = shell.exec(`${JSON.stringify(process.execPath)} -e "console.log(1234);"`);
const result = shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.log(1234);"`);
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, '1234\n');
});

test('check if stderr goes to output', t => {
const result = shell.exec(`${JSON.stringify(process.execPath)} -e "console.error(1234);"`);
const result = shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.error(1234);"`);
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, '');
t.is(result.stderr, '1234\n');
});

test('check if stdout + stderr go to output', t => {
const result = shell.exec(`${JSON.stringify(process.execPath)} -e "console.error(1234); console.log(666);"`);
const result = shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.error(1234); console.log(666);"`);
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, '666\n');
t.is(result.stderr, '1234\n');
});

test('check exit code', t => {
const result = shell.exec(`${JSON.stringify(process.execPath)} -e "process.exit(12);"`);
const result = shell.exec(`${JSON.stringify(common.config.execPath)} -e "process.exit(12);"`);
t.truthy(shell.error());
t.is(result.code, 12);
});

test('interaction with cd', t => {
shell.cd('resources/external');
const result = shell.exec(`${JSON.stringify(process.execPath)} node_script.js`);
const result = shell.exec(`${JSON.stringify(common.config.execPath)} node_script.js`);
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, 'node_script_1234\n');
});

test('check quotes escaping', t => {
const result = shell.exec(util.format(JSON.stringify(process.execPath) + ' -e "console.log(%s);"', "\\\"\\'+\\'_\\'+\\'\\\""));
const result = shell.exec(util.format(JSON.stringify(common.config.execPath) + ' -e "console.log(%s);"', "\\\"\\'+\\'_\\'+\\'\\\""));
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, "'+'_'+'\n");
Expand All @@ -108,12 +120,12 @@ test('set maxBuffer (very small)', t => {
});

test('set timeout option', t => {
const result = shell.exec(`${JSON.stringify(process.execPath)} resources/exec/slow.js 100`); // default timeout is ok
const result = shell.exec(`${JSON.stringify(common.config.execPath)} resources/exec/slow.js 100`); // default timeout is ok
t.falsy(shell.error());
t.is(result.code, 0);
if (process.version >= 'v0.11') {
// this option doesn't work on v0.10
shell.exec(`${JSON.stringify(process.execPath)} resources/exec/slow.js 100`, { timeout: 10 }); // times out
shell.exec(`${JSON.stringify(common.config.execPath)} resources/exec/slow.js 100`, { timeout: 10 }); // times out

t.truthy(shell.error());
}
Expand Down Expand Up @@ -159,14 +171,14 @@ test('exec returns a ShellString', t => {
//

test.cb('no callback', t => {
const c = shell.exec(`${JSON.stringify(process.execPath)} -e "console.log(1234)"`, { async: true });
const c = shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.log(1234)"`, { async: true });
t.falsy(shell.error());
t.truthy('stdout' in c, 'async exec returns child process object');
t.end();
});

test.cb('callback as 2nd argument', t => {
shell.exec(`${JSON.stringify(process.execPath)} -e "console.log(5678);"`, (code, stdout, stderr) => {
shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.log(5678);"`, (code, stdout, stderr) => {
t.is(code, 0);
t.is(stdout, '5678\n');
t.is(stderr, '');
Expand All @@ -175,7 +187,7 @@ test.cb('callback as 2nd argument', t => {
});

test.cb('callback as end argument', t => {
shell.exec(`${JSON.stringify(process.execPath)} -e "console.log(5566);"`, { async: true }, (code, stdout, stderr) => {
shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.log(5566);"`, { async: true }, (code, stdout, stderr) => {
t.is(code, 0);
t.is(stdout, '5566\n');
t.is(stderr, '');
Expand All @@ -184,7 +196,7 @@ test.cb('callback as end argument', t => {
});

test.cb('callback as 3rd argument (silent:true)', t => {
shell.exec(`${JSON.stringify(process.execPath)} -e "console.log(5678);"`, { silent: true }, (code, stdout, stderr) => {
shell.exec(`${JSON.stringify(common.config.execPath)} -e "console.log(5678);"`, { silent: true }, (code, stdout, stderr) => {
t.is(code, 0);
t.is(stdout, '5678\n');
t.is(stderr, '');
Expand Down
11 changes: 6 additions & 5 deletions test/set.js
View file
@@ -1,6 +1,7 @@
import test from 'ava';

import shell from '..';
import common from '../src/common';
import utils from './utils/utils';

const oldConfigSilent = shell.config.silent;
Expand Down Expand Up @@ -28,21 +29,21 @@ test('initial values', t => {
});

test('default behavior', t => {
const result = shell.exec(JSON.stringify(process.execPath) + ' -e "require(\'../global\'); ls(\'file_doesnt_exist\'); echo(1234);"');
const result = shell.exec(JSON.stringify(common.config.execPath) + ' -e "require(\'../global\'); ls(\'file_doesnt_exist\'); echo(1234);"');
t.is(result.code, 0);
t.is(result.stdout, '1234\n');
t.is(result.stderr, 'ls: no such file or directory: file_doesnt_exist\n');
});

test('set -e', t => {
const result = shell.exec(JSON.stringify(process.execPath) + ' -e "require(\'../global\'); set(\'-e\'); ls(\'file_doesnt_exist\'); echo(1234);"');
const result = shell.exec(JSON.stringify(common.config.execPath) + ' -e "require(\'../global\'); set(\'-e\'); ls(\'file_doesnt_exist\'); echo(1234);"');
t.is(result.code, uncaughtErrorExitCode);
t.is(result.stdout, '');
t.truthy(result.stderr.indexOf('Error: ls: no such file or directory: file_doesnt_exist') >= 0);
});

test('set -v', t => {
const result = shell.exec(JSON.stringify(process.execPath) + ' -e "require(\'../global\'); set(\'-v\'); ls(\'file_doesnt_exist\'); echo(1234);"');
const result = shell.exec(JSON.stringify(common.config.execPath) + ' -e "require(\'../global\'); set(\'-v\'); ls(\'file_doesnt_exist\'); echo(1234);"');
t.is(result.code, 0);
t.is(result.stdout, '1234\n');
t.is(
Expand All @@ -52,7 +53,7 @@ test('set -v', t => {
});

test('set -ev', t => {
const result = shell.exec(JSON.stringify(process.execPath) + ' -e "require(\'../global\'); set(\'-ev\'); ls(\'file_doesnt_exist\'); echo(1234);"');
const result = shell.exec(JSON.stringify(common.config.execPath) + ' -e "require(\'../global\'); set(\'-ev\'); ls(\'file_doesnt_exist\'); echo(1234);"');
t.is(result.code, uncaughtErrorExitCode);
t.is(result.stdout, '');
t.truthy(result.stderr.indexOf('Error: ls: no such file or directory: file_doesnt_exist') >= 0);
Expand All @@ -61,7 +62,7 @@ test('set -ev', t => {
});

test('set -e, set +e', t => {
const result = shell.exec(JSON.stringify(process.execPath) + ' -e "require(\'../global\'); set(\'-e\'); set(\'+e\'); ls(\'file_doesnt_exist\'); echo(1234);"');
const result = shell.exec(JSON.stringify(common.config.execPath) + ' -e "require(\'../global\'); set(\'-e\'); set(\'+e\'); ls(\'file_doesnt_exist\'); echo(1234);"');
t.is(result.code, 0);
t.is(result.stdout, '1234\n');
t.is(result.stderr, 'ls: no such file or directory: file_doesnt_exist\n');
Expand Down
3 changes: 2 additions & 1 deletion test/shjs.js
View file
Expand Up @@ -3,12 +3,13 @@ import path from 'path';
import test from 'ava';

import shell from '..';
import common from '../src/common';

function runWithShjs(name) {
// prefix with 'node ' for Windows, don't prefix for unix
const binPath = path.resolve(__dirname, '../bin/shjs');
const execPath = process.platform === 'win32'
? `${JSON.stringify(process.execPath)} `
? `${JSON.stringify(common.config.execPath)} `
: '';
const script = path.resolve(__dirname, 'resources', 'shjs', name);
return shell.exec(`${execPath}${binPath} ${script}`, { silent: true });
Expand Down
6 changes: 4 additions & 2 deletions test/utils/utils.js
View file
@@ -1,5 +1,7 @@
const child = require('child_process');

const common = require('../../src/common');

function numLines(str) {
return typeof str === 'string' ? (str.match(/\n/g) || []).length + 1 : 0;
}
Expand All @@ -26,11 +28,11 @@ function skipOnWinForEPERM(action, testCase) {
exports.skipOnWinForEPERM = skipOnWinForEPERM;

function runScript(script, cb) {
child.execFile(process.execPath, ['-e', script], cb);
child.execFile(common.config.execPath, ['-e', script], cb);
}
exports.runScript = runScript;

function sleep(time) {
child.execFileSync(process.execPath, ['resources/exec/slow.js', time.toString()]);
child.execFileSync(common.config.execPath, ['resources/exec/slow.js', time.toString()]);
}
exports.sleep = sleep;

0 comments on commit 4c48631

Please sign in to comment.