Code reviews

Author: Kartik Raj

.vscode/launch.json

@@ -144,7 +144,7 @@
                 "--ui=tdd",
                 "--recursive",
                 "--colors",
-                "--grep", "xExtension",
+                //"--grep", "<suite name>",
                 "--timeout=300000"
             ],
             "outFiles": [
@@ -209,4 +209,4 @@
             ]
         }
     ]
-}
+}

src/client/activation/extensionSurvey.ts

@@ -13,33 +13,33 @@ import {
 import { Common, ExtensionSurveyBanner, LanguageService } from '../common/utils/localize';
 import { sendTelemetryEvent } from '../telemetry';
 import { EventName } from '../telemetry/constants';
-import { IExtensionSurvey } from './types';
+import { IExtensionSingleActivationService } from './types';
 
 // persistent state names, exported to make use of in testing
 export enum extensionSurveyStateKeys {
     doNotShowAgain = 'doNotShowExtensionSurveyAgain',
     disableSurveyForTime = 'doNotShowExtensionSurveyUntilTime'
 }
 
-export const timeToDisableSurveyFor = 1000 * 60 * 60 * 24 * 7 * 12; // 12 weeks
-const waitTimeToShowSurvey = 1000 * 60 * 60 * 3; // 3 hours
+const timeToDisableSurveyFor = 1000 * 60 * 60 * 24 * 7 * 12; // 12 weeks
+const WAIT_TIME_TO_SHOW_SURVEY = 1000 * 60 * 60 * 3; // 3 hours
 
 @injectable()
-export class ExtensionSurveyPrompt implements IExtensionSurvey {
+export class ExtensionSurveyPrompt implements IExtensionSingleActivationService {
     constructor(
         @inject(IApplicationShell) private appShell: IApplicationShell,
         @inject(IBrowserService) private browserService: IBrowserService,
         @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory,
         @inject(IRandom) private random: IRandom,
         @optional() private sampleSizePerOneHundredUsers: number = 10,
-        @optional() private _waitTimeToShowSurvey: number = waitTimeToShowSurvey) { }
+        @optional() private waitTimeToShowSurvey: number = WAIT_TIME_TO_SHOW_SURVEY) { }
 
-    public async initialize(): Promise<void> {
+    public async activate(): Promise<void> {
         const show = this.shouldShowBanner();
         if (!show) {
             return;
         }
-        setTimeout(() => this.showSurvey().ignoreErrors(), this._waitTimeToShowSurvey);
+        setTimeout(() => this.showSurvey().ignoreErrors(), this.waitTimeToShowSurvey);
     }
 
     @traceDecorators.error('Failed to check whether to display prompt for extension survey')

src/client/activation/serviceRegistry.ts

@@ -27,7 +27,7 @@ import { BetaLanguageServerPackageRepository, DailyLanguageServerPackageReposito
 import { LanguageServerPackageService } from './languageServer/languageServerPackageService';
 import { LanguageServerManager } from './languageServer/manager';
 import { PlatformData } from './languageServer/platformData';
-import { IDownloadChannelRule, IExtensionActivationManager, IExtensionActivationService, IExtensionSurvey, ILanguageClientFactory, ILanguageServer, ILanguageServerActivator, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerDownloader, ILanguageServerExtension, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, IPlatformData, LanguageClientFactory, LanguageServerActivator } from './types';
+import { IDownloadChannelRule, IExtensionActivationManager, IExtensionActivationService, IExtensionSingleActivationService, ILanguageClientFactory, ILanguageServer, ILanguageServerActivator, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerDownloader, ILanguageServerExtension, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, IPlatformData, LanguageClientFactory, LanguageServerActivator } from './types';
 
 export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingleton<IExtensionActivationService>(IExtensionActivationService, LanguageServerExtensionActivationService);
@@ -56,5 +56,5 @@ export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.add<ILanguageServerAnalysisOptions>(ILanguageServerAnalysisOptions, LanguageServerAnalysisOptions);
     serviceManager.addSingleton<ILanguageServer>(ILanguageServer, LanguageServer);
     serviceManager.add<ILanguageServerManager>(ILanguageServerManager, LanguageServerManager);
-    serviceManager.addSingleton<IExtensionSurvey>(IExtensionSurvey, ExtensionSurveyPrompt);
+    serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ExtensionSurveyPrompt);
 }

src/client/activation/types.ts

@@ -18,7 +18,7 @@ export interface IExtensionActivationManager extends IDisposable {
 export const IExtensionActivationService = Symbol('IExtensionActivationService');
 /**
  * Classes implementing this interface will have their `activate` methods
- * invoked during the actiavtion of the extension.
+ * invoked for every resource during the actiavtion of the extension.
  * This is a great hook for extension activation code, i.e. you don't need to modify
  * the `extension.ts` file to invoke some code when extension gets activated.
  * @export
@@ -124,7 +124,15 @@ export interface IPlatformData {
     readonly engineExecutableName: string;
 }
 
-export const IExtensionSurvey = Symbol('IExtensionSurvey');
-export interface IExtensionSurvey {
-    initialize(): Promise<void>;
+export const IExtensionSingleActivationService = Symbol('IExtensionSingleActivationService');
+/**
+ * Classes implementing this interface will have their `activate` methods
+ * invoked during the activation of the extension.
+ * This is a great hook for extension activation code, i.e. you don't need to modify
+ * the `extension.ts` file to invoke some code when extension gets activated.
+ * @export
+ * @interface IExtensionSingleActivationService
+ */
+export interface IExtensionSingleActivationService {
+    activate(): Promise<void>;
 }

src/client/extension.ts

@@ -34,7 +34,7 @@ import {
 } from 'vscode';
 
 import { registerTypes as activationRegisterTypes } from './activation/serviceRegistry';
-import { IExtensionActivationManager, IExtensionSurvey, ILanguageServerExtension } from './activation/types';
+import { IExtensionActivationManager, IExtensionSingleActivationService, ILanguageServerExtension } from './activation/types';
 import { buildApi, IExtensionApi } from './api';
 import { registerTypes as appRegisterTypes } from './application/serviceRegistry';
 import { IApplicationDiagnostics } from './application/types';
@@ -263,7 +263,8 @@ function registerServices(context: ExtensionContext, serviceManager: ServiceMana
 async function initializeServices(context: ExtensionContext, serviceManager: ServiceManager, serviceContainer: ServiceContainer) {
     const abExperiments = serviceContainer.get<IExperimentsManager>(IExperimentsManager);
     await abExperiments.activate();
-    serviceContainer.get<IExtensionSurvey>(IExtensionSurvey).initialize().ignoreErrors();
+    const singleActivationServices = serviceContainer.getAll<IExtensionSingleActivationService>(IExtensionSingleActivationService);
+    Promise.all(singleActivationServices.map(item => item.activate())).ignoreErrors();
     const selector = serviceContainer.get<IInterpreterSelector>(IInterpreterSelector);
     selector.initialize();
     context.subscriptions.push(selector);

src/client/telemetry/index.ts

@@ -459,6 +459,9 @@ export interface IEventNamePropertyMapping {
          */
         error?: string;
     };
+    /**
+     * When user clicks a button in the python extension survey prompt, this telemetry event is sent with details
+     */
     [EventName.EXTENSION_SURVEY_PROMPT]: {
         /**
          * Carries the selection of user when they are asked to take the extension survey

src/test/activation/extensionSurvey.unit.test.ts

@@ -75,7 +75,8 @@ suite('xExtension survey prompt - shouldShowBanner()', () => {
         doNotShowAgain
             .setup(d => d.value)
             .returns(() => false);
-        for (let i = 11; i < 100; i = i + 1) {
+        // Default sample size is 10
+        for (let i = 10; i < 100; i = i + 1) {
             random
                 .setup(r => r.getRandomInt(0, 100))
                 .returns(() => i);
@@ -91,6 +92,7 @@ suite('xExtension survey prompt - shouldShowBanner()', () => {
         doNotShowAgain
             .setup(d => d.value)
             .returns(() => false);
+        // Default sample size is 10
         for (let i = 0; i < 10; i = i + 1) {
             random
                 .setup(r => r.getRandomInt(0, 100))