Skip to content

Commit a014d1f

Browse files
dgolovindgolovin
committed
Improve code coverage for JDK Installer
Fix adds platform specific tests to JDK unit tests, corrects wrong expect calls and gets coverage to back to 100%.
1 parent e76f3f7 commit a014d1f

3 files changed

Lines changed: 84 additions & 43 deletions

File tree

‎browser/model/installable-item.js‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class InstallableItem {
3737
this.detectedInstallLocation = '';
3838

3939
if (downloadUrl == null || downloadUrl == '') {
40-
throw(new Error(`No download URL set for ${keyName} Installer`));
40+
throw(new Error(`No download URL set for ${keyName} Installer`));
4141
}
4242

4343
this.downloadUrl = downloadUrl;
@@ -49,11 +49,11 @@ class InstallableItem {
4949

5050
this.isCollapsed = true;
5151
this.option = new Set();
52-
this.selectedOption = "install";
52+
this.selectedOption = 'install';
5353

5454
this.downloader = null;
55-
this.downloadFolder = path.normalize(path.join(__dirname,"../../../.."));
56-
this.downloadedFile = "";
55+
this.downloadFolder = path.normalize(path.join(__dirname,'../../../..'));
56+
this.downloadedFile = '';
5757

5858
this.installAfter = undefined;
5959
this.ipcRenderer = ipcRenderer;
@@ -176,13 +176,8 @@ class InstallableItem {
176176
}
177177
}
178178

179-
setup(progress, success, failure) {
180-
// To be overridden
181-
success();
182-
}
183-
184179
changeIsCollapsed() {
185-
this.isCollapsed = !this.isCollapsed;
180+
this.isCollapsed = !this.isCollapsed;
186181
}
187182

188183
hasOption(name) {

‎browser/model/jdk-install.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,21 @@ class JdkInstall extends InstallableItem {
5858
this.validateVersion();
5959
if(this.option['detected'].valid) {
6060
this.selectedOption = 'detected';
61-
} else if(process.platform !== 'darwin') {
61+
} else if(Platform.OS !== 'darwin') {
6262
this.selectedOption = 'install';
6363
}
6464
resolve(true);
6565
} else {
6666
reject('No java detected');
6767
}
6868
});
69-
}).then((result) => {
69+
}).then(() => {
7070
return Util.executeCommand(command, 2);
7171
}).then((output) => {
7272
let locationRegex = /java\.home*\s=*\s(.*)[\s\S]/;
7373
this.openJdk = output.includes('OpenJDK');
7474
var t = locationRegex.exec(output);
75-
if(t.length > 1) {
75+
if(t && t.length > 1) {
7676
this.option['detected'].location = t[1];
7777
} else {
7878
this.selectedOption = 'install';
@@ -151,7 +151,7 @@ class JdkInstall extends InstallableItem {
151151
this.installerDataSvc.jdkRoot = targetDir;
152152
}
153153
installer.succeed(true);
154-
}).catch((err)=>{
154+
}).catch(()=>{
155155
// location doesn't parsed correctly, nothing to verify just resolve and keep going
156156
installer.succeed(result);
157157
});

‎test/unit/model/jdk-install-test.js‎

Lines changed: 75 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ describe('JDK installer', function() {
7272
sandbox.restore();
7373
});
7474

75+
function mockDetectedJvm(version, location = 'java.home = /java/home\n') {
76+
sandbox.stub(JdkInstall.prototype, 'findMsiInstalledJava').returns(Promise.resolve(''));
77+
sandbox.stub(Util,'executeCommand')
78+
.onFirstCall().returns(Promise.resolve(`version "${version}"`))
79+
.onSecondCall().returns(Promise.resolve(location));
80+
sandbox.stub(Util,'writeFile').returns(Promise.resolve(true));
81+
sandbox.stub(Util,'executeFile').returns(Promise.resolve(true));
82+
}
83+
7584
describe('when instantiated', function() {
7685

7786
it('should not download jdk when an installation exists', function() {
@@ -99,19 +108,12 @@ describe('JDK installer', function() {
99108
expect(new JdkInstall(installerDataSvc, 'url', null).downloadedFile).to.equal(
100109
path.join('tempDirectory', 'jdk.msi'));
101110
});
102-
103111
});
104112

113+
// FIXME expect calls in done callback does not report errors
114+
// because if expect fails it gets cought in catch() and then
115+
// function callback done called again
105116
describe('when detecting existing installation',function() {
106-
let writeFileStub, executeFileStub;
107-
function mockDetectedJvm(version) {
108-
sandbox.stub(JdkInstall.prototype, 'findMsiInstalledJava').returns(Promise.resolve(''));
109-
sandbox.stub(Util,'executeCommand')
110-
.onFirstCall().returns(Promise.resolve(`version "${version}"`))
111-
.onSecondCall().returns(Promise.resolve('java.home = /java/home\n'));
112-
writeFileStub = sandbox.stub(Util,'writeFile').returns(Promise.resolve(true));
113-
executeFileStub = sandbox.stub(Util,'executeFile').returns(Promise.resolve(true));
114-
}
115117

116118
it('should detect java location if installed', function(done) {
117119
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
@@ -124,16 +126,6 @@ describe('JDK installer', function() {
124126
});
125127
});
126128

127-
it('should select openjdk for installation if older than supported version detected', function(done) {
128-
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
129-
mockDetectedJvm('1.7.0_111');
130-
return jdk.detectExistingInstall(function() {
131-
expect(jdk.selectedOption).to.be.equal('install');
132-
expect(jdk.getLocation()).to.be.equal('');
133-
done();
134-
});
135-
});
136-
137129
it('should create deafult empty callback if not provided', function() {
138130
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
139131
mockDetectedJvm('1.8.0_1');
@@ -144,8 +136,16 @@ describe('JDK installer', function() {
144136
}
145137
});
146138

139+
it('should not fail if selected option is not present in available options', function(){
140+
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
141+
jdk.selectedOption = 'detected';
142+
jdk.validateVersion();
143+
});
144+
145+
146+
147147
describe('on windows', function(){
148-
it('should select openjdk for installation if not java detected', function(done) {
148+
it('should select openjdk for installation if no java detected', function(done) {
149149
sandbox.stub(Platform,'getOS').returns('win32');
150150
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
151151
mockDetectedJvm('');
@@ -156,13 +156,34 @@ describe('JDK installer', function() {
156156
});
157157
});
158158

159-
it('should select openjdk for installation if newer than supported version detected', function(done) {
159+
160+
// FIXME is not the case for JDK 9, because version has different format
161+
it('should select openjdk for installation if newer than supported java version detected', function(done) {
160162
sandbox.stub(Platform,'getOS').returns('win32');
161163
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
162164
mockDetectedJvm('1.9.0_1');
165+
return jdk.detectExistingInstall(function() {
166+
expect(jdk.selectedOption).to.be.equal('detected');
167+
done();
168+
});
169+
});
170+
171+
it('should select openjdk for installation if older than supported java version detected', function(done) {
172+
sandbox.stub(Platform,'getOS').returns('win32');
173+
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
174+
mockDetectedJvm('1.7.0_1');
175+
return jdk.detectExistingInstall(function() {
176+
expect(jdk.selectedOption).to.be.equal('install');
177+
done();
178+
});
179+
});
180+
181+
it('should select openjdk for installation if location for java is not found', function(done) {
182+
sandbox.stub(Platform,'getOS').returns('win32');
183+
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
184+
mockDetectedJvm('1.8.0_1','');
163185
return jdk.detectExistingInstall(function() {
164186
expect(jdk.selectedOption).to.be.equal('install');
165-
expect(jdk.getLocation()).to.be.equal('');
166187
done();
167188
});
168189
});
@@ -173,15 +194,15 @@ describe('JDK installer', function() {
173194
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
174195
jdk.findMsiInstalledJava.restore();
175196
return jdk.detectExistingInstall(function() {
176-
expect(executeFileStub).to.have.been.calledOnce;
177-
expect(writeFileStub).to.have.been.calledOnce;
197+
expect(Util.executeFile).to.have.been.calledOnce;
198+
expect(Util.writeFile).to.have.been.calledOnce;
178199
done();
179200
});
180201
});
181202
});
182203

183204
describe('on macos', function() {
184-
it('should not select jdk for installation if not java detected', function(done) {
205+
it('should not select jdk for installation if no java detected', function(done) {
185206
sandbox.stub(Platform,'getOS').returns('darwin');
186207
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
187208
mockDetectedJvm('');
@@ -191,7 +212,7 @@ describe('JDK installer', function() {
191212
});
192213
});
193214

194-
it('should not select openjdk for installation if newer than supported version detected', function(done) {
215+
it('should not select openjdk for installation if newer than supported supported version detected', function(done) {
195216
sandbox.stub(Platform,'getOS').returns('darwin');
196217
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
197218
mockDetectedJvm('1.9.0_1');
@@ -201,14 +222,24 @@ describe('JDK installer', function() {
201222
});
202223
});
203224

225+
it('should not select openjdk for installation if older than supported supported java version detected', function(done) {
226+
sandbox.stub(Platform,'getOS').returns('darwin');
227+
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
228+
mockDetectedJvm('1.7.0_1');
229+
return jdk.detectExistingInstall(function() {
230+
expect(jdk.selectedOption).to.be.equal('detected');
231+
done();
232+
});
233+
});
234+
204235
it('should not check for available msi installtion', function(done) {
205236
sandbox.stub(Platform,'getOS').returns('darwin');
206237
mockDetectedJvm('1.8.0_1');
207238
let jdk = new JdkInstall(installerDataSvc, 'url', 'file');
208239
jdk.findMsiInstalledJava.restore();
209240
return jdk.detectExistingInstall(function() {
210-
expect(executeFileStub).to.have.not.been.called;
211-
expect(writeFileStub).to.have.not.been.called;
241+
expect(Util.executeFile).to.have.not.been.called;
242+
expect(Util.writeFile).to.have.not.been.called;
212243
done();
213244
});
214245
});
@@ -335,6 +366,21 @@ describe('JDK installer', function() {
335366
expect(calls).to.equal(1);
336367
});
337368

369+
it('should not change installerDataSvc.jdkRoot if the same location found in install log', function(done) {
370+
sandbox.stub(require('child_process'), 'execFile').yields();
371+
sandbox.stub(Installer.prototype,'execFile').returns(Promise.resolve(true));
372+
sandbox.stub(Util,'findText').returns(Promise.resolve('Dir \(target\): Key: INSTALLDIR , Object: target/install'));
373+
installer = new JdkInstall(installerDataSvc, downloadUrl, null);
374+
sandbox.stub(installer,'getLocation').returns('target/install');
375+
installerDataSvc.jdkRoot = 'install/jdk8';
376+
return installer.install(fakeProgress, function() {
377+
expect(installerDataSvc.jdkRoot).to.be.equal('install/jdk8');
378+
done();
379+
}, function(){
380+
expect.fail('it should not fail');
381+
});
382+
});
383+
338384
describe('files manipulation', function() {
339385
let err = new Error('critical error');
340386

0 commit comments

Comments
 (0)