microsoft · karrtikr · Sep 10, 2021 · Sep 10, 2021
diff --git a/news/2 Fixes/17370.md b/news/2 Fixes/17370.md
@@ -0,0 +1 @@
+Ensure we block getting active interpreter on auto-selection.
diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts
@@ -12,7 +12,7 @@ import { inDiscoveryExperiment } from '../experiments/helpers';
 import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { IFileSystem } from '../platform/types';
-import { IConfigurationService, IDisposableRegistry, IExperimentService } from '../types';
+import { IConfigurationService, IDisposableRegistry, IExperimentService, IInterpreterPathProxyService } from '../types';
 import { ProcessService } from './proc';
 import { createCondaEnv, createPythonEnv, createWindowsStoreEnv } from './pythonEnvironment';
 import { createPythonProcessService } from './pythonProcess';
@@ -27,6 +27,7 @@ import {
     IPythonExecutionService,
 } from './types';
 import { isWindowsStoreInterpreter } from '../../pythonEnvironments/discovery/locators/services/windowsStoreInterpreter';
+import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelection/types';
 
 // Minimum version number of conda required to be able to use 'conda run'
 export const CONDA_RUN_VERSION = '4.6.0';
@@ -48,6 +49,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         @inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
         @inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
         @inject(IExperimentService) private readonly experimentService: IExperimentService,
+        @inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
+        @inject(IInterpreterPathProxyService) private readonly interpreterPathExpHelper: IInterpreterPathProxyService,
     ) {
         // Acquire other objects here so that if we are called during dispose they are available.
         this.disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
@@ -56,6 +59,10 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
     }
 
     public async create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService> {
+        const interpreterPath = this.interpreterPathExpHelper.get(options.resource);
+        if (!interpreterPath || interpreterPath === 'python') {
+            await this.autoSelection.autoSelectInterpreter(options.resource); // Block on this only if no interpreter selected.
+        }
         const pythonPath = options.pythonPath
             ? options.pythonPath
             : this.configService.getSettings(options.resource).pythonPath;

diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts
@@ -3,17 +3,10 @@ import { Disposable, OutputChannel, StatusBarAlignment, StatusBarItem, Uri } fro
 import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
 import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
 import '../../common/extensions';
-import {
-    IDisposableRegistry,
-    IInterpreterPathProxyService,
-    IOutputChannel,
-    IPathUtils,
-    Resource,
-} from '../../common/types';
+import { IDisposableRegistry, IOutputChannel, IPathUtils, Resource } from '../../common/types';
 import { Interpreters } from '../../common/utils/localize';
 import { IServiceContainer } from '../../ioc/types';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
-import { IInterpreterAutoSelectionService } from '../autoSelection/types';
 import {
     IInterpreterDisplay,
     IInterpreterHelper,
@@ -29,10 +22,8 @@ export class InterpreterDisplay implements IInterpreterDisplay {
     private readonly workspaceService: IWorkspaceService;
     private readonly pathUtils: IPathUtils;
     private readonly interpreterService: IInterpreterService;
-    private readonly interpreterPathExpHelper: IInterpreterPathProxyService;
     private currentlySelectedInterpreterPath?: string;
     private currentlySelectedWorkspaceFolder: Resource;
-    private readonly autoSelection: IInterpreterAutoSelectionService;
     private interpreterPath: string | undefined;
     private statusBarCanBeDisplayed?: boolean;
     private visibilityFilters: IInterpreterStatusbarVisibilityFilter[] = [];
@@ -43,14 +34,10 @@ export class InterpreterDisplay implements IInterpreterDisplay {
         this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         this.pathUtils = serviceContainer.get<IPathUtils>(IPathUtils);
         this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
-        this.autoSelection = serviceContainer.get<IInterpreterAutoSelectionService>(IInterpreterAutoSelectionService);
         this.python27SupportPrompt = serviceContainer.get<IPython27SupportPrompt>(IPython27SupportPrompt);
 
         const application = serviceContainer.get<IApplicationShell>(IApplicationShell);
         const disposableRegistry = serviceContainer.get<Disposable[]>(IDisposableRegistry);
-        this.interpreterPathExpHelper = serviceContainer.get<IInterpreterPathProxyService>(
-            IInterpreterPathProxyService,
-        );
 
         this.statusBar = application.createStatusBarItem(StatusBarAlignment.Left, 100);
         this.statusBar.command = 'python.setInterpreter';
@@ -86,10 +73,6 @@ export class InterpreterDisplay implements IInterpreterDisplay {
         }
     }
     private async updateDisplay(workspaceFolder?: Uri) {
-        const interpreterPath = this.interpreterPathExpHelper.get(workspaceFolder);
-        if (!interpreterPath || interpreterPath === 'python') {
-            await this.autoSelection.autoSelectInterpreter(workspaceFolder); // Block on this only if no interpreter selected.
-        }
         const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder);
         this.currentlySelectedWorkspaceFolder = workspaceFolder;
         if (interpreter) {

diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts
@@ -103,6 +103,7 @@ import {
     IFileDownloader,
     IHttpClient,
     IInstaller,
+    IInterpreterPathProxyService,
     IInterpreterPathService,
     IPathUtils,
     IPersistentStateFactory,
@@ -122,6 +123,7 @@ import { MockModuleInstaller } from '../mocks/moduleInstaller';
 import { MockProcessService } from '../mocks/proc';
 import { UnitTestIocContainer } from '../testing/serviceRegistry';
 import { closeActiveWindows, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize';
+import { InterpreterPathProxyService } from '../../client/common/interpreterPathProxyService';
 
 suite('Installer', () => {
     let ioc: UnitTestIocContainer;
@@ -203,6 +205,10 @@ suite('Installer', () => {
         );
         ioc.serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
         ioc.serviceManager.addSingleton<IInterpreterPathService>(IInterpreterPathService, InterpreterPathService);
+        ioc.serviceManager.addSingleton<IInterpreterPathProxyService>(
+            IInterpreterPathProxyService,
+            InterpreterPathProxyService,
+        );
         ioc.serviceManager.addSingleton<IExtensions>(IExtensions, Extensions);
         ioc.serviceManager.addSingleton<IRandom>(IRandom, Random);
         ioc.serviceManager.addSingleton<ITerminalServiceFactory>(ITerminalServiceFactory, TerminalServiceFactory);

diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts
@@ -102,6 +102,7 @@ import {
     IFileDownloader,
     IHttpClient,
     IInstaller,
+    IInterpreterPathProxyService,
     IInterpreterPathService,
     IPathUtils,
     IPersistentStateFactory,
@@ -131,6 +132,7 @@ import { MockModuleInstaller } from '../mocks/moduleInstaller';
 import { MockProcessService } from '../mocks/proc';
 import { UnitTestIocContainer } from '../testing/serviceRegistry';
 import { closeActiveWindows, initializeTest } from '../initialize';
+import { InterpreterPathProxyService } from '../../client/common/interpreterPathProxyService';
 
 chaiUse(chaiAsPromised);
 
@@ -228,6 +230,10 @@ suite('Module Installer', () => {
 
             ioc.serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
             ioc.serviceManager.addSingleton<IInterpreterPathService>(IInterpreterPathService, InterpreterPathService);
+            ioc.serviceManager.addSingleton<IInterpreterPathProxyService>(
+                IInterpreterPathProxyService,
+                InterpreterPathProxyService,
+            );
             ioc.serviceManager.addSingleton<IExtensions>(IExtensions, Extensions);
             ioc.serviceManager.addSingleton<IRandom>(IRandom, Random);
             ioc.serviceManager.addSingleton<IApplicationShell>(IApplicationShell, ApplicationShell);

diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts
@@ -7,7 +7,7 @@ import * as assert from 'assert';
 import { expect } from 'chai';
 import { SemVer } from 'semver';
 import * as sinon from 'sinon';
-import { anyString, anything, instance, mock, verify, when } from 'ts-mockito';
+import { anyString, anything, instance, mock, reset, verify, when } from 'ts-mockito';
 import * as typemoq from 'typemoq';
 import { Uri } from 'vscode';
 
@@ -24,7 +24,12 @@ import {
     IProcessServiceFactory,
     IPythonExecutionService,
 } from '../../../client/common/process/types';
-import { IConfigurationService, IDisposableRegistry, IExperimentService } from '../../../client/common/types';
+import {
+    IConfigurationService,
+    IDisposableRegistry,
+    IExperimentService,
+    IInterpreterPathProxyService,
+} from '../../../client/common/types';
 import { Architecture } from '../../../client/common/utils/platform';
 import { EnvironmentActivationService } from '../../../client/interpreter/activation/service';
 import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types';
@@ -42,6 +47,7 @@ import * as ExperimentHelpers from '../../../client/common/experiments/helpers';
 import * as WindowsStoreInterpreter from '../../../client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter';
 import { ExperimentService } from '../../../client/common/experiments/service';
 import { DiscoveryVariants } from '../../../client/common/experiments/groups';
+import { IInterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection/types';
 
 const pythonInterpreter: PythonEnvironment = {
     path: '/foo/bar/python.exe',
@@ -96,7 +102,8 @@ suite('Process - PythonExecutionFactory', () => {
             let executionService: typemoq.IMock<IPythonExecutionService>;
             let isWindowsStoreInterpreterStub: sinon.SinonStub;
             let inDiscoveryExperimentStub: sinon.SinonStub;
-
+            let autoSelection: IInterpreterAutoSelectionService;
+            let interpreterPathExpHelper: IInterpreterPathProxyService;
             setup(() => {
                 bufferDecoder = mock(BufferDecoder);
                 activationHelper = mock(EnvironmentActivationService);
@@ -105,6 +112,9 @@ suite('Process - PythonExecutionFactory', () => {
                 condaService = mock(CondaService);
                 condaLocatorService = mock<ICondaLocatorService>();
                 processLogger = mock(ProcessLogger);
+                autoSelection = mock<IInterpreterAutoSelectionService>();
+                interpreterPathExpHelper = mock<IInterpreterPathProxyService>();
+                when(interpreterPathExpHelper.get(anything())).thenReturn('selected interpreter path');
                 experimentService = mock(ExperimentService);
                 when(experimentService.inExperiment(DiscoveryVariants.discoverWithFileWatching)).thenResolve(false);
                 when(experimentService.inExperiment(DiscoveryVariants.discoveryWithoutFileWatching)).thenResolve(false);
@@ -152,6 +162,8 @@ suite('Process - PythonExecutionFactory', () => {
                     instance(bufferDecoder),
                     instance(pyenvs),
                     instance(experimentService),
+                    instance(autoSelection),
+                    instance(interpreterPathExpHelper),
                 );
 
                 isWindowsStoreInterpreterStub = sinon.stub(WindowsStoreInterpreter, 'isWindowsStoreInterpreter');
@@ -175,6 +187,25 @@ suite('Process - PythonExecutionFactory', () => {
                 verify(processFactory.create(resource)).once();
                 verify(pythonSettings.pythonPath).once();
             });
+
+            test('If no interpreter is explicitly set, ensure we autoselect before PythonExecutionService is created', async () => {
+                const pythonSettings = mock(PythonSettings);
+                when(processFactory.create(resource)).thenResolve(processService.object);
+                when(activationHelper.getActivatedEnvironmentVariables(resource)).thenResolve({ x: '1' });
+                when(pythonSettings.pythonPath).thenReturn('HELLO');
+                reset(interpreterPathExpHelper);
+                when(interpreterPathExpHelper.get(anything())).thenReturn('python');
+                when(autoSelection.autoSelectInterpreter(anything())).thenResolve();
+                when(configService.getSettings(resource)).thenReturn(instance(pythonSettings));
+
+                const service = await factory.create({ resource });
+
+                expect(service).to.not.equal(undefined);
+                verify(autoSelection.autoSelectInterpreter(anything())).once();
+                verify(processFactory.create(resource)).once();
+                verify(pythonSettings.pythonPath).once();
+            });
+
             test('Ensure we use an existing `create` method if there are no environment variables for the activated env', async () => {
                 const pythonPath = 'path/to/python';
                 const pythonSettings = mock(PythonSettings);

diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts
@@ -1,7 +1,6 @@
 import { expect } from 'chai';
 import * as path from 'path';
 import { SemVer } from 'semver';
-import { anything, instance, mock, verify, when } from 'ts-mockito';
 import * as TypeMoq from 'typemoq';
 import {
     ConfigurationTarget,
@@ -15,17 +14,9 @@ import {
 import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types';
 import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants';
 import { IFileSystem } from '../../client/common/platform/types';
-import {
-    IInterpreterPathProxyService,
-    IDisposableRegistry,
-    IOutputChannel,
-    IPathUtils,
-    ReadWrite,
-} from '../../client/common/types';
+import { IDisposableRegistry, IOutputChannel, IPathUtils, ReadWrite } from '../../client/common/types';
 import { Interpreters } from '../../client/common/utils/localize';
 import { Architecture } from '../../client/common/utils/platform';
-import { InterpreterAutoSelectionService } from '../../client/interpreter/autoSelection';
-import { IInterpreterAutoSelectionService } from '../../client/interpreter/autoSelection/types';
 import {
     IInterpreterDisplay,
     IInterpreterHelper,
@@ -57,12 +48,10 @@ suite('Interpreters Display', () => {
     let fileSystem: TypeMoq.IMock<IFileSystem>;
     let disposableRegistry: Disposable[];
     let statusBar: TypeMoq.IMock<StatusBarItem>;
-    let interpreterPathProxyService: TypeMoq.IMock<IInterpreterPathProxyService>;
     let interpreterDisplay: IInterpreterDisplay;
     let interpreterHelper: TypeMoq.IMock<IInterpreterHelper>;
     let pathUtils: TypeMoq.IMock<IPathUtils>;
     let output: TypeMoq.IMock<IOutputChannel>;
-    let autoSelection: IInterpreterAutoSelectionService;
     let python27SupportPrompt: TypeMoq.IMock<IPython27SupportPrompt>;
 
     setup(() => {
@@ -74,11 +63,9 @@ suite('Interpreters Display', () => {
         interpreterHelper = TypeMoq.Mock.ofType<IInterpreterHelper>();
         disposableRegistry = [];
         statusBar = TypeMoq.Mock.ofType<StatusBarItem>();
-        interpreterPathProxyService = TypeMoq.Mock.ofType<IInterpreterPathProxyService>();
         pathUtils = TypeMoq.Mock.ofType<IPathUtils>();
         output = TypeMoq.Mock.ofType<IOutputChannel>();
         python27SupportPrompt = TypeMoq.Mock.ofType<IPython27SupportPrompt>();
-        autoSelection = mock(InterpreterAutoSelectionService);
 
         serviceContainer
             .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), STANDARD_OUTPUT_CHANNEL))
@@ -94,16 +81,10 @@ suite('Interpreters Display', () => {
             .returns(() => interpreterService.object);
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object);
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => disposableRegistry);
-        serviceContainer
-            .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterPathProxyService)))
-            .returns(() => interpreterPathProxyService.object);
         serviceContainer
             .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterHelper)))
             .returns(() => interpreterHelper.object);
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPathUtils))).returns(() => pathUtils.object);
-        serviceContainer
-            .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterAutoSelectionService)))
-            .returns(() => instance(autoSelection));
         serviceContainer
             .setup((c) => c.get(TypeMoq.It.isValue(IPython27SupportPrompt)))
             .returns(() => python27SupportPrompt.object);
@@ -148,7 +129,6 @@ suite('Interpreters Display', () => {
             path: path.join('user', 'development', 'env', 'bin', 'python'),
         };
         setupWorkspaceFolder(resource, workspaceFolder);
-        when(autoSelection.autoSelectInterpreter(anything())).thenResolve();
         interpreterService
             .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder)))
             .returns(() => Promise.resolve([]));
@@ -158,7 +138,6 @@ suite('Interpreters Display', () => {
 
         await interpreterDisplay.refresh(resource);
 
-        verify(autoSelection.autoSelectInterpreter(anything())).once();
         statusBar.verify((s) => (s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!), TypeMoq.Times.once());
         statusBar.verify((s) => (s.tooltip = TypeMoq.It.isValue(activeInterpreter.path)!), TypeMoq.Times.atLeastOnce());
     });
@@ -175,7 +154,6 @@ suite('Interpreters Display', () => {
             .setup((p) => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
             .returns(() => activeInterpreter.path);
         setupWorkspaceFolder(resource, workspaceFolder);
-        when(autoSelection.autoSelectInterpreter(anything())).thenResolve();
         interpreterService
             .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder)))
             .returns(() => Promise.resolve([]));
@@ -223,7 +201,6 @@ suite('Interpreters Display', () => {
         interpreterService
             .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder)))
             .returns(() => Promise.resolve(undefined));
-        interpreterPathProxyService.setup((c) => c.get(TypeMoq.It.isAny())).returns(() => pythonPath);
         fileSystem.setup((f) => f.fileExists(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(false));
         interpreterHelper
             .setup((v) => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath)))
@@ -278,7 +255,6 @@ suite('Interpreters Display', () => {
                 path: path.join('user', 'development', 'env', 'bin', 'python'),
             };
             setupWorkspaceFolder(resource, workspaceFolder);
-            when(autoSelection.autoSelectInterpreter(anything())).thenResolve();
             interpreterService
                 .setup((i) => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder)))
                 .returns(() => Promise.resolve([]));
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Ensure we block getting active interpreter on auto-selection.