Getting env vars of activated environments should ignore `term… (microsoft#11193)

* Disable activation of terminals higher up * News file * Fix tests

Author: DonJayamanne

news/2 Fixes/10370.md

@@ -0,0 +1 @@
+Getting environment variables of activated environments should ignore the setting `python.terminal.activateEnvironment`.

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

@@ -5,6 +5,7 @@
 
 import { inject, injectable, multiInject } from 'inversify';
 import { Terminal } from 'vscode';
+import { IConfigurationService } from '../../types';
 import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types';
 import { BaseTerminalActivator } from './base';
 
@@ -13,14 +14,21 @@ export class TerminalActivator implements ITerminalActivator {
  protected baseActivator!: ITerminalActivator;
  constructor(
  @inject(ITerminalHelper) readonly helper: ITerminalHelper,
- @multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[]
+ @multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[],
+ @inject(IConfigurationService) private readonly configurationService: IConfigurationService
  ) {
  this.initialize();
  }
  public async activateEnvironmentInTerminal(
  terminal: Terminal,
  options?: TerminalActivationOptions
  ): Promise<boolean> {
+ const settings = this.configurationService.getSettings(options?.resource);
+ const activateEnvironment = settings.terminal.activateEnvironment;
+ if (!activateEnvironment) {
+ return false;
+ }
+
  const activated = await this.baseActivator.activateEnvironmentInTerminal(terminal, options);
  this.handlers.forEach((handler) =>
  handler

src/client/common/terminal/helper.ts

@@ -125,10 +125,6 @@ export class TerminalHelper implements ITerminalHelper {
  providers: ITerminalActivationCommandProvider[]
  ): Promise<string[] | undefined> {
  const settings = this.configurationService.getSettings(resource);
- const activateEnvironment = settings.terminal.activateEnvironment;
- if (!activateEnvironment) {
- return;
- }
 
  // If we have a conda environment, then use that.
  const isCondaEnvironment = interpreter

src/test/common/terminals/activation.conda.unit.test.ts

@@ -112,16 +112,6 @@ suite('Terminal Environment Activation conda', () => {
  });
  });
 
- test('Ensure no activation commands are returned if the feature is disabled', async () => {
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => false);
-
- const activationCommands = await terminalHelper.getEnvironmentActivationCommands(
- TerminalShellType.bash,
- undefined
- );
- expect(activationCommands).to.equal(undefined, 'Activation commands should be undefined');
- });
-
  test('Conda activation for fish escapes spaces in conda filename', async () => {
  conda = 'path to conda';
  const envName = 'EnvA';
@@ -283,7 +273,6 @@ suite('Terminal Environment Activation conda', () => {
  shellType: TerminalShellType,
  envName: string
  ) {
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => true);
  platformService.setup((p) => p.isLinux).returns(() => isLinux);
  platformService.setup((p) => p.isWindows).returns(() => isWindows);
  platformService.setup((p) => p.isMac).returns(() => isOsx);
@@ -384,7 +373,6 @@ suite('Terminal Environment Activation conda', () => {
  isLinux: boolean,
  pythonPath: string
  ) {
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => true);
  platformService.setup((p) => p.isLinux).returns(() => isLinux);
  platformService.setup((p) => p.isWindows).returns(() => isWindows);
  platformService.setup((p) => p.isMac).returns(() => isOsx);
@@ -432,7 +420,6 @@ suite('Terminal Environment Activation conda', () => {
 
  test('Get activation script command if environment is not a conda environment', async () => {
  const pythonPath = path.join('users', 'xyz', '.conda', 'envs', 'enva', 'bin', 'python');
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => true);
  condaService.setup((c) => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false));
  pythonSettings.setup((s) => s.pythonPath).returns(() => pythonPath);
 
@@ -462,7 +449,6 @@ suite('Terminal Environment Activation conda', () => {
  isLinux: boolean,
  pythonPath: string
  ) {
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => true);
  platformService.setup((p) => p.isLinux).returns(() => isLinux);
  platformService.setup((p) => p.isWindows).returns(() => isWindows);
  platformService.setup((p) => p.isMac).returns(() => isOsx);
@@ -512,7 +498,6 @@ suite('Terminal Environment Activation conda', () => {
  test('Return undefined if unable to get activation command', async () => {
  const pythonPath = path.join('c', 'users', 'xyz', '.conda', 'envs', 'enva', 'python.exe');
 
- terminalSettings.setup((t) => t.activateEnvironment).returns(() => true);
  condaService.setup((c) => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false));
 
  pythonSettings.setup((s) => s.pythonPath).returns(() => pythonPath);

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

@@ -3,6 +3,7 @@
 
 'use strict';
 
+import { assert } from 'chai';
 import * as TypeMoq from 'typemoq';
 import { Terminal } from 'vscode';
 import { TerminalActivator } from '../../../../client/common/terminal/activator';
@@ -11,29 +12,43 @@ import {
  ITerminalActivator,
  ITerminalHelper
 } from '../../../../client/common/terminal/types';
+import { IConfigurationService, IPythonSettings, ITerminalSettings } from '../../../../client/common/types';
 
 // tslint:disable-next-line:max-func-body-length
 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>;
  setup(() => {
  baseActivator = TypeMoq.Mock.ofType<ITerminalActivator>();
+ terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
  handler1 = TypeMoq.Mock.ofType<ITerminalActivationHandler>();
  handler2 = TypeMoq.Mock.ofType<ITerminalActivationHandler>();
+ const configService = TypeMoq.Mock.ofType<IConfigurationService>();
+ configService
+ .setup((c) => c.getSettings(TypeMoq.It.isAny()))
+ .returns(() => {
+ return ({
+ terminal: terminalSettings.object
+ } as unknown) as IPythonSettings;
+ });
  activator = new (class extends TerminalActivator {
  protected initialize() {
  this.baseActivator = baseActivator.object;
  }
- })(TypeMoq.Mock.ofType<ITerminalHelper>().object, [handler1.object, handler2.object]);
+ })(TypeMoq.Mock.ofType<ITerminalHelper>().object, [handler1.object, handler2.object], configService.object);
  });
  async function testActivationAndHandlers(activationSuccessful: boolean) {
+ terminalSettings
+ .setup((b) => b.activateEnvironment)
+ .returns(() => activationSuccessful)
+ .verifiable(TypeMoq.Times.once());
  baseActivator
  .setup((b) => b.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
  .returns(() => Promise.resolve(activationSuccessful))
- .verifiable(TypeMoq.Times.once());
+ .verifiable(TypeMoq.Times.exactly(activationSuccessful ? 1 : 0));
  handler1
  .setup((h) =>
  h.handleActivation(
@@ -44,7 +59,7 @@ suite('Terminal Activator', () => {
  )
  )
  .returns(() => Promise.resolve())
- .verifiable(TypeMoq.Times.once());
+ .verifiable(TypeMoq.Times.exactly(activationSuccessful ? 1 : 0));
  handler2
  .setup((h) =>
  h.handleActivation(
@@ -55,11 +70,14 @@ suite('Terminal Activator', () => {
  )
  )
  .returns(() => Promise.resolve())
- .verifiable(TypeMoq.Times.once());
+ .verifiable(TypeMoq.Times.exactly(activationSuccessful ? 1 : 0));
 
  const terminal = TypeMoq.Mock.ofType<Terminal>();
- await activator.activateEnvironmentInTerminal(terminal.object, { preserveFocus: activationSuccessful });
+ const activated = await activator.activateEnvironmentInTerminal(terminal.object, {
+ preserveFocus: activationSuccessful
+ });
 
+ assert.equal(activated, activationSuccessful);
  baseActivator.verifyAll();
  handler1.verifyAll();
  handler2.verifyAll();

src/test/common/terminals/helper.unit.test.ts

@@ -167,14 +167,6 @@ suite('Terminal Service helpers', () => {
  doSetup();
  when(configurationService.getSettings(resource)).thenReturn(instance(pythonSettings));
  });
- test('Activation command must be empty if activation of terminals is disabled', async () => {
- when(pythonSettings.terminal).thenReturn({ activateEnvironment: false } as any);
-
- const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
-
- expect(cmd).to.equal(undefined, 'Command must be undefined');
- verify(pythonSettings.terminal).once();
- });
  function ensureCondaIsSupported(
  isSupported: boolean,
  pythonPath: string,
@@ -195,7 +187,6 @@ suite('Terminal Service helpers', () => {
  const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
 
  expect(cmd).to.equal(condaActivationCommands);
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(condaActivationProvider.getActivationCommands(resource, anything())).once();
@@ -215,7 +206,6 @@ suite('Terminal Service helpers', () => {
  );
 
  expect(cmd).to.equal(undefined, 'Command must be undefined');
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(bashActivationProvider.isShellSupported(anything())).atLeast(1);
@@ -238,7 +228,6 @@ suite('Terminal Service helpers', () => {
  const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
 
  expect(cmd).to.deep.equal(expectCommand);
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(bashActivationProvider.isShellSupported(anything())).atLeast(1);
@@ -265,7 +254,6 @@ suite('Terminal Service helpers', () => {
  const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
 
  expect(cmd).to.deep.equal(expectCommand);
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(bashActivationProvider.isShellSupported(anything())).atLeast(1);
@@ -290,7 +278,6 @@ suite('Terminal Service helpers', () => {
  const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
 
  expect(cmd).to.deep.equal(expectCommand);
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(bashActivationProvider.isShellSupported(anything())).atLeast(1);
@@ -315,7 +302,6 @@ suite('Terminal Service helpers', () => {
  const cmd = await helper.getEnvironmentActivationCommands(anything(), resource);
 
  expect(cmd).to.deep.equal(expectCommand);
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).once();
  verify(condaService.isCondaEnvironment(pythonPath)).once();
  verify(bashActivationProvider.isShellSupported(anything())).atLeast(1);
@@ -359,7 +345,6 @@ suite('Terminal Service helpers', () => {
  );
 
  expect(cmd).to.equal(undefined, 'Command must be undefined');
- verify(pythonSettings.terminal).once();
  verify(pythonSettings.pythonPath).times(interpreter ? 0 : 1);
  verify(condaService.isCondaEnvironment(pythonPath)).times(interpreter ? 0 : 1);
  verify(bashActivationProvider.isShellSupported(shellToExpect)).atLeast(1);