From 666c9b2c6aefb4f6718651438c40c63bb06441a3 Mon Sep 17 00:00:00 2001 From: Ventsislav Georgiev Date: Tue, 24 Oct 2017 14:06:39 +0300 Subject: [PATCH 01/14] Add cross client analytics tracking (#3176) * Make analytics-settings-service getClientId public * Allow tracking for multiple Google Analytic Projects * Add cross client custom dimensions * Add npmVersion and dayFromFirstRun custom dimension values * Update cross client analytics google tracking id * Remove unnecessary custom dimensions from the cross client project * Add doc and public method test --- PublicAPI.md | 22 ++++++++++ lib/bootstrap.ts | 2 +- lib/services/analytics-settings-service.ts | 2 + ...lytics-cross-client-custom-dimensions.d.ts | 6 +++ ...> google-analytics-custom-dimensions.d.ts} | 2 +- .../analytics/google-analytics-provider.ts | 44 ++++++++++++++++--- test/nativescript-cli-lib.ts | 3 +- 7 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 lib/services/analytics/google-analytics-cross-client-custom-dimensions.d.ts rename lib/services/analytics/{google-analytics-custom-dimensions.ts => google-analytics-custom-dimensions.d.ts} (69%) diff --git a/PublicAPI.md b/PublicAPI.md index 206287aee4..70542d1b3a 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -35,6 +35,8 @@ const tns = require("nativescript"); * [disableDebugging](#disableDebugging) * [getLiveSyncDeviceDescriptors](#getLiveSyncDeviceDescriptors) * [events](#events) +* [analyticsSettingsService](#analyticsSettingsService) + * [getClientId](#getClientId) ## Module projectService @@ -855,6 +857,26 @@ tns.liveSyncService.on("debuggerDetached", debugInfo => { console.log(`Detached debugger for device with id ${debugInfo.deviceIdentifier}`); }); ``` +## analyticsSettingsService +Provides methods for accessing the analytics settings file data. + +### getClientId +The `getClientId` method allows retrieving the clientId used in the analytics tracking + +* Definition: +```TypeScript +/** + * Gets the clientId used for analytics tracking + * @returns {Promise} Client identifier in UUIDv4 standard. + */ +getClientId(): Promise; +``` + +* Usage: +```JavaScript +tns.analyticsSettingsService.getClientId() + .then(clientId => console.log(clientId)); +``` ## How to add a new method to Public API CLI is designed as command line tool and when it is used as a library, it does not give you access to all of the methods. This is mainly implementation detail. Most of the CLI's code is created to work in command line, not as a library, so before adding method to public API, most probably it will require some modification. diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index 643a0ea7d2..aa52fd1939 100644 --- a/lib/bootstrap.ts +++ b/lib/bootstrap.ts @@ -29,7 +29,7 @@ $injector.require("iOSDebugService", "./services/ios-debug-service"); $injector.require("androidDebugService", "./services/android-debug-service"); $injector.require("userSettingsService", "./services/user-settings-service"); -$injector.require("analyticsSettingsService", "./services/analytics-settings-service"); +$injector.requirePublic("analyticsSettingsService", "./services/analytics-settings-service"); $injector.require("analyticsService", "./services/analytics/analytics-service"); $injector.require("eqatecAnalyticsProvider", "./services/analytics/eqatec-analytics-provider"); $injector.require("googleAnalyticsProvider", "./services/analytics/google-analytics-provider"); diff --git a/lib/services/analytics-settings-service.ts b/lib/services/analytics-settings-service.ts index 6ca50db27e..250e4b0db0 100644 --- a/lib/services/analytics-settings-service.ts +++ b/lib/services/analytics-settings-service.ts @@ -1,4 +1,5 @@ import { createGUID } from "../common/helpers"; +import { exported } from "../common/decorators"; class AnalyticsSettingsService implements IAnalyticsSettingsService { private static SESSIONS_STARTED_KEY_PREFIX = "SESSIONS_STARTED_"; @@ -15,6 +16,7 @@ class AnalyticsSettingsService implements IAnalyticsSettingsService { return this.getSettingValueOrDefault("USER_ID"); } + @exported("analyticsSettingsService") public getClientId(): Promise { return this.getSettingValueOrDefault(this.$staticConfig.ANALYTICS_INSTALLATION_ID_SETTING_NAME); } diff --git a/lib/services/analytics/google-analytics-cross-client-custom-dimensions.d.ts b/lib/services/analytics/google-analytics-cross-client-custom-dimensions.d.ts new file mode 100644 index 0000000000..c58e2b54c0 --- /dev/null +++ b/lib/services/analytics/google-analytics-cross-client-custom-dimensions.d.ts @@ -0,0 +1,6 @@ +// Sync indexes with the custom dimensions of the cross client analytics project +declare const enum GoogleAnalyticsCrossClientCustomDimensions { + sessionId = "cd9", + clientId = "cd10", + crossClientId = "cd12", +} diff --git a/lib/services/analytics/google-analytics-custom-dimensions.ts b/lib/services/analytics/google-analytics-custom-dimensions.d.ts similarity index 69% rename from lib/services/analytics/google-analytics-custom-dimensions.ts rename to lib/services/analytics/google-analytics-custom-dimensions.d.ts index 9e7c1d7007..488439b814 100644 --- a/lib/services/analytics/google-analytics-custom-dimensions.ts +++ b/lib/services/analytics/google-analytics-custom-dimensions.d.ts @@ -1,4 +1,4 @@ -const enum GoogleAnalyticsCustomDimensions { +declare const enum GoogleAnalyticsCustomDimensions { cliVersion = "cd1", projectType = "cd2", clientID = "cd3", diff --git a/lib/services/analytics/google-analytics-provider.ts b/lib/services/analytics/google-analytics-provider.ts index 1bb8d9fe13..8f93ab4388 100644 --- a/lib/services/analytics/google-analytics-provider.ts +++ b/lib/services/analytics/google-analytics-provider.ts @@ -4,24 +4,46 @@ import { AnalyticsClients } from "../../common/constants"; export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { private static GA_TRACKING_ID = "UA-111455-44"; + private static GA_CROSS_CLIENT_TRACKING_ID = "UA-111455-51"; private currentPage: string; constructor(private clientId: string, private $staticConfig: IStaticConfig, private $hostInfo: IHostInfo, - private $osInfo: IOsInfo) { + private $osInfo: IOsInfo, + private $logger: ILogger) { } public async trackHit(trackInfo: IGoogleAnalyticsData): Promise { + const trackingIds = [GoogleAnalyticsProvider.GA_TRACKING_ID, GoogleAnalyticsProvider.GA_CROSS_CLIENT_TRACKING_ID]; + const sessionId = uuid.v4(); + + for (const gaTrackingId of trackingIds) { + try { + await this.track(gaTrackingId, trackInfo, sessionId); + } catch (e) { + this.$logger.trace("Analytics exception: ", e); + } + } + } + + private async track(gaTrackingId: string, trackInfo: IGoogleAnalyticsData, sessionId: string): Promise { const visitor = ua({ - tid: GoogleAnalyticsProvider.GA_TRACKING_ID, + tid: gaTrackingId, cid: this.clientId, headers: { ["User-Agent"]: this.getUserAgentString() } }); - this.setCustomDimensions(visitor, trackInfo.customDimensions); + switch (gaTrackingId) { + case GoogleAnalyticsProvider.GA_CROSS_CLIENT_TRACKING_ID: + this.setCrossClientCustomDimensions(visitor, sessionId); + break; + default: + this.setCustomDimensions(visitor, trackInfo.customDimensions, sessionId); + break; + } switch (trackInfo.googleAnalyticsDataType) { case GoogleAnalyticsDataType.Page: @@ -33,13 +55,13 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { } } - private setCustomDimensions(visitor: ua.Visitor, customDimensions: IStringDictionary): void { + private setCustomDimensions(visitor: ua.Visitor, customDimensions: IStringDictionary, sessionId: string): void { const defaultValues: IStringDictionary = { [GoogleAnalyticsCustomDimensions.cliVersion]: this.$staticConfig.version, [GoogleAnalyticsCustomDimensions.nodeVersion]: process.version, [GoogleAnalyticsCustomDimensions.clientID]: this.clientId, [GoogleAnalyticsCustomDimensions.projectType]: null, - [GoogleAnalyticsCustomDimensions.sessionID]: uuid.v4(), + [GoogleAnalyticsCustomDimensions.sessionID]: sessionId, [GoogleAnalyticsCustomDimensions.client]: AnalyticsClients.Unknown }; @@ -50,6 +72,18 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { }); } + private async setCrossClientCustomDimensions(visitor: ua.Visitor, sessionId: string): Promise { + const customDimensions: IStringDictionary = { + [GoogleAnalyticsCrossClientCustomDimensions.sessionId]: sessionId, + [GoogleAnalyticsCrossClientCustomDimensions.clientId]: this.clientId, + [GoogleAnalyticsCrossClientCustomDimensions.crossClientId]: this.clientId, + }; + + _.each(customDimensions, (value, key) => { + visitor.set(key, value); + }); + } + private trackEvent(visitor: ua.Visitor, trackInfo: IGoogleAnalyticsEventData): Promise { return new Promise((resolve, reject) => { visitor.event(trackInfo.category, trackInfo.action, trackInfo.label, trackInfo.value, { p: this.currentPage }, (err: Error) => { diff --git a/test/nativescript-cli-lib.ts b/test/nativescript-cli-lib.ts index 10854e48f0..da541f223a 100644 --- a/test/nativescript-cli-lib.ts +++ b/test/nativescript-cli-lib.ts @@ -21,7 +21,8 @@ describe("nativescript-cli-lib", () => { npm: ["install", "uninstall", "view", "search"], extensibilityService: ["loadExtensions", "loadExtension", "getInstalledExtensions", "installExtension", "uninstallExtension"], liveSyncService: ["liveSync", "stopLiveSync", "enableDebugging", "disableDebugging", "attachDebugger"], - debugService: ["debug"] + debugService: ["debug"], + analyticsSettingsService: ["getClientId"] }; const pathToEntryPoint = path.join(__dirname, "..", "lib", "nativescript-cli-lib.js").replace(/\\/g, "\\\\"); From c79104040514cbb3b5508f22d947f0feaf677615 Mon Sep 17 00:00:00 2001 From: Ventsislav Georgiev Date: Thu, 2 Nov 2017 18:16:58 +0200 Subject: [PATCH 02/14] Expose getUserAgentString from the analyticsSettingsService (#3194) --- PublicAPI.md | 18 +++++++++ lib/services/analytics-settings-service.ts | 36 ++++++++++++++++++ .../analytics/google-analytics-provider.ts | 38 +------------------ 3 files changed, 56 insertions(+), 36 deletions(-) diff --git a/PublicAPI.md b/PublicAPI.md index 70542d1b3a..0b3b3547bd 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -878,6 +878,24 @@ tns.analyticsSettingsService.getClientId() .then(clientId => console.log(clientId)); ``` +### getUserAgentString +The `getUserAgentString` method allows retrieving a user agent string identifying the current system + +* Definition: +```TypeScript +/** + * Gets user agent string identifing the current system in the following format: `${identifier} (${systemInfo}) ${osArch}` + * @param {string} identifier The product identifier. + * @returns {string} The user agent string. + */ +getUserAgentString(identifier: string): string; +``` + +* Usage: +```JavaScript +const userAgentString = tns.analyticsSettingsService.getUserAgentString("tns/3.3.0"); +``` + ## How to add a new method to Public API CLI is designed as command line tool and when it is used as a library, it does not give you access to all of the methods. This is mainly implementation detail. Most of the CLI's code is created to work in command line, not as a library, so before adding method to public API, most probably it will require some modification. For example the `$options` injected module contains information about all `--` options passed on the terminal. When the CLI is used as a library, the options are not populated. Before adding method to public API, make sure its implementation does not rely on `$options`. diff --git a/lib/services/analytics-settings-service.ts b/lib/services/analytics-settings-service.ts index 250e4b0db0..8930796165 100644 --- a/lib/services/analytics-settings-service.ts +++ b/lib/services/analytics-settings-service.ts @@ -6,6 +6,8 @@ class AnalyticsSettingsService implements IAnalyticsSettingsService { constructor(private $userSettingsService: UserSettings.IUserSettingsService, private $staticConfig: IStaticConfig, + private $hostInfo: IHostInfo, + private $osInfo: IOsInfo, private $logger: ILogger) { } public async canDoRequest(): Promise { @@ -38,6 +40,40 @@ class AnalyticsSettingsService implements IAnalyticsSettingsService { return this.$userSettingsService.saveSetting(this.getSessionsProjectKey(projectName), count); } + @exported("analyticsSettingsService") + public getUserAgentString(identifier: string): string { + let osString = ""; + const osRelease = this.$osInfo.release(); + + if (this.$hostInfo.isWindows) { + osString = `Windows NT ${osRelease}`; + } else if (this.$hostInfo.isDarwin) { + osString = `Macintosh`; + const macRelease = this.getMacOSReleaseVersion(osRelease); + if (macRelease) { + osString += `; Intel Mac OS X ${macRelease}`; + } + } else { + osString = `Linux x86`; + if (this.$osInfo.arch() === "x64") { + osString += "_64"; + } + } + + const userAgent = `${identifier} (${osString}; ${this.$osInfo.arch()})`; + + return userAgent; + } + + private getMacOSReleaseVersion(osRelease: string): string { + // https://en.wikipedia.org/wiki/Darwin_(operating_system)#Release_history + // Each macOS version is labeled 10., where it looks like is taken from the major version returned by os.release() (16.x.x for example) and subtracting 4 from it. + // So the version becomes "10.12" in this case. + // Could be improved by spawning `system_profiler SPSoftwareDataType` and getting the System Version line from the result. + const majorVersion = osRelease && _.first(osRelease.split(".")); + return majorVersion && `10.${+majorVersion - 4}`; + } + private getSessionsProjectKey(projectName: string): string { return `${AnalyticsSettingsService.SESSIONS_STARTED_KEY_PREFIX}${projectName}`; } diff --git a/lib/services/analytics/google-analytics-provider.ts b/lib/services/analytics/google-analytics-provider.ts index 8f93ab4388..0fefd753b2 100644 --- a/lib/services/analytics/google-analytics-provider.ts +++ b/lib/services/analytics/google-analytics-provider.ts @@ -9,8 +9,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { constructor(private clientId: string, private $staticConfig: IStaticConfig, - private $hostInfo: IHostInfo, - private $osInfo: IOsInfo, + private $analyticsSettingsService: IAnalyticsSettingsService, private $logger: ILogger) { } @@ -32,7 +31,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { tid: gaTrackingId, cid: this.clientId, headers: { - ["User-Agent"]: this.getUserAgentString() + ["User-Agent"]: this.$analyticsSettingsService.getUserAgentString(`tnsCli/${this.$staticConfig.version}`) } }); @@ -116,39 +115,6 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { }); }); } - - private getUserAgentString(): string { - let osString = ""; - const osRelease = this.$osInfo.release(); - - if (this.$hostInfo.isWindows) { - osString = `Windows NT ${osRelease}`; - } else if (this.$hostInfo.isDarwin) { - osString = `Macintosh`; - const macRelease = this.getMacOSReleaseVersion(osRelease); - if (macRelease) { - osString += `; Intel Mac OS X ${macRelease}`; - } - } else { - osString = `Linux x86`; - if (this.$osInfo.arch() === "x64") { - osString += "_64"; - } - } - - const userAgent = `tnsCli/${this.$staticConfig.version} (${osString}; ${this.$osInfo.arch()})`; - - return userAgent; - } - - private getMacOSReleaseVersion(osRelease: string): string { - // https://en.wikipedia.org/wiki/Darwin_(operating_system)#Release_history - // Each macOS version is labeled 10., where it looks like is taken from the major version returned by os.release() (16.x.x for example) and subtracting 4 from it. - // So the version becomes "10.12" in this case. - // Could be improved by spawning `system_profiler SPSoftwareDataType` and getting the System Version line from the result. - const majorVersion = osRelease && _.first(osRelease.split(".")); - return majorVersion && `10.${+majorVersion - 4}`; - } } $injector.register("googleAnalyticsProvider", GoogleAnalyticsProvider); From 3842520f3040ea453b25844d8bcfd8be45bf1534 Mon Sep 17 00:00:00 2001 From: dtopuzov Date: Fri, 3 Nov 2017 11:32:51 +0200 Subject: [PATCH 03/14] Changelog for {N} 3.3.0 (#3190) Changelog for 3.3 --- CHANGELOG.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee07c39dcd..8f81f5d18c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,19 @@ NativeScript CLI Changelog ================ -3.2.0 - RC.1 (2017, August 29) +3.3.0 (2017, July 25) +== + +### New + +* [Implemented #3076](https://github.com/NativeScript/nativescript-cli/issues/3076): NativeScript setup scripts should have silent installer mode. + +### Fixed +* [Fixed #3141](https://github.com/NativeScript/nativescript-cli/issues/3141): No console.log output Xcode 9 iOS 11. +* [Fixed #3016](https://github.com/NativeScript/nativescript-cli/issues/3016): tns_modules randomly appears in app folder and breaks build. +* [Fixed #2967](https://github.com/NativeScript/nativescript-cli/issues/2967): Create plugin by static static libraries error. + +3.2.0 (2017, September 7) == ### Fixed From 0ef8a04fdb8d131cf025c284fbe59e085930af2f Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Tue, 7 Nov 2017 12:36:24 +0200 Subject: [PATCH 04/14] Fix missing console.logs on Android when CLI is used as library (#3198) In case CLI is used as a library, the device logs are no longer captured after application is restarted. The problem is the newly added logic for Android devices that stops the logcat process when CLI calls `stopApplication` and starts it when `startApplication` is called. However, in `startApplication` there's a check if `$options.justlaunch` is false - only then the device log operation is started again. When CLI is used as a library, we have hardcoded the `justlaunch` option to true in order to prevent duplicate logs for iOS devices. So the solution to resolve Android logs issues is to remove the setting of justlaunch option. However, this caused the duplicate logs issue for iOS devices. Investigating it further, it turned out to be a memory leak when iOS device is detached. It's fixed in the submodule: In case iOS Device is detached, the `IOSDevice` instance is kept alive. The reason is the handler for device logs, that `IOSDevice` passes to `iOSDeviceOperations`. As `iOSDeviceOperations` is still alive, the `IOSDevice` instance is also alive. Reattaching the same device will cause duplicate logs. Detaching and attaching it again will lead to additional logs. In order to fix the issue, convert `iOSDeviceOpertations` to event emitter and emit event when there's data for logging. Each `IOSDevice` will add handler for the event and will do its logic. In case device is detached (i.e. `DeviceLost` event is fired in `DevicesService`), call a newly added method to the specific `IDevice` instance. It's purpose is to clean the used resource. For iOS device, this method will remove the handler for `devceLogData` event of `iOSDeviceOperations`. --- lib/nativescript-cli-lib-bootstrap.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/nativescript-cli-lib-bootstrap.ts b/lib/nativescript-cli-lib-bootstrap.ts index 09ae34f712..0cefa37d6b 100644 --- a/lib/nativescript-cli-lib-bootstrap.ts +++ b/lib/nativescript-cli-lib-bootstrap.ts @@ -9,7 +9,4 @@ $injector.requirePublic("companionAppsService", "./common/appbuilder/services/li $injector.requirePublicClass("deviceEmitter", "./common/appbuilder/device-emitter"); $injector.requirePublicClass("deviceLogProvider", "./common/appbuilder/device-log-provider"); -// We need this because some services check if (!$options.justlaunch) to start the device log after some operation. -// We don't want this behaviour when the CLI is required as library. -$injector.resolve("options").justlaunch = true; $injector.resolve("staticConfig").disableAnalytics = true; From d4169a7d6badaec1d5fda549656101bc8acd78fa Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Tue, 7 Nov 2017 13:42:55 +0200 Subject: [PATCH 05/14] Do not track help command when other command fails (#3196) In case any command fails, we execute help command in order to show the help content. This leads to multiple trackings, i.e. - user executes only one command, but in Analytics we see two commands. Instead of executing help command, introduce new method in htmlHelpService, that prints the help to the terminal and call it instead. Use the same method in the help command itself. Rename htmlHelpService to helpService - it has been incorrectly named from the beginning. Remove `helpTextPath` from staticConfig interface - this property is not used for more than 2 years. Introduce tests for `helpService` - get the tests from AppBuilder CLI. --- lib/commands/generate-help.ts | 4 ++-- lib/commands/post-install.ts | 4 ++-- lib/config.ts | 4 ---- test/commands/post-install.ts | 2 +- test/platform-commands.ts | 5 ++++- test/platform-service.ts | 3 +++ test/plugins-service.ts | 3 +++ test/stubs.ts | 2 +- 8 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/commands/generate-help.ts b/lib/commands/generate-help.ts index 71cfa998d4..18fc8af9a0 100644 --- a/lib/commands/generate-help.ts +++ b/lib/commands/generate-help.ts @@ -1,10 +1,10 @@ export class GenerateHelpCommand implements ICommand { public allowedParameters: ICommandParameter[] = []; - constructor(private $htmlHelpService: IHtmlHelpService) { } + constructor(private $helpService: IHelpService) { } public async execute(args: string[]): Promise { - return this.$htmlHelpService.generateHtmlPages(); + return this.$helpService.generateHtmlPages(); } } diff --git a/lib/commands/post-install.ts b/lib/commands/post-install.ts index 779703e5c3..f574171789 100644 --- a/lib/commands/post-install.ts +++ b/lib/commands/post-install.ts @@ -5,12 +5,12 @@ export class PostInstallCliCommand extends PostInstallCommand { private $subscriptionService: ISubscriptionService, $staticConfig: Config.IStaticConfig, $commandsService: ICommandsService, - $htmlHelpService: IHtmlHelpService, + $helpService: IHelpService, $options: ICommonOptions, $doctorService: IDoctorService, $analyticsService: IAnalyticsService, $logger: ILogger) { - super($fs, $staticConfig, $commandsService, $htmlHelpService, $options, $doctorService, $analyticsService, $logger); + super($fs, $staticConfig, $commandsService, $helpService, $options, $doctorService, $analyticsService, $logger); } public async execute(args: string[]): Promise { diff --git a/lib/config.ts b/lib/config.ts index 4b6b21c942..dd4fcc8d13 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -60,10 +60,6 @@ export class StaticConfig extends StaticConfigBase implements IStaticConfig { public version = require("../package.json").version; - public get helpTextPath(): string { - return path.join(__dirname, "../resources/help.txt"); - } - public get HTML_CLI_HELPERS_DIR(): string { return path.join(__dirname, "../docs/helpers"); } diff --git a/test/commands/post-install.ts b/test/commands/post-install.ts index 5cadda5c0b..51f4dd8e02 100644 --- a/test/commands/post-install.ts +++ b/test/commands/post-install.ts @@ -18,7 +18,7 @@ const createTestInjector = (): IInjector => { tryExecuteCommand: async (commandName: string, commandArguments: string[]): Promise => undefined }); - testInjector.register("htmlHelpService", { + testInjector.register("helpService", { generateHtmlPages: async (): Promise => undefined }); diff --git a/test/platform-commands.ts b/test/platform-commands.ts index 07d11e6430..f862e6164c 100644 --- a/test/platform-commands.ts +++ b/test/platform-commands.ts @@ -54,7 +54,7 @@ class ErrorsNoFailStub implements IErrors { throw new Error(); } - async beginCommand(action: () => Promise, printHelpCommand: () => Promise): Promise { + async beginCommand(action: () => Promise, printHelpCommand: () => Promise): Promise { let result = false; try { result = await action(); @@ -146,6 +146,9 @@ function createTestInjector() { }); testInjector.register("messages", Messages); testInjector.register("devicePathProvider", {}); + testInjector.register("helpService", { + showCommandLineHelp: async (): Promise => (undefined) + }); return testInjector; } diff --git a/test/platform-service.ts b/test/platform-service.ts index 5c12c376fa..99ef2203d1 100644 --- a/test/platform-service.ts +++ b/test/platform-service.ts @@ -87,6 +87,9 @@ function createTestInjector() { }); testInjector.register("messages", Messages); testInjector.register("devicePathProvider", {}); + testInjector.register("helpService", { + showCommandLineHelp: async (): Promise => (undefined) + }); return testInjector; } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 6360e2ee0d..1f5953cec1 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -98,6 +98,9 @@ function createTestInjector() { }); testInjector.register("xmlValidator", XmlValidator); testInjector.register("config", StaticConfigLib.Configuration); + testInjector.register("helpService", { + showCommandLineHelp: async (): Promise => (undefined) + }); return testInjector; } diff --git a/test/stubs.ts b/test/stubs.ts index 04f128f8dc..99e7963a88 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -197,7 +197,7 @@ export class ErrorsStub implements IErrors { throw new Error(message); } - async beginCommand(action: () => Promise, printHelpCommand: () => Promise): Promise { + async beginCommand(action: () => Promise, printHelpCommand: () => Promise): Promise { throw new Error("not supported"); } From 59380562ff79f1e888bab38f70bd9ff0df22a6f4 Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Tue, 7 Nov 2017 15:18:36 +0200 Subject: [PATCH 06/14] Fix passing relative path for profileDir when extensions are used (#3201) Installing and loading extensions is based on the `--profileDir` value. In case a relative path is passed, the extensions cannot be loaded as the resolve is not correct. Resolve the full path, so the loading will work correctly. --- lib/services/extensibility-service.ts | 2 +- test/services/extensibility-service.ts | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index 0104e117fe..0ed6c3a8eb 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -4,7 +4,7 @@ import * as constants from "../constants"; export class ExtensibilityService implements IExtensibilityService { private get pathToExtensions(): string { - return path.join(this.$options.profileDir, "extensions"); + return path.join(path.resolve(this.$options.profileDir), "extensions"); } private get pathToPackageJson(): string { diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index 3243331703..26746cdf31 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -3,9 +3,18 @@ import { Yok } from "../../lib/common/yok"; import * as stubs from "../stubs"; import { assert } from "chai"; import * as constants from "../../lib/constants"; -import * as path from "path"; +const path = require("path"); +const originalResolve = path.resolve; describe("extensibilityService", () => { + before(() => { + path.resolve = (p: string) => p; + }); + + after(() => { + path.resolve = originalResolve; + }); + const getTestInjector = (): IInjector => { const testInjector = new Yok(); testInjector.register("fs", {}); @@ -450,7 +459,7 @@ describe("extensibilityService", () => { const fs: IFileSystem = testInjector.resolve("fs"); fs.exists = (pathToCheck: string): boolean => true; const npm: INodePackageManager = testInjector.resolve("npm"); - npm.uninstall = async (packageName: string, config?: any, path?: string): Promise => { + npm.uninstall = async (packageName: string, config?: any, p?: string): Promise => { throw new Error(expectedErrorMessage); }; @@ -469,9 +478,9 @@ describe("extensibilityService", () => { const npm: INodePackageManager = testInjector.resolve("npm"); const argsPassedToNpmInstall: any = {}; - npm.uninstall = async (packageName: string, config?: any, path?: string): Promise => { + npm.uninstall = async (packageName: string, config?: any, p?: string): Promise => { argsPassedToNpmInstall.packageName = packageName; - argsPassedToNpmInstall.pathToSave = path; + argsPassedToNpmInstall.pathToSave = p; argsPassedToNpmInstall.config = config; return [userSpecifiedValue]; }; @@ -523,7 +532,7 @@ describe("extensibilityService", () => { fs.readDirectory = (dir: string): string[] => [extensionName]; const npm: INodePackageManager = testInjector.resolve("npm"); - npm.uninstall = async (packageName: string, config?: any, path?: string): Promise => [extensionName]; + npm.uninstall = async (packageName: string, config?: any, p?: string): Promise => [extensionName]; const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); await extensibilityService.uninstallExtension(extensionName); From cd214db92eb473a4964c64e67939c79bc981a31f Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Tue, 7 Nov 2017 16:08:19 +0200 Subject: [PATCH 07/14] Do not break CLI process in case analytics fail (#3197) In case we are unable to start the Analytics Broker process, CLI will fail. But analytics errors should not break user's workflow, so catch the error and ensure the actions will continue. Add tests to verify the behavior. --- lib/services/analytics/analytics-service.ts | 11 +- test/services/analytics/analytics-service.ts | 295 +++++++++++++++++++ test/stubs.ts | 5 +- 3 files changed, 308 insertions(+), 3 deletions(-) create mode 100644 test/services/analytics/analytics-service.ts diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index 411e89b309..02227aea32 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -6,7 +6,7 @@ import { isInteractive } from '../../common/helpers'; import { DeviceTypes, AnalyticsClients } from "../../common/constants"; export class AnalyticsService extends AnalyticsServiceBase { - private static ANALYTICS_BROKER_START_TIMEOUT = 30 * 1000; + private static ANALYTICS_BROKER_START_TIMEOUT = 10 * 1000; private brokerProcess: ChildProcess; constructor(protected $logger: ILogger, @@ -182,7 +182,14 @@ export class AnalyticsService extends AnalyticsServiceBase { } private async sendMessageToBroker(message: ITrackingInformation): Promise { - const broker = await this.getAnalyticsBroker(); + let broker: ChildProcess; + try { + broker = await this.getAnalyticsBroker(); + } catch (err) { + this.$logger.trace("Unable to get broker instance due to error: ", err); + return; + } + return new Promise((resolve, reject) => { if (broker && broker.connected) { try { diff --git a/test/services/analytics/analytics-service.ts b/test/services/analytics/analytics-service.ts new file mode 100644 index 0000000000..eb3ae50f08 --- /dev/null +++ b/test/services/analytics/analytics-service.ts @@ -0,0 +1,295 @@ +import { AnalyticsService } from "../../../lib/services/analytics/analytics-service"; +import { Yok } from "../../../lib/common/yok"; +import * as stubs from "../../stubs"; +import { assert } from "chai"; +import { EventEmitter } from "events"; +import { AnalyticsClients } from "../../../lib/common/constants"; + +const helpers = require("../../../lib/common/helpers"); +const originalIsInteractive = helpers.isInteractive; + +const trackFeatureUsage = "TrackFeatureUsage"; +const createTestInjector = (): IInjector => { + const testInjector = new Yok(); + testInjector.register("options", {}); + testInjector.register("logger", stubs.LoggerStub); + + testInjector.register("staticConfig", { + disableAnalytics: false, + TRACK_FEATURE_USAGE_SETTING_NAME: trackFeatureUsage, + PATH_TO_BOOTSTRAP: "pathToBootstrap.js" + }); + + testInjector.register("prompter", { + + }); + + testInjector.register("userSettingsService", { + getSettingValue: async (settingName: string): Promise => { + return "true"; + } + }); + testInjector.register("analyticsSettingsService", { + canDoRequest: (): Promise => Promise.resolve(true) + }); + testInjector.register("osInfo", {}); + testInjector.register("childProcess", {}); + testInjector.register("processService", { + attachToProcessExitSignals: (context: any, callback: () => void): void => undefined + }); + testInjector.register("projectDataService", {}); + testInjector.register("mobileHelper", {}); + + return testInjector; +}; + +describe("analyticsService", () => { + afterEach(() => { + helpers.isInteractive = originalIsInteractive; + }); + + describe("trackInGoogleAnalytics", () => { + describe("does not track", () => { + const testScenario = async (configuration: { + disableAnalytics: boolean, + assertMessage: string, + userSettingsServiceOpts?: { trackFeatureUsageValue: string, defaultValue: string } + }) => { + const testInjector = createTestInjector(); + const staticConfig = testInjector.resolve("staticConfig"); + staticConfig.disableAnalytics = configuration.disableAnalytics; + + configuration.userSettingsServiceOpts = configuration.userSettingsServiceOpts || { trackFeatureUsageValue: "false", defaultValue: "true" }; + const userSettingsService = testInjector.resolve("userSettingsService"); + userSettingsService.getSettingValue = async (settingName: string): Promise => { + if (settingName === trackFeatureUsage) { + return configuration.userSettingsServiceOpts.trackFeatureUsageValue; + } + + return configuration.userSettingsServiceOpts.defaultValue; + }; + + let isChildProcessSpawned = false; + const childProcess = testInjector.resolve("childProcess"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + isChildProcessSpawned = true; + }; + + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics({ + googleAnalyticsDataType: GoogleAnalyticsDataType.Page, + customDimensions: { + customDimension1: "value1" + } + }); + + assert.isFalse(isChildProcessSpawned, configuration.assertMessage); + }; + + it("does not track when staticConfig's disableAnalytics is true", () => { + return testScenario({ + disableAnalytics: true, + assertMessage: "When staticConfig.disableAnalytics is true, no child process should be started, i.e. we should not track anything." + }); + }); + + it(`does not track when ${trackFeatureUsage} is not true`, async () => { + await testScenario({ + disableAnalytics: false, + assertMessage: `When ${trackFeatureUsage} is false, no child process should be started, i.e. we should not track anything.`, + userSettingsServiceOpts: { + trackFeatureUsageValue: "false", defaultValue: "true" + } + }); + + await testScenario({ + disableAnalytics: false, + assertMessage: `When ${trackFeatureUsage} is undefined, no child process should be started, i.e. we should not track anything.`, + userSettingsServiceOpts: { + trackFeatureUsageValue: undefined, defaultValue: "true" + } + }); + }); + + }); + + const getSpawnedProcess = (): any => { + const spawnedProcess: any = new EventEmitter(); + spawnedProcess.stdout = new EventEmitter(); + spawnedProcess.stderr = new EventEmitter(); + spawnedProcess.unref = (): void => undefined; + return spawnedProcess; + }; + + describe("does not fail", () => { + const assertExpectedError = async (testInjector: IInjector, opts: { isChildProcessSpawned: boolean, expectedErrorMessage: string }) => { + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics({ + googleAnalyticsDataType: GoogleAnalyticsDataType.Page, + customDimensions: { + customDimension1: "value1" + } + }); + + assert.isTrue(opts.isChildProcessSpawned); + const logger = testInjector.resolve("logger"); + assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1); + }; + + const setupTest = (expectedErrorMessage: string): any => { + const testInjector = createTestInjector(); + const opts = { + isChildProcessSpawned: false, + expectedErrorMessage + }; + + const childProcess = testInjector.resolve("childProcess"); + return { + testInjector, + opts, + childProcess + }; + }; + + it("when unable to start broker process", async () => { + const { testInjector, childProcess, opts } = setupTest("Unable to get broker instance due to error: Error: custom error"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + throw new Error("custom error"); + }; + + await assertExpectedError(testInjector, opts); + }); + + it("when broker cannot start for required timeout", async () => { + const { testInjector, childProcess, opts } = setupTest("Unable to get broker instance due to error: Error: Unable to start Analytics Broker process."); + const originalSetTimeout = setTimeout; + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + global.setTimeout = (callback: (...args: any[]) => void, ms: number, ...otherArgs: any[]) => originalSetTimeout(callback, 1); + return getSpawnedProcess(); + }; + + await assertExpectedError(testInjector, opts); + + global.setTimeout = originalSetTimeout; + }); + + it("when broker is not connected", async () => { + const { testInjector, childProcess, opts } = setupTest("Broker not found or not connected."); + + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = false; + spawnedProcess.send = (): void => undefined; + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + return spawnedProcess; + }; + + await assertExpectedError(testInjector, opts); + }); + + it("when sending message fails", async () => { + const { testInjector, childProcess, opts } = setupTest("Error while trying to send message to broker: Error: Failed to sent data."); + + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = true; + spawnedProcess.send = (): void => { + throw new Error("Failed to sent data."); + }; + + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + return spawnedProcess; + }; + + await assertExpectedError(testInjector, opts); + }); + }); + + describe("sends correct message to broker", () => { + const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }): { testInjector: IInjector, opts: any } => { + helpers.isInteractive = () => terminalOpts ? terminalOpts.isInteractive : true; + + const testInjector = createTestInjector(); + const opts = { + isChildProcessSpawned: false, + expectedResult, + dataToSend, + messageSent: null + }; + + const childProcess = testInjector.resolve("childProcess"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = true; + spawnedProcess.send = (msg: any, action: () => void): void => { + opts.messageSent = msg; + action(); + }; + + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + + return spawnedProcess; + }; + + return { + testInjector, + opts + }; + }; + + const assertExpectedResult = async (testInjector: IInjector, opts: { isChildProcessSpawned: boolean, expectedResult: any, messageSent: any, dataToSend: any }) => { + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics(opts.dataToSend); + + assert.isTrue(opts.isChildProcessSpawned); + assert.deepEqual(opts.messageSent, opts.expectedResult); + }; + + const getDataToSend = (gaDataType: string): any => ({ + googleAnalyticsDataType: gaDataType, + customDimensions: { + customDimension1: "value1" + } + }); + + const getExpectedResult = (gaDataType: string, analyticsClient?: string): any => ({ + type: "googleAnalyticsData", + category: "CLI", + googleAnalyticsDataType: gaDataType, + customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" } + }); + + _.each([GoogleAnalyticsDataType.Page, GoogleAnalyticsDataType.Event], (googleAnalyticsDataType: string) => { + it(`when data is ${googleAnalyticsDataType}`, async () => { + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType), getDataToSend(googleAnalyticsDataType)); + await assertExpectedResult(testInjector, opts); + }); + + it(`when data is ${googleAnalyticsDataType} and terminal is not interactive`, async () => { + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, AnalyticsClients.Unknown), getDataToSend(googleAnalyticsDataType), { isInteractive: false }); + await assertExpectedResult(testInjector, opts); + }); + + _.each([true, false], (isInteractive) => { + it(`when data is ${googleAnalyticsDataType} terminal is ${isInteractive ? "" : "not "}interactive and --analyticsClient is passed`, async () => { + const analyticsClient = "AnalyticsClient"; + + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, analyticsClient), getDataToSend(googleAnalyticsDataType), { isInteractive }); + const options = testInjector.resolve("options"); + options.analyticsClient = analyticsClient; + + await assertExpectedResult(testInjector, opts); + }); + }); + }); + }); + }); +}); diff --git a/test/stubs.ts b/test/stubs.ts index 99e7963a88..8a7c1e82e9 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -13,9 +13,12 @@ export class LoggerStub implements ILogger { warnWithLabel(...args: string[]): void { } info(...args: string[]): void { } debug(...args: string[]): void { } - trace(...args: string[]): void { } + trace(...args: string[]): void { + this.traceOutput += util.format.apply(null, args) + "\n"; + } public output = ""; + public traceOutput = ""; out(...args: string[]): void { this.output += util.format.apply(null, args) + "\n"; From 254ac9fb16af027d372fcbb8c8e3277ee381e06f Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Wed, 8 Nov 2017 17:13:37 +0200 Subject: [PATCH 08/14] Use profileDir from settingsService (#3203) Currently CLI's configuration directory is used from `$options.profileDir`. When user passes `--profileDir `, the `$options.profileDir` value is populated. In case user does not pass anything, a default value is set. All services that require configuration directory use the `$options.profileDir`. However, this causes several issues: - `$options` is intended for use only when CLI is used as a standalone command line. In case you are using it as a library, the `$options` object will not be populated. - Unable to test local installations of extensions when CLI is used as library - there's no way to set the custom profileDir when using CLI as a library. So the extensions are always loaded from the default location. In order to resolve these issues, move the logic for profileDir in `settingsService` and introduce a new method to get the profileDir. In order to ensure code is backwards compatible (i.e. extensions that use `$options.profileDir` should still work), modify `$options` to set the value of `profileDir` in `settingsService`. Whenever you want to test local extensions you can use: ```JavaScript const tns = require("nativescript"); tns.settingsService.setSettings({ profileDir: "my custom dir" }); Promise.all(tns.extensibilityService.loadExtensions()) .then((result) => { console.log("Loaded extensions:", result); // write your code here }); ``` Replace all places where `$options.profileDir` is used with `$settingsService.getProfileDir()`. --- PublicAPI.md | 2 +- lib/commands/post-install.ts | 4 ++-- lib/config.ts | 3 +++ lib/npm-installation-manager.ts | 3 ++- lib/options.ts | 24 +++------------------- lib/services/extensibility-service.ts | 4 ++-- lib/services/user-settings-service.ts | 4 ++-- test/android-project-properties-manager.ts | 2 ++ test/commands/post-install.ts | 3 +++ test/debug.ts | 3 +++ test/ios-project-service.ts | 3 +++ test/npm-installation-manager.ts | 2 ++ test/npm-support.ts | 3 ++- test/platform-commands.ts | 2 ++ test/platform-service.ts | 2 ++ test/plugin-variables-service.ts | 2 ++ test/plugins-service.ts | 2 ++ test/project-service.ts | 3 +++ test/services/extensibility-service.ts | 21 ++++++++++--------- 19 files changed, 52 insertions(+), 40 deletions(-) diff --git a/PublicAPI.md b/PublicAPI.md index 0b3b3547bd..8501addc33 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -273,7 +273,7 @@ interface ISettingsService { * Usage: ```JavaScript -tns.settingsService.setSettings({ userAgentName: "myUserAgent" }); +tns.settingsService.setSettings({ userAgentName: "myUserAgent", profileDir: "customProfileDir" }); ``` ## npm diff --git a/lib/commands/post-install.ts b/lib/commands/post-install.ts index f574171789..53d1c5f9a4 100644 --- a/lib/commands/post-install.ts +++ b/lib/commands/post-install.ts @@ -6,11 +6,11 @@ export class PostInstallCliCommand extends PostInstallCommand { $staticConfig: Config.IStaticConfig, $commandsService: ICommandsService, $helpService: IHelpService, - $options: ICommonOptions, + $settingsService: ISettingsService, $doctorService: IDoctorService, $analyticsService: IAnalyticsService, $logger: ILogger) { - super($fs, $staticConfig, $commandsService, $helpService, $options, $doctorService, $analyticsService, $logger); + super($fs, $staticConfig, $commandsService, $helpService, $settingsService, $doctorService, $analyticsService, $logger); } public async execute(args: string[]): Promise { diff --git a/lib/config.ts b/lib/config.ts index dd4fcc8d13..707ca1e3a2 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -28,6 +28,9 @@ export class StaticConfig extends StaticConfigBase implements IStaticConfig { public ERROR_REPORT_SETTING_NAME = "TrackExceptions"; public ANALYTICS_INSTALLATION_ID_SETTING_NAME = "AnalyticsInstallationID"; public INSTALLATION_SUCCESS_MESSAGE = "Installation successful. You are good to go. Connect with us on `http://twitter.com/NativeScript`."; + public get PROFILE_DIR_NAME(): string { + return ".nativescript-cli"; + } constructor($injector: IInjector) { super($injector); diff --git a/lib/npm-installation-manager.ts b/lib/npm-installation-manager.ts index 0911fcecd5..3c44556873 100644 --- a/lib/npm-installation-manager.ts +++ b/lib/npm-installation-manager.ts @@ -7,6 +7,7 @@ export class NpmInstallationManager implements INpmInstallationManager { private $childProcess: IChildProcess, private $logger: ILogger, private $options: IOptions, + private $settingsService: ISettingsService, private $fs: IFileSystem, private $staticConfig: IStaticConfig) { } @@ -59,7 +60,7 @@ export class NpmInstallationManager implements INpmInstallationManager { // local installation takes precedence over cache if (!this.inspectorAlreadyInstalled(inspectorPath)) { - const cachePath = path.join(this.$options.profileDir, constants.INSPECTOR_CACHE_DIRNAME); + const cachePath = path.join(this.$settingsService.getProfileDir(), constants.INSPECTOR_CACHE_DIRNAME); this.prepareCacheDir(cachePath); const pathToPackageInCache = path.join(cachePath, constants.NODE_MODULES_FOLDER_NAME, inspectorNpmPackageName); diff --git a/lib/options.ts b/lib/options.ts index e947c4cb9a..20c1eddd2b 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -1,11 +1,10 @@ import * as commonOptionsLibPath from "./common/options"; -import * as osenv from "osenv"; -import * as path from "path"; export class Options extends commonOptionsLibPath.OptionsBase { constructor($errors: IErrors, $staticConfig: IStaticConfig, - $hostInfo: IHostInfo) { + $hostInfo: IHostInfo, + $settingsService: ISettingsService) { super({ ipa: { type: OptionType.String }, frameworkPath: { type: OptionType.String }, @@ -39,24 +38,7 @@ export class Options extends commonOptionsLibPath.OptionsBase { clean: { type: OptionType.Boolean }, watch: { type: OptionType.Boolean, default: true } }, - path.join($hostInfo.isWindows ? process.env.AppData : path.join(osenv.home(), ".local/share"), ".nativescript-cli"), - $errors, $staticConfig); - - // On Windows we moved settings from LocalAppData to AppData. Move the existing file to keep the existing settings - // I guess we can remove this code after some grace period, say after 1.7 is out - if ($hostInfo.isWindows) { - try { - const shelljs = require("shelljs"), - oldSettings = path.join(process.env.LocalAppData, ".nativescript-cli", "user-settings.json"), - newSettings = path.join(process.env.AppData, ".nativescript-cli", "user-settings.json"); - if (shelljs.test("-e", oldSettings) && !shelljs.test("-e", newSettings)) { - shelljs.mkdir(path.join(process.env.AppData, ".nativescript-cli")); - shelljs.mv(oldSettings, newSettings); - } - } catch (err) { - // ignore the error - it is too early to use $logger here - } - } + $errors, $staticConfig, $settingsService); const that = (this); // if justlaunch is set, it takes precedence over the --watch flag and the default true value diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index 0ed6c3a8eb..c96df2117a 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -4,7 +4,7 @@ import * as constants from "../constants"; export class ExtensibilityService implements IExtensibilityService { private get pathToExtensions(): string { - return path.join(path.resolve(this.$options.profileDir), "extensions"); + return path.join(this.$settingsService.getProfileDir(), "extensions"); } private get pathToPackageJson(): string { @@ -14,7 +14,7 @@ export class ExtensibilityService implements IExtensibilityService { constructor(private $fs: IFileSystem, private $logger: ILogger, private $npm: INodePackageManager, - private $options: IOptions, + private $settingsService: ISettingsService, private $requireService: IRequireService) { } diff --git a/lib/services/user-settings-service.ts b/lib/services/user-settings-service.ts index a89c8495e1..ec0c0ca670 100644 --- a/lib/services/user-settings-service.ts +++ b/lib/services/user-settings-service.ts @@ -3,9 +3,9 @@ import * as userSettingsServiceBaseLib from "../common/services/user-settings-se class UserSettingsService extends userSettingsServiceBaseLib.UserSettingsServiceBase { constructor($fs: IFileSystem, - $options: IOptions, + $settingsService: ISettingsService, $lockfile: ILockFile) { - const userSettingsFilePath = path.join($options.profileDir, "user-settings.json"); + const userSettingsFilePath = path.join($settingsService.getProfileDir(), "user-settings.json"); super(userSettingsFilePath, $fs, $lockfile); } diff --git a/test/android-project-properties-manager.ts b/test/android-project-properties-manager.ts index a92ab97f40..1c51a3a75b 100644 --- a/test/android-project-properties-manager.ts +++ b/test/android-project-properties-manager.ts @@ -8,6 +8,7 @@ import * as LoggerLib from "../lib/common/logger"; import * as ConfigLib from "../lib/config"; import * as OptionsLib from "../lib/options"; import * as yok from "../lib/common/yok"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import * as path from "path"; import temp = require("temp"); temp.track(); @@ -23,6 +24,7 @@ function createTestInjector(): IInjector { testInjector.register("logger", LoggerLib.Logger); testInjector.register("config", ConfigLib.Configuration); testInjector.register("options", OptionsLib.Options); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/commands/post-install.ts b/test/commands/post-install.ts index 51f4dd8e02..81505cde77 100644 --- a/test/commands/post-install.ts +++ b/test/commands/post-install.ts @@ -1,6 +1,7 @@ import { Yok } from "../../lib/common/yok"; import { assert } from "chai"; import { PostInstallCliCommand } from "../../lib/commands/post-install"; +import { SettingsService } from "../../lib/common/test/unit-tests/stubs"; const createTestInjector = (): IInjector => { const testInjector = new Yok(); @@ -38,6 +39,8 @@ const createTestInjector = (): IInjector => { printMarkdown: (...args: any[]): void => undefined }); + testInjector.register("settingsService", SettingsService); + testInjector.registerCommand("post-install-cli", PostInstallCliCommand); return testInjector; diff --git a/test/debug.ts b/test/debug.ts index a5753a12c6..0d529a6f9b 100644 --- a/test/debug.ts +++ b/test/debug.ts @@ -11,6 +11,8 @@ import { AndroidDebugBridge } from "../lib/common/mobile/android/android-debug-b import { AndroidDebugBridgeResultHandler } from "../lib/common/mobile/android/android-debug-bridge-result-handler"; import { DebugCommandErrors } from "../lib/constants"; import { CONNECTED_STATUS, UNREACHABLE_STATUS } from "../lib/common/constants"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; + const helpers = require("../lib/common/helpers"); const originalIsInteracive = helpers.isInteractive; @@ -73,6 +75,7 @@ function createTestInjector(): IInjector { return null; } }); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index 30b7e58629..dfccaaf237 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -33,6 +33,7 @@ import * as constants from "../lib/constants"; import { assert } from "chai"; import { IOSProvisionService } from "../lib/services/ios-provision-service"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import temp = require("temp"); temp.track(); @@ -114,6 +115,8 @@ function createTestInjector(projectPath: string, projectName: string): IInjector testInjector.register("npmInstallationManager", NpmInstallationManager); testInjector.register("npm", NodePackageManager); testInjector.register("xCConfigService", XCConfigService); + testInjector.register("settingsService", SettingsService); + return testInjector; } diff --git a/test/npm-installation-manager.ts b/test/npm-installation-manager.ts index fa06505ca0..1c35146e37 100644 --- a/test/npm-installation-manager.ts +++ b/test/npm-installation-manager.ts @@ -9,6 +9,7 @@ import * as OptionsLib from "../lib/options"; import * as StaticConfigLib from "../lib/config"; import * as yok from "../lib/common/yok"; import ChildProcessLib = require("../lib/common/child-process"); +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; function createTestInjector(): IInjector { const testInjector = new yok.Yok(); @@ -21,6 +22,7 @@ function createTestInjector(): IInjector { testInjector.register("hostInfo", HostInfoLib.HostInfo); testInjector.register("staticConfig", StaticConfigLib.StaticConfig); testInjector.register("childProcess", ChildProcessLib.ChildProcess); + testInjector.register("settingsService", SettingsService); testInjector.register("npmInstallationManager", NpmInstallationManagerLib.NpmInstallationManager); diff --git a/test/npm-support.ts b/test/npm-support.ts index b4757e924c..34e13d2133 100644 --- a/test/npm-support.ts +++ b/test/npm-support.ts @@ -26,6 +26,7 @@ import { XmlValidator } from "../lib/xml-validator"; import ProjectChangesLib = require("../lib/services/project-changes-service"); import { Messages } from "../lib/common/messages/messages"; import { NodeModulesDependenciesBuilder } from "../lib/tools/node-modules/node-modules-dependencies-builder"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import path = require("path"); import temp = require("temp"); @@ -82,7 +83,7 @@ function createTestInjector(): IInjector { }); testInjector.register("messages", Messages); testInjector.register("nodeModulesDependenciesBuilder", NodeModulesDependenciesBuilder); - + testInjector.register("settingsService", SettingsService); testInjector.register("devicePathProvider", {}); return testInjector; diff --git a/test/platform-commands.ts b/test/platform-commands.ts index f862e6164c..7953219e09 100644 --- a/test/platform-commands.ts +++ b/test/platform-commands.ts @@ -21,6 +21,7 @@ import { XmlValidator } from "../lib/xml-validator"; import * as ChildProcessLib from "../lib/common/child-process"; import ProjectChangesLib = require("../lib/services/project-changes-service"); import { Messages } from "../lib/common/messages/messages"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; let isCommandExecuted = true; @@ -149,6 +150,7 @@ function createTestInjector() { testInjector.register("helpService", { showCommandLineHelp: async (): Promise => (undefined) }); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/platform-service.ts b/test/platform-service.ts index 99ef2203d1..41ca8d1a77 100644 --- a/test/platform-service.ts +++ b/test/platform-service.ts @@ -19,6 +19,7 @@ import { XmlValidator } from "../lib/xml-validator"; import * as ChildProcessLib from "../lib/common/child-process"; import ProjectChangesLib = require("../lib/services/project-changes-service"); import { Messages } from "../lib/common/messages/messages"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; require("should"); const temp = require("temp"); @@ -90,6 +91,7 @@ function createTestInjector() { testInjector.register("helpService", { showCommandLineHelp: async (): Promise => (undefined) }); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/plugin-variables-service.ts b/test/plugin-variables-service.ts index 2beb1737e0..6e51f32899 100644 --- a/test/plugin-variables-service.ts +++ b/test/plugin-variables-service.ts @@ -11,6 +11,7 @@ import { ProjectHelper } from "../lib/common/project-helper"; import { StaticConfig } from "../lib/config"; import { MessagesService } from "../lib/common/services/messages-service"; import { Yok } from '../lib/common/yok'; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import * as stubs from './stubs'; import * as path from "path"; import * as temp from "temp"; @@ -37,6 +38,7 @@ function createTestInjector(): IInjector { } }); testInjector.register("staticConfig", StaticConfig); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 1f5953cec1..7a07e480a1 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -30,6 +30,7 @@ import { ProjectFilesProvider } from "../lib/providers/project-files-provider"; import { MobilePlatformsCapabilities } from "../lib/mobile-platforms-capabilities"; import { DevicePlatformsConstants } from "../lib/common/mobile/device-platforms-constants"; import { XmlValidator } from "../lib/xml-validator"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; import StaticConfigLib = require("../lib/config"); import * as path from "path"; import * as temp from "temp"; @@ -101,6 +102,7 @@ function createTestInjector() { testInjector.register("helpService", { showCommandLineHelp: async (): Promise => (undefined) }); + testInjector.register("settingsService", SettingsService); return testInjector; } diff --git a/test/project-service.ts b/test/project-service.ts index 2ed5961274..94add49581 100644 --- a/test/project-service.ts +++ b/test/project-service.ts @@ -18,6 +18,7 @@ import { assert } from "chai"; import { Options } from "../lib/options"; import { HostInfo } from "../lib/common/host-info"; import { ProjectTemplatesService } from "../lib/services/project-templates-service"; +import { SettingsService } from "../lib/common/test/unit-tests/stubs"; const mockProjectNameValidator = { validate: () => true @@ -156,6 +157,7 @@ class ProjectIntegrationTest { } }); this.testInjector.register("npmInstallationManager", NpmInstallationManager); + this.testInjector.register("settingsService", SettingsService); } } @@ -429,6 +431,7 @@ describe("Project Service Tests", () => { testInjector.register("staticConfig", {}); testInjector.register("projectHelper", {}); testInjector.register("npmInstallationManager", {}); + testInjector.register("settingsService", SettingsService); return testInjector; }; diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index 26746cdf31..87978b60c3 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -3,6 +3,7 @@ import { Yok } from "../../lib/common/yok"; import * as stubs from "../stubs"; import { assert } from "chai"; import * as constants from "../../lib/constants"; +import { SettingsService } from "../../lib/common/test/unit-tests/stubs"; const path = require("path"); const originalResolve = path.resolve; @@ -20,9 +21,7 @@ describe("extensibilityService", () => { testInjector.register("fs", {}); testInjector.register("logger", stubs.LoggerStub); testInjector.register("npm", {}); - testInjector.register("options", { - profileDir: "profileDir" - }); + testInjector.register("settingsService", SettingsService); testInjector.register("requireService", { require: (pathToRequire: string): any => undefined }); @@ -121,10 +120,11 @@ describe("extensibilityService", () => { it("passes full path to extensions dir for installation", async () => { const extensionName = "extension1"; const testInjector = getTestInjector(); - const options: IOptions = testInjector.resolve("options"); - options.profileDir = "my-profile-dir"; + const settingsService: ISettingsService = testInjector.resolve("settingsService"); + const profileDir = "my-profile-dir"; + settingsService.getProfileDir = () => profileDir; - const expectedDirForInstallation = path.join(options.profileDir, "extensions"); + const expectedDirForInstallation = path.join(profileDir, "extensions"); const argsPassedToNpmInstall = await getArgsPassedToNpmInstallDuringInstallExtensionCall(extensionName, testInjector); assert.deepEqual(argsPassedToNpmInstall.pathToSave, expectedDirForInstallation); }); @@ -514,12 +514,13 @@ describe("extensibilityService", () => { it("passes full path to extensions dir for uninstallation", async () => { const extensionName = "extension1"; const testInjector = getTestInjector(); - const options: IOptions = testInjector.resolve("options"); - options.profileDir = "my-profile-dir"; + const settingsService: ISettingsService = testInjector.resolve("settingsService"); + const profileDir = "my-profile-dir"; + settingsService.getProfileDir = () => profileDir; - const expectedDirForInstallation = path.join(options.profileDir, "extensions"); + const expectedDirForUninstall = path.join(profileDir, "extensions"); const argsPassedToNpmUninstall = await getArgsPassedToNpmUninstallDuringUninstallExtensionCall(extensionName, testInjector); - assert.deepEqual(argsPassedToNpmUninstall.pathToSave, expectedDirForInstallation); + assert.deepEqual(argsPassedToNpmUninstall.pathToSave, expectedDirForUninstall); }); }); From 6ae1d4c01ddf3c15998b16f5ace94b2670fb3777 Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Thu, 9 Nov 2017 13:57:33 +0200 Subject: [PATCH 09/14] Fix showing default help (from index.md) (#3205) In case user executes: - `tns help` - we should show the default index.html help with all commands - `tns --help` - we should show the default command line help generated from index.md file with all commands This has been broken during changing html-help-service to help-service. Fix the default behavior and add tests for it. From 6e9731e1b20896fcfe8c26220057c66efa574d2c Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Mon, 13 Nov 2017 10:55:06 +0200 Subject: [PATCH 10/14] Fix hanging commands when webpack and unit tests are used (#3212) In case you try using npm scripts that come with `nativescript-dev-webpack` and you also have `nativescript-unit-test-runner` in your project, the scripts hang. The problem is that `nativescript-unit-test-runner` has a hook that resolves `testExecutionService`. In its constructor it sets that analytics broker should not be disposed. However, this is incorrect - it should not be disposed only in case we execute tests with livesync enabled. In order to fix this, move the logic in the commands. This way the hook will not change the disposing of the analytics broker, so once CLI process finishes, analytics broker process will be disconnected and CLI's process will die gracefully. Also update submodule, where the following change is applied: Fix execution of hooks outside of process In case you need a hook to be executed outside of process, you should place correct shebang at the beginning of the file and CLI should spawn it. However, this is not working at the moment, as when CLI detects that the hook should be spawned, it just does nothing. The code is incorrectly placed inside the `if` for in process execution. This causes another issue - in case a hook returns a function, which returns falsey value, we decide the hook should be executed outside of process and spawn a new Node.js process to execute the hook. So we execute it twice. Fix the if-else logic, so the hooks will work correctly. --- lib/commands/test.ts | 7 +++++-- lib/services/test-execution-service.ts | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/commands/test.ts b/lib/commands/test.ts index b990a02d44..1fe1ea00e0 100644 --- a/lib/commands/test.ts +++ b/lib/commands/test.ts @@ -4,8 +4,10 @@ function RunTestCommandFactory(platform: string) { return function RunTestCommand( $options: IOptions, $testExecutionService: ITestExecutionService, - $projectData: IProjectData) { + $projectData: IProjectData, + $analyticsService: IAnalyticsService) { $projectData.initializeProjectData(); + $analyticsService.setShouldDispose($options.justlaunch || !$options.watch); const projectFilesConfig = helpers.getProjectFilesConfig({ isReleaseBuild: $options.release }); this.execute = (args: string[]): Promise => $testExecutionService.startTestRunner(platform, $projectData, projectFilesConfig); this.allowedParameters = []; @@ -16,8 +18,9 @@ $injector.registerCommand("dev-test|android", RunTestCommandFactory('android')); $injector.registerCommand("dev-test|ios", RunTestCommandFactory('iOS')); function RunKarmaTestCommandFactory(platform: string) { - return function RunKarmaTestCommand($options: IOptions, $testExecutionService: ITestExecutionService, $projectData: IProjectData) { + return function RunKarmaTestCommand($options: IOptions, $testExecutionService: ITestExecutionService, $projectData: IProjectData, $analyticsService: IAnalyticsService) { $projectData.initializeProjectData(); + $analyticsService.setShouldDispose($options.justlaunch || !$options.watch); const projectFilesConfig = helpers.getProjectFilesConfig({ isReleaseBuild: $options.release }); this.execute = (args: string[]): Promise => $testExecutionService.startKarmaServer(platform, $projectData, projectFilesConfig); this.allowedParameters = []; diff --git a/lib/services/test-execution-service.ts b/lib/services/test-execution-service.ts index 4694b0ea8f..62a54e7373 100644 --- a/lib/services/test-execution-service.ts +++ b/lib/services/test-execution-service.ts @@ -26,9 +26,7 @@ class TestExecutionService implements ITestExecutionService { private $errors: IErrors, private $debugService: IDebugService, private $devicesService: Mobile.IDevicesService, - private $analyticsService: IAnalyticsService, private $childProcess: IChildProcess) { - this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); } public platform: string; From 133399f33b7c83bfbc4f06cafb2104d71fde216b Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 13 Nov 2017 12:15:59 +0200 Subject: [PATCH 11/14] Update submodule --- lib/common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common b/lib/common index d20d42dbd4..6fbcb97d63 160000 --- a/lib/common +++ b/lib/common @@ -1 +1 @@ -Subproject commit d20d42dbd4bd04715de9725c811534448f3c470a +Subproject commit 6fbcb97d63751af1c2cdba440f09ab102468d401 From 9222383e3a6ad222ab9b9f7a73659209b1025af3 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Wed, 15 Nov 2017 12:16:50 +0100 Subject: [PATCH 12/14] find the correct dependency by searching for the `userSpecifiedPackageName` (#3213) --- lib/node-package-manager.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/node-package-manager.ts b/lib/node-package-manager.ts index 60ca9c9d54..77bb61e056 100644 --- a/lib/node-package-manager.ts +++ b/lib/node-package-manager.ts @@ -156,10 +156,12 @@ export class NodePackageManager implements INodePackageManager { }); // Npm 5 return different object after performing `npm install --dry-run`. - // Considering that the dependency is already installed we should - // find it in the `updated` key as a first element of the array. + // We find the correct dependency by searching for the `userSpecifiedPackageName` in the + // `npm5Output.updated` array and as a fallback, considering that the dependency is already installed, + // we find it as the first element. if (!name && npm5Output.updated) { - const updatedDependency = npm5Output.updated[0]; + const packageNameWithoutVersion = userSpecifiedPackageName.split('@')[0]; + const updatedDependency = _.find(npm5Output.updated, ['name', packageNameWithoutVersion]) || npm5Output.updated[0]; return { name: updatedDependency.name, originalOutput, From f8b729222bbfc88e967cd93107c8e49cb5ae2ccb Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 13 Nov 2017 12:32:19 +0200 Subject: [PATCH 13/14] Set version to 3.3.1 --- npm-shrinkwrap.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index dfd7612404..0259f0b524 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "nativescript", - "version": "3.3.0", + "version": "3.3.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index b6f852f8a5..8eb813764d 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nativescript", "preferGlobal": true, - "version": "3.3.0", + "version": "3.3.1", "author": "Telerik ", "description": "Command-line interface for building NativeScript projects", "bin": { From 7b8685b02dc3b8a8fd7bfea28c99e16dab213a43 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 13 Nov 2017 13:02:23 +0200 Subject: [PATCH 14/14] Update Changelog for 3.3.1 release --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f81f5d18c..8adca28f11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,12 @@ NativeScript CLI Changelog ================ -3.3.0 (2017, July 25) +3.3.1 (2017, November 17) +### Fixed +* [Fixed #3164](https://github.com/NativeScript/nativescript-cli/issues/3164): `npm run build-*-bundle` gets stuck at nativescript-unit-test-runner hook. +* [Fixed #3182](https://github.com/NativeScript/nativescript-cli/issues/3182): CLI fails when unable to start Analytics Broker process. + +3.3.0 (2017, October 26) == ### New