Applied changes requested in pull review

src/server/bin/kibana.bat

@@ -9,7 +9,6 @@ set NODE=%DIR%\node\node.exe
 set SERVER=%DIR%\src\bin\kibana.js
 set NODE_ENV="production"
 set CONFIG_PATH=%DIR%\config\kibana.yml
-set NPM=npm
 
 TITLE Kibana Server @@version
 

src/server/bin/kibana.js

@@ -1,10 +1,10 @@
 #!/usr/bin/env node
 
 var program = require('commander');
-require('../lib/commanderExtensions.js')(program);
+require('../lib/commanderExtensions')(program);
 var path = require('path');
-var startupOptions = require('./startup/startupOptions.js');
-var startup = require('./startup/startup.js');
+var startupOptions = require('./startup/startupOptions');
+var startup = require('./startup/startup');
 var pluginProgram = require('./plugin/plugin');
 
 var env = (process.env.NODE_ENV) ? process.env.NODE_ENV : 'development';

src/server/bin/plugin/npmInstall.js

@@ -16,8 +16,7 @@ module.exports = function (dest, logger) {
       return reject(new Error('Plugin does not contain package.json file'));
     }
 
-    var cmd = (process.env.NPM) ? process.env.NPM : 'npm';
-    cmd += ' install';
+    var cmd = '"' + path.resolve(path.dirname(process.execPath), 'npm').replace(/\\/g, '/') + '" install --production';
 
     var child = exec(cmd, { cwd: dest });
     child.on('error', function (err) {
@@ -27,7 +26,7 @@ module.exports = function (dest, logger) {
       if (code === 0) {
         resolve();
       } else {
-        reject(new Error('npm install failed.'));
+        reject(new Error('npm install failed with code ' + code));
       }
     });
 

src/server/bin/plugin/plugin.js

@@ -24,10 +24,26 @@ module.exports = function (program) {
     }
   }
 
+  var installDesc =
+    'The plugin to install\n\n' +
+    '\tCommon examples:\n' +
+    '\t  -i username/sample\n' +
+    '\t    attempts to download the latest version from the following urls:\n' +
+    '\t      https://download.elastic.co/username/sample/sample-latest.tar.gz\n' +
+    '\t      https://github.com/username/sample/archive/master.tar.gz\n\n' +
+    '\t  -i username/sample/v1.1.1\n' +
+    '\t    attempts to download from the following urls:\n' +
+    '\t      https://download.elastic.co/username/sample/sample-v1.1.1.tar.gz\n' +
+    '\t      https://github.com/username/sample/archive/v1.1.1.tar.gz\n\n' +
+    '\t  -i sample -u http://www.example.com/other_name.tar.gz\n' +
+    '\t    attempts to download from the specified url,\n' +
+    '\t    and installs the plugin found at that url as "sample"' +
+    '\n';
+
   program
     .command('plugin')
     .description('Maintain Plugins')
-    .option('-i, --install <org>/<plugin>/<version>', 'The plugin to install')
+    .option('-i, --install <org>/<plugin>/<version>', installDesc)
     .option('-r, --remove <plugin>', 'The plugin to remove')
     .option('-s, --silent', 'Disable process messaging')
     .option('-u, --url <url>', 'Specify download url')

src/server/bin/plugin/pluginDownloader.js

@@ -3,7 +3,7 @@ var zlib = require('zlib');
 var Promise = require('bluebird');
 var request = require('request');
 var tar = require('tar');
-var progressReporter = require('./progressReporter.js');
+var progressReporter = require('./progressReporter');
 
 module.exports = function (settings, logger) {
 

src/server/bin/plugin/pluginInstaller.js

@@ -1,6 +1,6 @@
-var pluginDownloader = require('./pluginDownloader.js');
-var pluginCleaner = require('./pluginCleaner.js');
-var npmInstall = require('./npmInstall.js');
+var pluginDownloader = require('./pluginDownloader');
+var pluginCleaner = require('./pluginCleaner');
+var npmInstall = require('./npmInstall');
 var fs = require('fs');
 
 module.exports = {
@@ -35,8 +35,7 @@ function install(settings, logger) {
     logger.log('Plugin installation complete!');
   })
   .catch(function (e) {
-    logger.error('Plugin installation was unsuccessful.');
-    logger.error(e.message);
+    logger.error('Plugin installation was unsuccessful due to error "' + e.message + '"');
     cleaner.cleanError();
     process.exit(70);
   });

src/server/bin/plugin/pluginLogger.js

@@ -15,12 +15,14 @@
       return;
     }
 
-    if (!sameLine) data += '\n';
     process.stdout.write(data);
+    if (!sameLine) process.stdout.write('\n');
     previousLineEnded = !sameLine;
   }
 
   function error(data) {
+    if (silent) return;
+
     if (!previousLineEnded) {
       process.stderr.write('\n');
     }

src/server/bin/plugin/pluginRemover.js

@@ -18,8 +18,9 @@ function remove(settings, logger) {
     logger.log('Removing ' + settings.package + '...');
 
     rimraf.sync(settings.pluginPath);
-  } catch (ex) {
-    logger.error(ex.message);
+  } catch (err) {
+    var message = 'Unable to remove plugin "' + settings.package + '" because of error: "' + err.message + '"';
+    logger.error(message);
     process.exit(74);
   }
 }

test/unit/server/bin/plugin/plugin.js

@@ -12,52 +12,43 @@ describe('kibana cli', function () {
 
     describe('settings.action', function () {
 
-      var program = {
-        command: function () { return program; },
-        description: function () { return program; },
-        option: function () { return program; },
-        action: function (processCommand) {
-          processCommand();
-        }
-      };
-
-      beforeEach(function () {
-        sinon.stub(remover, 'remove');
-        sinon.stub(installer, 'install');
-      });
-
-      afterEach(function () {
-        remover.remove.restore();
-        installer.install.restore();
-        settingParser.parse.restore();
-      });
-
-      it('should call remove if settings.action is "remove"', function () {
-        sinon.stub(settingParser, 'parse', function () {
-          return {
-            action: 'remove'
-          };
+      function testUsageOfAction(action) {
+        describe('when action is ' + action, function () {
+          before(function () {
+            var program = {
+              command: function () { return program; },
+              description: function () { return program; },
+              option: function () { return program; },
+              action: function (processCommand) {
+                processCommand();
+              }
+            };
+
+            sinon.stub(remover, 'remove');
+            sinon.stub(installer, 'install');
+
+            sinon
+            .stub(settingParser, 'parse')
+            .returns({ action: action });
+
+            plugin(program);
+          });
+
+          it('calls the right function', function () {
+            expect(remover.remove.called).to.be(action === 'remove');
+            expect(installer.install.called).to.be(action === 'install');
+          });
+
+          after(function () {
+            remover.remove.restore();
+            installer.install.restore();
+            settingParser.parse.restore();
+          });
         });
+      }
 
-        plugin(program);
-
-        expect(remover.remove.called).to.be(true);
-        expect(installer.install.called).to.be(false);
-      });
-
-      it('should call install if settings.action is "install"', function () {
-        sinon.stub(settingParser, 'parse', function () {
-          return {
-            action: 'install'
-          };
-        });
-
-        plugin(program);
-
-        expect(remover.remove.called).to.be(false);
-        expect(installer.install.called).to.be(true);
-      });
-
+      testUsageOfAction('remove');
+      testUsageOfAction('install');
     });
 
     describe('commander options', function () {

test/unit/server/bin/plugin/pluginLogger.js

@@ -26,7 +26,10 @@ describe('kibana cli', function () {
           var message = 'this is my message';
 
           logger.log(message);
-          expect(process.stdout.write.calledWith(message + '\n')).to.be(true);
+
+          var callCount = process.stdout.write.callCount;
+          expect(process.stdout.write.getCall(callCount - 2).args[0]).to.be(message);
+          expect(process.stdout.write.getCall(callCount - 1).args[0]).to.be('\n');
         });
 
         it('should log messages to the console and append not append a new line', function () {
@@ -35,7 +38,7 @@ describe('kibana cli', function () {
           }
           logger.log('Done!');
 
-          expect(process.stdout.write.callCount).to.be(12);
+          expect(process.stdout.write.callCount).to.be(13);
           expect(process.stdout.write.getCall(0).args[0]).to.be('.');
           expect(process.stdout.write.getCall(1).args[0]).to.be('.');
           expect(process.stdout.write.getCall(2).args[0]).to.be('.');
@@ -47,7 +50,8 @@ describe('kibana cli', function () {
           expect(process.stdout.write.getCall(8).args[0]).to.be('.');
           expect(process.stdout.write.getCall(9).args[0]).to.be('.');
           expect(process.stdout.write.getCall(10).args[0]).to.be('\n');
-          expect(process.stdout.write.getCall(11).args[0]).to.be('Done!\n');
+          expect(process.stdout.write.getCall(11).args[0]).to.be('Done!');
+          expect(process.stdout.write.getCall(12).args[0]).to.be('\n');
         });
 
         it('should not log any messages when silent is set', function () {
@@ -84,13 +88,13 @@ describe('kibana cli', function () {
           expect(process.stderr.write.calledWith(message + '\n')).to.be(true);
         });
 
-
-        it('should log error messages to the console regardless of silent setting', function () {
+        it('should not log any error messages when silent is set', function () {
           logger = pluginLogger(true);
           var message = 'this is my error';
 
           logger.error(message);
-          expect(process.stderr.write.calledWith(message + '\n')).to.be(true);
+
+          expect(process.stderr.write.callCount).to.be(0);
         });
 
       });