From ea6d350dc96cf276e9019614cd4aae3602ade616 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 2 Mar 2021 14:39:22 -0800 Subject: [PATCH 1/4] Remove UseTerminalToGetActivatedEnvVars experiment --- experiments.json | 24 ---- package.json | 2 - src/client/common/experiments/groups.ts | 11 -- .../wrapperEnvironmentActivationService.ts | 42 +------ ...rEnvironmentActivationService.unit.test.ts | 117 +----------------- 5 files changed, 3 insertions(+), 193 deletions(-) diff --git a/experiments.json b/experiments.json index 04bafc9f7596..bf1e2570bf9f 100644 --- a/experiments.json +++ b/experiments.json @@ -1,16 +1,4 @@ [ - { - "name": "AlwaysDisplayTestExplorer - experiment", - "salt": "AlwaysDisplayTestExplorer", - "min": 80, - "max": 100 - }, - { - "name": "AlwaysDisplayTestExplorer - control", - "salt": "AlwaysDisplayTestExplorer", - "min": 0, - "max": 20 - }, { "name": "ShowPlayIcon - start", "salt": "ShowPlayIcon", @@ -65,18 +53,6 @@ "min": 0, "max": 0 }, - { - "name": "UseTerminalToGetActivatedEnvVars - experiment", - "salt": "UseTerminalToGetActivatedEnvVars", - "min": 0, - "max": 0 - }, - { - "name": "UseTerminalToGetActivatedEnvVars - control", - "salt": "UseTerminalToGetActivatedEnvVars", - "min": 0, - "max": 100 - }, { "name": "AA_testing - control", "salt": "AA_testing", diff --git a/package.json b/package.json index 15352bfacd6c..37e71799fba0 100644 --- a/package.json +++ b/package.json @@ -1042,7 +1042,6 @@ "LocalZMQKernel - experiment", "NativeNotebook - experiment", "CustomEditorSupport - experiment", - "UseTerminalToGetActivatedEnvVars - experiment", "CollectLSRequestTiming - experiment", "CollectNodeLSRequestTiming - experiment", "DeprecatePythonPath - experiment", @@ -1071,7 +1070,6 @@ "LocalZMQKernel - experiment", "NativeNotebook - experiment", "CustomEditorSupport - experiment", - "UseTerminalToGetActivatedEnvVars - experiment", "CollectLSRequestTiming - experiment", "CollectNodeLSRequestTiming - experiment", "DeprecatePythonPath - experiment", diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 8ad5257c4396..43b471427699 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -16,17 +16,6 @@ export enum WebAppReload { experiment = 'Reload - experiment', } -/** - * Experiment to check whether to to use a terminal to generate the environment variables of activated environments. - * - * @export - * @enum {number} - */ -export enum UseTerminalToGetActivatedEnvVars { - control = 'UseTerminalToGetActivatedEnvVars - control', - experiment = 'UseTerminalToGetActivatedEnvVars - experiment', -} - // Dummy experiment added to validate metrics of A/B testing export enum ValidateABTesting { control = 'AA_testing - control', diff --git a/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts b/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts index ccb2cada81af..583b995d789d 100644 --- a/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts +++ b/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts @@ -5,17 +5,10 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { UseTerminalToGetActivatedEnvVars } from '../../common/experiments/groups'; import '../../common/extensions'; import { traceError } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { - ICryptoUtils, - IDisposableRegistry, - IExperimentsManager, - IExtensionContext, - Resource, -} from '../../common/types'; +import { ICryptoUtils, IDisposableRegistry, IExtensionContext, Resource } from '../../common/types'; import { createDeferredFromPromise } from '../../common/utils/async'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -23,7 +16,6 @@ import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { IInterpreterService } from '../contracts'; import { EnvironmentActivationService } from './service'; -import { TerminalEnvironmentActivationService } from './terminalEnvironmentActivationService'; import { IEnvironmentActivationService } from './types'; // We have code in terminal activation that waits for a min of 500ms. @@ -39,9 +31,6 @@ export class WrapperEnvironmentActivationService implements IEnvironmentActivati private readonly cachePerResourceAndInterpreter = new Map>(); constructor( @inject(EnvironmentActivationService) private readonly procActivation: IEnvironmentActivationService, - @inject(TerminalEnvironmentActivationService) - private readonly terminalActivation: IEnvironmentActivationService, - @inject(IExperimentsManager) private readonly experiment: IExperimentsManager, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IEnvironmentVariablesProvider) private readonly envVarsProvider: IEnvironmentVariablesProvider, @inject(IExtensionContext) private readonly context: IExtensionContext, @@ -76,9 +65,7 @@ export class WrapperEnvironmentActivationService implements IEnvironmentActivati const procEnvVarsPromise = this.cacheCallback(procCacheKey, () => this.getActivatedEnvVarsFromProc(resource, interpreter, allowExceptions), ); - const terminalEnvVarsPromise = this.cacheCallback(terminalCacheKey, () => - this.getActivatedEnvVarsFromTerminal(procEnvVarsPromise, resource, interpreter, allowExceptions), - ); + const terminalEnvVarsPromise = this.cacheCallback(terminalCacheKey, () => procEnvVarsPromise); const procEnvVars = createDeferredFromPromise(procEnvVarsPromise); const terminalEnvVars = createDeferredFromPromise(terminalEnvVarsPromise); @@ -164,32 +151,7 @@ export class WrapperEnvironmentActivationService implements IEnvironmentActivati ): Promise { return this.procActivation.getActivatedEnvironmentVariables(resource, interpreter, allowExceptions); } - /** - * Get environment variables by activating a terminal. - * As a fallback use the `fallback` promise passed in (old approach). - */ - private async getActivatedEnvVarsFromTerminal( - fallback: Promise, - resource: Resource, - interpreter?: PythonEnvironment, - allowExceptions?: boolean, - ): Promise { - if (!this.experiment.inExperiment(UseTerminalToGetActivatedEnvVars.experiment)) { - return fallback; - } - return this.terminalActivation - .getActivatedEnvironmentVariables(resource, interpreter, allowExceptions) - .then((vars) => { - // If no variables in terminal, then revert to old approach. - return vars || fallback; - }) - .catch((ex) => { - // Swallow exceptions when using terminal env and revert to using old approach. - traceError('Failed to get variables using Terminal Service', ex); - return fallback; - }); - } /** * Computes a key used to cache environment variables. * 1. If resource changes, then environment variables could be different, as user could have `.env` files or similar. diff --git a/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts b/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts index 8b57d3eb70a0..15c08d30232f 100644 --- a/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts +++ b/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts @@ -8,15 +8,13 @@ import { EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; import { CryptoUtils } from '../../../client/common/crypto'; -import { ExperimentsManager } from '../../../client/common/experiments/manager'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { ICryptoUtils, IExperimentsManager, IExtensionContext, Resource } from '../../../client/common/types'; +import { ICryptoUtils, IExtensionContext, Resource } from '../../../client/common/types'; import { Architecture } from '../../../client/common/utils/platform'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; -import { TerminalEnvironmentActivationService } from '../../../client/interpreter/activation/terminalEnvironmentActivationService'; import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { WrapperEnvironmentActivationService } from '../../../client/interpreter/activation/wrapperEnvironmentActivationService'; import { IInterpreterService } from '../../../client/interpreter/contracts'; @@ -26,8 +24,6 @@ import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnviro suite('Interpreters Activation - Python Environment Variables (wrap terminal and proc approach)', () => { let envActivationService: IEnvironmentActivationService; let procActivation: IEnvironmentActivationService; - let termActivation: IEnvironmentActivationService; - let experiment: IExperimentsManager; let interpreterService: IInterpreterService; let workspace: IWorkspaceService; let envVarsProvider: IEnvironmentVariablesProvider; @@ -51,8 +47,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and onDidChangeEnvVars = new EventEmitter(); envVarsProvider = mock(EnvironmentVariablesProvider); procActivation = mock(EnvironmentActivationService); - termActivation = mock(TerminalEnvironmentActivationService); - experiment = mock(ExperimentsManager); interpreterService = mock(InterpreterService); workspace = mock(WorkspaceService); crypto = mock(CryptoUtils); @@ -63,7 +57,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and }, } as any; when(crypto.createHash(anything(), anything(), anything())).thenCall((value) => value); - when(experiment.inExperiment(anything())).thenReturn(true); when(envVarsProvider.getCustomEnvironmentVariables(anything())).thenCall((value) => Promise.resolve({ key: (value || {}).toString(), @@ -77,8 +70,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); envActivationService = new WrapperEnvironmentActivationService( instance(procActivation), - instance(termActivation), - instance(experiment), instance(interpreterService), instance(envVarsProvider), extContext, @@ -91,9 +82,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and suite(interpreter ? 'With an interpreter' : 'Without an interpreter', () => { test('Environment variables returned by process provider should be used if terminal provider crashes', async () => { const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenReject(new Error('kaboom')); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve(expectedVars); @@ -104,18 +92,12 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); }); test('Use cached variables returned by process provider should be used if terminal provider crashes', async () => { const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenReject(new Error('kaboom')); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve(expectedVars); @@ -126,9 +108,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); @@ -138,18 +117,12 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and interpreter, ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); }); test('Environment variables returned by terminal provider should be used if that returns any variables', async () => { const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve({ somethingElse: '1' }); @@ -160,18 +133,12 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); }); test('Environment variables returned by terminal provider should be used if that returns any variables', async () => { const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve({ somethingElse: '1' }); @@ -182,9 +149,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); @@ -194,18 +158,12 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and interpreter, ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); }); test('Will not use cached info, if passing different resource or interpreter', async () => { const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve({ somethingElse: '1' }); @@ -216,9 +174,6 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); @@ -229,85 +184,15 @@ suite('Interpreters Activation - Python Environment Variables (wrap terminal and interpreter, ); assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); verify( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).once(); - - // Invoke again with a different resource. - const newResource = Uri.file('New Resource'); - when( - termActivation.getActivatedEnvironmentVariables( - newResource, - anything(), - anything(), - ), - ).thenResolve(undefined); - when( - procActivation.getActivatedEnvironmentVariables( - newResource, - anything(), - anything(), - ), - ).thenResolve({ NewVars: '1' }); - - vars = await envActivationService.getActivatedEnvironmentVariables( - newResource, - undefined, - ); - assert.deepEqual(vars, { NewVars: '1' }); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).twice(); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).twice(); - - // Invoke again with a different python interpreter. - const newInterpreter: PythonEnvironment = { - architecture: Architecture.x64, - path: 'New', - sysPrefix: '', - sysVersion: '', - envType: EnvironmentType.Pipenv, - }; - when( - termActivation.getActivatedEnvironmentVariables( - anything(), - newInterpreter, - anything(), - ), - ).thenResolve({ NewPythonVars: '1' }); - when( - procActivation.getActivatedEnvironmentVariables( - anything(), - newInterpreter, - anything(), - ), - ).thenResolve(undefined); - - vars = await envActivationService.getActivatedEnvironmentVariables( - newResource, - newInterpreter, - ); - assert.deepEqual(vars, { NewPythonVars: '1' }); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thrice(); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thrice(); }); test('Use variables from file cache', async function () { if (!storagePath) { return this.skip(); } const expectedVars = { WOW: '1' }; - when( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(undefined); when( procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), ).thenResolve(undefined); From 815abad15cc97f32d611a23c78fb640e153f41af Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 2 Mar 2021 15:16:32 -0800 Subject: [PATCH 2/4] Remove UseTerminalToGetActivatedEnvVars experiment --- .eslintignore | 4 - .../activation/preWarmVariables.ts | 24 ---- .../terminalEnvironmentActivationService.ts | 76 ----------- src/client/interpreter/serviceRegistry.ts | 11 -- src/client/telemetry/index.ts | 13 -- .../envVarsProvider.multiroot.test.ts | 4 - .../activation/preWarmVariables.unit.test.ts | 48 ------- ...lEnvironmentActivationService.unit.test.ts | 124 ------------------ .../interpreters/serviceRegistry.unit.test.ts | 2 - src/test/serviceRegistry.ts | 8 -- src/test/testing/unittest/unittest.test.ts | 4 - 11 files changed, 318 deletions(-) delete mode 100644 src/client/interpreter/activation/preWarmVariables.ts delete mode 100644 src/client/interpreter/activation/terminalEnvironmentActivationService.ts delete mode 100644 src/test/interpreters/activation/preWarmVariables.unit.test.ts delete mode 100644 src/test/interpreters/activation/terminalEnvironmentActivationService.unit.test.ts diff --git a/.eslintignore b/.eslintignore index 84d6372abc44..4279c36e7463 100644 --- a/.eslintignore +++ b/.eslintignore @@ -52,7 +52,6 @@ src/test/interpreters/autoSelection/rules/system.unit.test.ts src/test/interpreters/virtualEnvManager.unit.test.ts src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts src/test/interpreters/activation/service.unit.test.ts -src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts src/test/interpreters/helpers.unit.test.ts src/test/interpreters/currentPathService.unit.test.ts src/test/interpreters/display/interpreterSelectionTip.unit.test.ts @@ -313,9 +312,6 @@ src/client/interpreter/autoSelection/rules/settings.ts src/client/interpreter/autoSelection/rules/currentPath.ts src/client/interpreter/autoSelection/rules/cached.ts src/client/interpreter/autoSelection/rules/system.ts -src/client/interpreter/activation/wrapperEnvironmentActivationService.ts -src/client/interpreter/activation/terminalEnvironmentActivationService.ts -src/client/interpreter/activation/preWarmVariables.ts src/client/interpreter/activation/service.ts src/client/interpreter/display/shebangCodeLensProvider.ts src/client/interpreter/display/index.ts diff --git a/src/client/interpreter/activation/preWarmVariables.ts b/src/client/interpreter/activation/preWarmVariables.ts deleted file mode 100644 index 301404716e65..000000000000 --- a/src/client/interpreter/activation/preWarmVariables.ts +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import { IExtensionSingleActivationService } from '../../activation/types'; -import '../../common/extensions'; -import { IInterpreterService } from '../contracts'; -import { IEnvironmentActivationService } from './types'; - -@injectable() -export class PreWarmActivatedEnvironmentVariables implements IExtensionSingleActivationService { - constructor( - @inject(IEnvironmentActivationService) private readonly activationService: IEnvironmentActivationService, - @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - ) {} - public async activate(): Promise { - this.interpreterService.onDidChangeInterpreter(() => - this.activationService.getActivatedEnvironmentVariables(undefined).ignoreErrors(), - ); - this.activationService.getActivatedEnvironmentVariables(undefined).ignoreErrors(); - } -} diff --git a/src/client/interpreter/activation/terminalEnvironmentActivationService.ts b/src/client/interpreter/activation/terminalEnvironmentActivationService.ts deleted file mode 100644 index 8b6e037b7203..000000000000 --- a/src/client/interpreter/activation/terminalEnvironmentActivationService.ts +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import { CancellationTokenSource } from 'vscode'; -import { LogOptions, traceDecorators } from '../../common/logger'; -import { IFileSystem } from '../../common/platform/types'; -import * as internalScripts from '../../common/process/internal/scripts'; -import { ITerminalServiceFactory } from '../../common/terminal/types'; -import { Resource } from '../../common/types'; -import { IEnvironmentVariablesProvider } from '../../common/variables/types'; -import { PythonEnvironment } from '../../pythonEnvironments/info'; -import { captureTelemetry } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; -import { IEnvironmentActivationService } from './types'; - -/** - * This class will provide the environment variables of an interpreter by activating it in a terminal. - * This has the following benefit: - * - Using a shell that's configured by the user (using their default shell). - * - Environment variables are dumped into a file instead of reading from stdout. - * - * @export - * @class TerminalEnvironmentActivationService - * @implements {IEnvironmentActivationService} - * @implements {IDisposable} - */ -@injectable() -export class TerminalEnvironmentActivationService implements IEnvironmentActivationService { - constructor( - @inject(ITerminalServiceFactory) private readonly terminalFactory: ITerminalServiceFactory, - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(IEnvironmentVariablesProvider) private readonly envVarsProvider: IEnvironmentVariablesProvider, - ) {} - @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments) - @captureTelemetry( - EventName.PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, - { failed: false, activatedInTerminal: true }, - true, - ) - public async getActivatedEnvironmentVariables( - resource: Resource, - interpreter?: PythonEnvironment | undefined, - _allowExceptions?: boolean | undefined, - ): Promise { - const env = (await this.envVarsProvider.getCustomEnvironmentVariables(resource)) as - | { [key: string]: string | null } - | undefined; - const terminal = this.terminalFactory.getTerminalService({ - env, - hideFromUser: true, - interpreter, - resource, - title: `${interpreter?.displayName}${new Date().getTime()}`, - }); - - const command = interpreter?.path || 'python'; - const jsonFile = await this.fs.createTemporaryFile('.json'); - try { - const [args, parse] = internalScripts.printEnvVariablesToFile(jsonFile.filePath); - - // Pass a cancellation token to ensure we wait until command has completed. - // If there are any errors in executing in the terminal, throw them so they get logged and bubbled up. - await terminal.sendCommand(command, args, new CancellationTokenSource().token, false); - - const contents = await this.fs.readFile(jsonFile.filePath); - return parse(contents); - } finally { - // We created a hidden terminal for temp usage, hence dispose when done. - terminal.dispose(); - jsonFile.dispose(); - } - } -} diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 1e3088e59027..86150e343fb3 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -5,9 +5,7 @@ import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types'; import { IServiceManager } from '../ioc/types'; -import { PreWarmActivatedEnvironmentVariables } from './activation/preWarmVariables'; import { EnvironmentActivationService } from './activation/service'; -import { TerminalEnvironmentActivationService } from './activation/terminalEnvironmentActivationService'; import { IEnvironmentActivationService } from './activation/types'; import { InterpreterAutoSelectionService } from './autoSelection/index'; import { InterpreterEvaluation } from './autoSelection/interpreterSecurity/interpreterEvaluation'; @@ -154,11 +152,6 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void ); serviceManager.addSingleton(IExtensionActivationService, CondaInheritEnvPrompt); - - serviceManager.addSingleton( - IExtensionSingleActivationService, - PreWarmActivatedEnvironmentVariables, - ); } export function registerTypes(serviceManager: IServiceManager): void { @@ -171,10 +164,6 @@ export function registerTypes(serviceManager: IServiceManager): void { EnvironmentActivationService, EnvironmentActivationService, ); - serviceManager.addSingleton( - TerminalEnvironmentActivationService, - TerminalEnvironmentActivationService, - ); serviceManager.addSingleton( IEnvironmentActivationService, EnvironmentActivationService, diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 2e6273684d30..9f0208594f95 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -984,19 +984,6 @@ export interface IEventNamePropertyMapping { * @type {boolean} */ failed?: boolean; - /** - * Whether the environment was activated within a terminal or not. - * - * @type {boolean} - */ - activatedInTerminal?: boolean; - /** - * Whether the environment was activated by the wrapper class. - * If `true`, this telemetry is sent by the class that wraps the two activation providers . - * - * @type {boolean} - */ - activatedByWrapper?: boolean; }; /** * Telemetry event sent when getting activation commands for active interpreter diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 63a3dd8e411d..ccdca42c54a0 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -49,10 +49,6 @@ suite('Multiroot Environment Variables Provider', () => { await ioc.registerMockInterpreterTypes(); const mockEnvironmentActivationService = mock(EnvironmentActivationService); when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything())).thenResolve(); - when( - mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(); ioc.serviceManager.rebindInstance( IEnvironmentActivationService, instance(mockEnvironmentActivationService), diff --git a/src/test/interpreters/activation/preWarmVariables.unit.test.ts b/src/test/interpreters/activation/preWarmVariables.unit.test.ts deleted file mode 100644 index 824977e0fe88..000000000000 --- a/src/test/interpreters/activation/preWarmVariables.unit.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { EventEmitter } from 'vscode'; -import { IExtensionSingleActivationService } from '../../../client/activation/types'; -import { PreWarmActivatedEnvironmentVariables } from '../../../client/interpreter/activation/preWarmVariables'; -import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; -import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; -import { IInterpreterService } from '../../../client/interpreter/contracts'; -import { InterpreterService } from '../../../client/interpreter/interpreterService'; - -suite('Interpreters Activation - Env Variables', () => { - let activationService: IExtensionSingleActivationService; - let envActivationService: IEnvironmentActivationService; - let interpreterService: IInterpreterService; - let onDidChangeInterpreter: EventEmitter; - setup(() => { - onDidChangeInterpreter = new EventEmitter(); - envActivationService = mock(EnvironmentActivationService); - interpreterService = mock(InterpreterService); - when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); - activationService = new PreWarmActivatedEnvironmentVariables( - instance(envActivationService), - instance(interpreterService), - ); - }); - test('Should pre-warm env variables', async () => { - when(envActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - - await activationService.activate(); - - verify(envActivationService.getActivatedEnvironmentVariables(undefined)).once(); - }); - test('Should pre-warm env variables when interpreter changes', async () => { - when(envActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - - await activationService.activate(); - - verify(envActivationService.getActivatedEnvironmentVariables(undefined)).once(); - - onDidChangeInterpreter.fire(); - - verify(envActivationService.getActivatedEnvironmentVariables(undefined)).twice(); - }); -}); diff --git a/src/test/interpreters/activation/terminalEnvironmentActivationService.unit.test.ts b/src/test/interpreters/activation/terminalEnvironmentActivationService.unit.test.ts deleted file mode 100644 index 441092d11cc8..000000000000 --- a/src/test/interpreters/activation/terminalEnvironmentActivationService.unit.test.ts +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { assert } from 'chai'; -import * as path from 'path'; -import { anything, capture, deepEqual, instance, mock, verify, when } from 'ts-mockito'; -import { Uri } from 'vscode'; -import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; -import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { IFileSystem } from '../../../client/common/platform/types'; -import { TerminalServiceFactory } from '../../../client/common/terminal/factory'; -import { TerminalService } from '../../../client/common/terminal/service'; -import { ITerminalService, ITerminalServiceFactory } from '../../../client/common/terminal/types'; -import { Architecture } from '../../../client/common/utils/platform'; -import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; -import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; -import { TerminalEnvironmentActivationService } from '../../../client/interpreter/activation/terminalEnvironmentActivationService'; -import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; -import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info'; -import { noop } from '../../core'; - -suite('Interpreters Activation - Python Environment Variables (using terminals)', () => { - let envActivationService: IEnvironmentActivationService; - let terminalFactory: ITerminalServiceFactory; - let fs: IFileSystem; - let envVarsProvider: IEnvironmentVariablesProvider; - const jsonFile = path.join('hello', 'output.json'); - let terminal: ITerminalService; - const mockInterpreter: PythonEnvironment = { - architecture: Architecture.Unknown, - path: '', - sysPrefix: '', - sysVersion: '', - envType: EnvironmentType.Conda, - }; - setup(() => { - terminalFactory = mock(TerminalServiceFactory); - terminal = mock(TerminalService); - fs = mock(FileSystem); - envVarsProvider = mock(EnvironmentVariablesProvider); - - when(terminalFactory.getTerminalService(anything())).thenReturn(instance(terminal)); - when(fs.createTemporaryFile(anything())).thenResolve({ dispose: noop, filePath: jsonFile }); - when(terminal.sendCommand(anything(), anything(), anything(), anything())).thenResolve(); - envActivationService = new TerminalEnvironmentActivationService( - instance(terminalFactory), - instance(fs), - instance(envVarsProvider), - ); - }); - - [undefined, Uri.file('some Resource')].forEach((resource) => { - [undefined, mockInterpreter].forEach((interpreter) => { - suite(resource ? 'With a resource' : 'Without a resource', () => { - suite(interpreter ? 'With an interpreter' : 'Without an interpreter', () => { - test('Should create a terminal with user defined custom env vars', async () => { - const customEnv = { HELLO: '1' }; - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(customEnv); - - await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - const termArgs = capture(terminalFactory.getTerminalService).first()[0]; - assert.equal(termArgs?.env, customEnv); - }); - test('Should destroy the created terminal', async () => { - const customEnv = { HELLO: '1' }; - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(customEnv); - - await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - verify(terminal.dispose()).once(); - }); - test('Should create a terminal with correct arguments', async () => { - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(undefined); - - await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - const termArgs = capture(terminalFactory.getTerminalService).first()[0]; - assert.isUndefined(termArgs?.env); - assert.equal(termArgs?.resource, resource); - assert.deepEqual(termArgs?.interpreter, interpreter); - assert.isTrue(termArgs?.hideFromUser); - }); - test('Should create a terminal with correct arguments', async () => { - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(undefined); - - await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - const termArgs = capture(terminalFactory.getTerminalService).first()[0]; - assert.isUndefined(termArgs?.env); - assert.equal(termArgs?.resource, resource); - assert.deepEqual(termArgs?.interpreter, interpreter); - assert.isTrue(termArgs?.hideFromUser); - }); - test('Should execute python file in terminal (that is what dumps variables into json)', async () => { - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(undefined); - const isolated = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'); - const pyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariablesToFile.py'); - - await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - const cmd = interpreter?.path || 'python'; - verify( - terminal.sendCommand( - cmd, - deepEqual([isolated, pyFile, jsonFile.fileToCommandArgument()]), - anything(), - false, - ), - ).once(); - }); - test('Should return activated environment variables', async () => { - when(envVarsProvider.getCustomEnvironmentVariables(resource)).thenResolve(undefined); - when(fs.readFile(jsonFile)).thenResolve(JSON.stringify({ WOW: '1' })); - - const vars = await envActivationService.getActivatedEnvironmentVariables(resource, interpreter); - - assert.deepEqual(vars, { WOW: '1' }); - }); - }); - }); - }); - }); -}); diff --git a/src/test/interpreters/serviceRegistry.unit.test.ts b/src/test/interpreters/serviceRegistry.unit.test.ts index fccdb537428a..093ded571443 100644 --- a/src/test/interpreters/serviceRegistry.unit.test.ts +++ b/src/test/interpreters/serviceRegistry.unit.test.ts @@ -6,7 +6,6 @@ import { instance, mock, verify } from 'ts-mockito'; import { IExtensionActivationService, IExtensionSingleActivationService } from '../../client/activation/types'; import { EnvironmentActivationService } from '../../client/interpreter/activation/service'; -import { TerminalEnvironmentActivationService } from '../../client/interpreter/activation/terminalEnvironmentActivationService'; import { IEnvironmentActivationService } from '../../client/interpreter/activation/types'; import { InterpreterAutoSelectionService } from '../../client/interpreter/autoSelection'; import { InterpreterEvaluation } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation'; @@ -110,7 +109,6 @@ suite('Interpreters - Service Registry', () => { [IInterpreterAutoSelectionService, InterpreterAutoSelectionService], [EnvironmentActivationService, EnvironmentActivationService], - [TerminalEnvironmentActivationService, TerminalEnvironmentActivationService], [IEnvironmentActivationService, EnvironmentActivationService], [IExtensionActivationService, CondaInheritEnvPrompt], ].forEach((mapping) => { diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 9ced7f9532e4..d97670782d96 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -137,10 +137,6 @@ export class IocContainer { processRegisterTypes(this.serviceManager); const mockEnvironmentActivationService = mock(EnvironmentActivationService); when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything())).thenResolve(); - when( - mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(); this.serviceManager.addSingletonInstance( IEnvironmentActivationService, instance(mockEnvironmentActivationService), @@ -194,10 +190,6 @@ export class IocContainer { ); const mockEnvironmentActivationService = mock(EnvironmentActivationService); when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything())).thenResolve(); - when( - mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(); this.serviceManager.rebindInstance( IEnvironmentActivationService, instance(mockEnvironmentActivationService), diff --git a/src/test/testing/unittest/unittest.test.ts b/src/test/testing/unittest/unittest.test.ts index ff275f534ef9..ab6c853f9303 100644 --- a/src/test/testing/unittest/unittest.test.ts +++ b/src/test/testing/unittest/unittest.test.ts @@ -57,10 +57,6 @@ suite('Unit Tests - unittest - discovery against actual python process', () => { ioc.serviceManager.rebindInstance(IInterpreterService, instance(mock(InterpreterService))); const mockEnvironmentActivationService = mock(EnvironmentActivationService); when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); - when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything())).thenResolve(); - when( - mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(); ioc.serviceManager.rebindInstance( IEnvironmentActivationService, instance(mockEnvironmentActivationService), From 33b7e07932c61a6ed77de486776f5ebe2cfee0af Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Mar 2021 11:50:54 -0800 Subject: [PATCH 3/4] Why did this file not get deleted --- .../wrapperEnvironmentActivationService.ts | 176 -------------- ...rEnvironmentActivationService.unit.test.ts | 214 ------------------ 2 files changed, 390 deletions(-) delete mode 100644 src/client/interpreter/activation/wrapperEnvironmentActivationService.ts delete mode 100644 src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts diff --git a/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts b/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts deleted file mode 100644 index 583b995d789d..000000000000 --- a/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts +++ /dev/null @@ -1,176 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import '../../common/extensions'; -import { traceError } from '../../common/logger'; -import { IFileSystem } from '../../common/platform/types'; -import { ICryptoUtils, IDisposableRegistry, IExtensionContext, Resource } from '../../common/types'; -import { createDeferredFromPromise } from '../../common/utils/async'; -import { IEnvironmentVariablesProvider } from '../../common/variables/types'; -import { PythonEnvironment } from '../../pythonEnvironments/info'; -import { captureTelemetry } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; -import { IInterpreterService } from '../contracts'; -import { EnvironmentActivationService } from './service'; -import { IEnvironmentActivationService } from './types'; - -// We have code in terminal activation that waits for a min of 500ms. -// Observed on a Mac that it can take up to 2.5s (the delay is caused by initialization scripts running in the shell). -// On some other machines (windows) with Conda, this could take up to 40s. -// To get around this we will: -// 1. Load the variables when extension loads -// 2. Cache variables in a file (so its available when VSC re-loads). - -type EnvVariablesInCachedFile = { env?: NodeJS.ProcessEnv }; -@injectable() -export class WrapperEnvironmentActivationService implements IEnvironmentActivationService { - private readonly cachePerResourceAndInterpreter = new Map>(); - constructor( - @inject(EnvironmentActivationService) private readonly procActivation: IEnvironmentActivationService, - @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - @inject(IEnvironmentVariablesProvider) private readonly envVarsProvider: IEnvironmentVariablesProvider, - @inject(IExtensionContext) private readonly context: IExtensionContext, - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(ICryptoUtils) private readonly crypto: ICryptoUtils, - - @inject(IDisposableRegistry) disposables: IDisposableRegistry, - ) { - // Environment variables rely on custom variables defined by the user in `.env` files. - disposables.push( - envVarsProvider.onDidEnvironmentVariablesChange(() => this.cachePerResourceAndInterpreter.clear()), - ); - } - @captureTelemetry( - EventName.PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, - { failed: false, activatedByWrapper: true }, - true, - ) - public async getActivatedEnvironmentVariables( - resource: Resource, - interpreter?: PythonEnvironment | undefined, - allowExceptions?: boolean | undefined, - ): Promise { - let key: string; - [key, interpreter] = await Promise.all([ - this.getCacheKey(resource, interpreter), - interpreter || (await this.interpreterService.getActiveInterpreter(undefined)), - ]); - - const procCacheKey = `Process${key}`; - const terminalCacheKey = `Terminal${key}`; - const procEnvVarsPromise = this.cacheCallback(procCacheKey, () => - this.getActivatedEnvVarsFromProc(resource, interpreter, allowExceptions), - ); - const terminalEnvVarsPromise = this.cacheCallback(terminalCacheKey, () => procEnvVarsPromise); - - const procEnvVars = createDeferredFromPromise(procEnvVarsPromise); - const terminalEnvVars = createDeferredFromPromise(terminalEnvVarsPromise); - - // Do not return this value, its possible both complete almost at the same time. - // Hence wait for another tick, then check and return. - await Promise.race([terminalEnvVars.promise, procEnvVars.promise]); - - // Give preference to the terminal environment variables promise. - return terminalEnvVars.completed ? terminalEnvVars.promise : procEnvVars.promise; - } - /** - * Cache the implementation so it can be used in the future. - * If a cached entry already exists, then ignore the implementation. - * - * @private - * @param {string} cacheKey - * @param {(() => Promise)} implementation - * @returns {(Promise)} - * @memberof WrapperEnvironmentActivationService - */ - private async cacheCallback( - cacheKey: string, - implementation: () => Promise, - ): Promise { - const contents = await this.getDataCachedInFile(cacheKey); - if (contents) { - // If we have it in file cache, then blow away in memory cache, we don't need that anymore. - this.cachePerResourceAndInterpreter.delete(cacheKey); - return contents.env; - } - - // If we don't have this cached in file, we need to ensure the request is cached in memory. - // This way if two different parts of the extension request variables for the same resource + interpreter, they get the same result (promise). - if (!this.cachePerResourceAndInterpreter.get(cacheKey)) { - const promise = implementation(); - this.cachePerResourceAndInterpreter.set(cacheKey, promise); - - // What ever result we get back, store that in file (cache it for other VSC sessions). - promise - .then((env) => this.writeDataToCacheFile(cacheKey, { env })) - .catch((ex) => traceError('Failed to write Env Vars to disc', ex)); - } - - return this.cachePerResourceAndInterpreter.get(cacheKey)!; - } - private getCacheFile(cacheKey: string): string | undefined { - return this.context.storagePath - ? path.join(this.context.storagePath, `pvscEnvVariables${cacheKey}.json`) - : undefined; - } - private async getDataCachedInFile(cacheKey: string): Promise { - const cacheFile = this.getCacheFile(cacheKey); - if (!cacheFile) { - return; - } - return this.fs - .readFile(cacheFile) - .then((data) => JSON.parse(data) as EnvVariablesInCachedFile) - .catch(() => undefined); - } - /** - * Writes the environment variables to disc. - * This way it is available to other VSC Sessions (between VSC reloads). - */ - private async writeDataToCacheFile(cacheKey: string, data: EnvVariablesInCachedFile): Promise { - const cacheFile = this.getCacheFile(cacheKey); - if (!cacheFile || !this.context.storagePath) { - return; - } - if (!(await this.fs.directoryExists(this.context.storagePath))) { - await this.fs.createDirectory(this.context.storagePath); - } - await this.fs.writeFile(cacheFile, JSON.stringify(data)); - } - /** - * Get environment variables by spawning a process (old approach). - */ - private async getActivatedEnvVarsFromProc( - resource: Resource, - interpreter?: PythonEnvironment, - allowExceptions?: boolean, - ): Promise { - return this.procActivation.getActivatedEnvironmentVariables(resource, interpreter, allowExceptions); - } - - /** - * Computes a key used to cache environment variables. - * 1. If resource changes, then environment variables could be different, as user could have `.env` files or similar. - * (this might not be necessary, if there are no custom variables, but paths such as PYTHONPATH, PATH might be different too when computed). - * 2. If interpreter changes, then environment variables could be different (conda has its own env variables). - * 3. Similarly, each workspace could have its own env variables defined in `.env` files, and these could change as well. - * Hence the key is computed based off of these three. - */ - private async getCacheKey(resource: Resource, interpreter?: PythonEnvironment | undefined): Promise { - // Get the custom environment variables as a string (if any errors, ignore and use empty string). - const customEnvVariables = await this.envVarsProvider - .getCustomEnvironmentVariables(resource) - .then((item) => (item ? JSON.stringify(item) : '')) - .catch(() => ''); - - return this.crypto.createHash( - `${customEnvVariables}${interpreter?.path}${interpreter?.version?.raw}`, - 'string', - 'SHA256', - ); - } -} diff --git a/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts b/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts deleted file mode 100644 index 15c08d30232f..000000000000 --- a/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts +++ /dev/null @@ -1,214 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { assert } from 'chai'; -import * as path from 'path'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { EventEmitter, Uri } from 'vscode'; -import { IWorkspaceService } from '../../../client/common/application/types'; -import { WorkspaceService } from '../../../client/common/application/workspace'; -import { CryptoUtils } from '../../../client/common/crypto'; -import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { IFileSystem } from '../../../client/common/platform/types'; -import { ICryptoUtils, IExtensionContext, Resource } from '../../../client/common/types'; -import { Architecture } from '../../../client/common/utils/platform'; -import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; -import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; -import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; -import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; -import { WrapperEnvironmentActivationService } from '../../../client/interpreter/activation/wrapperEnvironmentActivationService'; -import { IInterpreterService } from '../../../client/interpreter/contracts'; -import { InterpreterService } from '../../../client/interpreter/interpreterService'; -import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info'; - -suite('Interpreters Activation - Python Environment Variables (wrap terminal and proc approach)', () => { - let envActivationService: IEnvironmentActivationService; - let procActivation: IEnvironmentActivationService; - let interpreterService: IInterpreterService; - let workspace: IWorkspaceService; - let envVarsProvider: IEnvironmentVariablesProvider; - let onDidChangeEnvVars: EventEmitter; - let crypto: ICryptoUtils; - let fs: IFileSystem; - const mockInterpreter: PythonEnvironment = { - architecture: Architecture.Unknown, - path: '', - sysPrefix: '', - sysVersion: '', - envType: EnvironmentType.Conda, - }; - - [undefined, Uri.file('some Resource')].forEach((resource) => { - [undefined, mockInterpreter].forEach((interpreter) => { - [undefined, path.join('a')].forEach((storagePath) => { - suite(resource ? 'With an extension storagepath' : 'Without an extension storagepath', () => { - suite(resource ? 'With a resource' : 'Without a resource', () => { - setup(() => { - onDidChangeEnvVars = new EventEmitter(); - envVarsProvider = mock(EnvironmentVariablesProvider); - procActivation = mock(EnvironmentActivationService); - interpreterService = mock(InterpreterService); - workspace = mock(WorkspaceService); - crypto = mock(CryptoUtils); - fs = mock(FileSystem); - const extContext: IExtensionContext = { - get storagePath() { - return storagePath; - }, - } as any; - when(crypto.createHash(anything(), anything(), anything())).thenCall((value) => value); - when(envVarsProvider.getCustomEnvironmentVariables(anything())).thenCall((value) => - Promise.resolve({ - key: (value || {}).toString(), - }), - ); - when(envVarsProvider.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVars.event); - when(fs.readFile(anything())).thenReject(new Error('kaboom')); - // Generate a unique key based on resource. - when(workspace.getWorkspaceFolderIdentifier(anything())).thenCall( - (identifier: Resource) => identifier?.fsPath || '', - ); - envActivationService = new WrapperEnvironmentActivationService( - instance(procActivation), - instance(interpreterService), - instance(envVarsProvider), - extContext, - instance(fs), - instance(crypto), - [], - ); - }); - - suite(interpreter ? 'With an interpreter' : 'Without an interpreter', () => { - test('Environment variables returned by process provider should be used if terminal provider crashes', async () => { - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - }); - test('Use cached variables returned by process provider should be used if terminal provider crashes', async () => { - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - }); - test('Environment variables returned by terminal provider should be used if that returns any variables', async () => { - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve({ somethingElse: '1' }); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - }); - test('Environment variables returned by terminal provider should be used if that returns any variables', async () => { - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve({ somethingElse: '1' }); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - }); - test('Will not use cached info, if passing different resource or interpreter', async () => { - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve({ somethingElse: '1' }); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - // Same resource, hence return cached info. - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - assert.deepEqual(vars, expectedVars); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - }); - test('Use variables from file cache', async function () { - if (!storagePath) { - return this.skip(); - } - const expectedVars = { WOW: '1' }; - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(undefined); - when(fs.readFile(anything())).thenResolve(JSON.stringify({ env: expectedVars })); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - }); - }); - }); - }); - }); - }); - }); -}); From 77cdb90d14641effe3273625530b6478e3342f35 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Mar 2021 11:52:11 -0800 Subject: [PATCH 4/4] How did this disappear --- experiments.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/experiments.json b/experiments.json index bf1e2570bf9f..29644f07141d 100644 --- a/experiments.json +++ b/experiments.json @@ -1,4 +1,16 @@ [ + { + "name": "AlwaysDisplayTestExplorer - experiment", + "salt": "AlwaysDisplayTestExplorer", + "min": 80, + "max": 100 + }, + { + "name": "AlwaysDisplayTestExplorer - control", + "salt": "AlwaysDisplayTestExplorer", + "min": 0, + "max": 20 + }, { "name": "ShowPlayIcon - start", "salt": "ShowPlayIcon",