From 8a80ebe68c1ad5b53735c720e6616be4c7ccac1e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 8 Mar 2023 13:25:45 -0800 Subject: [PATCH] Add experiment to implicitly use environment variables for environment activation (#20651) --- .eslintignore | 1 - package-lock.json | 16 +- package.json | 17 +- package.nls.json | 1 + .../application/applicationEnvironment.ts | 22 +- src/client/common/application/types.ts | 4 + src/client/common/experiments/groups.ts | 4 + src/client/common/experiments/helpers.ts | 21 ++ src/client/common/process/logger.ts | 9 +- src/client/common/terminal/activator/index.ts | 7 +- .../shellDetectors/baseShellDetector.ts | 34 +-- src/client/common/utils/localize.ts | 5 +- .../configuration/resolvers/helper.ts | 13 +- .../configuration/resolvers/launch.ts | 13 +- src/client/interpreter/activation/service.ts | 64 +++-- .../terminalEnvVarCollectionService.ts | 150 +++++++++++ src/client/interpreter/activation/types.ts | 1 + src/client/interpreter/contracts.ts | 2 +- src/client/interpreter/interpreterService.ts | 6 +- src/client/interpreter/serviceRegistry.ts | 5 + src/client/jupyter/jupyterIntegration.ts | 2 +- src/client/providers/terminalProvider.ts | 10 +- .../common/environmentManagers/conda.ts | 41 +-- src/test/common/process/logger.unit.test.ts | 57 ++--- .../terminals/activator/index.unit.test.ts | 17 +- src/test/debugger/envVars.test.ts | 21 ++ .../resolvers/launch.unit.test.ts | 11 +- .../activation/service.unit.test.ts | 9 +- ...rminalEnvVarCollectionService.unit.test.ts | 236 ++++++++++++++++++ src/test/linters/lint.provider.test.ts | 5 +- src/test/providers/terminal.unit.test.ts | 12 +- .../testing/common/debugLauncher.unit.test.ts | 10 +- types/vscode.proposed.envShellEvent.d.ts | 16 ++ 33 files changed, 688 insertions(+), 154 deletions(-) create mode 100644 src/client/common/experiments/helpers.ts create mode 100644 src/client/interpreter/activation/terminalEnvVarCollectionService.ts create mode 100644 src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts create mode 100644 types/vscode.proposed.envShellEvent.d.ts diff --git a/.eslintignore b/.eslintignore index 083b9d650d0c..bce20d67c2c9 100644 --- a/.eslintignore +++ b/.eslintignore @@ -146,7 +146,6 @@ src/client/interpreter/configuration/services/workspaceUpdaterService.ts src/client/interpreter/configuration/services/workspaceFolderUpdaterService.ts src/client/interpreter/helpers.ts src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts -src/client/interpreter/activation/service.ts src/client/interpreter/display/index.ts src/client/api.ts diff --git a/package-lock.json b/package-lock.json index 83184c787aca..ef50646d7457 100644 --- a/package-lock.json +++ b/package-lock.json @@ -69,7 +69,7 @@ "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.74.0", + "@types/vscode": "^1.75.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", @@ -129,7 +129,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.75.0-20230123" + "vscode": "^1.75.0" } }, "node_modules/@azure/abort-controller": { @@ -1140,9 +1140,9 @@ "dev": true }, "node_modules/@types/vscode": { - "version": "1.74.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.74.0.tgz", - "integrity": "sha512-LyeCIU3jb9d38w0MXFwta9r0Jx23ugujkAxdwLTNCyspdZTKUc43t7ppPbCiPoQ/Ivd/pnDFZrb4hWd45wrsgA==", + "version": "1.75.1", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.75.1.tgz", + "integrity": "sha512-emg7wdsTFzdi+elvoyoA+Q8keEautdQHyY5LNmHVM4PTpY8JgOTVADrGVyXGepJ6dVW2OS5/xnLUWh+nZxvdiA==", "dev": true }, "node_modules/@types/which": { @@ -16329,9 +16329,9 @@ "dev": true }, "@types/vscode": { - "version": "1.74.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.74.0.tgz", - "integrity": "sha512-LyeCIU3jb9d38w0MXFwta9r0Jx23ugujkAxdwLTNCyspdZTKUc43t7ppPbCiPoQ/Ivd/pnDFZrb4hWd45wrsgA==", + "version": "1.75.1", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.75.1.tgz", + "integrity": "sha512-emg7wdsTFzdi+elvoyoA+Q8keEautdQHyY5LNmHVM4PTpY8JgOTVADrGVyXGepJ6dVW2OS5/xnLUWh+nZxvdiA==", "dev": true }, "@types/which": { diff --git a/package.json b/package.json index 6ac7bbde56a5..a10840f2a6da 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "publisher": "ms-python", "enabledApiProposals": [ "quickPickSortByLabel", + "envShellEvent", "testObserver" ], "author": { @@ -40,7 +41,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.75.0-20230123" + "vscode": "^1.76.0" }, "keywords": [ "python", @@ -426,12 +427,14 @@ "enum": [ "All", "pythonSurveyNotification", - "pythonPromptNewToolsExt" + "pythonPromptNewToolsExt", + "pythonTerminalEnvVarActivation" ], "enumDescriptions": [ "%python.experiments.All.description%", "%python.experiments.pythonSurveyNotification.description%", - "%python.experiments.pythonPromptNewToolsExt.description%" + "%python.experiments.pythonPromptNewToolsExt.description%", + "%python.experiments.pythonTerminalEnvVarActivation.description%" ] }, "scope": "machine", @@ -445,12 +448,14 @@ "enum": [ "All", "pythonSurveyNotification", - "pythonPromptNewToolsExt" + "pythonPromptNewToolsExt", + "pythonTerminalEnvVarActivation" ], "enumDescriptions": [ "%python.experiments.All.description%", "%python.experiments.pythonSurveyNotification.description%", - "%python.experiments.pythonPromptNewToolsExt.description%" + "%python.experiments.pythonPromptNewToolsExt.description%", + "%python.experiments.pythonTerminalEnvVarActivation.description%" ] }, "scope": "machine", @@ -1868,7 +1873,7 @@ "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.74.0", + "@types/vscode": "^1.75.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", diff --git a/package.nls.json b/package.nls.json index 3265ce86f2cd..2d0f005f61d5 100644 --- a/package.nls.json +++ b/package.nls.json @@ -37,6 +37,7 @@ "python.experiments.All.description": "Combined list of all experiments.", "python.experiments.pythonSurveyNotification.description": "Denotes the Python Survey Notification experiment.", "python.experiments.pythonPromptNewToolsExt.description": "Denotes the Python Prompt New Tools Extension experiment.", + "python.experiments.pythonTerminalEnvVarActivation.description": "Enables use of environment variables to activate terminals instead of sending activation commands.", "python.formatting.autopep8Args.description": "Arguments passed in. Each argument is a separate item in the array.", "python.formatting.autopep8Path.description": "Path to autopep8, you can use a custom version of autopep8 by modifying this setting to include the full path.", "python.formatting.blackArgs.description": "Arguments passed in. Each argument is a separate item in the array.", diff --git a/src/client/common/application/applicationEnvironment.ts b/src/client/common/application/applicationEnvironment.ts index e3d78477996d..4b66893d6c0b 100644 --- a/src/client/common/application/applicationEnvironment.ts +++ b/src/client/common/application/applicationEnvironment.ts @@ -7,6 +7,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { parse } from 'semver'; import * as vscode from 'vscode'; +import { traceError } from '../../logging'; import { Channel } from '../constants'; import { IPlatformService } from '../platform/types'; import { ICurrentProcess, IPathUtils } from '../types'; @@ -70,19 +71,22 @@ export class ApplicationEnvironment implements IApplicationEnvironment { public get extensionName(): string { return this.packageJson.displayName; } - /** - * At the time of writing this API, the vscode.env.shell isn't officially released in stable version of VS Code. - * Using this in stable version seems to throw errors in VSC with messages being displayed to the user about use of - * unstable API. - * Solution - log and suppress the errors. - * @readonly - * @type {(string)} - * @memberof ApplicationEnvironment - */ + public get shell(): string { return vscode.env.shell; } + public get onDidChangeShell(): vscode.Event { + try { + return vscode.env.onDidChangeShell; + } catch (ex) { + traceError('Failed to get onDidChangeShell API', ex); + // `onDidChangeShell` is a proposed API at the time of writing this, so wrap this in a try...catch + // block in case the API is removed or changed. + return new vscode.EventEmitter().event; + } + } + public get packageJson(): any { return require('../../../../package.json'); } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index 69caf30a261b..1b054eda687c 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -1048,6 +1048,10 @@ export interface IApplicationEnvironment { * @memberof IApplicationShell */ readonly shell: string; + /** + * An {@link Event} which fires when the default shell changes. + */ + readonly onDidChangeShell: Event; /** * Gets the vscode channel (whether 'insiders' or 'stable'). */ diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 7d9c27bf33e9..b575a116096c 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -6,3 +6,7 @@ export enum ShowExtensionSurveyPrompt { export enum ShowToolsExtensionPrompt { experiment = 'pythonPromptNewToolsExt', } + +export enum TerminalEnvVarActivation { + experiment = 'pythonTerminalEnvVarActivation', +} diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts new file mode 100644 index 000000000000..4aed04da3fd0 --- /dev/null +++ b/src/client/common/experiments/helpers.ts @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { workspace } from 'vscode'; +import { isTestExecution } from '../constants'; +import { IExperimentService } from '../types'; +import { TerminalEnvVarActivation } from './groups'; + +export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean { + if (workspace.workspaceFile && !isTestExecution()) { + // Don't run experiment in multi-root workspaces for now, requires work on VSCode: + // https://github.com/microsoft/vscode/issues/171173 + return false; + } + if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) { + return false; + } + return true; +} diff --git a/src/client/common/process/logger.ts b/src/client/common/process/logger.ts index ebb1ad019a48..5c8f04cbec30 100644 --- a/src/client/common/process/logger.ts +++ b/src/client/common/process/logger.ts @@ -7,11 +7,11 @@ import { inject, injectable } from 'inversify'; import { traceLog } from '../../logging'; import { IWorkspaceService } from '../application/types'; import { isCI, isTestExecution } from '../constants'; -import { Logging } from '../utils/localize'; import { getOSType, getUserHomeDir, OSType } from '../utils/platform'; import { IProcessLogger, SpawnOptions } from './types'; import { escapeRegExp } from 'lodash'; import { replaceAll } from '../stringUtils'; +import { identifyShellFromShellPath } from '../terminal/shellDetectors/baseShellDetector'; @injectable() export class ProcessLogger implements IProcessLogger { @@ -27,8 +27,11 @@ export class ProcessLogger implements IProcessLogger { ? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgumentForPythonExt()).join(' ') : fileOrCommand; const info = [`> ${this.getDisplayCommands(command)}`]; - if (options && options.cwd) { - info.push(`${Logging.currentWorkingDirectory} ${this.getDisplayCommands(options.cwd)}`); + if (options?.cwd) { + info.push(`cwd: ${this.getDisplayCommands(options.cwd)}`); + } + if (typeof options?.shell === 'string') { + info.push(`shell: ${identifyShellFromShellPath(options?.shell)}`); } info.forEach((line) => { diff --git a/src/client/common/terminal/activator/index.ts b/src/client/common/terminal/activator/index.ts index 5bc76c0cb0f8..752d512d4c30 100644 --- a/src/client/common/terminal/activator/index.ts +++ b/src/client/common/terminal/activator/index.ts @@ -5,7 +5,8 @@ import { inject, injectable, multiInject } from 'inversify'; import { Terminal } from 'vscode'; -import { IConfigurationService } from '../../types'; +import { inTerminalEnvVarExperiment } from '../../experiments/helpers'; +import { IConfigurationService, IExperimentService } from '../../types'; import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types'; import { BaseTerminalActivator } from './base'; @@ -17,6 +18,7 @@ export class TerminalActivator implements ITerminalActivator { @inject(ITerminalHelper) readonly helper: ITerminalHelper, @multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[], @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { this.initialize(); } @@ -37,7 +39,8 @@ export class TerminalActivator implements ITerminalActivator { options?: TerminalActivationOptions, ): Promise { const settings = this.configurationService.getSettings(options?.resource); - const activateEnvironment = settings.terminal.activateEnvironment; + const activateEnvironment = + settings.terminal.activateEnvironment && !inTerminalEnvVarExperiment(this.experimentService); if (!activateEnvironment || options?.hideFromUser) { return false; } diff --git a/src/client/common/terminal/shellDetectors/baseShellDetector.ts b/src/client/common/terminal/shellDetectors/baseShellDetector.ts index 4716af1a34e4..657c51e82af6 100644 --- a/src/client/common/terminal/shellDetectors/baseShellDetector.ts +++ b/src/client/common/terminal/shellDetectors/baseShellDetector.ts @@ -54,22 +54,26 @@ export abstract class BaseShellDetector implements IShellDetector { terminal?: Terminal, ): TerminalShellType | undefined; public identifyShellFromShellPath(shellPath: string): TerminalShellType { - // Remove .exe extension so shells can be more consistently detected - // on Windows (including Cygwin). - const basePath = shellPath.replace(/\.exe$/, ''); + return identifyShellFromShellPath(shellPath); + } +} - const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => { - if (matchedShell === TerminalShellType.other) { - const pat = detectableShells.get(shellToDetect); - if (pat && pat.test(basePath)) { - return shellToDetect; - } +export function identifyShellFromShellPath(shellPath: string): TerminalShellType { + // Remove .exe extension so shells can be more consistently detected + // on Windows (including Cygwin). + const basePath = shellPath.replace(/\.exe$/, ''); + + const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => { + if (matchedShell === TerminalShellType.other) { + const pat = detectableShells.get(shellToDetect); + if (pat && pat.test(basePath)) { + return shellToDetect; } - return matchedShell; - }, TerminalShellType.other); + } + return matchedShell; + }, TerminalShellType.other); - traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`); - traceVerbose(`Shell path identified as shell '${shell}'`); - return shell; - } + traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`); + traceVerbose(`Shell path identified as shell '${shell}'`); + return shell; } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 5037494463f2..8673fe7cc8cd 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -193,6 +193,7 @@ export namespace Interpreters { export const condaInheritEnvMessage = l10n.t( 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings. [Learn more](https://aka.ms/AA66i8f).', ); + export const activatingTerminals = l10n.t('Reactivating terminals...'); export const activatedCondaEnvLaunch = l10n.t( 'We noticed VS Code was launched from an activated conda environment, would you like to select it?', ); @@ -243,10 +244,6 @@ export namespace OutputChannelNames { export const pythonTest = l10n.t('Python Test Log'); } -export namespace Logging { - export const currentWorkingDirectory = l10n.t('cwd:'); -} - export namespace Linters { export const selectLinter = l10n.t('Select Linter'); } diff --git a/src/client/debugger/extension/configuration/resolvers/helper.ts b/src/client/debugger/extension/configuration/resolvers/helper.ts index 781b25a26510..0ccaa9964054 100644 --- a/src/client/debugger/extension/configuration/resolvers/helper.ts +++ b/src/client/debugger/extension/configuration/resolvers/helper.ts @@ -13,7 +13,10 @@ import { getSearchPathEnvVarNames } from '../../../../common/utils/exec'; export const IDebugEnvironmentVariablesService = Symbol('IDebugEnvironmentVariablesService'); export interface IDebugEnvironmentVariablesService { - getEnvironmentVariables(args: LaunchRequestArguments): Promise; + getEnvironmentVariables( + args: LaunchRequestArguments, + baseVars?: EnvironmentVariables, + ): Promise; } @injectable() @@ -23,7 +26,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl @inject(ICurrentProcess) private process: ICurrentProcess, ) {} - public async getEnvironmentVariables(args: LaunchRequestArguments): Promise { + public async getEnvironmentVariables( + args: LaunchRequestArguments, + baseVars?: EnvironmentVariables, + ): Promise { const pathVariableName = getSearchPathEnvVarNames()[0]; // Merge variables from both .env file and env json variables. @@ -37,6 +43,9 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl // "overwrite: true" to ensure that debug-configuration env variable values // take precedence over env file. this.envParser.mergeVariables(debugLaunchEnvVars, env, { overwrite: true }); + if (baseVars) { + this.envParser.mergeVariables(baseVars, env); + } // Append the PYTHONPATH and PATH variables. this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]); diff --git a/src/client/debugger/extension/configuration/resolvers/launch.ts b/src/client/debugger/extension/configuration/resolvers/launch.ts index 485091119eb3..6a28075d4353 100644 --- a/src/client/debugger/extension/configuration/resolvers/launch.ts +++ b/src/client/debugger/extension/configuration/resolvers/launch.ts @@ -9,6 +9,8 @@ import { InvalidPythonPathInDebuggerServiceId } from '../../../../application/di import { IDiagnosticsService, IInvalidPythonPathInDebuggerService } from '../../../../application/diagnostics/types'; import { IConfigurationService } from '../../../../common/types'; import { getOSType, OSType } from '../../../../common/utils/platform'; +import { EnvironmentVariables } from '../../../../common/variables/types'; +import { IEnvironmentActivationService } from '../../../../interpreter/activation/types'; import { IInterpreterService } from '../../../../interpreter/contracts'; import { DebuggerTypeName } from '../../../constants'; import { DebugOptions, DebugPurpose, LaunchRequestArguments } from '../../../types'; @@ -24,6 +26,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver { + const isPythonSet = debugConfiguration.python !== undefined; if (debugConfiguration.python === undefined) { debugConfiguration.python = debugConfiguration.pythonPath; } @@ -99,10 +103,17 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver>(); + private normalMap = new Map>(); - public static forceUseStatic() { + public static forceUseStatic(): void { EnvironmentActivationServiceCache.useStatic = true; } - public static forceUseNormal() { + + public static forceUseNormal(): void { EnvironmentActivationServiceCache.useStatic = false; } + public get(key: string): InMemoryCache | undefined { if (EnvironmentActivationServiceCache.useStatic) { return EnvironmentActivationServiceCache.staticMap.get(key); @@ -75,7 +83,7 @@ export class EnvironmentActivationServiceCache { return this.normalMap.get(key); } - public set(key: string, value: InMemoryCache) { + public set(key: string, value: InMemoryCache): void { if (EnvironmentActivationServiceCache.useStatic) { EnvironmentActivationServiceCache.staticMap.set(key, value); } else { @@ -83,7 +91,7 @@ export class EnvironmentActivationServiceCache { } } - public delete(key: string) { + public delete(key: string): void { if (EnvironmentActivationServiceCache.useStatic) { EnvironmentActivationServiceCache.staticMap.delete(key); } else { @@ -91,7 +99,7 @@ export class EnvironmentActivationServiceCache { } } - public clear() { + public clear(): void { // Don't clear during a test as the environment isn't going to change if (!EnvironmentActivationServiceCache.useStatic) { this.normalMap.clear(); @@ -102,7 +110,9 @@ export class EnvironmentActivationServiceCache { @injectable() export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable { private readonly disposables: IDisposable[] = []; + private readonly activatedEnvVariablesCache = new EnvironmentActivationServiceCache(); + constructor( @inject(ITerminalHelper) private readonly helper: ITerminalHelper, @inject(IPlatformService) private readonly platform: IPlatformService, @@ -117,29 +127,25 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi this, this.disposables, ); - - this.interpreterService.onDidChangeInterpreter( - () => this.activatedEnvVariablesCache.clear(), - this, - this.disposables, - ); } public dispose(): void { this.disposables.forEach((d) => d.dispose()); } + @traceDecoratorVerbose('getActivatedEnvironmentVariables', TraceOptions.Arguments) public async getActivatedEnvironmentVariables( resource: Resource, interpreter?: PythonEnvironment, allowExceptions?: boolean, + shell?: string, ): Promise { const stopWatch = new StopWatch(); // Cache key = resource + interpreter. const workspaceKey = this.workspace.getWorkspaceFolderIdentifier(resource); interpreter = interpreter ?? (await this.interpreterService.getActiveInterpreter(resource)); const interpreterPath = this.platform.isWindows ? interpreter?.path.toLowerCase() : interpreter?.path; - const cacheKey = `${workspaceKey}_${interpreterPath}`; + const cacheKey = `${workspaceKey}_${interpreterPath}_${shell}`; if (this.activatedEnvVariablesCache.get(cacheKey)?.hasData) { return this.activatedEnvVariablesCache.get(cacheKey)!.data; @@ -147,7 +153,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // Cache only if successful, else keep trying & failing if necessary. const cache = new InMemoryCache(CACHE_DURATION); - return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions) + return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions, shell) .then((vars) => { cache.data = vars; this.activatedEnvVariablesCache.set(cacheKey, cache); @@ -167,6 +173,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi throw ex; }); } + public async getEnvironmentActivationShellCommands( resource: Resource, interpreter?: PythonEnvironment, @@ -177,23 +184,29 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } return this.helper.getEnvironmentActivationShellCommands(resource, shellInfo.shellType, interpreter); } + public async getActivatedEnvironmentVariablesImpl( resource: Resource, interpreter?: PythonEnvironment, allowExceptions?: boolean, + shell?: string, ): Promise { - const shellInfo = defaultShells[this.platform.osType]; + let shellInfo = defaultShells[this.platform.osType]; if (!shellInfo) { - return; + return undefined; + } + if (shell) { + const customShellType = identifyShellFromShellPath(shell); + shellInfo = { shellType: customShellType, shell }; } try { let command: string | undefined; - let [args, parse] = internalScripts.printEnvVariables(); + const [args, parse] = internalScripts.printEnvVariables(); args.forEach((arg, i) => { args[i] = arg.toCommandArgumentForPythonExt(); }); if (interpreter?.envType === EnvironmentType.Conda) { - const conda = await Conda.getConda(); + const conda = await Conda.getConda(shell); const pythonArgv = await conda?.getRunPythonArgs({ name: interpreter.envName, prefix: interpreter.envPath ?? '', @@ -211,10 +224,10 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi ); traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`); if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) { - return; + return undefined; } // Run the activate command collect the environment from it. - const activationCommand = this.fixActivationCommands(activationCommands).join(' && '); + const activationCommand = fixActivationCommands(activationCommands).join(' && '); // In order to make sure we know where the environment output is, // put in a dummy echo we can look for command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`; @@ -306,15 +319,13 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi throw e; } } + return undefined; } - protected fixActivationCommands(commands: string[]): string[] { - // Replace 'source ' with '. ' as that works in shell exec - return commands.map((cmd) => cmd.replace(/^source\s+/, '. ')); - } @traceDecoratorError('Failed to parse Environment variables') @traceDecoratorVerbose('parseEnvironmentOutput', TraceOptions.None) - protected parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) { + // eslint-disable-next-line class-methods-use-this + private parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) { if (output.indexOf(ENVIRONMENT_PREFIX) === -1) { return parse(output); } @@ -323,3 +334,8 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi return parse(js); } } + +function fixActivationCommands(commands: string[]): string[] { + // Replace 'source ' with '. ' as that works in shell exec + return commands.map((cmd) => cmd.replace(/^source\s+/, '. ')); +} diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts new file mode 100644 index 000000000000..f5af71b3f2ca --- /dev/null +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -0,0 +1,150 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { inject, injectable } from 'inversify'; +import { ProgressOptions, ProgressLocation } from 'vscode'; +import { IExtensionSingleActivationService } from '../../activation/types'; +import { IApplicationShell, IApplicationEnvironment } from '../../common/application/types'; +import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; +import { IPlatformService } from '../../common/platform/types'; +import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector'; +import { IExtensionContext, IExperimentService, Resource, IDisposableRegistry } from '../../common/types'; +import { Deferred, createDeferred } from '../../common/utils/async'; +import { Interpreters } from '../../common/utils/localize'; +import { traceDecoratorVerbose, traceVerbose } from '../../logging'; +import { IInterpreterService } from '../contracts'; +import { defaultShells } from './service'; +import { IEnvironmentActivationService } from './types'; + +@injectable() +export class TerminalEnvVarCollectionService implements IExtensionSingleActivationService { + public readonly supportedWorkspaceTypes = { + untrustedWorkspace: false, + virtualWorkspace: false, + }; + + private deferred: Deferred | undefined; + + private previousEnvVars = _normCaseKeys(process.env); + + constructor( + @inject(IPlatformService) private readonly platform: IPlatformService, + @inject(IInterpreterService) private interpreterService: IInterpreterService, + @inject(IExtensionContext) private context: IExtensionContext, + @inject(IApplicationShell) private shell: IApplicationShell, + @inject(IExperimentService) private experimentService: IExperimentService, + @inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment, + @inject(IDisposableRegistry) private disposables: IDisposableRegistry, + @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, + ) {} + + public async activate(): Promise { + if (!inTerminalEnvVarExperiment(this.experimentService)) { + this.context.environmentVariableCollection.clear(); + return; + } + this.interpreterService.onDidChangeInterpreter( + async (resource) => { + this.showProgress(); + await this._applyCollection(resource); + this.hideProgress(); + }, + this, + this.disposables, + ); + this.applicationEnvironment.onDidChangeShell( + async (shell: string) => { + this.showProgress(); + // Pass in the shell where known instead of relying on the application environment, because of bug + // on VSCode: https://github.com/microsoft/vscode/issues/160694 + await this._applyCollection(undefined, shell); + this.hideProgress(); + }, + this, + this.disposables, + ); + + this._applyCollection(undefined).ignoreErrors(); + } + + public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { + const env = await this.environmentActivationService.getActivatedEnvironmentVariables( + resource, + undefined, + undefined, + shell, + ); + if (!env) { + const shellType = identifyShellFromShellPath(shell); + const defaultShell = defaultShells[this.platform.osType]; + if (defaultShell?.shellType !== shellType) { + // Commands to fetch env vars may fail in custom shells due to unknown reasons, in that case + // fallback to default shells as they are known to work better. + await this._applyCollection(resource, defaultShell?.shell); + return; + } + this.context.environmentVariableCollection.clear(); + this.previousEnvVars = _normCaseKeys(process.env); + return; + } + const previousEnv = this.previousEnvVars; + this.previousEnvVars = env; + Object.keys(env).forEach((key) => { + const value = env[key]; + const prevValue = previousEnv[key]; + if (prevValue !== value) { + if (value !== undefined) { + traceVerbose(`Setting environment variable ${key} in collection to ${value}`); + this.context.environmentVariableCollection.replace(key, value); + } else { + traceVerbose(`Clearing environment variable ${key} from collection`); + this.context.environmentVariableCollection.delete(key); + } + } + }); + Object.keys(previousEnv).forEach((key) => { + // If the previous env var is not in the current env, clear it from collection. + if (!(key in env)) { + traceVerbose(`Clearing environment variable ${key} from collection`); + this.context.environmentVariableCollection.delete(key); + } + }); + } + + @traceDecoratorVerbose('Display activating terminals') + private showProgress(): void { + if (!this.deferred) { + this.createProgress(); + } + } + + @traceDecoratorVerbose('Hide activating terminals') + private hideProgress(): void { + if (this.deferred) { + this.deferred.resolve(); + this.deferred = undefined; + } + } + + private createProgress() { + const progressOptions: ProgressOptions = { + location: ProgressLocation.Window, + title: Interpreters.activatingTerminals, + }; + this.shell.withProgress(progressOptions, () => { + this.deferred = createDeferred(); + return this.deferred.promise; + }); + } +} + +export function _normCaseKeys(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const result: NodeJS.ProcessEnv = {}; + Object.keys(env).forEach((key) => { + // `os.environ` script used to get env vars normalizes keys to upper case: + // https://github.com/python/cpython/issues/101754 + // So convert `process.env` keys to upper case to match. + result[key.toUpperCase()] = env[key]; + }); + return result; +} diff --git a/src/client/interpreter/activation/types.ts b/src/client/interpreter/activation/types.ts index 9508147a3552..d8e4ae16dbca 100644 --- a/src/client/interpreter/activation/types.ts +++ b/src/client/interpreter/activation/types.ts @@ -12,6 +12,7 @@ export interface IEnvironmentActivationService { resource: Resource, interpreter?: PythonEnvironment, allowExceptions?: boolean, + shell?: string, ): Promise; getEnvironmentActivationShellCommands( resource: Resource, diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index ec504802bcfc..bfaebd235f19 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -76,7 +76,7 @@ export interface IInterpreterService { readonly refreshPromise: Promise | undefined; readonly onDidChangeInterpreters: Event; onDidChangeInterpreterConfiguration: Event; - onDidChangeInterpreter: Event; + onDidChangeInterpreter: Event; onDidChangeInterpreterInformation: Event; /** * Note this API does not trigger the refresh but only works with the current refresh if any. Information diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 151b86508b8c..2fee9aaec22e 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -58,7 +58,7 @@ export class InterpreterService implements Disposable, IInterpreterService { return this.pyenvs.getRefreshPromise(); } - public get onDidChangeInterpreter(): Event { + public get onDidChangeInterpreter(): Event { return this.didChangeInterpreterEmitter.event; } @@ -80,7 +80,7 @@ export class InterpreterService implements Disposable, IInterpreterService { private readonly interpreterPathService: IInterpreterPathService; - private readonly didChangeInterpreterEmitter = new EventEmitter(); + private readonly didChangeInterpreterEmitter = new EventEmitter(); private readonly didChangeInterpreterInformation = new EventEmitter(); @@ -226,7 +226,7 @@ export class InterpreterService implements Disposable, IInterpreterService { this.didChangeInterpreterConfigurationEmitter.fire(resource); if (this._pythonPathSetting === '' || this._pythonPathSetting !== pySettings.pythonPath) { this._pythonPathSetting = pySettings.pythonPath; - this.didChangeInterpreterEmitter.fire(); + this.didChangeInterpreterEmitter.fire(resource); reportActiveInterpreterChanged({ path: pySettings.pythonPath, resource: this.serviceContainer.get(IWorkspaceService).getWorkspaceFolder(resource), diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 422776bd5e43..15dd6de7b722 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -6,6 +6,7 @@ import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types'; import { IServiceManager } from '../ioc/types'; import { EnvironmentActivationService } from './activation/service'; +import { TerminalEnvVarCollectionService } from './activation/terminalEnvVarCollectionService'; import { IEnvironmentActivationService } from './activation/types'; import { InterpreterAutoSelectionService } from './autoSelection/index'; import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy'; @@ -108,4 +109,8 @@ export function registerTypes(serviceManager: IServiceManager): void { IEnvironmentActivationService, EnvironmentActivationService, ); + serviceManager.addSingleton( + IExtensionSingleActivationService, + TerminalEnvVarCollectionService, + ); } diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index 556ff93f240a..aedc24b1e8bf 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -63,7 +63,7 @@ type PythonApiForJupyterExtension = { /** * IInterpreterService */ - onDidChangeInterpreter: Event; + onDidChangeInterpreter: Event; /** * IInterpreterService */ diff --git a/src/client/providers/terminalProvider.ts b/src/client/providers/terminalProvider.ts index ee1de62acd8c..d047ea4b6d82 100644 --- a/src/client/providers/terminalProvider.ts +++ b/src/client/providers/terminalProvider.ts @@ -4,8 +4,9 @@ import { Disposable, Terminal } from 'vscode'; import { IActiveResourceService, ICommandManager } from '../common/application/types'; import { Commands } from '../common/constants'; +import { inTerminalEnvVarExperiment } from '../common/experiments/helpers'; import { ITerminalActivator, ITerminalServiceFactory } from '../common/terminal/types'; -import { IConfigurationService } from '../common/types'; +import { IConfigurationService, IExperimentService } from '../common/types'; import { swallowExceptions } from '../common/utils/decorators'; import { IServiceContainer } from '../ioc/types'; import { captureTelemetry, sendTelemetryEvent } from '../telemetry'; @@ -24,9 +25,14 @@ export class TerminalProvider implements Disposable { @swallowExceptions('Failed to initialize terminal provider') public async initialize(currentTerminal: Terminal | undefined): Promise { const configuration = this.serviceContainer.get(IConfigurationService); + const experimentService = this.serviceContainer.get(IExperimentService); const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource()); - if (currentTerminal && pythonSettings.terminal.activateEnvInCurrentTerminal) { + if ( + currentTerminal && + pythonSettings.terminal.activateEnvInCurrentTerminal && + !inTerminalEnvVarExperiment(experimentService) + ) { const hideFromUser = 'hideFromUser' in currentTerminal.creationOptions && currentTerminal.creationOptions.hideFromUser; if (!hideFromUser) { diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index bb5720a15312..88178d02d58a 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -22,6 +22,7 @@ import { isTestExecution } from '../../../common/constants'; import { traceError, traceVerbose } from '../../../logging'; import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts'; import { splitLines } from '../../../common/stringUtils'; +import { SpawnOptions } from '../../../common/process/types'; export const AnacondaCompanyName = 'Anaconda, Inc.'; export const CONDAPATH_SETTING_KEY = 'condaPath'; @@ -248,9 +249,9 @@ export class Conda { * need a Conda instance should use getConda() to obtain it, and should never access * this property directly. */ - private static condaPromise: Promise | undefined; + private static condaPromise = new Map>(); - private condaInfoCached: Promise | undefined; + private condaInfoCached = new Map | undefined>(); /** * Carries path to conda binary to be used for shell execution. @@ -263,18 +264,18 @@ export class Conda { * @param command - Command used to spawn conda. This has the same meaning as the * first argument of spawn() - i.e. it can be a full path, or just a binary name. */ - constructor(readonly command: string, shellCommand?: string) { + constructor(readonly command: string, shellCommand?: string, private readonly shellPath?: string) { this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { - Conda.condaPromise = undefined; + Conda.condaPromise = new Map>(); }); } - public static async getConda(): Promise { - if (Conda.condaPromise === undefined || isTestExecution()) { - Conda.condaPromise = Conda.locate(); + public static async getConda(shellPath?: string): Promise { + if (Conda.condaPromise.get(shellPath) === undefined || isTestExecution()) { + Conda.condaPromise.set(shellPath, Conda.locate(shellPath)); } - return Conda.condaPromise; + return Conda.condaPromise.get(shellPath); } /** @@ -283,7 +284,7 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(): Promise { + private static async locate(shellPath?: string): Promise { traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); const customCondaPath = getPythonSetting(CONDAPATH_SETTING_KEY); @@ -383,7 +384,7 @@ export class Conda { // Probe the candidates, and pick the first one that exists and does what we need. for await (const condaPath of getCandidates()) { traceVerbose(`Probing conda binary: ${condaPath}`); - let conda = new Conda(condaPath); + let conda = new Conda(condaPath, undefined, shellPath); try { await conda.getInfo(); if (getOSType() === OSType.Windows && (isTestExecution() || condaPath !== customCondaPath)) { @@ -392,9 +393,9 @@ export class Conda { const condaBatFile = await getCondaBatFile(condaPath); try { if (condaBatFile) { - const condaBat = new Conda(condaBatFile); + const condaBat = new Conda(condaBatFile, undefined, shellPath); await condaBat.getInfo(); - conda = new Conda(condaPath, condaBatFile); + conda = new Conda(condaPath, condaBatFile, shellPath); } } catch (ex) { traceVerbose('Failed to spawn conda bat file', condaBatFile, ex); @@ -420,10 +421,12 @@ export class Conda { * Corresponds to "conda info --json". */ public async getInfo(useCache?: boolean): Promise { - if (!useCache || !this.condaInfoCached) { - this.condaInfoCached = this.getInfoImpl(this.command); + let condaInfoCached = this.condaInfoCached.get(this.shellPath); + if (!useCache || !condaInfoCached) { + condaInfoCached = this.getInfoImpl(this.command, this.shellPath); + this.condaInfoCached.set(this.shellPath, condaInfoCached); } - return this.condaInfoCached; + return condaInfoCached; } /** @@ -431,8 +434,12 @@ export class Conda { */ @cache(30_000, true, 10_000) // eslint-disable-next-line class-methods-use-this - private async getInfoImpl(command: string): Promise { - const result = await exec(command, ['info', '--json'], { timeout: CONDA_GENERAL_TIMEOUT }); + private async getInfoImpl(command: string, shellPath: string | undefined): Promise { + const options: SpawnOptions = { timeout: CONDA_GENERAL_TIMEOUT }; + if (shellPath) { + options.shell = shellPath; + } + const result = await exec(command, ['info', '--json'], options); traceVerbose(`${command} info --json: ${result.stdout}`); return JSON.parse(result.stdout); } diff --git a/src/test/common/process/logger.unit.test.ts b/src/test/common/process/logger.unit.test.ts index 3b0fc239e183..ebce120b7e6c 100644 --- a/src/test/common/process/logger.unit.test.ts +++ b/src/test/common/process/logger.unit.test.ts @@ -11,7 +11,6 @@ import untildify = require('untildify'); import { WorkspaceFolder } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { ProcessLogger } from '../../../client/common/process/logger'; -import { Logging } from '../../../client/common/utils/localize'; import { getOSType, OSType } from '../../../client/common/utils/platform'; import * as logging from '../../../client/logging'; @@ -41,7 +40,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar'], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger adds quotes around arguments if they contain spaces', async () => { @@ -49,10 +48,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar', 'import test'], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} ${path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${path.join('debug', 'path')}`); }); test('Logger preserves quotes around arguments if they contain spaces', async () => { @@ -60,10 +56,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar', '"import test"'], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} ${path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${path.join('debug', 'path')}`); }); test('Logger converts single quotes around arguments to double quotes if they contain spaces', async () => { @@ -71,10 +64,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar', "'import test'"], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} ${path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${path.join('debug', 'path')}`); }); test('Logger removes single quotes around arguments if they do not contain spaces', async () => { @@ -82,10 +72,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar', "'importtest'"], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar importtest`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} ${path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${path.join('debug', 'path')}`); }); test('Logger replaces the path/to/home with ~ in the current working directory', async () => { @@ -93,10 +80,7 @@ suite('ProcessLogger suite', () => { logger.logProcess('test', ['--foo', '--bar'], options); sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} ${path.join('~', 'debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${path.join('~', 'debug', 'path')}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS at the beginning of the path', async () => { @@ -104,7 +88,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('~', 'test')} --foo --bar`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS at the beginning of the path but another arg contains other ref to home folder', async () => { @@ -112,7 +96,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', path.join(untildify('~'), 'boo')], options); sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('~', 'test')} --foo ${path.join('~', 'boo')}`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS at the beginning of the path between doble quotes', async () => { @@ -120,7 +104,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join(untildify('~'), 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" "--foo" "--bar"`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS NOT at the beginning of the path', async () => { @@ -128,7 +112,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(path.join('net', untildify('~'), 'test'), ['--foo', '--bar'], options); sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('net', '~', 'test')} --foo --bar`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS NOT at the beginning of the path but another arg contains other ref to home folder', async () => { @@ -143,7 +127,7 @@ suite('ProcessLogger suite', () => { traceLogStub, `> ${path.join('net', '~', 'test')} --foo ${path.join('~', 'boo')}`, ); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ in the command path where the home path IS NOT at the beginning of the path between doble quotes', async () => { @@ -151,7 +135,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join('net', untildify('~'), 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('net', '~', 'test')}" "--foo" "--bar"`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ if shell command is provided', async () => { @@ -159,7 +143,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join(untildify('~'), 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" "--foo" "--bar"`); - sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `cwd: ${options.cwd}`); }); test('Logger replaces the path to workspace with . if exactly one workspace folder is opened', async () => { @@ -167,10 +151,7 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} .${path.sep + path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: .${path.sep + path.join('debug', 'path')}`); }); test('On Windows, logger replaces both backwards and forward slash version of path to workspace with . if exactly one workspace folder is opened', async function () { @@ -182,20 +163,14 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} .${path.sep + path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: .${path.sep + path.join('debug', 'path')}`); traceLogStub.resetHistory(); options = { cwd: path.join('path\\to\\workspace', 'debug', 'path') }; logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledWithExactly( - traceLogStub, - `${Logging.currentWorkingDirectory} .${path.sep + path.join('debug', 'path')}`, - ); + sinon.assert.calledWithExactly(traceLogStub, `cwd: .${path.sep + path.join('debug', 'path')}`); }); test("Logger doesn't display the working directory line if there is no options parameter", async () => { diff --git a/src/test/common/terminals/activator/index.unit.test.ts b/src/test/common/terminals/activator/index.unit.test.ts index 9dff5a800cad..6ed47fdbe2d3 100644 --- a/src/test/common/terminals/activator/index.unit.test.ts +++ b/src/test/common/terminals/activator/index.unit.test.ts @@ -12,7 +12,12 @@ import { ITerminalActivator, ITerminalHelper, } from '../../../../client/common/terminal/types'; -import { IConfigurationService, IPythonSettings, ITerminalSettings } from '../../../../client/common/types'; +import { + IConfigurationService, + IExperimentService, + IPythonSettings, + ITerminalSettings, +} from '../../../../client/common/types'; suite('Terminal Activator', () => { let activator: TerminalActivator; @@ -20,12 +25,14 @@ suite('Terminal Activator', () => { let handler1: TypeMoq.IMock; let handler2: TypeMoq.IMock; let terminalSettings: TypeMoq.IMock; + let experimentService: TypeMoq.IMock; setup(() => { baseActivator = TypeMoq.Mock.ofType(); terminalSettings = TypeMoq.Mock.ofType(); handler1 = TypeMoq.Mock.ofType(); handler2 = TypeMoq.Mock.ofType(); const configService = TypeMoq.Mock.ofType(); + experimentService = TypeMoq.Mock.ofType(); configService .setup((c) => c.getSettings(TypeMoq.It.isAny())) .returns(() => { @@ -33,11 +40,17 @@ suite('Terminal Activator', () => { terminal: terminalSettings.object, } as unknown) as IPythonSettings; }); + experimentService.setup((e) => e.inExperimentSync(TypeMoq.It.isAny())).returns(() => false); activator = new (class extends TerminalActivator { protected initialize() { this.baseActivator = baseActivator.object; } - })(TypeMoq.Mock.ofType().object, [handler1.object, handler2.object], configService.object); + })( + TypeMoq.Mock.ofType().object, + [handler1.object, handler2.object], + configService.object, + experimentService.object, + ); }); async function testActivationAndHandlers( activationSuccessful: boolean, diff --git a/src/test/debugger/envVars.test.ts b/src/test/debugger/envVars.test.ts index 71c5b8e62650..6aa0dea4d8c6 100644 --- a/src/test/debugger/envVars.test.ts +++ b/src/test/debugger/envVars.test.ts @@ -76,6 +76,27 @@ suite('Resolving Environment Variables when Debugging', () => { test('Confirm basic environment variables exist when launched in intergrated terminal', () => testBasicProperties('integratedTerminal', 2)); + test('Confirm base environment variables are merged without overwriting when provided', async () => { + const env: Record = { DO_NOT_OVERWRITE: '1' }; + const args = ({ + program: '', + pythonPath: '', + args: [], + envFile: '', + console, + env, + } as any) as LaunchRequestArguments; + + const baseEnvVars = { CONDA_PREFIX: 'path/to/conda/env', DO_NOT_OVERWRITE: '0' }; + const envVars = await debugEnvParser.getEnvironmentVariables(args, baseEnvVars); + expect(envVars).not.be.undefined; + expect(Object.keys(envVars)).lengthOf(4, 'Incorrect number of variables'); + expect(envVars).to.have.property('PYTHONUNBUFFERED', '1', 'Property not found'); + expect(envVars).to.have.property('PYTHONIOENCODING', 'UTF-8', 'Property not found'); + expect(envVars).to.have.property('CONDA_PREFIX', 'path/to/conda/env', 'Property not found'); + expect(envVars).to.have.property('DO_NOT_OVERWRITE', '1', 'Property not found'); + }); + test('Confirm basic environment variables exist when launched in debug console', async () => { let expectedNumberOfVariables = Object.keys(mockProcess.env).length; if (mockProcess.env['PYTHONUNBUFFERED'] === undefined) { diff --git a/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts b/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts index 4830b88a34fb..2aec3dcfd041 100644 --- a/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts +++ b/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts @@ -21,6 +21,7 @@ import { getInfoPerOS } from './common'; import * as platform from '../../../../../client/common/utils/platform'; import * as windowApis from '../../../../../client/common/vscodeApis/windowApis'; import * as workspaceApis from '../../../../../client/common/vscodeApis/workspaceApis'; +import { IEnvironmentActivationService } from '../../../../../client/interpreter/activation/types'; getInfoPerOS().forEach(([osName, osType, path]) => { if (osType === platform.OSType.Unknown) { @@ -31,12 +32,13 @@ getInfoPerOS().forEach(([osName, osType, path]) => { let debugProvider: DebugConfigurationProvider; let pythonExecutionService: TypeMoq.IMock; let helper: TypeMoq.IMock; + const envVars = { FOO: 'BAR' }; let diagnosticsService: TypeMoq.IMock; let configService: TypeMoq.IMock; let debugEnvHelper: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; - + let environmentActivationService: TypeMoq.IMock; let getActiveTextEditorStub: sinon.SinonStub; let getOSTypeStub: sinon.SinonStub; let getWorkspaceFolderStub: sinon.SinonStub; @@ -63,6 +65,10 @@ getInfoPerOS().forEach(([osName, osType, path]) => { } function setupIoc(pythonPath: string, workspaceFolder?: WorkspaceFolder) { + environmentActivationService = TypeMoq.Mock.ofType(); + environmentActivationService + .setup((e) => e.getActivatedEnvironmentVariables(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(envVars)); configService = TypeMoq.Mock.ofType(); diagnosticsService = TypeMoq.Mock.ofType(); debugEnvHelper = TypeMoq.Mock.ofType(); @@ -88,7 +94,7 @@ getInfoPerOS().forEach(([osName, osType, path]) => { } configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); debugEnvHelper - .setup((x) => x.getEnvironmentVariables(TypeMoq.It.isAny())) + .setup((x) => x.getEnvironmentVariables(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve({})); debugProvider = new LaunchConfigurationResolver( @@ -96,6 +102,7 @@ getInfoPerOS().forEach(([osName, osType, path]) => { configService.object, debugEnvHelper.object, interpreterService.object, + environmentActivationService.object, ); } diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts index d50b2b5d5995..002189d412db 100644 --- a/src/test/interpreters/activation/service.unit.test.ts +++ b/src/test/interpreters/activation/service.unit.test.ts @@ -18,7 +18,7 @@ import { ProcessServiceFactory } from '../../../client/common/process/processFac import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types'; import { TerminalHelper } from '../../../client/common/terminal/helper'; import { ITerminalHelper } from '../../../client/common/terminal/types'; -import { ICurrentProcess } from '../../../client/common/types'; +import { ICurrentProcess, Resource } from '../../../client/common/types'; import { getNamesAndValues } from '../../../client/common/utils/enum'; import { Architecture, OSType } from '../../../client/common/utils/platform'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; @@ -48,7 +48,7 @@ suite('Interpreters Activation - Python Environment Variables', () => { let workspace: IWorkspaceService; let interpreterService: IInterpreterService; let onDidChangeEnvVariables: EventEmitter; - let onDidChangeInterpreter: EventEmitter; + let onDidChangeInterpreter: EventEmitter; const pythonInterpreter: PythonEnvironment = { path: '/foo/bar/python.exe', version: new SemVer('3.6.6-final'), @@ -68,7 +68,7 @@ suite('Interpreters Activation - Python Environment Variables', () => { interpreterService = mock(InterpreterService); workspace = mock(WorkspaceService); onDidChangeEnvVariables = new EventEmitter(); - onDidChangeInterpreter = new EventEmitter(); + onDidChangeInterpreter = new EventEmitter(); when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event); when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter); @@ -322,9 +322,6 @@ suite('Interpreters Activation - Python Environment Variables', () => { verify(envVarsService.getEnvironmentVariables(resource)).twice(); verify(processService.shellExec(anything(), anything())).twice(); } - test('Cache Variables get cleared when changing interpreter', async () => { - await testClearingCache(onDidChangeInterpreter.fire.bind(onDidChangeInterpreter)); - }); test('Cache Variables get cleared when changing env variables file', async () => { await testClearingCache(onDidChangeEnvVariables.fire.bind(onDidChangeEnvVariables)); }); diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts new file mode 100644 index 000000000000..4ac04cf1ee22 --- /dev/null +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -0,0 +1,236 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as sinon from 'sinon'; +import { assert, expect } from 'chai'; +import { cloneDeep } from 'lodash'; +import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; +import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; +import { IApplicationShell, IApplicationEnvironment } from '../../../client/common/application/types'; +import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; +import { IPlatformService } from '../../../client/common/platform/types'; +import { IExtensionContext, IExperimentService, Resource } from '../../../client/common/types'; +import { Interpreters } from '../../../client/common/utils/localize'; +import { getOSType } from '../../../client/common/utils/platform'; +import { defaultShells } from '../../../client/interpreter/activation/service'; +import { + TerminalEnvVarCollectionService, + _normCaseKeys, +} from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; +import { IInterpreterService } from '../../../client/interpreter/contracts'; + +suite('Terminal Environment Variable Collection Service', () => { + let platform: IPlatformService; + let interpreterService: IInterpreterService; + let context: IExtensionContext; + let shell: IApplicationShell; + let experimentService: IExperimentService; + let collection: EnvironmentVariableCollection; + let applicationEnvironment: IApplicationEnvironment; + let environmentActivationService: IEnvironmentActivationService; + let terminalEnvVarCollectionService: TerminalEnvVarCollectionService; + const progressOptions = { + location: ProgressLocation.Window, + title: Interpreters.activatingTerminals, + }; + const customShell = 'powershell'; + const defaultShell = defaultShells[getOSType()]; + + setup(() => { + platform = mock(); + when(platform.osType).thenReturn(getOSType()); + interpreterService = mock(); + context = mock(); + shell = mock(); + collection = mock(); + when(context.environmentVariableCollection).thenReturn(instance(collection)); + experimentService = mock(); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); + applicationEnvironment = mock(); + when(applicationEnvironment.shell).thenReturn(customShell); + when(shell.withProgress(anything(), anything())) + .thenCall((options, _) => { + expect(options).to.deep.equal(progressOptions); + }) + .thenResolve(); + environmentActivationService = mock(); + terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( + instance(platform), + instance(interpreterService), + instance(context), + instance(shell), + instance(experimentService), + instance(applicationEnvironment), + [], + instance(environmentActivationService), + ); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Apply activated variables to the collection on activation', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(); + assert(applyCollectionStub.calledOnce, 'Collection not applied on activation'); + }); + + test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { + reset(experimentService); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + + await terminalEnvVarCollectionService.activate(); + + verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never(); + verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); + assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); + + verify(collection.clear()).once(); + }); + + test('When interpreter changes, apply new activated variables to the collection', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + const resource = Uri.file('x'); + let callback: (resource: Resource) => Promise; + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenCall((cb) => { + callback = cb; + }); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(); + + await callback!(resource); + assert(applyCollectionStub.calledWithExactly(resource)); + }); + + test('When selected shell changes, apply new activated variables to the collection', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + let callback: (shell: string) => Promise; + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenCall((cb) => { + callback = cb; + }); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(); + + await callback!(customShell); + assert(applyCollectionStub.calledWithExactly(undefined, customShell)); + }); + + test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + delete envVars.PATH; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything())).thenResolve(); + when(collection.delete(anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); + verify(collection.delete('PATH')).once(); + }); + + test('Only relative changes to previously applied variables are applied to the collection', async () => { + const envVars: NodeJS.ProcessEnv = { + RANDOM_VAR: 'random', + CONDA_PREFIX: 'prefix/to/conda', + ..._normCaseKeys(process.env), + }; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything())).thenResolve(); + when(collection.delete(anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + const newEnvVars = cloneDeep(envVars); + delete newEnvVars.CONDA_PREFIX; + newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. + reset(environmentActivationService); + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(newEnvVars); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.delete('CONDA_PREFIX')).once(); + verify(collection.delete('RANDOM_VAR')).once(); + }); + + test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(undefined); + const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + defaultShell?.shell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything())).thenResolve(); + when(collection.delete(anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); + verify(collection.delete(anything())).never(); + }); + + test('If no activated variables are returned for default shell, clear collection', async () => { + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + defaultShell?.shell, + ), + ).thenResolve(undefined); + + when(collection.replace(anything(), anything())).thenResolve(); + when(collection.delete(anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); + + verify(collection.clear()).once(); + }); +}); diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 680dfecc0277..760c2282ba05 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -24,6 +24,7 @@ import { IPersistentStateFactory, IPythonSettings, Product, + Resource, WORKSPACE_MEMENTO, } from '../../client/common/types'; import { createDeferred } from '../../client/common/utils/async'; @@ -171,12 +172,12 @@ suite('Linting - Provider', () => { }); test('Lint on change interpreters', async () => { - const e = new vscode.EventEmitter(); + const e = new vscode.EventEmitter(); interpreterService.setup((x) => x.onDidChangeInterpreter).returns(() => e.event); const linterProvider = new LinterProvider(serviceContainer); await linterProvider.activate(); - e.fire(); + e.fire(undefined); engine.verify((x) => x.lintOpenPythonFiles(), TypeMoq.Times.once()); }); diff --git a/src/test/providers/terminal.unit.test.ts b/src/test/providers/terminal.unit.test.ts index 9a62b560dc99..603c0710f8c5 100644 --- a/src/test/providers/terminal.unit.test.ts +++ b/src/test/providers/terminal.unit.test.ts @@ -7,9 +7,15 @@ import * as TypeMoq from 'typemoq'; import { Disposable, Terminal, Uri } from 'vscode'; import { IActiveResourceService, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; import { Commands } from '../../client/common/constants'; +import { TerminalEnvVarActivation } from '../../client/common/experiments/groups'; import { TerminalService } from '../../client/common/terminal/service'; import { ITerminalActivator, ITerminalServiceFactory } from '../../client/common/terminal/types'; -import { IConfigurationService, IPythonSettings, ITerminalSettings } from '../../client/common/types'; +import { + IConfigurationService, + IExperimentService, + IPythonSettings, + ITerminalSettings, +} from '../../client/common/types'; import { IServiceContainer } from '../../client/ioc/types'; import { TerminalProvider } from '../../client/providers/terminalProvider'; @@ -18,13 +24,17 @@ suite('Terminal Provider', () => { let commandManager: TypeMoq.IMock; let workspace: TypeMoq.IMock; let activeResourceService: TypeMoq.IMock; + let experimentService: TypeMoq.IMock; let terminalProvider: TerminalProvider; const resource = Uri.parse('a'); setup(() => { serviceContainer = TypeMoq.Mock.ofType(); commandManager = TypeMoq.Mock.ofType(); + experimentService = TypeMoq.Mock.ofType(); + experimentService.setup((e) => e.inExperimentSync(TerminalEnvVarActivation.experiment)).returns(() => false); activeResourceService = TypeMoq.Mock.ofType(); workspace = TypeMoq.Mock.ofType(); + serviceContainer.setup((c) => c.get(IExperimentService)).returns(() => experimentService.object); serviceContainer.setup((c) => c.get(ICommandManager)).returns(() => commandManager.object); serviceContainer.setup((c) => c.get(IWorkspaceService)).returns(() => workspace.object); serviceContainer.setup((c) => c.get(IActiveResourceService)).returns(() => activeResourceService.object); diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index b8841c380308..b8b7d5c55130 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -28,6 +28,7 @@ import { LaunchOptions } from '../../../client/testing/common/types'; import { ITestingSettings } from '../../../client/testing/configuration/types'; import { TestProvider } from '../../../client/testing/types'; import { isOs, OSType } from '../../common'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; use(chaiAsPromised); @@ -39,12 +40,18 @@ suite('Unit Tests - Debug Launcher', () => { let settings: TypeMoq.IMock; let debugEnvHelper: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; + let environmentActivationService: TypeMoq.IMock; let getWorkspaceFolderStub: sinon.SinonStub; let getWorkspaceFoldersStub: sinon.SinonStub; let pathExistsStub: sinon.SinonStub; let readFileStub: sinon.SinonStub; + const envVars = { FOO: 'BAR' }; setup(async () => { + environmentActivationService = TypeMoq.Mock.ofType(); + environmentActivationService + .setup((e) => e.getActivatedEnvironmentVariables(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(envVars)); interpreterService = TypeMoq.Mock.ofType(); serviceContainer = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const configService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); @@ -94,6 +101,7 @@ suite('Unit Tests - Debug Launcher', () => { configService, debugEnvHelper.object, interpreterService.object, + environmentActivationService.object, ); } function setupDebugManager( @@ -110,7 +118,7 @@ suite('Unit Tests - Debug Launcher', () => { expected.args = debugArgs; debugEnvHelper - .setup((d) => d.getEnvironmentVariables(TypeMoq.It.isAny())) + .setup((x) => x.getEnvironmentVariables(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(expected.env)); debugService diff --git a/types/vscode.proposed.envShellEvent.d.ts b/types/vscode.proposed.envShellEvent.d.ts new file mode 100644 index 000000000000..8fed971ef711 --- /dev/null +++ b/types/vscode.proposed.envShellEvent.d.ts @@ -0,0 +1,16 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + // See https://github.com/microsoft/vscode/issues/160694 + export namespace env { + + /** + * An {@link Event} which fires when the default shell changes. + */ + export const onDidChangeShell: Event; + } +}