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/experiments.json b/experiments.json index 685f83fecb53..18c91e8bb7e0 100644 --- a/experiments.json +++ b/experiments.json @@ -11,18 +11,6 @@ "min": 0, "max": 40 }, - { - "name": "UseTerminalToGetActivatedEnvVars - experiment", - "salt": "UseTerminalToGetActivatedEnvVars", - "min": 0, - "max": 0 - }, - { - "name": "UseTerminalToGetActivatedEnvVars - control", - "salt": "UseTerminalToGetActivatedEnvVars", - "min": 0, - "max": 100 - }, { "name": "CollectLSRequestTiming - experiment", "salt": "CollectLSRequestTiming", diff --git a/package.json b/package.json index d1a93b0e06c0..c31f2e19edc1 100644 --- a/package.json +++ b/package.json @@ -1050,7 +1050,6 @@ "enum": [ "ShowExtensionSurveyPrompt - enabled", "Reload - experiment", - "UseTerminalToGetActivatedEnvVars - experiment", "CollectLSRequestTiming - experiment", "CollectNodeLSRequestTiming - experiment", "DeprecatePythonPath - experiment", @@ -1073,7 +1072,6 @@ "enum": [ "ShowExtensionSurveyPrompt - enabled", "Reload - 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 3c0662ddd91b..aef878a1b86b 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -10,17 +10,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', -} - // Collect language server request timings. export enum CollectLSRequestTiming { control = 'CollectLSRequestTiming - control', 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/activation/wrapperEnvironmentActivationService.ts b/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts deleted file mode 100644 index ccb2cada81af..000000000000 --- a/src/client/interpreter/activation/wrapperEnvironmentActivationService.ts +++ /dev/null @@ -1,214 +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 { 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 { 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 { TerminalEnvironmentActivationService } from './terminalEnvironmentActivationService'; -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(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, - @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, () => - this.getActivatedEnvVarsFromTerminal(procEnvVarsPromise, resource, interpreter, allowExceptions), - ); - - 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); - } - /** - * 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. - * (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/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 7faa308374a9..538d8bffd288 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/activation/wrapperEnvironmentActivationService.unit.test.ts b/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts deleted file mode 100644 index 8b57d3eb70a0..000000000000 --- a/src/test/interpreters/activation/wrapperEnvironmentActivationService.unit.test.ts +++ /dev/null @@ -1,329 +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 { 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 { 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'; -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 termActivation: IEnvironmentActivationService; - let experiment: IExperimentsManager; - 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); - termActivation = mock(TerminalEnvironmentActivationService); - experiment = mock(ExperimentsManager); - 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(experiment.inExperiment(anything())).thenReturn(true); - 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(termActivation), - instance(experiment), - 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( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenReject(new Error('kaboom')); - when( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).thenResolve(expectedVars); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - 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); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - 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' }); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - 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' }); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - 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' }); - - let vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - verify( - termActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - verify( - procActivation.getActivatedEnvironmentVariables(anything(), anything(), anything()), - ).once(); - - // Same resource, hence return cached info. - vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - 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); - when(fs.readFile(anything())).thenResolve(JSON.stringify({ env: expectedVars })); - - const vars = await envActivationService.getActivatedEnvironmentVariables( - resource, - interpreter, - ); - - assert.deepEqual(vars, expectedVars); - }); - }); - }); - }); - }); - }); - }); -}); 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),