Make fixes to pythonTerminalEnvVarActivation experiment (#21036)

For #20822

src/client/common/terminal/activator/index.ts

@@ -5,9 +5,10 @@
 
 import { inject, injectable, multiInject } from 'inversify';
 import { Terminal } from 'vscode';
-import { IConfigurationService } from '../../types';
+import { IConfigurationService, IExperimentService } from '../../types';
 import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types';
 import { BaseTerminalActivator } from './base';
+import { inTerminalEnvVarExperiment } from '../../experiments/helpers';
 
 @injectable()
 export class TerminalActivator implements ITerminalActivator {
@@ -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<boolean> {
         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;
         }

src/client/common/variables/environment.ts

@@ -57,7 +57,7 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
     public mergeVariables(
         source: EnvironmentVariables,
         target: EnvironmentVariables,
-        options?: { overwrite?: boolean },
+        options?: { overwrite?: boolean; mergeAll?: boolean },
     ) {
         if (!target) {
             return;
@@ -67,7 +67,7 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         source = normCaseKeys(source);
         const settingsNotToMerge = ['PYTHONPATH', this.pathVariable];
         Object.keys(source).forEach((setting) => {
-            if (settingsNotToMerge.indexOf(setting) >= 0) {
+            if (!options?.mergeAll && settingsNotToMerge.indexOf(setting) >= 0) {
                 return;
             }
             if (target[setting] === undefined || options?.overwrite) {

src/client/common/variables/types.ts

@@ -10,7 +10,11 @@ export const IEnvironmentVariablesService = Symbol('IEnvironmentVariablesService
 export interface IEnvironmentVariablesService {
     parseFile(filePath?: string, baseVars?: EnvironmentVariables): Promise<EnvironmentVariables | undefined>;
     parseFileSync(filePath?: string, baseVars?: EnvironmentVariables): EnvironmentVariables | undefined;
-    mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables, options?: { overwrite?: boolean }): void;
+    mergeVariables(
+        source: EnvironmentVariables,
+        target: EnvironmentVariables,
+        options?: { overwrite?: boolean; mergeAll?: boolean },
+    ): void;
     appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]): void;
     appendPath(vars: EnvironmentVariables, ...paths: string[]): void;
 }

src/client/debugger/extension/configuration/resolvers/helper.ts

@@ -44,7 +44,7 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
         // take precedence over env file.
         this.envParser.mergeVariables(debugLaunchEnvVars, env, { overwrite: true });
         if (baseVars) {
-            this.envParser.mergeVariables(baseVars, env);
+            this.envParser.mergeVariables(baseVars, env, { mergeAll: true });
         }
 
         // Append the PYTHONPATH and PATH variables.

src/client/debugger/extension/configuration/resolvers/launch.ts

@@ -19,6 +19,8 @@ import { getProgram, IDebugEnvironmentVariablesService } from './helper';
 
 @injectable()
 export class LaunchConfigurationResolver extends BaseConfigurationResolver<LaunchRequestArguments> {
+    private isPythonSet = false;
+
     constructor(
         @inject(IDiagnosticsService)
         @named(InvalidPythonPathInDebuggerServiceId)
@@ -36,6 +38,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
         debugConfiguration: LaunchRequestArguments,
         _token?: CancellationToken,
     ): Promise<LaunchRequestArguments | undefined> {
+        this.isPythonSet = debugConfiguration.python !== undefined;
         if (
             debugConfiguration.name === undefined &&
             debugConfiguration.type === undefined &&
@@ -84,7 +87,6 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
         workspaceFolder: Uri | undefined,
         debugConfiguration: LaunchRequestArguments,
     ): Promise<void> {
-        const isPythonSet = debugConfiguration.python !== undefined;
         if (debugConfiguration.python === undefined) {
             debugConfiguration.python = debugConfiguration.pythonPath;
         }
@@ -104,7 +106,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
             debugConfiguration.envFile = settings.envFile;
         }
         let baseEnvVars: EnvironmentVariables | undefined;
-        if (isPythonSet) {
+        if (this.isPythonSet) {
             baseEnvVars = await this.environmentActivationService.getActivatedEnvironmentVariables(
                 workspaceFolder,
                 await this.interpreterService.getInterpreterDetails(debugConfiguration.python ?? ''),

src/client/interpreter/activation/terminalEnvVarCollectionService.ts

@@ -8,7 +8,13 @@ import { IApplicationShell, IApplicationEnvironment } from '../../common/applica
 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 {
+    IExtensionContext,
+    IExperimentService,
+    Resource,
+    IDisposableRegistry,
+    IConfigurationService,
+} from '../../common/types';
 import { Deferred, createDeferred } from '../../common/utils/async';
 import { Interpreters } from '../../common/utils/localize';
 import { traceDecoratorVerbose, traceVerbose } from '../../logging';
@@ -36,6 +42,7 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati
         @inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment,
         @inject(IDisposableRegistry) private disposables: IDisposableRegistry,
         @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService,
+        @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
     ) {}
 
     public async activate(): Promise<void> {
@@ -68,6 +75,11 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati
     }
 
     public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
+        const settings = this.configurationService.getSettings(resource);
+        if (!settings.terminal.activateEnvironment) {
+            traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath);
+            return;
+        }
         const env = await this.environmentActivationService.getActivatedEnvironmentVariables(
             resource,
             undefined,

src/test/common/terminals/activator/index.unit.test.ts

@@ -12,17 +12,25 @@ 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;
     let baseActivator: TypeMoq.IMock<ITerminalActivator>;
     let handler1: TypeMoq.IMock<ITerminalActivationHandler>;
     let handler2: TypeMoq.IMock<ITerminalActivationHandler>;
     let terminalSettings: TypeMoq.IMock<ITerminalSettings>;
+    let experimentService: TypeMoq.IMock<IExperimentService>;
     setup(() => {
         baseActivator = TypeMoq.Mock.ofType<ITerminalActivator>();
         terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
+        experimentService = TypeMoq.Mock.ofType<IExperimentService>();
+        experimentService.setup((e) => e.inExperimentSync(TypeMoq.It.isAny())).returns(() => false);
         handler1 = TypeMoq.Mock.ofType<ITerminalActivationHandler>();
         handler2 = TypeMoq.Mock.ofType<ITerminalActivationHandler>();
         const configService = TypeMoq.Mock.ofType<IConfigurationService>();
@@ -37,7 +45,12 @@ suite('Terminal Activator', () => {
             protected initialize() {
                 this.baseActivator = baseActivator.object;
             }
-        })(TypeMoq.Mock.ofType<ITerminalHelper>().object, [handler1.object, handler2.object], configService.object);
+        })(
+            TypeMoq.Mock.ofType<ITerminalHelper>().object,
+            [handler1.object, handler2.object],
+            configService.object,
+            experimentService.object,
+        );
     });
     async function testActivationAndHandlers(
         activationSuccessful: boolean,

src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

@@ -11,7 +11,13 @@ 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 {
+    IExtensionContext,
+    IExperimentService,
+    Resource,
+    IConfigurationService,
+    IPythonSettings,
+} 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';
@@ -57,6 +63,10 @@ suite('Terminal Environment Variable Collection Service', () => {
             })
             .thenResolve();
         environmentActivationService = mock<IEnvironmentActivationService>();
+        const configService = mock<IConfigurationService>();
+        when(configService.getSettings(anything())).thenReturn(({
+            terminal: { activateEnvironment: true },
+        } as unknown) as IPythonSettings);
         terminalEnvVarCollectionService = new TerminalEnvVarCollectionService(
             instance(platform),
             instance(interpreterService),
@@ -66,6 +76,7 @@ suite('Terminal Environment Variable Collection Service', () => {
             instance(applicationEnvironment),
             [],
             instance(environmentActivationService),
+            instance(configService),
         );
     });