feat: doctor improvements

lib/common/commands/doctor.ts

@@ -6,7 +6,7 @@ export class DoctorCommand implements ICommand {
 	public allowedParameters: ICommandParameter[] = [];
 
 	public execute(args: string[]): Promise<void> {
-		return this.$doctorService.printWarnings({ trackResult: false, projectDir: this.$projectHelper.projectDir });
+		return this.$doctorService.printWarnings({ trackResult: false, projectDir: this.$projectHelper.projectDir, forceCheck: true });
 	}
 }
 

lib/common/declarations.d.ts

@@ -1195,7 +1195,7 @@ interface IDoctorService {
 	 * @param configOptions: defines if the result should be tracked by Analytics
 	 * @returns {Promise<void>}
 	 */
-	printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void>;
+	printWarnings(configOptions?: { trackResult?: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions, forceCheck?: boolean }): Promise<void>;
 	/**
 	 * Runs the setup script on host machine
 	 * @returns {Promise<ISpawnResult>}
@@ -1206,7 +1206,7 @@ interface IDoctorService {
 	 * @param platform @optional The current platform
 	 * @returns {Promise<boolean>} true if the environment is properly configured for local builds
 	 */
-	canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean>;
+	canExecuteLocalBuild(configuration?: { platform?: string, projectDir?: string, runtimeVersion?: string, forceCheck?: boolean }): Promise<boolean>;
 
 	/**
 	 * Checks and notifies users for deprecated short imports in their applications.

lib/definitions/platform.d.ts

@@ -80,6 +80,7 @@ interface ICheckEnvironmentRequirementsInput {
 	runtimeVersion?: string;
 	options?: IOptions;
 	notConfiguredEnvOptions?: INotConfiguredEnvOptions;
+	forceCheck?: boolean;
 }
 
 interface ICheckEnvironmentRequirementsOutput {

lib/services/doctor-service.ts

@@ -1,6 +1,7 @@
 import { EOL } from "os";
 import * as path from "path";
 import * as helpers from "../common/helpers";
+import { cache } from "../common/decorators";
 import { TrackActionNames, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME } from "../constants";
 import { doctor, constants } from "nativescript-doctor";
 
@@ -9,6 +10,16 @@ export class DoctorService implements IDoctorService {
 	private static WindowsSetupScriptExecutable = "powershell.exe";
 	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\'))"'];
 
+	@cache()
+	private get jsonFileSettingsPath(): string {
+		return path.join(this.$settingsService.getProfileDir(), "doctor-cache.json");
+	}
+
+	@cache()
+	private get $jsonFileSettingsService(): IJsonFileSettingsService {
+		return this.$injector.resolve<IJsonFileSettingsService>("jsonFileSettingsService", { jsonFileSettingsPath: this.jsonFileSettingsPath });
+	}
+
 	constructor(private $analyticsService: IAnalyticsService,
 		private $hostInfo: IHostInfo,
 		private $logger: ILogger,
@@ -17,12 +28,15 @@ export class DoctorService implements IDoctorService {
 		private $projectDataService: IProjectDataService,
 		private $fs: IFileSystem,
 		private $terminalSpinnerService: ITerminalSpinnerService,
-		private $versionsService: IVersionsService) { }
+		private $versionsService: IVersionsService,
+		private $settingsService: ISettingsService) { }
 
-	public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
+	public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions, forceCheck?: boolean }): Promise<void> {
+		configOptions = configOptions || <any>{};
+		const getInfosData: any = { projectDir: configOptions.projectDir, androidRuntimeVersion: configOptions.runtimeVersion };
 		const infos = await this.$terminalSpinnerService.execute<NativeScriptDoctor.IInfo[]>({
 			text: `Getting environment information ${EOL}`
-		}, () => doctor.getInfos({ projectDir: configOptions && configOptions.projectDir, androidRuntimeVersion: configOptions && configOptions.runtimeVersion }));
+		}, () => this.getInfos({ forceCheck: configOptions.forceCheck }, getInfosData));
 
 		const warnings = infos.filter(info => info.type === constants.WARNING_TYPE_NAME);
 		const hasWarnings = warnings.length > 0;
@@ -39,8 +53,12 @@ export class DoctorService implements IDoctorService {
 
 		if (hasWarnings) {
 			this.$logger.info("There seem to be issues with your configuration.");
+			// cleanup the cache file as there seems to be issues with the current config
+			// all projects need to be rechecked
+			this.$fs.deleteFile(this.jsonFileSettingsPath);
 		} else {
 			this.$logger.info("No issues were detected.".bold);
+			await this.$jsonFileSettingsService.saveSetting(this.getKeyForConfiguration(getInfosData), infos);
 			this.printInfosCore(infos);
 		}
 
@@ -54,9 +72,9 @@ export class DoctorService implements IDoctorService {
 
 		await this.$injector.resolve<IPlatformEnvironmentRequirements>("platformEnvironmentRequirements").checkEnvironmentRequirements({
 			platform: null,
-			projectDir: configOptions && configOptions.projectDir,
-			runtimeVersion: configOptions && configOptions.runtimeVersion,
-			options: configOptions && configOptions.options
+			projectDir: configOptions.projectDir,
+			runtimeVersion: configOptions.runtimeVersion,
+			options: configOptions.options
 		});
 	}
 
@@ -90,23 +108,27 @@ export class DoctorService implements IDoctorService {
 		});
 	}
 
-	public async canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean> {
+	public async canExecuteLocalBuild(configuration?: { platform?: string, projectDir?: string, runtimeVersion?: string, forceCheck?: boolean }): Promise<boolean> {
 		await this.$analyticsService.trackEventActionInGoogleAnalytics({
 			action: TrackActionNames.CheckLocalBuildSetup,
 			additionalData: "Starting",
 		});
-		const infos = await doctor.getInfos({ platform, projectDir, androidRuntimeVersion: runtimeVersion });
-
+		const sysInfoConfig: NativeScriptDoctor.ISysInfoConfig = { platform: configuration.platform, projectDir: configuration.projectDir, androidRuntimeVersion: configuration.runtimeVersion };
+		const infos = await this.getInfos({ forceCheck: configuration && configuration.forceCheck }, sysInfoConfig);
 		const warnings = this.filterInfosByType(infos, constants.WARNING_TYPE_NAME);
 		const hasWarnings = warnings.length > 0;
 		if (hasWarnings) {
+			// cleanup the cache file as there seems to be issues with the current config
+			// all projects need to be rechecked
+			this.$fs.deleteFile(this.jsonFileSettingsPath);
 			await this.$analyticsService.trackEventActionInGoogleAnalytics({
 				action: TrackActionNames.CheckLocalBuildSetup,
 				additionalData: `Warnings:${warnings.map(w => w.message).join("__")}`,
 			});
 			this.printInfosCore(infos);
 		} else {
 			infos.map(info => this.$logger.trace(info.message));
+			await this.$jsonFileSettingsService.saveSetting(this.getKeyForConfiguration(sysInfoConfig), infos);
 		}
 
 		await this.$analyticsService.trackEventActionInGoogleAnalytics({
@@ -221,5 +243,40 @@ export class DoctorService implements IDoctorService {
 	private filterInfosByType(infos: NativeScriptDoctor.IInfo[], type: string): NativeScriptDoctor.IInfo[] {
 		return infos.filter(info => info.type === type);
 	}
+
+	private getKeyForConfiguration(sysInfoConfig?: NativeScriptDoctor.ISysInfoConfig): string {
+		const nativeScriptData = sysInfoConfig && sysInfoConfig.projectDir && JSON.stringify(this.$fs.readJson(path.join(sysInfoConfig.projectDir, "package.json")).nativescript);
+		const delimiter = "__";
+		const key = [
+			JSON.stringify(sysInfoConfig),
+			process.env.ANDROID_HOME,
+			process.env.JAVA_HOME,
+			process.env["CommonProgramFiles(x86)"],
+			process.env["CommonProgramFiles"],
+			process.env.PROCESSOR_ARCHITEW6432,
+			process.env.ProgramFiles,
+			process.env["ProgramFiles(x86)"],
+			nativeScriptData
+		]
+			.filter(a => !!a)
+			.join(delimiter);
+
+		const data = helpers.getHash(key, { algorithm: "md5" });
+		return data;
+	}
+
+	private async getInfos(cacheConfig: { forceCheck: boolean }, sysInfoConfig?: NativeScriptDoctor.ISysInfoConfig): Promise<NativeScriptDoctor.IInfo[]> {
+		const key = this.getKeyForConfiguration(sysInfoConfig);
+
+		const infosFromCache = cacheConfig.forceCheck ?
+			null :
+			await this.$jsonFileSettingsService.getSettingValue<NativeScriptDoctor.IInfo[]>(key);
+
+		this.$logger.trace(`getInfos cacheConfig options:`, cacheConfig, " current info from cache: ", infosFromCache);
+
+		const infos = infosFromCache || await doctor.getInfos(sysInfoConfig);
+
+		return infos;
+	}
 }
 $injector.register("doctorService", DoctorService);

lib/services/platform-environment-requirements.ts

@@ -57,7 +57,8 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
 			};
 		}
 
-		const canExecute = await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion);
+		const canExecute = await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck });
+
 		if (!canExecute) {
 			if (!isInteractive()) {
 				await this.$analyticsService.trackEventActionInGoogleAnalytics({
@@ -80,7 +81,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
 			if (selectedOption === PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME) {
 				await this.$doctorService.runSetupScript();
 
-				if (await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion)) {
+				if (await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck })) {
 					return {
 						canExecute: true,
 						selectedOption
@@ -114,7 +115,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ
 
 			if (selectedOption === PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME) {
 				await this.processBothCloudBuildsAndSetupScript();
-				if (await this.$doctorService.canExecuteLocalBuild(platform, projectDir, runtimeVersion)) {
+				if (await this.$doctorService.canExecuteLocalBuild({ platform, projectDir, runtimeVersion, forceCheck: input.forceCheck })) {
 					return {
 						canExecute: true,
 						selectedOption

test/services/doctor-service.ts

@@ -1,8 +1,10 @@
 import { DoctorService } from "../../lib/services/doctor-service";
 import { Yok } from "../../lib/common/yok";
-import { LoggerStub } from "../stubs";
+import { LoggerStub, FileSystemStub } from "../stubs";
 import { assert } from "chai";
 import * as path from "path";
+import * as sinon from "sinon";
+const nativescriptDoctor = require("nativescript-doctor");
 
 class DoctorServiceInheritor extends DoctorService {
 	constructor($analyticsService: IAnalyticsService,
@@ -13,8 +15,9 @@ class DoctorServiceInheritor extends DoctorService {
 		$projectDataService: IProjectDataService,
 		$fs: IFileSystem,
 		$terminalSpinnerService: ITerminalSpinnerService,
-		$versionsService: IVersionsService) {
-		super($analyticsService, $hostInfo, $logger, $childProcess, $injector, $projectDataService, $fs, $terminalSpinnerService, $versionsService);
+		$versionsService: IVersionsService,
+		$settingsService: ISettingsService) {
+		super($analyticsService, $hostInfo, $logger, $childProcess, $injector, $projectDataService, $fs, $terminalSpinnerService, $versionsService, $settingsService);
 	}
 
 	public getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
@@ -31,9 +34,26 @@ describe("doctorService", () => {
 		testInjector.register("logger", LoggerStub);
 		testInjector.register("childProcess", {});
 		testInjector.register("projectDataService", {});
-		testInjector.register("fs", {});
-		testInjector.register("terminalSpinnerService", {});
+		testInjector.register("fs", FileSystemStub);
+		testInjector.register("terminalSpinnerService", {
+			execute: (spinnerOptions: ITerminalSpinnerOptions, action: () => Promise<any>): Promise<any> => action(),
+			createSpinner: (spinnerOptions?: ITerminalSpinnerOptions): ITerminalSpinner => (<any>{
+				text: '',
+				succeed: (): any => undefined,
+				fail: (): any => undefined
+			})
+		});
 		testInjector.register("versionsService", {});
+		testInjector.register("settingsService", {
+			getProfileDir: (): string => ""
+		});
+		testInjector.register("jsonFileSettingsService", {
+			getSettingValue: async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => undefined,
+			saveSetting: async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => undefined
+		});
+		testInjector.register("platformEnvironmentRequirements", {
+			checkEnvironmentRequirements: async (input: ICheckEnvironmentRequirementsInput): Promise<ICheckEnvironmentRequirementsOutput> => (<any>{})
+		});
 
 		return testInjector;
 	};
@@ -254,4 +274,136 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl
 			});
 		});
 	});
+
+	describe("printWarnings", () => {
+		let sandbox: sinon.SinonSandbox;
+
+		beforeEach(() => {
+			sandbox = sinon.sandbox.create();
+		});
+
+		afterEach(() => {
+			sandbox.restore();
+		});
+		const successGetInfosResult = [{
+			message:
+				'Your ANDROID_HOME environment variable is set and points to correct directory.',
+			platforms: ['Android'],
+			type: 'info'
+		},
+		{
+			message: 'Xcode is installed and is configured properly.',
+			platforms: ['iOS'],
+			type: 'info'
+		}];
+
+		const failedGetInfosResult = [{
+			message:
+				'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.',
+			additionalInformation:
+				'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.',
+			platforms: ['Android'],
+			type: 'warning'
+		},
+		{
+			message:
+				'WARNING: adb from the Android SDK is not installed or is not configured properly. ',
+			additionalInformation:
+				'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',
+			platforms: ['Android'],
+			type: 'warning'
+		},
+		{
+			message: 'Xcode is installed and is configured properly.',
+			platforms: ['iOS'],
+			type: 'info'
+		}];
+
+		it("prints correct message when no issues are detected", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.returns(successGetInfosResult);
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings();
+			assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
+		});
+
+		it("prints correct message when issues are detected", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.returns(failedGetInfosResult);
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings();
+			assert.isTrue(logger.output.indexOf("There seem to be issues with your configuration.") !== -1);
+		});
+
+		it("returns result from cached file when they exist and the forceCheck is not passed", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.throws(new Error("We should not call nativescript-doctor package when we have results in the file."));
+
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
+			jsonFileSettingsService.getSettingValue = async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => successGetInfosResult;
+			let saveSettingValue: any = null;
+			jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings();
+			assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
+			assert.deepEqual(saveSettingValue, successGetInfosResult);
+		});
+
+		it("saves results in cache when there are no warnings", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.returns(successGetInfosResult);
+
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
+			let saveSettingValue: any = null;
+			jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings();
+			assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
+			assert.deepEqual(saveSettingValue, successGetInfosResult);
+		});
+
+		it("returns result from nativescript-doctor and saves them in cache when the forceCheck is passed", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.returns(successGetInfosResult);
+
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const jsonFileSettingsService = testInjector.resolve<IJsonFileSettingsService>("jsonFileSettingsService");
+			let saveSettingValue: any = null;
+			let isGetSettingValueCalled = false;
+			jsonFileSettingsService.getSettingValue = async (settingName: string, cacheOpts?: ICacheTimeoutOpts): Promise<any> => {
+				isGetSettingValueCalled = true;
+				return null;
+			};
+			jsonFileSettingsService.saveSetting = async (key: string, value: any, cacheOpts?: IUseCacheOpts): Promise<void> => saveSettingValue = value;
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings({ forceCheck: true });
+			assert.isTrue(logger.output.indexOf("No issues were detected.") !== -1);
+			assert.deepEqual(saveSettingValue, successGetInfosResult);
+			assert.isTrue(nsDoctorStub.calledOnce);
+			assert.isFalse(isGetSettingValueCalled, "When forceCheck is passed, we should not read the cache file.");
+		});
+
+		it("deletes the cache file when issues are detected", async () => {
+			const nsDoctorStub = sandbox.stub(nativescriptDoctor.doctor, "getInfos");
+			nsDoctorStub.returns(failedGetInfosResult);
+			const testInjector = createTestInjector();
+			const doctorService = testInjector.resolve<IDoctorService>("doctorService");
+			const fs = testInjector.resolve<IFileSystem>("fs");
+			let deletedPath = "";
+			fs.deleteFile = (filePath: string): void => {deletedPath = filePath; };
+			const logger = testInjector.resolve<LoggerStub>("logger");
+			await doctorService.printWarnings();
+			assert.isTrue(logger.output.indexOf("There seem to be issues with your configuration.") !== -1);
+			assert.isTrue(deletedPath.indexOf("doctor-cache.json") !== -1);
+		});
+	});
 });