From c8cccbf2517ad7a9ea882ccf96d0fd37cff6cc29 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 17:24:07 -0700 Subject: [PATCH 01/18] Mark various timed intervals. --- src/client/extension.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/client/extension.ts b/src/client/extension.ts index b13956d714c2..9d0eba4d476f 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -13,10 +13,19 @@ initialize(require('vscode')); // Initialize the logger first. require('./common/logger'); +//=============================================== +// We start tracking the extension's startup time at this point. The +// locations at which we record various Intervals are marked below in +// the same way as this. + const durations: Record = {}; import { StopWatch } from './common/utils/stopWatch'; // Do not move this line of code (used to measure extension load times). const stopWatch = new StopWatch(); + +//=============================================== +// loading starts here + import { Container } from 'inversify'; import { CodeActionKind, @@ -113,6 +122,10 @@ import { ITestCodeNavigatorCommandHandler, ITestExplorerCommandHandler } from '. import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; durations.codeLoadingTime = stopWatch.elapsedTime; + +//=============================================== +// loading ends here + const activationDeferred = createDeferred(); let activatedServiceContainer: ServiceContainer | undefined; @@ -129,6 +142,10 @@ export async function activate(context: ExtensionContext): Promise { displayProgress(activationDeferred.promise); durations.startActivateTime = stopWatch.elapsedTime; + + //=============================================== + // activation starts here + const cont = new Container(); const serviceManager = new ServiceManager(cont); const serviceContainer = new ServiceContainer(cont); @@ -202,6 +219,10 @@ async function activateUnsafe(context: ExtensionContext): Promise }); serviceContainer.get(IDebuggerBanner).initialize(); + + //=============================================== + // activation ends here + durations.endActivateTime = stopWatch.elapsedTime; activationDeferred.resolve(); From 3deac30a6cd8072875c77229cd4340b010aa3d66 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 17:29:32 -0700 Subject: [PATCH 02/18] Move sendStartupTelemetry() down to the bottom. --- src/client/extension.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 9d0eba4d476f..aec7d7a9f20e 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -169,15 +169,6 @@ async function activateUnsafe(context: ExtensionContext): Promise serviceManager.get(ICodeExecutionManager).registerCommands(); - if (!isTestExecution()) { - // tslint:disable-next-line:no-suspicious-comment - // TODO: Move this down to right before durations.endActivateTime is set. - sendStartupTelemetry( - Promise.all([activationDeferred.promise, activationPromise]), - serviceContainer - ).ignoreErrors(); - } - const workspaceService = serviceContainer.get(IWorkspaceService); const interpreterManager = serviceContainer.get(IInterpreterService); interpreterManager @@ -220,6 +211,13 @@ async function activateUnsafe(context: ExtensionContext): Promise serviceContainer.get(IDebuggerBanner).initialize(); + if (!isTestExecution()) { + sendStartupTelemetry( + Promise.all([activationDeferred.promise, activationPromise]), + serviceContainer + ).ignoreErrors(); + } + //=============================================== // activation ends here From 4c0b6e866a1d997005fa035fccb1d34116d25138 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 18:11:05 -0700 Subject: [PATCH 03/18] Move startup telemetry code to its own module. --- src/client/extension.ts | 137 ++----------------------------- src/client/startupTelemetry.ts | 142 +++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 130 deletions(-) create mode 100644 src/client/startupTelemetry.ts diff --git a/src/client/extension.ts b/src/client/extension.ts index aec7d7a9f20e..8395cbd84d38 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -54,7 +54,6 @@ import { traceError } from './common/logger'; import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; -import { ITerminalHelper } from './common/terminal/types'; import { GLOBAL_MEMENTO, IAsyncDisposableRegistry, @@ -65,7 +64,6 @@ import { IFeatureDeprecationManager, IMemento, IOutputChannel, - Resource, WORKSPACE_MEMENTO } from './common/types'; import { createDeferred } from './common/utils/async'; @@ -84,18 +82,11 @@ import { IDebuggerBanner } from './debugger/extension/types'; import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry'; -import { - AutoSelectionRule, - IInterpreterAutoSelectionRule, - IInterpreterAutoSelectionService -} from './interpreter/autoSelection/types'; import { IInterpreterSelector } from './interpreter/configuration/types'; import { - ICondaService, IInterpreterLocatorProgressHandler, IInterpreterLocatorProgressService, - IInterpreterService, - PythonInterpreter + IInterpreterService } from './interpreter/contracts'; import { registerTypes as interpretersRegisterTypes } from './interpreter/serviceRegistry'; import { ServiceContainer } from './ioc/container'; @@ -111,9 +102,7 @@ import { registerTypes as providersRegisterTypes } from './providers/serviceRegi import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; import { TerminalProvider } from './providers/terminalProvider'; import { ISortImportsEditingProvider } from './providers/types'; -import { sendTelemetryEvent } from './telemetry'; -import { EventName } from './telemetry/constants'; -import { EditorLoadTelemetry } from './telemetry/types'; +import { sendErrorTelemetry, sendStartupTelemetry } from './startupTelemetry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { TEST_OUTPUT_CHANNEL } from './testing/common/constants'; @@ -211,19 +200,16 @@ async function activateUnsafe(context: ExtensionContext): Promise serviceContainer.get(IDebuggerBanner).initialize(); - if (!isTestExecution()) { - sendStartupTelemetry( - Promise.all([activationDeferred.promise, activationPromise]), - serviceContainer - ).ignoreErrors(); - } - //=============================================== // activation ends here durations.endActivateTime = stopWatch.elapsedTime; activationDeferred.resolve(); + sendStartupTelemetry(activationPromise, durations, stopWatch, serviceContainer) + // It will run in the background. + .ignoreErrors(); + const api = buildApi( Promise.all([activationDeferred.promise, activationPromise]), serviceContainer.get(IExperimentsManager), @@ -330,98 +316,6 @@ async function initializeServices( serviceContainer.get(ITestContextService).register(); } -// tslint:disable-next-line:no-any -async function sendStartupTelemetry(activatedPromise: Promise, serviceContainer: IServiceContainer) { - try { - await activatedPromise; - durations.totalActivateTime = stopWatch.elapsedTime; - const props = await getActivationTelemetryProps(serviceContainer); - sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props); - } catch (ex) { - traceError('sendStartupTelemetry() failed.', ex); - } -} -function isUsingGlobalInterpreterInWorkspace(currentPythonPath: string, serviceContainer: IServiceContainer): boolean { - const service = serviceContainer.get(IInterpreterAutoSelectionService); - const globalInterpreter = service.getAutoSelectedInterpreter(undefined); - if (!globalInterpreter) { - return false; - } - return currentPythonPath === globalInterpreter.path; -} -function hasUserDefinedPythonPath(resource: Resource, serviceContainer: IServiceContainer) { - const workspaceService = serviceContainer.get(IWorkspaceService); - const settings = workspaceService.getConfiguration('python', resource)!.inspect('pythonPath')!; - return (settings.workspaceFolderValue && settings.workspaceFolderValue !== 'python') || - (settings.workspaceValue && settings.workspaceValue !== 'python') || - (settings.globalValue && settings.globalValue !== 'python') - ? true - : false; -} - -function getPreferredWorkspaceInterpreter(resource: Resource, serviceContainer: IServiceContainer) { - const workspaceInterpreterSelector = serviceContainer.get( - IInterpreterAutoSelectionRule, - AutoSelectionRule.workspaceVirtualEnvs - ); - const interpreter = workspaceInterpreterSelector.getPreviouslyAutoSelectedInterpreter(resource); - return interpreter ? interpreter.path : undefined; -} - -///////////////////////////// -// telemetry - -// tslint:disable-next-line:no-any -async function getActivationTelemetryProps(serviceContainer: IServiceContainer): Promise { - // tslint:disable-next-line:no-suspicious-comment - // TODO: Not all of this data is showing up in the database... - // tslint:disable-next-line:no-suspicious-comment - // TODO: If any one of these parts fails we send no info. We should - // be able to partially populate as much as possible instead - // (through granular try-catch statements). - const terminalHelper = serviceContainer.get(ITerminalHelper); - const terminalShellType = terminalHelper.identifyTerminalShell(); - const condaLocator = serviceContainer.get(ICondaService); - const interpreterService = serviceContainer.get(IInterpreterService); - const workspaceService = serviceContainer.get(IWorkspaceService); - const configurationService = serviceContainer.get(IConfigurationService); - const mainWorkspaceUri = workspaceService.hasWorkspaceFolders - ? workspaceService.workspaceFolders![0].uri - : undefined; - const settings = configurationService.getSettings(mainWorkspaceUri); - const [condaVersion, interpreter, interpreters] = await Promise.all([ - condaLocator - .getCondaVersion() - .then(ver => (ver ? ver.raw : '')) - .catch(() => ''), - interpreterService.getActiveInterpreter().catch(() => undefined), - interpreterService.getInterpreters(mainWorkspaceUri).catch(() => []) - ]); - const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0; - const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined; - const interpreterType = interpreter ? interpreter.type : undefined; - const usingUserDefinedInterpreter = hasUserDefinedPythonPath(mainWorkspaceUri, serviceContainer); - const preferredWorkspaceInterpreter = getPreferredWorkspaceInterpreter(mainWorkspaceUri, serviceContainer); - const usingGlobalInterpreter = isUsingGlobalInterpreterInWorkspace(settings.pythonPath, serviceContainer); - const usingAutoSelectedWorkspaceInterpreter = preferredWorkspaceInterpreter - ? settings.pythonPath === getPreferredWorkspaceInterpreter(mainWorkspaceUri, serviceContainer) - : false; - const hasPython3 = - interpreters!.filter(item => (item && item.version ? item.version.major === 3 : false)).length > 0; - - return { - condaVersion, - terminal: terminalShellType, - pythonVersion, - interpreterType, - workspaceFolderCount, - hasPython3, - usingUserDefinedInterpreter, - usingAutoSelectedWorkspaceInterpreter, - usingGlobalInterpreter - }; -} - ///////////////////////////// // error handling @@ -430,7 +324,7 @@ function handleError(ex: Error) { "Extension activation failed, run the 'Developer: Toggle Developer Tools' command for more information." ); traceError('extension activation failed', ex); - sendErrorTelemetry(ex).ignoreErrors(); + sendErrorTelemetry(ex, durations, activatedServiceContainer).ignoreErrors(); } interface IAppShell { @@ -450,20 +344,3 @@ function notifyUser(msg: string) { // ignore } } - -async function sendErrorTelemetry(ex: Error) { - try { - // tslint:disable-next-line:no-any - let props: any = {}; - if (activatedServiceContainer) { - try { - props = await getActivationTelemetryProps(activatedServiceContainer); - } catch (ex) { - // ignore - } - } - sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props, ex); - } catch (exc2) { - traceError('sendErrorTelemetry() failed.', exc2); - } -} diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts new file mode 100644 index 000000000000..4533b083cf4f --- /dev/null +++ b/src/client/startupTelemetry.ts @@ -0,0 +1,142 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { IWorkspaceService } from './common/application/types'; +import { isTestExecution } from './common/constants'; +import { traceError } from './common/logger'; +import { ITerminalHelper } from './common/terminal/types'; +import { IConfigurationService, Resource } from './common/types'; +import { + AutoSelectionRule, + IInterpreterAutoSelectionRule, + IInterpreterAutoSelectionService +} from './interpreter/autoSelection/types'; +import { ICondaService, IInterpreterService, PythonInterpreter } from './interpreter/contracts'; +import { IServiceContainer } from './ioc/types'; +import { sendTelemetryEvent } from './telemetry'; +import { EventName } from './telemetry/constants'; +import { EditorLoadTelemetry } from './telemetry/types'; + +interface IStopWatch { + elapsedTime: number; +} + +export async function sendStartupTelemetry( + // tslint:disable-next-line:no-any + activatedPromise: Promise, + durations: Record, + stopWatch: IStopWatch, + serviceContainer: IServiceContainer +) { + if (isTestExecution()) { + return; + } + + try { + await activatedPromise; + durations.totalActivateTime = stopWatch.elapsedTime; + const props = await getActivationTelemetryProps(serviceContainer); + sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props); + } catch (ex) { + traceError('sendStartupTelemetry() failed.', ex); + } +} + +export async function sendErrorTelemetry( + ex: Error, + durations: Record, + serviceContainer?: IServiceContainer +) { + try { + // tslint:disable-next-line:no-any + let props: any = {}; + if (serviceContainer) { + try { + props = await getActivationTelemetryProps(serviceContainer); + } catch (ex) { + // ignore + } + } + sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props, ex); + } catch (exc2) { + traceError('sendErrorTelemetry() failed.', exc2); + } +} + +function isUsingGlobalInterpreterInWorkspace(currentPythonPath: string, serviceContainer: IServiceContainer): boolean { + const service = serviceContainer.get(IInterpreterAutoSelectionService); + const globalInterpreter = service.getAutoSelectedInterpreter(undefined); + if (!globalInterpreter) { + return false; + } + return currentPythonPath === globalInterpreter.path; +} + +function hasUserDefinedPythonPath(resource: Resource, serviceContainer: IServiceContainer) { + const workspaceService = serviceContainer.get(IWorkspaceService); + const settings = workspaceService.getConfiguration('python', resource)!.inspect('pythonPath')!; + return (settings.workspaceFolderValue && settings.workspaceFolderValue !== 'python') || + (settings.workspaceValue && settings.workspaceValue !== 'python') || + (settings.globalValue && settings.globalValue !== 'python') + ? true + : false; +} + +function getPreferredWorkspaceInterpreter(resource: Resource, serviceContainer: IServiceContainer) { + const workspaceInterpreterSelector = serviceContainer.get( + IInterpreterAutoSelectionRule, + AutoSelectionRule.workspaceVirtualEnvs + ); + const interpreter = workspaceInterpreterSelector.getPreviouslyAutoSelectedInterpreter(resource); + return interpreter ? interpreter.path : undefined; +} + +async function getActivationTelemetryProps(serviceContainer: IServiceContainer): Promise { + // tslint:disable-next-line:no-suspicious-comment + // TODO: Not all of this data is showing up in the database... + // tslint:disable-next-line:no-suspicious-comment + // TODO: If any one of these parts fails we send no info. We should + // be able to partially populate as much as possible instead + // (through granular try-catch statements). + const terminalHelper = serviceContainer.get(ITerminalHelper); + const terminalShellType = terminalHelper.identifyTerminalShell(); + const condaLocator = serviceContainer.get(ICondaService); + const interpreterService = serviceContainer.get(IInterpreterService); + const workspaceService = serviceContainer.get(IWorkspaceService); + const configurationService = serviceContainer.get(IConfigurationService); + const mainWorkspaceUri = workspaceService.hasWorkspaceFolders + ? workspaceService.workspaceFolders![0].uri + : undefined; + const settings = configurationService.getSettings(mainWorkspaceUri); + const [condaVersion, interpreter, interpreters] = await Promise.all([ + condaLocator + .getCondaVersion() + .then(ver => (ver ? ver.raw : '')) + .catch(() => ''), + interpreterService.getActiveInterpreter().catch(() => undefined), + interpreterService.getInterpreters(mainWorkspaceUri).catch(() => []) + ]); + const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0; + const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined; + const interpreterType = interpreter ? interpreter.type : undefined; + const usingUserDefinedInterpreter = hasUserDefinedPythonPath(mainWorkspaceUri, serviceContainer); + const preferredWorkspaceInterpreter = getPreferredWorkspaceInterpreter(mainWorkspaceUri, serviceContainer); + const usingGlobalInterpreter = isUsingGlobalInterpreterInWorkspace(settings.pythonPath, serviceContainer); + const usingAutoSelectedWorkspaceInterpreter = preferredWorkspaceInterpreter + ? settings.pythonPath === getPreferredWorkspaceInterpreter(mainWorkspaceUri, serviceContainer) + : false; + const hasPython3 = + interpreters!.filter(item => (item && item.version ? item.version.major === 3 : false)).length > 0; + + return { + condaVersion, + terminal: terminalShellType, + pythonVersion, + interpreterType, + workspaceFolderCount, + hasPython3, + usingUserDefinedInterpreter, + usingAutoSelectedWorkspaceInterpreter, + usingGlobalInterpreter + }; +} From 957818040d0eb4a84b0e8bcf552def55d0cc1924 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 18:32:10 -0700 Subject: [PATCH 04/18] Simplify the buildApi() call in activateUnsafe(). --- src/client/api.ts | 20 +++++++++++++++++--- src/client/extension.ts | 22 +++------------------- src/test/extension.unit.test.ts | 30 ++++++++++++++++++++++-------- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/client/api.ts b/src/client/api.ts index d84858f8acf0..85b8b6b162b0 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -3,11 +3,13 @@ 'use strict'; +import { isTestExecution } from './common/constants'; import { DebugAdapterNewPtvsd } from './common/experimentGroups'; import { traceError } from './common/logger'; import { IExperimentsManager } from './common/types'; import { RemoteDebuggerExternalLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider'; import { IDebugAdapterDescriptorFactory } from './debugger/extension/types'; +import { IServiceContainer, IServiceManager } from './ioc/types'; /* * Do not introduce any breaking changes to this API. @@ -38,10 +40,13 @@ export interface IExtensionApi { export function buildApi( // tslint:disable-next-line:no-any ready: Promise, - experimentsManager: IExperimentsManager, - debugFactory: IDebugAdapterDescriptorFactory + serviceManager: IServiceManager, + serviceContainer: IServiceContainer ) { - return { + const experimentsManager = serviceContainer.get(IExperimentsManager); + const debugFactory = serviceContainer.get(IDebugAdapterDescriptorFactory); + + const api = { // 'ready' will propagate the exception, but we must log it here first. ready: ready.catch(ex => { traceError('Failure during activation.', ex); @@ -69,4 +74,13 @@ export function buildApi( } } }; + + // In test environment return the DI Container. + if (isTestExecution()) { + // tslint:disable:no-any + (api as any).serviceContainer = serviceContainer; + (api as any).serviceManager = serviceManager; + // tslint:enable:no-any + } + return api; } diff --git a/src/client/extension.ts b/src/client/extension.ts index 8395cbd84d38..cf3a0b47accf 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -48,7 +48,7 @@ import { registerTypes as appRegisterTypes } from './application/serviceRegistry import { IApplicationDiagnostics } from './application/types'; import { DebugService } from './common/application/debugService'; import { IApplicationShell, ICommandManager, IWorkspaceService } from './common/application/types'; -import { Commands, isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; +import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; import { traceError } from './common/logger'; import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; @@ -76,11 +76,7 @@ import { DebuggerTypeName } from './debugger/constants'; import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher'; import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types'; import { registerTypes as debugConfigurationRegisterTypes } from './debugger/extension/serviceRegistry'; -import { - IDebugAdapterDescriptorFactory, - IDebugConfigurationService, - IDebuggerBanner -} from './debugger/extension/types'; +import { IDebugConfigurationService, IDebuggerBanner } from './debugger/extension/types'; import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry'; import { IInterpreterSelector } from './interpreter/configuration/types'; import { @@ -210,19 +206,7 @@ async function activateUnsafe(context: ExtensionContext): Promise // It will run in the background. .ignoreErrors(); - const api = buildApi( - Promise.all([activationDeferred.promise, activationPromise]), - serviceContainer.get(IExperimentsManager), - serviceContainer.get(IDebugAdapterDescriptorFactory) - ); - // In test environment return the DI Container. - if (isTestExecution()) { - // tslint:disable:no-any - (api as any).serviceContainer = serviceContainer; - (api as any).serviceManager = serviceManager; - // tslint:enable:no-any - } - return api; + return buildApi(activationPromise, serviceManager, serviceContainer); } export function deactivate(): Thenable { diff --git a/src/test/extension.unit.test.ts b/src/test/extension.unit.test.ts index 12d28ed09bc8..df3565de8408 100644 --- a/src/test/extension.unit.test.ts +++ b/src/test/extension.unit.test.ts @@ -18,6 +18,9 @@ import { ExperimentsManager } from '../client/common/experiments'; import { IExperimentsManager } from '../client/common/types'; import { DebugAdapterDescriptorFactory } from '../client/debugger/extension/adapter/factory'; import { IDebugAdapterDescriptorFactory } from '../client/debugger/extension/types'; +import { ServiceContainer } from '../client/ioc/container'; +import { ServiceManager } from '../client/ioc/serviceManager'; +import { IServiceContainer, IServiceManager } from '../client/ioc/types'; // tslint:disable-next-line: max-func-body-length suite('Extension API Debugger', () => { @@ -26,12 +29,23 @@ suite('Extension API Debugger', () => { const ptvsdHost = 'somehost'; const ptvsdPort = 12345; + let serviceManager: IServiceManager; + let serviceContainer: IServiceContainer; let experimentsManager: IExperimentsManager; let debugAdapterFactory: IDebugAdapterDescriptorFactory; setup(() => { + serviceManager = mock(ServiceManager); + serviceContainer = mock(ServiceContainer); experimentsManager = mock(ExperimentsManager); debugAdapterFactory = mock(DebugAdapterDescriptorFactory); + + when(serviceManager.get(IExperimentsManager)) + // Return the mock. + .thenReturn(Promise.resolve(instance(experimentsManager))); + when(serviceManager.get(IDebugAdapterDescriptorFactory)) + // Return the mock. + .thenReturn(Promise.resolve(instance(debugAdapterFactory))); }); function mockInExperiment(host: string, port: number, wait: boolean) { @@ -85,8 +99,8 @@ suite('Extension API Debugger', () => { const args = await buildApi( Promise.resolve(), - instance(experimentsManager), - instance(debugAdapterFactory) + instance(serviceManager), + instance(serviceContainer) ).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach); const expectedArgs = [expectedLauncherPath, '--default', '--host', ptvsdHost, '--port', ptvsdPort.toString()]; @@ -99,8 +113,8 @@ suite('Extension API Debugger', () => { const args = await buildApi( Promise.resolve(), - instance(experimentsManager), - instance(debugAdapterFactory) + instance(serviceManager), + instance(serviceContainer) ).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach); const expectedArgs = [ptvsdPath, '--host', ptvsdHost, '--port', ptvsdPort.toString()]; @@ -113,8 +127,8 @@ suite('Extension API Debugger', () => { const args = await buildApi( Promise.resolve(), - instance(experimentsManager), - instance(debugAdapterFactory) + instance(serviceManager), + instance(serviceContainer) ).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach); const expectedArgs = [ expectedLauncherPath, @@ -135,8 +149,8 @@ suite('Extension API Debugger', () => { const args = await buildApi( Promise.resolve(), - instance(experimentsManager), - instance(debugAdapterFactory) + instance(serviceManager), + instance(serviceContainer) ).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach); const expectedArgs = [ptvsdPath, '--host', ptvsdHost, '--port', ptvsdPort.toString(), '--wait']; From db994ec602e831ef7694fd8de383192ec78bc3d7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 18:37:54 -0700 Subject: [PATCH 05/18] Keep the expoerted functions adjacent. --- src/client/extension.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index cf3a0b47accf..be16a2172e36 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -123,6 +123,21 @@ export async function activate(context: ExtensionContext): Promise { + // Make sure to shutdown anybody who needs it. + if (activatedServiceContainer) { + const registry = activatedServiceContainer.get(IAsyncDisposableRegistry); + if (registry) { + return registry.dispose(); + } + } + + return Promise.resolve(); +} + +///////////////////////////// +// activation + // tslint:disable-next-line:max-func-body-length async function activateUnsafe(context: ExtensionContext): Promise { displayProgress(activationDeferred.promise); @@ -209,18 +224,6 @@ async function activateUnsafe(context: ExtensionContext): Promise return buildApi(activationPromise, serviceManager, serviceContainer); } -export function deactivate(): Thenable { - // Make sure to shutdown anybody who needs it. - if (activatedServiceContainer) { - const registry = activatedServiceContainer.get(IAsyncDisposableRegistry); - if (registry) { - return registry.dispose(); - } - } - - return Promise.resolve(); -} - // tslint:disable-next-line:no-any function displayProgress(promise: Promise) { const progressOptions: ProgressOptions = { location: ProgressLocation.Window, title: Common.loadingExtension() }; From 1253891186c4c7c601b674a4bd1b6b7286afcada Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 19:06:08 -0700 Subject: [PATCH 06/18] Factor out initializeGlobals(), initializeComponents(), and activateComponents(). --- src/client/extension.ts | 87 +++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index be16a2172e36..b6303009ad24 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -146,11 +146,65 @@ async function activateUnsafe(context: ExtensionContext): Promise //=============================================== // activation starts here + const [serviceManager, serviceContainer] = initializeGlobals(context); + initializeComponents(context, serviceManager, serviceContainer); + const activationPromise = activateComponents(context, serviceManager, serviceContainer); + + //=============================================== + // activation ends here + + durations.endActivateTime = stopWatch.elapsedTime; + activationDeferred.resolve(); + + sendStartupTelemetry(activationPromise, durations, stopWatch, serviceContainer) + // It will run in the background. + .ignoreErrors(); + + return buildApi(activationPromise, serviceManager, serviceContainer); +} + +function initializeGlobals(context: ExtensionContext): [IServiceManager, IServiceContainer] { const cont = new Container(); const serviceManager = new ServiceManager(cont); const serviceContainer = new ServiceContainer(cont); activatedServiceContainer = serviceContainer; - registerServices(context, serviceManager, serviceContainer); + + serviceManager.addSingletonInstance(IServiceContainer, serviceContainer); + serviceManager.addSingletonInstance(IServiceManager, serviceManager); + + serviceManager.addSingletonInstance(IDisposableRegistry, context.subscriptions); + serviceManager.addSingletonInstance(IMemento, context.globalState, GLOBAL_MEMENTO); + serviceManager.addSingletonInstance(IMemento, context.workspaceState, WORKSPACE_MEMENTO); + serviceManager.addSingletonInstance(IExtensionContext, context); + + return [serviceManager, serviceContainer]; +} + +///////////////////////////// +// component init + +function initializeComponents( + _context: ExtensionContext, + _serviceManager: IServiceManager, + _serviceContainer: IServiceContainer +) { + // We will be pulling code over from activateComponents. +} + +///////////////////////////// +// component activation + +async function activateComponents( + context: ExtensionContext, + serviceManager: IServiceManager, + serviceContainer: IServiceContainer +) { + // tslint:disable-next-line:no-suspicious-comment + // TODO(GH-10454): Gradually move simple initialization + // and DI registration currently in this function over + // to initializeComponents(). + + registerServices(serviceManager); await initializeServices(context, serviceManager, serviceContainer); const manager = serviceContainer.get(IExtensionActivationManager); @@ -169,6 +223,8 @@ async function activateUnsafe(context: ExtensionContext): Promise serviceManager.get(ICodeExecutionManager).registerCommands(); + // At this point we have enough information to send valid telemetry. + const workspaceService = serviceContainer.get(IWorkspaceService); const interpreterManager = serviceContainer.get(IInterpreterService); interpreterManager @@ -211,17 +267,7 @@ async function activateUnsafe(context: ExtensionContext): Promise serviceContainer.get(IDebuggerBanner).initialize(); - //=============================================== - // activation ends here - - durations.endActivateTime = stopWatch.elapsedTime; - activationDeferred.resolve(); - - sendStartupTelemetry(activationPromise, durations, stopWatch, serviceContainer) - // It will run in the background. - .ignoreErrors(); - - return buildApi(activationPromise, serviceManager, serviceContainer); + return activationPromise; } // tslint:disable-next-line:no-any @@ -230,18 +276,7 @@ function displayProgress(promise: Promise) { window.withProgress(progressOptions, () => promise); } -function registerServices( - context: ExtensionContext, - serviceManager: ServiceManager, - serviceContainer: ServiceContainer -) { - serviceManager.addSingletonInstance(IServiceContainer, serviceContainer); - serviceManager.addSingletonInstance(IServiceManager, serviceManager); - serviceManager.addSingletonInstance(IDisposableRegistry, context.subscriptions); - serviceManager.addSingletonInstance(IMemento, context.globalState, GLOBAL_MEMENTO); - serviceManager.addSingletonInstance(IMemento, context.workspaceState, WORKSPACE_MEMENTO); - serviceManager.addSingletonInstance(IExtensionContext, context); - +function registerServices(serviceManager: IServiceManager) { const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python()); const unitTestOutChannel = window.createOutputChannel(OutputChannelNames.pythonTest()); const jupyterOutputChannel = window.createOutputChannel(OutputChannelNames.jupyter()); @@ -272,8 +307,8 @@ function registerServices( async function initializeServices( context: ExtensionContext, - serviceManager: ServiceManager, - serviceContainer: ServiceContainer + serviceManager: IServiceManager, + serviceContainer: IServiceContainer ) { const abExperiments = serviceContainer.get(IExperimentsManager); await abExperiments.activate(); From b7f127b102c9a20dafad38640f341d2e664a64a1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 19:14:59 -0700 Subject: [PATCH 07/18] Factor out activateLegacy(). --- src/client/extension.ts | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index b6303009ad24..e5486f655656 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -136,7 +136,7 @@ export function deactivate(): Thenable { } ///////////////////////////// -// activation +// activation helpers // tslint:disable-next-line:max-func-body-length async function activateUnsafe(context: ExtensionContext): Promise { @@ -163,6 +163,12 @@ async function activateUnsafe(context: ExtensionContext): Promise return buildApi(activationPromise, serviceManager, serviceContainer); } +// tslint:disable-next-line:no-any +function displayProgress(promise: Promise) { + const progressOptions: ProgressOptions = { location: ProgressLocation.Window, title: Common.loadingExtension() }; + window.withProgress(progressOptions, () => promise); +} + function initializeGlobals(context: ExtensionContext): [IServiceManager, IServiceContainer] { const cont = new Container(); const serviceManager = new ServiceManager(cont); @@ -188,7 +194,7 @@ function initializeComponents( _serviceManager: IServiceManager, _serviceContainer: IServiceContainer ) { - // We will be pulling code over from activateComponents. + // We will be pulling code over from activateLegacy(). } ///////////////////////////// @@ -199,11 +205,25 @@ async function activateComponents( serviceManager: IServiceManager, serviceContainer: IServiceContainer ) { - // tslint:disable-next-line:no-suspicious-comment - // TODO(GH-10454): Gradually move simple initialization - // and DI registration currently in this function over - // to initializeComponents(). + // We will be pulling code over from activateLegacy(). + + return activateLegacy(context, serviceManager, serviceContainer); +} + +///////////////////////////// +// old activation code + +// tslint:disable-next-line:no-suspicious-comment +// TODO(GH-10454): Gradually move simple initialization +// and DI registration currently in this function over +// to initializeComponents(). Likewise with complex +// init and activation: move them to activateComponents(). +async function activateLegacy( + context: ExtensionContext, + serviceManager: IServiceManager, + serviceContainer: IServiceContainer +) { registerServices(serviceManager); await initializeServices(context, serviceManager, serviceContainer); @@ -270,12 +290,6 @@ async function activateComponents( return activationPromise; } -// tslint:disable-next-line:no-any -function displayProgress(promise: Promise) { - const progressOptions: ProgressOptions = { location: ProgressLocation.Window, title: Common.loadingExtension() }; - window.withProgress(progressOptions, () => promise); -} - function registerServices(serviceManager: IServiceManager) { const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python()); const unitTestOutChannel = window.createOutputChannel(OutputChannelNames.pythonTest()); From 0f75f6366d311b0d61a8328922d1170f6c9b1a27 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 19:22:06 -0700 Subject: [PATCH 08/18] Add notes about what happens during "initialization". --- src/client/extension.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index e5486f655656..c24ed45f45ab 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -169,6 +169,14 @@ function displayProgress(promise: Promise) { window.withProgress(progressOptions, () => promise); } +///////////////////////////// +// intialization (globals & components) + +// The code in this section should do nothing more complex than register +// objects to DI and simple init (e.g. no side effects). That implies +// that constructors are likewise simple and do no work. It also means +// that it is inherently synchronous. + function initializeGlobals(context: ExtensionContext): [IServiceManager, IServiceContainer] { const cont = new Container(); const serviceManager = new ServiceManager(cont); @@ -186,9 +194,6 @@ function initializeGlobals(context: ExtensionContext): [IServiceManager, IServic return [serviceManager, serviceContainer]; } -///////////////////////////// -// component init - function initializeComponents( _context: ExtensionContext, _serviceManager: IServiceManager, From c9cbb95df96fafd496e58a4f5c908159810f7ef4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 5 Mar 2020 19:24:10 -0700 Subject: [PATCH 09/18] Add a NEWS entry. --- news/3 Code Health/10454.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/10454.md diff --git a/news/3 Code Health/10454.md b/news/3 Code Health/10454.md new file mode 100644 index 000000000000..698c4bbbea30 --- /dev/null +++ b/news/3 Code Health/10454.md @@ -0,0 +1 @@ +Refactor the extension activation code to split on phases. From 5b3d57d44581b1fad9004f95224b94585228e97d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 9 Mar 2020 11:46:18 -0600 Subject: [PATCH 10/18] Factor out sendStartupTelemetryInBackground(). --- src/client/extension.ts | 14 +++++++------- src/client/startupTelemetry.ts | 12 +++++++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index c24ed45f45ab..9ead95134cb5 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -98,7 +98,7 @@ import { registerTypes as providersRegisterTypes } from './providers/serviceRegi import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; import { TerminalProvider } from './providers/terminalProvider'; import { ISortImportsEditingProvider } from './providers/types'; -import { sendErrorTelemetry, sendStartupTelemetry } from './startupTelemetry'; +import { sendErrorTelemetry, sendStartupTelemetryInBackground } from './startupTelemetry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { TEST_OUTPUT_CHANNEL } from './testing/common/constants'; @@ -118,7 +118,9 @@ export async function activate(context: ExtensionContext): Promise durations.endActivateTime = stopWatch.elapsedTime; activationDeferred.resolve(); - sendStartupTelemetry(activationPromise, durations, stopWatch, serviceContainer) - // It will run in the background. - .ignoreErrors(); + sendStartupTelemetryInBackground(activationPromise, durations, stopWatch, serviceContainer); return buildApi(activationPromise, serviceManager, serviceContainer); } @@ -360,12 +360,12 @@ async function initializeServices( ///////////////////////////// // error handling -function handleError(ex: Error) { +async function handleError(ex: Error) { notifyUser( "Extension activation failed, run the 'Developer: Toggle Developer Tools' command for more information." ); traceError('extension activation failed', ex); - sendErrorTelemetry(ex, durations, activatedServiceContainer).ignoreErrors(); + await sendErrorTelemetry(ex, durations, activatedServiceContainer); } interface IAppShell { diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts index 4533b083cf4f..ef32d6f994a6 100644 --- a/src/client/startupTelemetry.ts +++ b/src/client/startupTelemetry.ts @@ -21,7 +21,17 @@ interface IStopWatch { elapsedTime: number; } -export async function sendStartupTelemetry( +export function sendStartupTelemetryInBackground( + // tslint:disable-next-line:no-any + activatedPromise: Promise, + durations: Record, + stopWatch: IStopWatch, + serviceContainer: IServiceContainer +) { + sendStartupTelemetry(activatedPromise, durations, stopWatch, serviceContainer).ignoreErrors(); +} + +async function sendStartupTelemetry( // tslint:disable-next-line:no-any activatedPromise: Promise, durations: Record, From 2d30f101ea747390b7aab72e5e7ece1eaae42c39 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 9 Mar 2020 12:41:22 -0600 Subject: [PATCH 11/18] Factor out extension-init.ts and extension-activation.ts. --- src/client/extension-activation.ts | 208 +++++++++++++++++++++ src/client/extension-init.ts | 43 +++++ src/client/extension.ts | 281 ++--------------------------- 3 files changed, 267 insertions(+), 265 deletions(-) create mode 100644 src/client/extension-activation.ts create mode 100644 src/client/extension-init.ts diff --git a/src/client/extension-activation.ts b/src/client/extension-activation.ts new file mode 100644 index 000000000000..16b3f15ddca6 --- /dev/null +++ b/src/client/extension-activation.ts @@ -0,0 +1,208 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:max-func-body-length + +import { CodeActionKind, debug, DebugConfigurationProvider, languages, OutputChannel, window } from 'vscode'; + +import { registerTypes as activationRegisterTypes } from './activation/serviceRegistry'; +import { IExtensionActivationManager, ILanguageServerExtension } from './activation/types'; +import { registerTypes as appRegisterTypes } from './application/serviceRegistry'; +import { IApplicationDiagnostics } from './application/types'; +import { DebugService } from './common/application/debugService'; +import { ICommandManager, IWorkspaceService } from './common/application/types'; +import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; +import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; +import { traceError } from './common/logger'; +import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; +import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; +import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; +import { + IConfigurationService, + IDisposableRegistry, + IExperimentsManager, + IExtensionContext, + IFeatureDeprecationManager, + IOutputChannel +} from './common/types'; +import { OutputChannelNames } from './common/utils/localize'; +import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; +import { JUPYTER_OUTPUT_CHANNEL } from './datascience/constants'; +import { registerTypes as dataScienceRegisterTypes } from './datascience/serviceRegistry'; +import { IDataScience } from './datascience/types'; +import { DebuggerTypeName } from './debugger/constants'; +import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher'; +import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types'; +import { registerTypes as debugConfigurationRegisterTypes } from './debugger/extension/serviceRegistry'; +import { IDebugConfigurationService, IDebuggerBanner } from './debugger/extension/types'; +import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry'; +import { IInterpreterSelector } from './interpreter/configuration/types'; +import { + IInterpreterLocatorProgressHandler, + IInterpreterLocatorProgressService, + IInterpreterService +} from './interpreter/contracts'; +import { registerTypes as interpretersRegisterTypes } from './interpreter/serviceRegistry'; +import { IServiceContainer, IServiceManager } from './ioc/types'; +import { getLanguageConfiguration } from './language/languageConfiguration'; +import { LinterCommands } from './linters/linterCommands'; +import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry'; +import { PythonCodeActionProvider } from './providers/codeActionProvider/pythonCodeActionProvider'; +import { PythonFormattingEditProvider } from './providers/formatProvider'; +import { ReplProvider } from './providers/replProvider'; +import { registerTypes as providersRegisterTypes } from './providers/serviceRegistry'; +import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; +import { TerminalProvider } from './providers/terminalProvider'; +import { ISortImportsEditingProvider } from './providers/types'; +import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; +import { ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; +import { TEST_OUTPUT_CHANNEL } from './testing/common/constants'; +import { ITestContextService } from './testing/common/types'; +import { ITestCodeNavigatorCommandHandler, ITestExplorerCommandHandler } from './testing/navigation/types'; +import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; + +export async function activateComponents( + context: IExtensionContext, + serviceManager: IServiceManager, + serviceContainer: IServiceContainer +) { + // We will be pulling code over from activateLegacy(). + + return activateLegacy(context, serviceManager, serviceContainer); +} + +///////////////////////////// +// old activation code + +// tslint:disable-next-line:no-suspicious-comment +// TODO(GH-10454): Gradually move simple initialization +// and DI registration currently in this function over +// to initializeComponents(). Likewise with complex +// init and activation: move them to activateComponents(). + +async function activateLegacy( + context: IExtensionContext, + serviceManager: IServiceManager, + serviceContainer: IServiceContainer +) { + // register "services" + + const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python()); + const unitTestOutChannel = window.createOutputChannel(OutputChannelNames.pythonTest()); + const jupyterOutputChannel = window.createOutputChannel(OutputChannelNames.jupyter()); + serviceManager.addSingletonInstance(IOutputChannel, standardOutputChannel, STANDARD_OUTPUT_CHANNEL); + serviceManager.addSingletonInstance(IOutputChannel, unitTestOutChannel, TEST_OUTPUT_CHANNEL); + serviceManager.addSingletonInstance(IOutputChannel, jupyterOutputChannel, JUPYTER_OUTPUT_CHANNEL); + + commonRegisterTypes(serviceManager); + processRegisterTypes(serviceManager); + variableRegisterTypes(serviceManager); + unitTestsRegisterTypes(serviceManager); + lintersRegisterTypes(serviceManager); + interpretersRegisterTypes(serviceManager); + formattersRegisterTypes(serviceManager); + platformRegisterTypes(serviceManager); + installerRegisterTypes(serviceManager); + commonRegisterTerminalTypes(serviceManager); + dataScienceRegisterTypes(serviceManager); + debugConfigurationRegisterTypes(serviceManager); + + const configuration = serviceManager.get(IConfigurationService); + const languageServerType = configuration.getSettings().languageServer; + + appRegisterTypes(serviceManager, languageServerType); + providersRegisterTypes(serviceManager); + activationRegisterTypes(serviceManager, languageServerType); + + // "initialize" "services" + + const abExperiments = serviceContainer.get(IExperimentsManager); + await abExperiments.activate(); + const selector = serviceContainer.get(IInterpreterSelector); + selector.initialize(); + context.subscriptions.push(selector); + + const interpreterManager = serviceContainer.get(IInterpreterService); + interpreterManager.initialize(); + + const handlers = serviceManager.getAll(IDebugSessionEventHandlers); + const disposables = serviceManager.get(IDisposableRegistry); + const dispatcher = new DebugSessionEventDispatcher(handlers, DebugService.instance, disposables); + dispatcher.registerEventHandlers(); + + const cmdManager = serviceContainer.get(ICommandManager); + const outputChannel = serviceManager.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + disposables.push(cmdManager.registerCommand(Commands.ViewOutput, () => outputChannel.show())); + + // Display progress of interpreter refreshes only after extension has activated. + serviceContainer.get(IInterpreterLocatorProgressHandler).register(); + serviceContainer.get(IInterpreterLocatorProgressService).register(); + serviceContainer.get(IApplicationDiagnostics).register(); + serviceContainer.get(ITestCodeNavigatorCommandHandler).register(); + serviceContainer.get(ITestExplorerCommandHandler).register(); + serviceContainer.get(ILanguageServerExtension).register(); + serviceContainer.get(ITestContextService).register(); + + // "activate" everything else + + const manager = serviceContainer.get(IExtensionActivationManager); + context.subscriptions.push(manager); + const activationPromise = manager.activate(); + + serviceManager.get(ITerminalAutoActivation).register(); + const pythonSettings = configuration.getSettings(); + + activateSimplePythonRefactorProvider(context, standardOutputChannel, serviceContainer); + + const sortImports = serviceContainer.get(ISortImportsEditingProvider); + sortImports.registerCommands(); + + serviceManager.get(ICodeExecutionManager).registerCommands(); + + // At this point we have enough information to send valid telemetry. + + const workspaceService = serviceContainer.get(IWorkspaceService); + interpreterManager + .refresh(workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders![0].uri : undefined) + .catch(ex => traceError('Python Extension: interpreterManager.refresh', ex)); + + // Activate data science features + const dataScience = serviceManager.get(IDataScience); + dataScience.activate().ignoreErrors(); + + context.subscriptions.push(new LinterCommands(serviceManager)); + + languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration()); + + if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') { + const formatProvider = new PythonFormattingEditProvider(context, serviceContainer); + context.subscriptions.push(languages.registerDocumentFormattingEditProvider(PYTHON, formatProvider)); + context.subscriptions.push(languages.registerDocumentRangeFormattingEditProvider(PYTHON, formatProvider)); + } + + const deprecationMgr = serviceContainer.get(IFeatureDeprecationManager); + deprecationMgr.initialize(); + context.subscriptions.push(deprecationMgr); + + context.subscriptions.push(new ReplProvider(serviceContainer)); + + const terminalProvider = new TerminalProvider(serviceContainer); + terminalProvider.initialize(window.activeTerminal).ignoreErrors(); + context.subscriptions.push(terminalProvider); + + context.subscriptions.push( + languages.registerCodeActionsProvider(PYTHON, new PythonCodeActionProvider(), { + providedCodeActionKinds: [CodeActionKind.SourceOrganizeImports] + }) + ); + + serviceContainer.getAll(IDebugConfigurationService).forEach(debugConfigProvider => { + context.subscriptions.push(debug.registerDebugConfigurationProvider(DebuggerTypeName, debugConfigProvider)); + }); + + serviceContainer.get(IDebuggerBanner).initialize(); + + return activationPromise; +} diff --git a/src/client/extension-init.ts b/src/client/extension-init.ts new file mode 100644 index 000000000000..27e155cd8d12 --- /dev/null +++ b/src/client/extension-init.ts @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:max-func-body-length + +import { Container } from 'inversify'; +import { Disposable, Memento } from 'vscode'; + +import { GLOBAL_MEMENTO, IDisposableRegistry, IExtensionContext, IMemento, WORKSPACE_MEMENTO } from './common/types'; +import { ServiceContainer } from './ioc/container'; +import { ServiceManager } from './ioc/serviceManager'; +import { IServiceContainer, IServiceManager } from './ioc/types'; + +// The code in this module should do nothing more complex than register +// objects to DI and simple init (e.g. no side effects). That implies +// that constructors are likewise simple and do no work. It also means +// that it is inherently synchronous. + +export function initializeGlobals(context: IExtensionContext): [IServiceManager, IServiceContainer] { + const cont = new Container(); + const serviceManager = new ServiceManager(cont); + const serviceContainer = new ServiceContainer(cont); + + serviceManager.addSingletonInstance(IServiceContainer, serviceContainer); + serviceManager.addSingletonInstance(IServiceManager, serviceManager); + + serviceManager.addSingletonInstance(IDisposableRegistry, context.subscriptions); + serviceManager.addSingletonInstance(IMemento, context.globalState, GLOBAL_MEMENTO); + serviceManager.addSingletonInstance(IMemento, context.workspaceState, WORKSPACE_MEMENTO); + serviceManager.addSingletonInstance(IExtensionContext, context); + + return [serviceManager, serviceContainer]; +} + +export function initializeComponents( + _context: IExtensionContext, + _serviceManager: IServiceManager, + _serviceContainer: IServiceContainer +) { + // We will be pulling code over from activateLegacy(). +} diff --git a/src/client/extension.ts b/src/client/extension.ts index 9ead95134cb5..7b466f0d1d50 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -1,4 +1,5 @@ 'use strict'; + // tslint:disable:no-var-requires no-require-imports // This line should always be right on top. @@ -26,95 +27,32 @@ const stopWatch = new StopWatch(); //=============================================== // loading starts here -import { Container } from 'inversify'; -import { - CodeActionKind, - debug, - DebugConfigurationProvider, - Disposable, - ExtensionContext, - languages, - Memento, - OutputChannel, - ProgressLocation, - ProgressOptions, - window -} from 'vscode'; +import { ProgressLocation, ProgressOptions, window } from 'vscode'; -import { registerTypes as activationRegisterTypes } from './activation/serviceRegistry'; -import { IExtensionActivationManager, ILanguageServerExtension } from './activation/types'; import { buildApi, IExtensionApi } from './api'; -import { registerTypes as appRegisterTypes } from './application/serviceRegistry'; -import { IApplicationDiagnostics } from './application/types'; -import { DebugService } from './common/application/debugService'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from './common/application/types'; -import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; -import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; +import { IApplicationShell } from './common/application/types'; import { traceError } from './common/logger'; -import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; -import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; -import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; -import { - GLOBAL_MEMENTO, - IAsyncDisposableRegistry, - IConfigurationService, - IDisposableRegistry, - IExperimentsManager, - IExtensionContext, - IFeatureDeprecationManager, - IMemento, - IOutputChannel, - WORKSPACE_MEMENTO -} from './common/types'; +import { IAsyncDisposableRegistry, IExtensionContext } from './common/types'; import { createDeferred } from './common/utils/async'; -import { Common, OutputChannelNames } from './common/utils/localize'; -import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; -import { JUPYTER_OUTPUT_CHANNEL } from './datascience/constants'; -import { registerTypes as dataScienceRegisterTypes } from './datascience/serviceRegistry'; -import { IDataScience } from './datascience/types'; -import { DebuggerTypeName } from './debugger/constants'; -import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher'; -import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types'; -import { registerTypes as debugConfigurationRegisterTypes } from './debugger/extension/serviceRegistry'; -import { IDebugConfigurationService, IDebuggerBanner } from './debugger/extension/types'; -import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry'; -import { IInterpreterSelector } from './interpreter/configuration/types'; -import { - IInterpreterLocatorProgressHandler, - IInterpreterLocatorProgressService, - IInterpreterService -} from './interpreter/contracts'; -import { registerTypes as interpretersRegisterTypes } from './interpreter/serviceRegistry'; -import { ServiceContainer } from './ioc/container'; -import { ServiceManager } from './ioc/serviceManager'; -import { IServiceContainer, IServiceManager } from './ioc/types'; -import { getLanguageConfiguration } from './language/languageConfiguration'; -import { LinterCommands } from './linters/linterCommands'; -import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry'; -import { PythonCodeActionProvider } from './providers/codeActionProvider/pythonCodeActionProvider'; -import { PythonFormattingEditProvider } from './providers/formatProvider'; -import { ReplProvider } from './providers/replProvider'; -import { registerTypes as providersRegisterTypes } from './providers/serviceRegistry'; -import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; -import { TerminalProvider } from './providers/terminalProvider'; -import { ISortImportsEditingProvider } from './providers/types'; +import { Common } from './common/utils/localize'; +import { activateComponents } from './extension-activation'; +import { initializeComponents, initializeGlobals } from './extension-init'; +import { IServiceContainer } from './ioc/types'; import { sendErrorTelemetry, sendStartupTelemetryInBackground } from './startupTelemetry'; -import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; -import { ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; -import { TEST_OUTPUT_CHANNEL } from './testing/common/constants'; -import { ITestContextService } from './testing/common/types'; -import { ITestCodeNavigatorCommandHandler, ITestExplorerCommandHandler } from './testing/navigation/types'; -import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; durations.codeLoadingTime = stopWatch.elapsedTime; //=============================================== // loading ends here +// These persist between activations: const activationDeferred = createDeferred(); -let activatedServiceContainer: ServiceContainer | undefined; +let activatedServiceContainer: IServiceContainer | undefined; + +///////////////////////////// +// public functions -export async function activate(context: ExtensionContext): Promise { +export async function activate(context: IExtensionContext): Promise { try { return await activateUnsafe(context); } catch (ex) { @@ -141,7 +79,7 @@ export function deactivate(): Thenable { // activation helpers // tslint:disable-next-line:max-func-body-length -async function activateUnsafe(context: ExtensionContext): Promise { +async function activateUnsafe(context: IExtensionContext): Promise { displayProgress(activationDeferred.promise); durations.startActivateTime = stopWatch.elapsedTime; @@ -149,6 +87,7 @@ async function activateUnsafe(context: ExtensionContext): Promise // activation starts here const [serviceManager, serviceContainer] = initializeGlobals(context); + activatedServiceContainer = serviceContainer; initializeComponents(context, serviceManager, serviceContainer); const activationPromise = activateComponents(context, serviceManager, serviceContainer); @@ -169,194 +108,6 @@ function displayProgress(promise: Promise) { window.withProgress(progressOptions, () => promise); } -///////////////////////////// -// intialization (globals & components) - -// The code in this section should do nothing more complex than register -// objects to DI and simple init (e.g. no side effects). That implies -// that constructors are likewise simple and do no work. It also means -// that it is inherently synchronous. - -function initializeGlobals(context: ExtensionContext): [IServiceManager, IServiceContainer] { - const cont = new Container(); - const serviceManager = new ServiceManager(cont); - const serviceContainer = new ServiceContainer(cont); - activatedServiceContainer = serviceContainer; - - serviceManager.addSingletonInstance(IServiceContainer, serviceContainer); - serviceManager.addSingletonInstance(IServiceManager, serviceManager); - - serviceManager.addSingletonInstance(IDisposableRegistry, context.subscriptions); - serviceManager.addSingletonInstance(IMemento, context.globalState, GLOBAL_MEMENTO); - serviceManager.addSingletonInstance(IMemento, context.workspaceState, WORKSPACE_MEMENTO); - serviceManager.addSingletonInstance(IExtensionContext, context); - - return [serviceManager, serviceContainer]; -} - -function initializeComponents( - _context: ExtensionContext, - _serviceManager: IServiceManager, - _serviceContainer: IServiceContainer -) { - // We will be pulling code over from activateLegacy(). -} - -///////////////////////////// -// component activation - -async function activateComponents( - context: ExtensionContext, - serviceManager: IServiceManager, - serviceContainer: IServiceContainer -) { - // We will be pulling code over from activateLegacy(). - - return activateLegacy(context, serviceManager, serviceContainer); -} - -///////////////////////////// -// old activation code - -// tslint:disable-next-line:no-suspicious-comment -// TODO(GH-10454): Gradually move simple initialization -// and DI registration currently in this function over -// to initializeComponents(). Likewise with complex -// init and activation: move them to activateComponents(). - -async function activateLegacy( - context: ExtensionContext, - serviceManager: IServiceManager, - serviceContainer: IServiceContainer -) { - registerServices(serviceManager); - await initializeServices(context, serviceManager, serviceContainer); - - const manager = serviceContainer.get(IExtensionActivationManager); - context.subscriptions.push(manager); - const activationPromise = manager.activate(); - - serviceManager.get(ITerminalAutoActivation).register(); - const configuration = serviceManager.get(IConfigurationService); - const pythonSettings = configuration.getSettings(); - - const standardOutputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - activateSimplePythonRefactorProvider(context, standardOutputChannel, serviceContainer); - - const sortImports = serviceContainer.get(ISortImportsEditingProvider); - sortImports.registerCommands(); - - serviceManager.get(ICodeExecutionManager).registerCommands(); - - // At this point we have enough information to send valid telemetry. - - const workspaceService = serviceContainer.get(IWorkspaceService); - const interpreterManager = serviceContainer.get(IInterpreterService); - interpreterManager - .refresh(workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders![0].uri : undefined) - .catch(ex => traceError('Python Extension: interpreterManager.refresh', ex)); - - // Activate data science features - const dataScience = serviceManager.get(IDataScience); - dataScience.activate().ignoreErrors(); - - context.subscriptions.push(new LinterCommands(serviceManager)); - - languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration()); - - if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') { - const formatProvider = new PythonFormattingEditProvider(context, serviceContainer); - context.subscriptions.push(languages.registerDocumentFormattingEditProvider(PYTHON, formatProvider)); - context.subscriptions.push(languages.registerDocumentRangeFormattingEditProvider(PYTHON, formatProvider)); - } - - const deprecationMgr = serviceContainer.get(IFeatureDeprecationManager); - deprecationMgr.initialize(); - context.subscriptions.push(deprecationMgr); - - context.subscriptions.push(new ReplProvider(serviceContainer)); - - const terminalProvider = new TerminalProvider(serviceContainer); - terminalProvider.initialize(window.activeTerminal).ignoreErrors(); - context.subscriptions.push(terminalProvider); - - context.subscriptions.push( - languages.registerCodeActionsProvider(PYTHON, new PythonCodeActionProvider(), { - providedCodeActionKinds: [CodeActionKind.SourceOrganizeImports] - }) - ); - - serviceContainer.getAll(IDebugConfigurationService).forEach(debugConfigProvider => { - context.subscriptions.push(debug.registerDebugConfigurationProvider(DebuggerTypeName, debugConfigProvider)); - }); - - serviceContainer.get(IDebuggerBanner).initialize(); - - return activationPromise; -} - -function registerServices(serviceManager: IServiceManager) { - const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python()); - const unitTestOutChannel = window.createOutputChannel(OutputChannelNames.pythonTest()); - const jupyterOutputChannel = window.createOutputChannel(OutputChannelNames.jupyter()); - serviceManager.addSingletonInstance(IOutputChannel, standardOutputChannel, STANDARD_OUTPUT_CHANNEL); - serviceManager.addSingletonInstance(IOutputChannel, unitTestOutChannel, TEST_OUTPUT_CHANNEL); - serviceManager.addSingletonInstance(IOutputChannel, jupyterOutputChannel, JUPYTER_OUTPUT_CHANNEL); - - commonRegisterTypes(serviceManager); - processRegisterTypes(serviceManager); - variableRegisterTypes(serviceManager); - unitTestsRegisterTypes(serviceManager); - lintersRegisterTypes(serviceManager); - interpretersRegisterTypes(serviceManager); - formattersRegisterTypes(serviceManager); - platformRegisterTypes(serviceManager); - installerRegisterTypes(serviceManager); - commonRegisterTerminalTypes(serviceManager); - dataScienceRegisterTypes(serviceManager); - debugConfigurationRegisterTypes(serviceManager); - - const configuration = serviceManager.get(IConfigurationService); - const languageServerType = configuration.getSettings().languageServer; - - appRegisterTypes(serviceManager, languageServerType); - providersRegisterTypes(serviceManager); - activationRegisterTypes(serviceManager, languageServerType); -} - -async function initializeServices( - context: ExtensionContext, - serviceManager: IServiceManager, - serviceContainer: IServiceContainer -) { - const abExperiments = serviceContainer.get(IExperimentsManager); - await abExperiments.activate(); - const selector = serviceContainer.get(IInterpreterSelector); - selector.initialize(); - context.subscriptions.push(selector); - - const interpreterManager = serviceContainer.get(IInterpreterService); - interpreterManager.initialize(); - - const handlers = serviceManager.getAll(IDebugSessionEventHandlers); - const disposables = serviceManager.get(IDisposableRegistry); - const dispatcher = new DebugSessionEventDispatcher(handlers, DebugService.instance, disposables); - dispatcher.registerEventHandlers(); - - const cmdManager = serviceContainer.get(ICommandManager); - const outputChannel = serviceManager.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - disposables.push(cmdManager.registerCommand(Commands.ViewOutput, () => outputChannel.show())); - - // Display progress of interpreter refreshes only after extension has activated. - serviceContainer.get(IInterpreterLocatorProgressHandler).register(); - serviceContainer.get(IInterpreterLocatorProgressService).register(); - serviceContainer.get(IApplicationDiagnostics).register(); - serviceContainer.get(ITestCodeNavigatorCommandHandler).register(); - serviceContainer.get(ITestExplorerCommandHandler).register(); - serviceContainer.get(ILanguageServerExtension).register(); - serviceContainer.get(ITestContextService).register(); -} - ///////////////////////////// // error handling From ef6d0b410288640ec0c33701f8e4b045f17e8f75 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 9 Mar 2020 13:13:14 -0600 Subject: [PATCH 12/18] Fix the extension API tests. --- ...on.unit.test.ts => api.functional.test.ts} | 75 ++----------------- src/test/extension-version.functional.test.ts | 72 ++++++++++++++++++ 2 files changed, 78 insertions(+), 69 deletions(-) rename src/test/{extension.unit.test.ts => api.functional.test.ts} (66%) create mode 100644 src/test/extension-version.functional.test.ts diff --git a/src/test/extension.unit.test.ts b/src/test/api.functional.test.ts similarity index 66% rename from src/test/extension.unit.test.ts rename to src/test/api.functional.test.ts index df3565de8408..7fd600caacc3 100644 --- a/src/test/extension.unit.test.ts +++ b/src/test/api.functional.test.ts @@ -3,16 +3,12 @@ 'use strict'; -// tslint:disable:no-any +// tslint:disable:no-any max-func-body-length import { expect } from 'chai'; -import * as fs from 'fs'; -import * as glob from 'glob'; import * as path from 'path'; import { anyString, anything, instance, mock, when } from 'ts-mockito'; import { buildApi } from '../client/api'; -import { ApplicationEnvironment } from '../client/common/application/applicationEnvironment'; -import { IApplicationEnvironment } from '../client/common/application/types'; import { EXTENSION_ROOT_DIR } from '../client/common/constants'; import { ExperimentsManager } from '../client/common/experiments'; import { IExperimentsManager } from '../client/common/types'; @@ -22,8 +18,7 @@ import { ServiceContainer } from '../client/ioc/container'; import { ServiceManager } from '../client/ioc/serviceManager'; import { IServiceContainer, IServiceManager } from '../client/ioc/types'; -// tslint:disable-next-line: max-func-body-length -suite('Extension API Debugger', () => { +suite('Extension API - Debugger', () => { const expectedLauncherPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`; const ptvsdPath = path.join('path', 'to', 'ptvsd'); const ptvsdHost = 'somehost'; @@ -40,12 +35,12 @@ suite('Extension API Debugger', () => { experimentsManager = mock(ExperimentsManager); debugAdapterFactory = mock(DebugAdapterDescriptorFactory); - when(serviceManager.get(IExperimentsManager)) + when(serviceContainer.get(IExperimentsManager)) // Return the mock. - .thenReturn(Promise.resolve(instance(experimentsManager))); - when(serviceManager.get(IDebugAdapterDescriptorFactory)) + .thenReturn(instance(experimentsManager)); + when(serviceContainer.get(IDebugAdapterDescriptorFactory)) // Return the mock. - .thenReturn(Promise.resolve(instance(debugAdapterFactory))); + .thenReturn(instance(debugAdapterFactory)); }); function mockInExperiment(host: string, port: number, wait: boolean) { @@ -157,61 +152,3 @@ suite('Extension API Debugger', () => { expect(args).to.be.deep.equal(expectedArgs); }); }); - -suite('Extension version tests', () => { - let version: string; - let applicationEnvironment: IApplicationEnvironment; - const branchName = process.env.CI_BRANCH_NAME; - - suiteSetup(async function() { - // Skip the entire suite if running locally - if (!branchName) { - // tslint:disable-next-line: no-invalid-this - return this.skip(); - } - }); - - setup(() => { - applicationEnvironment = new ApplicationEnvironment(undefined as any, undefined as any, undefined as any); - version = applicationEnvironment.packageJson.version; - }); - - test('If we are running a pipeline in the master branch, the extension version in `package.json` should have the "-dev" suffix', async function() { - if (branchName !== 'master') { - // tslint:disable-next-line: no-invalid-this - return this.skip(); - } - - return expect( - version.endsWith('-dev'), - 'When running a pipeline in the master branch, the extension version in package.json should have the -dev suffix' - ).to.be.true; - }); - - test('If we are running a pipeline in the release branch, the extension version in `package.json` should not have the "-dev" suffix', async function() { - if (!branchName!.startsWith('release')) { - // tslint:disable-next-line: no-invalid-this - return this.skip(); - } - - return expect( - version.endsWith('-dev'), - 'When running a pipeline in the release branch, the extension version in package.json should not have the -dev suffix' - ).to.be.false; - }); -}); - -suite('Extension localization files', () => { - test('Load localization file', () => { - const filesFailed: string[] = []; - glob.sync('package.nls.*.json', { sync: true, cwd: EXTENSION_ROOT_DIR }).forEach(localizationFile => { - try { - JSON.parse(fs.readFileSync(path.join(EXTENSION_ROOT_DIR, localizationFile)).toString()); - } catch { - filesFailed.push(localizationFile); - } - }); - - expect(filesFailed).to.be.lengthOf(0, `Failed to load JSON for ${filesFailed.join(', ')}`); - }); -}); diff --git a/src/test/extension-version.functional.test.ts b/src/test/extension-version.functional.test.ts new file mode 100644 index 000000000000..70adee3a9489 --- /dev/null +++ b/src/test/extension-version.functional.test.ts @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:no-any max-func-body-length + +import { expect } from 'chai'; +import * as fs from 'fs'; +import * as glob from 'glob'; +import * as path from 'path'; +import { ApplicationEnvironment } from '../client/common/application/applicationEnvironment'; +import { IApplicationEnvironment } from '../client/common/application/types'; +import { EXTENSION_ROOT_DIR } from '../client/common/constants'; + +suite('Extension version tests', () => { + let version: string; + let applicationEnvironment: IApplicationEnvironment; + const branchName = process.env.CI_BRANCH_NAME; + + suiteSetup(async function() { + // Skip the entire suite if running locally + if (!branchName) { + // tslint:disable-next-line: no-invalid-this + return this.skip(); + } + }); + + setup(() => { + applicationEnvironment = new ApplicationEnvironment(undefined as any, undefined as any, undefined as any); + version = applicationEnvironment.packageJson.version; + }); + + test('If we are running a pipeline in the master branch, the extension version in `package.json` should have the "-dev" suffix', async function() { + if (branchName !== 'master') { + // tslint:disable-next-line: no-invalid-this + return this.skip(); + } + + return expect( + version.endsWith('-dev'), + 'When running a pipeline in the master branch, the extension version in package.json should have the -dev suffix' + ).to.be.true; + }); + + test('If we are running a pipeline in the release branch, the extension version in `package.json` should not have the "-dev" suffix', async function() { + if (!branchName!.startsWith('release')) { + // tslint:disable-next-line: no-invalid-this + return this.skip(); + } + + return expect( + version.endsWith('-dev'), + 'When running a pipeline in the release branch, the extension version in package.json should not have the -dev suffix' + ).to.be.false; + }); +}); + +suite('Extension localization files', () => { + test('Load localization file', () => { + const filesFailed: string[] = []; + glob.sync('package.nls.*.json', { sync: true, cwd: EXTENSION_ROOT_DIR }).forEach(localizationFile => { + try { + JSON.parse(fs.readFileSync(path.join(EXTENSION_ROOT_DIR, localizationFile)).toString()); + } catch { + filesFailed.push(localizationFile); + } + }); + + expect(filesFailed).to.be.lengthOf(0, `Failed to load JSON for ${filesFailed.join(', ')}`); + }); +}); From b1f8eb9c1a68eb812373430578cb645946c53603 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 9 Mar 2020 13:17:32 -0600 Subject: [PATCH 13/18] Log an error instead of ignoring. --- src/client/extension.ts | 2 +- src/client/startupTelemetry.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 7b466f0d1d50..536f228a0232 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -133,6 +133,6 @@ function notifyUser(msg: string) { } appShell.showErrorMessage(msg).ignoreErrors(); } catch (ex) { - // ignore + traceError('failed to notify user', ex); } } diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts index ef32d6f994a6..abbe737c093d 100644 --- a/src/client/startupTelemetry.ts +++ b/src/client/startupTelemetry.ts @@ -64,7 +64,7 @@ export async function sendErrorTelemetry( try { props = await getActivationTelemetryProps(serviceContainer); } catch (ex) { - // ignore + traceError('getActivationTelemetryProps() failed.', ex); } } sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props, ex); From cb2f6190127f300f87d38347c5733588341cdde3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 9 Mar 2020 13:19:20 -0600 Subject: [PATCH 14/18] Drop a superfluous comment. --- src/client/extension-activation.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/client/extension-activation.ts b/src/client/extension-activation.ts index 16b3f15ddca6..4097a08127de 100644 --- a/src/client/extension-activation.ts +++ b/src/client/extension-activation.ts @@ -161,8 +161,6 @@ async function activateLegacy( serviceManager.get(ICodeExecutionManager).registerCommands(); - // At this point we have enough information to send valid telemetry. - const workspaceService = serviceContainer.get(IWorkspaceService); interpreterManager .refresh(workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders![0].uri : undefined) From 24ce4eaca15ea2f5e61c1080fe4272f21d3e6bf8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Mar 2020 08:04:47 -0600 Subject: [PATCH 15/18] Fix module names. --- src/client/extension.ts | 4 ++-- .../{extension-activation.ts => extensionActivation.ts} | 0 src/client/{extension-init.ts => extensionInit.ts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/client/{extension-activation.ts => extensionActivation.ts} (100%) rename src/client/{extension-init.ts => extensionInit.ts} (100%) diff --git a/src/client/extension.ts b/src/client/extension.ts index 536f228a0232..229a09a432a2 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -35,8 +35,8 @@ import { traceError } from './common/logger'; import { IAsyncDisposableRegistry, IExtensionContext } from './common/types'; import { createDeferred } from './common/utils/async'; import { Common } from './common/utils/localize'; -import { activateComponents } from './extension-activation'; -import { initializeComponents, initializeGlobals } from './extension-init'; +import { activateComponents } from './extensionActivation'; +import { initializeComponents, initializeGlobals } from './extensionInit'; import { IServiceContainer } from './ioc/types'; import { sendErrorTelemetry, sendStartupTelemetryInBackground } from './startupTelemetry'; diff --git a/src/client/extension-activation.ts b/src/client/extensionActivation.ts similarity index 100% rename from src/client/extension-activation.ts rename to src/client/extensionActivation.ts diff --git a/src/client/extension-init.ts b/src/client/extensionInit.ts similarity index 100% rename from src/client/extension-init.ts rename to src/client/extensionInit.ts From 3f5aad9e33bee2ff9908e5dc47f903a41e6ecc01 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Mar 2020 09:06:09 -0600 Subject: [PATCH 16/18] Eliminate sendStartupTelemetryInBackground(). --- src/client/extension.ts | 20 ++++++++++++++------ src/client/startupTelemetry.ts | 12 +----------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 229a09a432a2..7d887d55127a 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -38,7 +38,7 @@ import { Common } from './common/utils/localize'; import { activateComponents } from './extensionActivation'; import { initializeComponents, initializeGlobals } from './extensionInit'; import { IServiceContainer } from './ioc/types'; -import { sendErrorTelemetry, sendStartupTelemetryInBackground } from './startupTelemetry'; +import { sendErrorTelemetry, sendStartupTelemetry } from './startupTelemetry'; durations.codeLoadingTime = stopWatch.elapsedTime; @@ -53,14 +53,23 @@ let activatedServiceContainer: IServiceContainer | undefined; // public functions export async function activate(context: IExtensionContext): Promise { + let api: IExtensionApi; + let ready: Promise; + let serviceContainer: IServiceContainer; try { - return await activateUnsafe(context); + [api, ready, serviceContainer] = await activateUnsafe(context); } catch (ex) { // We want to completely handle the error // before notifying VS Code. await handleError(ex); throw ex; // re-raise } + // Send the "success" telemetry only if activation did not fail. + // Otherwise Telemetry is send via the error handler. + sendStartupTelemetry(ready, durations, stopWatch, serviceContainer) + // Run in the background. + .ignoreErrors(); + return api; } export function deactivate(): Thenable { @@ -79,7 +88,7 @@ export function deactivate(): Thenable { // activation helpers // tslint:disable-next-line:max-func-body-length -async function activateUnsafe(context: IExtensionContext): Promise { +async function activateUnsafe(context: IExtensionContext): Promise<[IExtensionApi, Promise, IServiceContainer]> { displayProgress(activationDeferred.promise); durations.startActivateTime = stopWatch.elapsedTime; @@ -97,9 +106,8 @@ async function activateUnsafe(context: IExtensionContext): Promise, - durations: Record, - stopWatch: IStopWatch, - serviceContainer: IServiceContainer -) { - sendStartupTelemetry(activatedPromise, durations, stopWatch, serviceContainer).ignoreErrors(); -} - -async function sendStartupTelemetry( +export async function sendStartupTelemetry( // tslint:disable-next-line:no-any activatedPromise: Promise, durations: Record, From e6f38355e45c6a9c686e6c3d45092ab1c8b22584 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Mar 2020 09:25:03 -0600 Subject: [PATCH 17/18] Pass the stopwatch and durations explicitly. --- src/client/extension.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 7d887d55127a..65eef98fd78a 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -57,11 +57,11 @@ export async function activate(context: IExtensionContext): Promise; let serviceContainer: IServiceContainer; try { - [api, ready, serviceContainer] = await activateUnsafe(context); + [api, ready, serviceContainer] = await activateUnsafe(context, stopWatch, durations); } catch (ex) { // We want to completely handle the error // before notifying VS Code. - await handleError(ex); + await handleError(ex, durations); throw ex; // re-raise } // Send the "success" telemetry only if activation did not fail. @@ -88,9 +88,13 @@ export function deactivate(): Thenable { // activation helpers // tslint:disable-next-line:max-func-body-length -async function activateUnsafe(context: IExtensionContext): Promise<[IExtensionApi, Promise, IServiceContainer]> { +async function activateUnsafe( + context: IExtensionContext, + startupStopWatch: StopWatch, + startupDurations: Record +): Promise<[IExtensionApi, Promise, IServiceContainer]> { displayProgress(activationDeferred.promise); - durations.startActivateTime = stopWatch.elapsedTime; + startupDurations.startActivateTime = startupStopWatch.elapsedTime; //=============================================== // activation starts here @@ -103,7 +107,7 @@ async function activateUnsafe(context: IExtensionContext): Promise<[IExtensionAp //=============================================== // activation ends here - durations.endActivateTime = stopWatch.elapsedTime; + startupDurations.endActivateTime = startupStopWatch.elapsedTime; activationDeferred.resolve(); const api = buildApi(activationPromise, serviceManager, serviceContainer); @@ -119,12 +123,12 @@ function displayProgress(promise: Promise) { ///////////////////////////// // error handling -async function handleError(ex: Error) { +async function handleError(ex: Error, startupDurations: Record) { notifyUser( "Extension activation failed, run the 'Developer: Toggle Developer Tools' command for more information." ); traceError('extension activation failed', ex); - await sendErrorTelemetry(ex, durations, activatedServiceContainer); + await sendErrorTelemetry(ex, startupDurations, activatedServiceContainer); } interface IAppShell { From ecb576697efd19d9c6df953107717fb125f238aa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Mar 2020 09:30:38 -0600 Subject: [PATCH 18/18] Make activationDeferred a local variable. --- src/client/extension.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 65eef98fd78a..adebd5e9ec1d 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -46,7 +46,6 @@ durations.codeLoadingTime = stopWatch.elapsedTime; // loading ends here // These persist between activations: -const activationDeferred = createDeferred(); let activatedServiceContainer: IServiceContainer | undefined; ///////////////////////////// @@ -93,6 +92,7 @@ async function activateUnsafe( startupStopWatch: StopWatch, startupDurations: Record ): Promise<[IExtensionApi, Promise, IServiceContainer]> { + const activationDeferred = createDeferred(); displayProgress(activationDeferred.promise); startupDurations.startActivateTime = startupStopWatch.elapsedTime;