Skip to content

Commit

Permalink
Disambiguate cli options from spawn options - Fix #754
Browse files Browse the repository at this point in the history
  • Loading branch information
SBoudrias committed Nov 26, 2016
1 parent 687b9be commit a3cab64
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 18 deletions.
32 changes: 19 additions & 13 deletions lib/actions/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@ var install = module.exports;
* run loop. (So don't combine the callback with `this.async()`)
*
* @param {String} installer Which package manager to use
* @param {String|Array} [paths] Packages to install.Use an empty string for `npm install`
* @param {Object} [options] Options to pass to `dargs` as arguments, then to `child_process.spawn`
* @param {String|Array} [paths] Packages to install. Use an empty string for `npm install`
* @param {Object} [options] Options to pass to `dargs` as arguments
* @param {Function} [cb]
* @param {Object} [spawnOptions] Options to pass `child_process.spawn`. ref
* https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options
*/

install.runInstall = function (installer, paths, options, cb) {
install.runInstall = function (installer, paths, options, cb, spawnOptions) {
if (!cb && _.isFunction(options)) {
cb = options;
options = {};
}

options = options || {};
spawnOptions = spawnOptions || {};
cb = cb || function () {};
paths = Array.isArray(paths) ? paths : paths && paths.split(' ') || [];

Expand All @@ -53,7 +56,7 @@ install.runInstall = function (installer, paths, options, cb) {

this.env.runLoop.add('install', function (done) {
this.emit(installer + 'Install', paths);
this.spawnCommand(installer, args, options)
this.spawnCommand(installer, args, spawnOptions)
.on('error', function (err) {
console.log(chalk.red('Could not finish installation. \n') +
'Please install ' + installer + ' with ' +
Expand Down Expand Up @@ -150,12 +153,13 @@ install.installDependencies = function (options) {
* The installation will automatically run during the run loop `install` phase.
*
* @param {String|Array} [cmpnt] Components to install
* @param {Object} [options] Options to pass to `child_process.spawn` when invoking bower.
* @param {Object} [options] Options to pass to `dargs` as arguments
* @param {Function} [cb]
* @param {Object} [spawnOptions] Options to pass `child_process.spawn`.
*/

install.bowerInstall = function install(cmpnt, options, cb) {
return this.runInstall('bower', cmpnt, options, cb);
install.bowerInstall = function install(cmpnt, options, cb, spawnOptions) {
return this.runInstall('bower', cmpnt, options, cb, spawnOptions);
};

/**
Expand All @@ -164,23 +168,25 @@ install.bowerInstall = function install(cmpnt, options, cb) {
* The installation will automatically run during the run loop `install` phase.
*
* @param {String|Array} [pkgs] Packages to install
* @param {Object} [options] Options to pass to `child_process.spawn` when invoking npm.
* @param {Object} [options] Options to pass to `dargs` as arguments
* @param {Function} [cb]
* @param {Object} [spawnOptions] Options to pass `child_process.spawn`.
*/

install.npmInstall = function install(pkgs, options, cb) {
return this.runInstall('npm', pkgs, options, cb);
install.npmInstall = function install(pkgs, options, cb, spawnOptions) {
return this.runInstall('npm', pkgs, options, cb, spawnOptions);
};
/**
* Receives a list of `packages` and an `options` object to install through npm.
*
* The installation will automatically run during the run loop `install` phase.
*
* @param {String|Array} [pkgs] Packages to install
* @param {Object} [options] Options to pass to `child_process.spawn` when invoking npm.
* @param {Object} [options] Options to pass to `dargs` as arguments
* @param {Function} [cb]
* @param {Object} [spawnOptions] Options to pass `child_process.spawn`.
*/

install.yarnInstall = function install(pkgs, options, cb) {
return this.runInstall('yarn', pkgs, options, cb);
install.yarnInstall = function install(pkgs, options, cb, spawnOptions) {
return this.runInstall('yarn', pkgs, options, cb, spawnOptions);
};
13 changes: 8 additions & 5 deletions test/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,22 @@ describe('Base (actions/install mixin)', function () {
describe('#runInstall()', function () {
it('takes a config object and passes it to the spawned process', function (done) {
var callbackSpy = sinon.spy();
var options = {
save: true
};
var spawnEnv = {
env: {
PATH: '/path/to/bin'
}
};

// args: installer, paths, options, cb
this.dummy.runInstall('nestedScript', ['path1', 'path2'], spawnEnv, callbackSpy);
this.dummy.runInstall('nestedScript', ['path1', 'path2'], options, callbackSpy, spawnEnv);
this.dummy.run(function () {
sinon.assert.calledWithExactly(
this.spawnCommandStub,
'nestedScript',
['install', 'path1', 'path2'],
['install', 'path1', 'path2', '--save'],
spawnEnv
);

Expand Down Expand Up @@ -116,7 +119,7 @@ describe('Base (actions/install mixin)', function () {
this.spawnCommandStub,
'bower',
['install', 'jquery', '--save-dev'],
{ saveDev: true }
{}
);

done();
Expand Down Expand Up @@ -168,7 +171,7 @@ describe('Base (actions/install mixin)', function () {
this.dummy.yarnInstall('yo', {dev: true});
this.dummy.run(function () {
sinon.assert.calledOnce(this.spawnCommandStub);
sinon.assert.calledWithExactly(this.spawnCommandStub, 'yarn', ['add', 'yo', '--dev'], {dev: true});
sinon.assert.calledWithExactly(this.spawnCommandStub, 'yarn', ['add', 'yo', '--dev'], {});
done();
}.bind(this));
});
Expand All @@ -195,7 +198,7 @@ describe('Base (actions/install mixin)', function () {
});

it('execute a callback after installs', function (done) {
this.dummy.installDependencies({ callback: done });
this.dummy.installDependencies({callback: done});
this.dummy.run();
});

Expand Down

0 comments on commit a3cab64

Please sign in to comment.