Skip to content

Commit daec506

Browse files
committed
JBDS-3951 Installer needs to add the VirtualBox install directory to the PATH
Fix resolves test misconceptons and adds additional test to cover case when nothing detected
1 parent f0d630f commit daec506

2 files changed

Lines changed: 57 additions & 25 deletions

File tree

browser/model/virtualbox.js

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,16 @@ class VirtualBoxInstallWindows extends VirtualBoxInstall {
130130
installAfterRequirements(progress, success, failure) {
131131
let installer = new Installer(VirtualBoxInstall.KEY, progress, success, failure);
132132
if(this.selectedOption === 'install') {
133-
installer.execFile(this.downloadedFile,
134-
['--extract',
135-
'-path',
136-
this.installerDataSvc.tempDir(),
137-
'--silent'])
138-
.then((result) => {
139-
return this.configure(installer, result);
140-
})
141-
.then((result) => {
142-
return installer.succeed(result);
143-
})
144-
.catch((error) => {
145-
return installer.fail(error);
146-
});
133+
installer.execFile(
134+
this.downloadedFile, ['--extract', '-path', this.installerDataSvc.tempDir(), '--silent']
135+
).then(() => {
136+
return this.configure(installer);
137+
}).then((result) => {
138+
Platform.addToUserPath([this.option['install'].location]);
139+
return installer.succeed(result);
140+
}).catch((error) => {
141+
return installer.fail(error);
142+
});
147143
} else {
148144
success();
149145
}

test/unit/model/virtualbox-test.js

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import InstallableItem from 'browser/model/installable-item';
1717
import Util from 'browser/model/helpers/util';
1818
import InstallerDataService from 'browser/services/data';
1919
import {ProgressState} from 'browser/pages/install/controller';
20+
import 'sinon-as-promised';
2021
chai.use(sinonChai);
2122

2223
let child_process = require('child_process');
@@ -143,11 +144,12 @@ describe('Virtualbox installer', function() {
143144

144145
describe('on windows', function() {
145146
let installer;
147+
let helper;
146148
beforeEach(function() {
147149
sandbox.stub(Platform, 'getOS').returns('win32');
148150
installer = new VirtualBoxInstallWindows(version, revision, installerDataSvc, downloadUrl, null);
149151
installer.ipcRenderer = { on: function() {} };
150-
})
152+
});
151153

152154
it('should execute the silent extract', function() {
153155
sandbox.stub(child_process, 'execFile').yields('done');
@@ -248,6 +250,24 @@ describe('Virtualbox installer', function() {
248250
}
249251
});
250252

253+
it('should add virtualbox target install folder to user PATH environment variable', function() {
254+
sandbox.stub(child_process, 'execFile').yields(undefined, "", "");
255+
sandbox.stub(installer, 'configure').resolves(true);
256+
sandbox.stub(Platform, 'addToUserPath').resolves(true);
257+
258+
installer.selectedOption = 'install';
259+
installer.addOption('install', installer.version, 'targetLocation', true);
260+
261+
return new Promise((resolve, reject)=> {
262+
installer.install(fakeProgress, resolve, reject);
263+
}).then(()=>{
264+
expect(Platform.addToUserPath).to.be.calledOnce;
265+
expect(Platform.addToUserPath).calledWith(['targetLocation']);
266+
}).catch(()=>{
267+
expect.fail()
268+
});
269+
});
270+
251271
it('should skip installation when an existing version is used', function() {
252272
installer.selectedOption = 'detect';
253273
let spy = sandbox.spy(Installer.prototype, 'execFile');
@@ -262,32 +282,48 @@ describe('Virtualbox installer', function() {
262282

263283
describe('detection', function() {
264284
let validateStub;
285+
const VERSION = '5.0.26r1234';
286+
const VERSION_PARSED = '5.0.26';
287+
const LOCATION = 'folder/vbox';
288+
265289

266290
beforeEach(function() {
267291
let stub = sandbox.stub(Util, 'executeCommand');
268292
if (process.platform === 'win32') {
269293
stub.onCall(0).resolves('%VBOX_INSTALL_PATH%');
270-
stub.onCall(1).resolves('folder/vbox');
271-
stub.onCall(2).resolves('5.0.26r1234');
294+
stub.onCall(1).resolves(LOCATION);
295+
stub.onCall(2).resolves(VERSION);
296+
272297
} else {
273-
stub.onCall(0).resolves('folder/vbox');
274-
stub.onCall(1).resolves('5.0.26r1234');
298+
stub.onCall(0).resolves(LOCATION);
299+
stub.onCall(1).resolves(VERSION);
275300
sandbox.stub(Util, 'findText').resolves('dir=folder/vbox');
276301
}
277-
sandbox.stub(Util, 'folderContains').resolves('folder/vbox');
302+
sandbox.stub(Util, 'folderContains').resolves(LOCATION);
278303
validateStub = sandbox.stub(installer, 'validateVersion').returns();
279304
});
280305

281-
it('should set virtualbox as detected in the appropriate folder when found', function(done) {
282-
return installer.detectExistingInstall(function() {
283-
expect(installer.option['detected'].location).to.equal('folder/vbox');
284-
done();
306+
it('should add option \'detected\' with detected version and location', function() {
307+
return new Promise((resolve, rejects)=> {
308+
installer.detectExistingInstall(resolve);
309+
}).then(()=>{
310+
expect(installer.option['detected'].location).to.equal(LOCATION);
311+
expect(installer.option['detected'].version).to.equal(VERSION_PARSED);
312+
});
313+
});
314+
315+
it('should add option \'install\' when nothing detected', function() {
316+
return new Promise((resolve, rejects)=> {
317+
Util.executeCommand.onCall(2).rejects();
318+
installer.detectExistingInstall(resolve);
319+
}).then(()=>{
320+
expect(installer.option['install']).is.not.undefined;
285321
});
286322
});
287323

288324
it('should check the detected version', function(done) {
289325
return installer.detectExistingInstall(function() {
290-
expect(installer.option['detected'].version).to.equal('5.0.26');
326+
expect(installer.option['detected'].version).to.equal(VERSION_PARSED);
291327
done();
292328
});
293329
});

0 commit comments

Comments
 (0)