Skip to content

Commit cf583a5

Browse files
Merge pull request #5178 from NativeScript/vladimirov/doctor-improvements
feat: doctor improvements
2 parents 65278fa + c601915 commit cf583a5

File tree

6 files changed

+231
-20
lines changed

6 files changed

+231
-20
lines changed

lib/common/commands/doctor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export class DoctorCommand implements ICommand {
66
public allowedParameters: ICommandParameter[] = [];
77

88
public execute(args: string[]): Promise<void> {
9-
return this.$doctorService.printWarnings({ trackResult: false, projectDir: this.$projectHelper.projectDir });
9+
return this.$doctorService.printWarnings({ trackResult: false, projectDir: this.$projectHelper.projectDir, forceCheck: true });
1010
}
1111
}
1212

lib/common/declarations.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,7 @@ interface IDoctorService {
11951195
* @param configOptions: defines if the result should be tracked by Analytics
11961196
* @returns {Promise<void>}
11971197
*/
1198-
printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void>;
1198+
printWarnings(configOptions?: { trackResult?: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions, forceCheck?: boolean }): Promise<void>;
11991199
/**
12001200
* Runs the setup script on host machine
12011201
* @returns {Promise<ISpawnResult>}
@@ -1206,7 +1206,7 @@ interface IDoctorService {
12061206
* @param platform @optional The current platform
12071207
* @returns {Promise<boolean>} true if the environment is properly configured for local builds
12081208
*/
1209-
canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean>;
1209+
canExecuteLocalBuild(configuration?: { platform?: string, projectDir?: string, runtimeVersion?: string, forceCheck?: boolean }): Promise<boolean>;
12101210

12111211
/**
12121212
* Checks and notifies users for deprecated short imports in their applications.

lib/definitions/platform.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ interface ICheckEnvironmentRequirementsInput {
8080
runtimeVersion?: string;
8181
options?: IOptions;
8282
notConfiguredEnvOptions?: INotConfiguredEnvOptions;
83+
forceCheck?: boolean;
8384
}
8485

8586
interface ICheckEnvironmentRequirementsOutput {

lib/services/doctor-service.ts

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { EOL } from "os";
22
import * as path from "path";
33
import * as helpers from "../common/helpers";
4+
import { cache } from "../common/decorators";
45
import { TrackActionNames, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME } from "../constants";
56
import { doctor, constants } from "nativescript-doctor";
67

@@ -9,6 +10,16 @@ export class DoctorService implements IDoctorService {
910
private static WindowsSetupScriptExecutable = "powershell.exe";
1011
private static WindowsSetupScriptArguments = ["start-process", "-FilePath", "PowerShell.exe", "-NoNewWindow", "-Wait", "-ArgumentList", '"-NoProfile -ExecutionPolicy Bypass -Command iex ((new-object net.webclient).DownloadString(\'https://www.nativescript.org/setup/win\'))"'];
1112

13+
@cache()
14+
private get jsonFileSettingsPath(): string {
15+
return path.join(this.$settingsService.getProfileDir(), "doctor-cache.json");
16+
}
17+
18+
@cache()
19+
private get $jsonFileSettingsService(): IJsonFileSettingsService {
20+
return this.$injector.resolve<IJsonFileSettingsService>("jsonFileSettingsService", { jsonFileSettingsPath: this.jsonFileSettingsPath });
21+
}
22+
1223
constructor(private $analyticsService: IAnalyticsService,
1324
private $hostInfo: IHostInfo,
1425
private $logger: ILogger,
@@ -17,12 +28,15 @@ export class DoctorService implements IDoctorService {
1728
private $projectDataService: IProjectDataService,
1829
private $fs: IFileSystem,
1930
private $terminalSpinnerService: ITerminalSpinnerService,
20-
private $versionsService: IVersionsService) { }
31+
private $versionsService: IVersionsService,
32+
private $settingsService: ISettingsService) { }
2133

22-
public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
34+
public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions, forceCheck?: boolean }): Promise<void> {
35+
configOptions = configOptions || <any>{};
36+
const getInfosData: any = { projectDir: configOptions.projectDir, androidRuntimeVersion: configOptions.runtimeVersion };
2337
const infos = await this.$terminalSpinnerService.execute<NativeScriptDoctor.IInfo[]>({
2438
text: `Getting environment information ${EOL}`
25-
}, () => doctor.getInfos({ projectDir: configOptions && configOptions.projectDir, androidRuntimeVersion: configOptions && configOptions.runtimeVersion }));
39+
}, () => this.getInfos({ forceCheck: configOptions.forceCheck }, getInfosData));
2640

2741
const warnings = infos.filter(info => info.type === constants.WARNING_TYPE_NAME);
2842
const hasWarnings = warnings.length > 0;
@@ -39,8 +53,12 @@ export class DoctorService implements IDoctorService {
3953

4054
if (hasWarnings) {
4155
this.$logger.info("There seem to be issues with your configuration.");
56+
// cleanup the cache file as there seems to be issues with the current config
57+
// all projects need to be rechecked
58+
this.$fs.deleteFile(this.jsonFileSettingsPath);
4259
} else {
4360
this.$logger.info("No issues were detected.".bold);
61+
await this.$jsonFileSettingsService.saveSetting(this.getKeyForConfiguration(getInfosData), infos);
4462
this.printInfosCore(infos);
4563
}
4664

@@ -54,9 +72,9 @@ export class DoctorService implements IDoctorService {
5472

5573
await this.$injector.resolve<IPlatformEnvironmentRequirements>("platformEnvironmentRequirements").checkEnvironmentRequirements({
5674
platform: null,
57-
projectDir: configOptions && configOptions.projectDir,
58-
runtimeVersion: configOptions && configOptions.runtimeVersion,
59-
options: configOptions && configOptions.options
75+
projectDir: configOptions.projectDir,
76+
runtimeVersion: configOptions.runtimeVersion,
77+
options: configOptions.options
6078
});
6179
}
6280

@@ -90,23 +108,27 @@ export class DoctorService implements IDoctorService {
90108
});
91109
}
92110

93-
public async canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean> {
111+
public async canExecuteLocalBuild(configuration?: { platform?: string, projectDir?: string, runtimeVersion?: string, forceCheck?: boolean }): Promise<boolean> {
94112
await this.$analyticsService.trackEventActionInGoogleAnalytics({
95113
action: TrackActionNames.CheckLocalBuildSetup,
96114
additionalData: "Starting",
97115
});
98-
const infos = await doctor.getInfos({ platform, projectDir, androidRuntimeVersion: runtimeVersion });
99-
116+
const sysInfoConfig: NativeScriptDoctor.ISysInfoConfig = { platform: configuration.platform, projectDir: configuration.projectDir, androidRuntimeVersion: configuration.runtimeVersion };
117+
const infos = await this.getInfos({ forceCheck: configuration && configuration.forceCheck }, sysInfoConfig);
100118
const warnings = this.filterInfosByType(infos, constants.WARNING_TYPE_NAME);
101119
const hasWarnings = warnings.length > 0;
102120
if (hasWarnings) {
121+
// cleanup the cache file as there seems to be issues with the current config
122+
// all projects need to be rechecked
123+
this.$fs.deleteFile(this.jsonFileSettingsPath);
103124
await this.$analyticsService.trackEventActionInGoogleAnalytics({
104125
action: TrackActionNames.CheckLocalBuildSetup,
105126
additionalData: `Warnings:${warnings.map(w => w.message).join("__")}`,
106127
});
107128
this.printInfosCore(infos);
108129
} else {
109130
infos.map(info => this.$logger.trace(info.message));
131+
await this.$jsonFileSettingsService.saveSetting(this.getKeyForConfiguration(sysInfoConfig), infos);
110132
}
111133

112134
await this.$analyticsService.trackEventActionInGoogleAnalytics({
@@ -221,5 +243,40 @@ export class DoctorService implements IDoctorService {
221243
private filterInfosByType(infos: NativeScriptDoctor.IInfo[], type: string): NativeScriptDoctor.IInfo[] {
222244
return infos.filter(info => info.type === type);
223245
}
246+
247+
private getKeyForConfiguration(sysInfoConfig?: NativeScriptDoctor.ISysInfoConfig): string {
248+
const nativeScriptData = sysInfoConfig && sysInfoConfig.projectDir && JSON.stringify(this.$fs.readJson(path.join(sysInfoConfig.projectDir, "package.json")).nativescript);
249+
const delimiter = "__";
250+
const key = [
251+
JSON.stringify(sysInfoConfig),
252+
process.env.ANDROID_HOME,
253+
process.env.JAVA_HOME,
254+
process.env["CommonProgramFiles(x86)"],
255+
process.env["CommonProgramFiles"],
256+
process.env.PROCESSOR_ARCHITEW6432,
257+
process.env.ProgramFiles,
258+
process.env["ProgramFiles(x86)"],
259+
nativeScriptData
260+
]
261+
.filter(a => !!a)
262+
.join(delimiter);
263+
264+
const data = helpers.getHash(key, { algorithm: "md5" });
265+
return data;
266+
}
267+
268+
private async getInfos(cacheConfig: { forceCheck: boolean }, sysInfoConfig?: NativeScriptDoctor.ISysInfoConfig): Promise<NativeScriptDoctor.IInfo[]> {
269+
const key = this.getKeyForConfiguration(sysInfoConfig);
270+
271+
const infosFromCache = cacheConfig.forceCheck ?
272+
null :
273+
await this.$jsonFileSettingsService.getSettingValue<NativeScriptDoctor.IInfo[]>(key);
274+
275+
this.$logger.trace(`getInfos cacheConfig options:`, cacheConfig, " current info from cache: ", infosFromCache);
276+
277+
const infos = infosFromCache || await doctor.getInfos(sysInfoConfig);
278+
279+
return infos;
280+
}
224281
}
225282
$injector.register("doctorService", DoctorService);

lib/services/platform-environment-requirements.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
5757
};
5858
}
5959

60-
const canExecute = await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion);
60+
const canExecute = await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck });
61+
6162
if (!canExecute) {
6263
if (!isInteractive()) {
6364
await this.$analyticsService.trackEventActionInGoogleAnalytics({
@@ -80,7 +81,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
8081
if (selectedOption === PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME) {
8182
await this.$doctorService.runSetupScript();
8283

83-
if (await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion)) {
84+
if (await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck })) {
8485
return {
8586
canExecute: true,
8687
selectedOption
@@ -114,7 +115,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
114115

115116
if (selectedOption === PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME) {
116117
await this.processBothCloudBuildsAndSetupScript();
117-
if (await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion)) {
118+
if (await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck })) {
118119
return {
119120
canExecute: true,
120121
selectedOption

test/services/doctor-service.ts

Lines changed: 157 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { DoctorService } from "../../lib/services/doctor-service";
22
import { Yok } from "../../lib/common/yok";
3-
import { LoggerStub } from "../stubs";
3+
import { LoggerStub, FileSystemStub } from "../stubs";
44
import { assert } from "chai";
55
import * as path from "path";
6+
import * as sinon from "sinon";
7+
const nativescriptDoctor = require("nativescript-doctor");
68

79
class DoctorServiceInheritor extends DoctorService {
810
constructor($analyticsService: IAnalyticsService,
@@ -13,8 +15,9 @@ class DoctorServiceInheritor extends DoctorService {
1315
$projectDataService: IProjectDataService,
1416
$fs: IFileSystem,
1517
$terminalSpinnerService: ITerminalSpinnerService,
16-
$versionsService: IVersionsService) {
17-
super($analyticsService, $hostInfo, $logger, $childProcess, $injector, $projectDataService, $fs, $terminalSpinnerService, $versionsService);
18+
$versionsService: IVersionsService,
19+
$settingsService: ISettingsService) {
20+
super($analyticsService, $hostInfo, $logger, $childProcess, $injector, $projectDataService, $fs, $terminalSpinnerService, $versionsService, $settingsService);
1821
}
1922

2023
public getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
@@ -31,9 +34,26 @@ describe("doctorService", () => {
3134
testInjector.register("logger", LoggerStub);
3235
testInjector.register("childProcess", {});
3336
testInjector.register("projectDataService", {});
34-
testInjector.register("fs", {});
35-
testInjector.register("terminalSpinnerService", {});
37+
testInjector.register("fs", FileSystemStub);
38+
testInjector.register("terminalSpinnerService", {
39+
execute: (spinnerOptions: ITerminalSpinnerOptions, action: () => Promise<any>): Promise<any> => action(),
40+
createSpinner: (spinnerOptions?: ITerminalSpinnerOptions): ITerminalSpinner => (<any>{
41+
text: '',
42+
succeed: (): any => undefined,
43+
fail: (): any => undefined
44+
})
45+
});
3646
testInjector.register("versionsService", {});
47+
testInjector.register("settingsService", {
48+
getProfileDir: (): string => ""
49+
});
50+
testInjector.register("jsonFileSettingsService", {
51+
getSettingValue: async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => undefined,
52+
saveSetting: async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => undefined
53+
});
54+
testInjector.register("platformEnvironmentRequirements", {
55+
checkEnvironmentRequirements: async (input: ICheckEnvironmentRequirementsInput): Promise<ICheckEnvironmentRequirementsOutput> => (<any>{})
56+
});
3757

3858
return testInjector;
3959
};
@@ -254,4 +274,136 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl
254274
});
255275
});
256276
});
277+
278+
describe("printWarnings", () => {
279+
let sandbox: sinon.SinonSandbox;
280+
281+
beforeEach(() => {
282+
sandbox = sinon.sandbox.create();
283+
});
284+
285+
afterEach(() => {
286+
sandbox.restore();
287+
});
288+
const successGetInfosResult = [{
289+
message:
290+
'Your ANDROID_HOME environment variable is set and points to correct directory.',
291+
platforms: ['Android'],
292+
type: 'info'
293+
},
294+
{
295+
message: 'Xcode is installed and is configured properly.',
296+
platforms: ['iOS'],
297+
type: 'info'
298+
}];
299+
300+
const failedGetInfosResult = [{
301+
message:
302+
'The ANDROID_HOME environment variable is not set or it points to a non-existent directory. You will not be able to perform any build-related operations for Android.',
303+
additionalInformation:
304+
'To be able to perform Android build-related operations, set the `ANDROID_HOME` variable to point to the root of your Android SDK installation directory.',
305+
platforms: ['Android'],
306+
type: 'warning'
307+
},
308+
{
309+
message:
310+
'WARNING: adb from the Android SDK is not installed or is not configured properly. ',
311+
additionalInformation:
312+
'For Android-related operations, the NativeScript CLI will use a built-in version of adb.\nTo avoid possible issues with the native Android emulator, Genymotion or connected\nAndroid devices, verify that you have installed the latest Android SDK and\nits dependencies as described in http://developer.android.com/sdk/index.html#Requirements',
313+
platforms: ['Android'],
314+
type: 'warning'
315+
},
316+
{
317+
message: 'Xcode is installed and is configured properly.',
318+
platforms: ['iOS'],
319+
type: 'info'
320+
}];
321+
322+
it("prints correct message when no issues are detected", async () => {
323+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
324+
nsDoctorStub.returns(successGetInfosResult);
325+
const testInjector = createTestInjector();
326+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
327+
const logger = testInjector.resolve<LoggerStub>("logger");
328+
await doctorService.printWarnings();
329+
assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
330+
});
331+
332+
it("prints correct message when issues are detected", async () => {
333+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
334+
nsDoctorStub.returns(failedGetInfosResult);
335+
const testInjector = createTestInjector();
336+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
337+
const logger = testInjector.resolve<LoggerStub>("logger");
338+
await doctorService.printWarnings();
339+
assert.isTrue(logger.output.indexOf("There seem to be issues with your configuration.") !== -1);
340+
});
341+
342+
it("returns result from cached file when they exist and the forceCheck is not passed", async () => {
343+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
344+
nsDoctorStub.throws(new Error("We should not call nativescript-doctor package when we have results in the file."));
345+
346+
const testInjector = createTestInjector();
347+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
348+
const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
349+
jsonFileSettingsService.getSettingValue = async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => successGetInfosResult;
350+
let saveSettingValue: any = null;
351+
jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
352+
const logger = testInjector.resolve<LoggerStub>("logger");
353+
await doctorService.printWarnings();
354+
assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
355+
assert.deepEqual(saveSettingValue, successGetInfosResult);
356+
});
357+
358+
it("saves results in cache when there are no warnings", async () => {
359+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
360+
nsDoctorStub.returns(successGetInfosResult);
361+
362+
const testInjector = createTestInjector();
363+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
364+
const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
365+
let saveSettingValue: any = null;
366+
jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
367+
const logger = testInjector.resolve<LoggerStub>("logger");
368+
await doctorService.printWarnings();
369+
assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
370+
assert.deepEqual(saveSettingValue, successGetInfosResult);
371+
});
372+
373+
it("returns result from nativescript-doctor and saves them in cache when the forceCheck is passed", async () => {
374+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
375+
nsDoctorStub.returns(successGetInfosResult);
376+
377+
const testInjector = createTestInjector();
378+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
379+
const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
380+
let saveSettingValue: any = null;
381+
let isGetSettingValueCalled = false;
382+
jsonFileSettingsService.getSettingValue = async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => {
383+
isGetSettingValueCalled = true;
384+
return null;
385+
};
386+
jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
387+
const logger = testInjector.resolve<LoggerStub>("logger");
388+
await doctorService.printWarnings({ forceCheck: true });
389+
assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
390+
assert.deepEqual(saveSettingValue, successGetInfosResult);
391+
assert.isTrue(nsDoctorStub.calledOnce);
392+
assert.isFalse(isGetSettingValueCalled, "When forceCheck is passed, we should not read the cache file.");
393+
});
394+
395+
it("deletes the cache file when issues are detected", async () => {
396+
const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
397+
nsDoctorStub.returns(failedGetInfosResult);
398+
const testInjector = createTestInjector();
399+
const doctorService = testInjector.resolve<IDoctorService>("doctorService");
400+
const fs = testInjector.resolve<IFileSystem>("fs");
401+
let deletedPath = "";
402+
fs.deleteFile = (filePath: string): void => {deletedPath = filePath; };
403+
const logger = testInjector.resolve<LoggerStub>("logger");
404+
await doctorService.printWarnings();
405+
assert.isTrue(logger.output.indexOf("There seem to be issues with your configuration.") !== -1);
406+
assert.isTrue(deletedPath.indexOf("doctor-cache.json") !== -1);
407+
});
408+
});
257409
});

0 commit comments

Comments
 (0)