Refactoring for Util and Installer API

Fix adds options parameter to Util.executeCommand with macOS default
to process.env and proces.env.PATH altered to have /usr/local/bin.

Fix refactors installer.wrteFile to use Util.writeFile.

Fix increases unit test code coverage for util.js to 100%.

Author: dgolovin

browser/model/helpers/util.js

@@ -1,19 +1,26 @@
 'use strict';
 
 let child_process = require('child_process');
-let fs = require('fs');
+let fs = require('fs-extra');
 
 import Platform from '../../services/platform';
 
 class Util {
 
-  static executeCommand(command, outputCode=1) {
+  static executeCommand(command, outputCode=1,options) {
     return new Promise((resolve, reject) => {
-      let options = {
-        env : Object.assign({},Platform.ENV)
-      };
       if (Platform.OS == 'darwin') {
-        options.env.PATH = options.env.PATH + ':/usr/local/bin';
+        if(options === undefined) {
+          options = {};
+        }
+        if(options['env'] === undefined) {
+          options.env = Object.assign({},Platform.ENV);
+        }
+        if(options.env['PATH']) {
+          options.env.PATH = options.env.PATH + ':/usr/local/bin';
+        } else {
+          options.env.PATH = '/usr/local/bin';
+        }
       }
       child_process.exec(command, options, defaultCallback(resolve, reject, outputCode));
     });
@@ -25,7 +32,7 @@ class Util {
     });
   }
 
-  static writeFile(key, file, data) {
+  static writeFile(file, data) {
     return new Promise((resolve, reject) => {
       fs.writeFile(file, data, (err) => {
         if (err) {

browser/pages/start/controller.js

@@ -34,7 +34,7 @@ class StartController {
 
   launchDevstudio_darwin() {
     let devStudioAppPath = path.join(this.installerDataSvc.jbdsDir(), 'Devstudio.app');
-    Util.executeCommand(`open ${devStudioAppPath}`,1).then((result)=>{
+    Util.executeCommand(`open ${devStudioAppPath}`,1).then(()=>{
       Logger.info('devstudio started sucessfully');
       this.exit();
     }).catch((error)=>{

test/unit/model/helpers/util-test.js

@@ -4,8 +4,9 @@ import chai, { expect } from 'chai';
 import sinon from 'sinon';
 import { default as sinonChai } from 'sinon-chai';
 import Util from 'browser/model/helpers/util';
+import Platform from 'browser/services/platform';
 import child_process from 'child_process';
-import fs from 'fs';
+import fs from 'fs-extra';
 chai.use(sinonChai);
 
 describe('Util', function() {
@@ -39,6 +40,15 @@ describe('Util', function() {
       });
     });
 
+    it('should resolve to standard output when called with only one param', function() {
+      sandbox.stub(child_process, 'exec').yields(null, 'stdout', 'stderr');
+
+      return Util.executeCommand('command')
+      .then((result) => {
+        expect(result).to.equal('stdout');
+      });
+    });
+
     it('should resolve to error output with 2 as second param', function() {
       sandbox.stub(child_process, 'exec').yields(null, 'stdout', 'stderr');
 
@@ -60,6 +70,58 @@ describe('Util', function() {
         expect(error).to.equal(err);
       });
     });
+
+    describe('on macos',function(){
+      describe('when options parameter is undefined', function(){
+        it('should set options.env.PATH to "/usr/local/bin" if process.env.PATH is not present or empty', function(){
+          sandbox.stub(Platform,'getOS').returns('darwin');
+          sandbox.stub(Platform,'getEnv').returns({PATH:''});
+          let mock = sandbox.mock(child_process);
+          let execExpect = mock.expects('exec');
+          execExpect.withArgs('command', {env: {PATH:'/usr/local/bin'}});
+          execExpect.yields(null,'stdout','stderr');
+          return Util.executeCommand('command', 1).then(()=>{
+            mock.verify();
+          });
+        });
+
+        it('should add ":/usr/local/bin" to the options.env.PATH if process.env.PATH is not empty', function(){
+          sandbox.stub(Platform,'getOS').returns('darwin');
+          sandbox.stub(Platform,'getEnv').returns({PATH:'/bin'});
+          let mock = sandbox.mock(child_process);
+          let execExpect = mock.expects('exec');
+          execExpect.withArgs('command', {env: {PATH:'/bin:/usr/local/bin'}});
+          execExpect.yields(null,'stdout','stderr');
+          return Util.executeCommand('command', 1).then(()=>{
+            mock.verify();
+          });
+        });
+      });
+
+      describe('when options parameter is provided', function(){
+        it('should set options.env.PATH to "/usr/local/bin" if options.env.PATH is not present or empty', function(){
+          sandbox.stub(Platform,'getOS').returns('darwin');
+          let mock = sandbox.mock(child_process);
+          let execExpect = mock.expects('exec');
+          execExpect.withArgs('command', {env: {PATH:'/usr/local/bin'}});
+          execExpect.yields(null,'stdout','stderr');
+          return Util.executeCommand('command', 1, {env: {PATH:''}}).then(()=>{
+            mock.verify();
+          });
+        });
+
+        it('should add ":/usr/local/bin" to the options.env.PATH if options.env.PATH is not empty', function(){
+          sandbox.stub(Platform,'getOS').returns('darwin');
+          let mock = sandbox.mock(child_process);
+          let execExpect = mock.expects('exec');
+          execExpect.withArgs('command', {env: {PATH:'/bin:/usr/local/bin'}});
+          execExpect.yields(null,'stdout','stderr');
+          return Util.executeCommand('command', 1, {env: {PATH:'/bin'}}).then(()=>{
+            mock.verify();
+          });
+        });
+      });
+    });
   });
 
   describe('executeFile', function() {
@@ -82,6 +144,15 @@ describe('Util', function() {
       });
     });
 
+    it('should resolve to standard output when called with only one param', function() {
+      sandbox.stub(child_process, 'execFile').yields(null, 'stdout', 'stderr');
+
+      return Util.executeFile('file', 'arguments')
+      .then((result) => {
+        expect(result).to.equal('stdout');
+      });
+    });
+
     it('should resolve to error output with 2 as second param', function() {
       sandbox.stub(child_process, 'execFile').yields(null, 'stdout', 'stderr');
 
@@ -210,4 +281,16 @@ describe('Util', function() {
       });
     });
   });
+
+  describe('writeFile', function(){
+    it('calls fs#writeFile with correct arguments', function(){
+      sandbox.stub(fs, 'writeFile').yields();
+
+      return Util.writeFile('file','data')
+      .then(function() {
+        expect(fs.writeFile).to.have.been.calledOnce;
+        expect(fs.writeFile).to.have.been.calledWith('file','data');
+      });
+    });
+  });
 });