From 70f3e60e24d07f6a0bbdb1e77ce9ebb78213d92a Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 23 Apr 2020 13:50:40 -0700 Subject: [PATCH 1/8] Add onActivation option to getInterpreters() --- src/client/activation/activationManager.ts | 2 +- .../interpreter/autoSelection/rules/system.ts | 2 +- .../autoSelection/rules/workspaceEnv.ts | 20 +-- src/client/interpreter/contracts.ts | 11 +- src/client/interpreter/interpreterService.ts | 5 +- src/client/interpreter/locators/index.ts | 13 +- .../services/cacheableLocatorService.ts | 13 +- src/client/startupTelemetry.ts | 4 +- .../activation/activationManager.unit.test.ts | 8 +- .../rules/currentPath.unit.test.ts | 2 +- .../autoSelection/rules/system.unit.test.ts | 15 ++- .../rules/workspaceEnv.unit.test.ts | 121 ++---------------- 12 files changed, 62 insertions(+), 154 deletions(-) diff --git a/src/client/activation/activationManager.ts b/src/client/activation/activationManager.ts index f570eb05aa3a..4a5333c6111b 100644 --- a/src/client/activation/activationManager.ts +++ b/src/client/activation/activationManager.ts @@ -73,7 +73,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); // Get latest interpreter list in the background. - this.interpreterService.getInterpreters(resource).ignoreErrors(); + this.interpreterService.getInterpreters(resource, { onActivation: true }).ignoreErrors(); await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource); diff --git a/src/client/interpreter/autoSelection/rules/system.ts b/src/client/interpreter/autoSelection/rules/system.ts index 9feb4925b9a4..6785d83a9fa8 100644 --- a/src/client/interpreter/autoSelection/rules/system.ts +++ b/src/client/interpreter/autoSelection/rules/system.ts @@ -25,7 +25,7 @@ export class SystemWideInterpretersAutoSelectionRule extends BaseRuleService { resource: Resource, manager?: IInterpreterAutoSelectionService ): Promise { - const interpreters = await this.interpreterService.getInterpreters(resource); + const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true }); // Exclude non-local interpreters. const filteredInterpreters = interpreters.filter( (int) => diff --git a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts index 7233dce8308e..92ec8c067749 100644 --- a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts +++ b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts @@ -15,7 +15,6 @@ import { OSType } from '../../../common/utils/platform'; import { IInterpreterHelper, IInterpreterLocatorService, - PIPENV_SERVICE, PythonInterpreter, WORKSPACE_VIRTUAL_ENV_SERVICE } from '../../contracts'; @@ -31,9 +30,6 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IInterpreterLocatorService) - @named(PIPENV_SERVICE) - private readonly pipEnvInterpreterLocator: IInterpreterLocatorService, - @inject(IInterpreterLocatorService) @named(WORKSPACE_VIRTUAL_ENV_SERVICE) private readonly workspaceVirtualEnvInterpreterLocator: IInterpreterLocatorService, @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, @@ -59,24 +55,18 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe if (pythonPathInConfig.workspaceFolderValue || pythonPathInConfig.workspaceValue) { return NextAction.runNextRule; } - const pipEnvPromise = createDeferredFromPromise( - this.pipEnvInterpreterLocator.getInterpreters(workspacePath.folderUri, true) - ); const virtualEnvPromise = createDeferredFromPromise( this.getWorkspaceVirtualEnvInterpreters(workspacePath.folderUri) ); - // Use only one, we currently do not have support for both pipenv and virtual env in same workspace. - // If users have this, then theu can specify which one is to be used. - const interpreters = await Promise.race([pipEnvPromise.promise, virtualEnvPromise.promise]); + const interpreters = await virtualEnvPromise.promise; let bestInterpreter: PythonInterpreter | undefined; if (Array.isArray(interpreters) && interpreters.length > 0) { bestInterpreter = this.helper.getBestInterpreter(interpreters); } else { - const [pipEnv, virtualEnv] = await Promise.all([pipEnvPromise.promise, virtualEnvPromise.promise]); - const pipEnvList = Array.isArray(pipEnv) ? pipEnv : []; + const virtualEnv = await virtualEnvPromise.promise; const virtualEnvList = Array.isArray(virtualEnv) ? virtualEnv : []; - bestInterpreter = this.helper.getBestInterpreter(pipEnvList.concat(virtualEnvList)); + bestInterpreter = this.helper.getBestInterpreter(virtualEnvList); } if (bestInterpreter && manager) { await super.cacheSelectedInterpreter(workspacePath.folderUri, bestInterpreter); @@ -99,7 +89,9 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe return; } // Now check virtual environments under the workspace root - const interpreters = await this.workspaceVirtualEnvInterpreterLocator.getInterpreters(resource, true); + const interpreters = await this.workspaceVirtualEnvInterpreterLocator.getInterpreters(resource, { + ignoreCache: true + }); const workspacePath = this.platform.osType === OSType.Windows ? workspaceFolder.uri.fsPath.toUpperCase() diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index 32837df1b8bd..cfea89200a91 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -26,12 +26,19 @@ export const IVirtualEnvironmentsSearchPathProvider = Symbol('IVirtualEnvironmen export interface IVirtualEnvironmentsSearchPathProvider { getSearchPaths(resource?: Uri): Promise; } + +export type GetInterpreterOptions = { + onActivation?: boolean; +}; + +export type GetInterpreterLocatorOptions = GetInterpreterOptions & { ignoreCache?: boolean }; + export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService'); export interface IInterpreterLocatorService extends Disposable { readonly onLocating: Event>; readonly hasInterpreters: Promise; - getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise; + getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise; } export type CondaInfo = { @@ -91,7 +98,7 @@ export interface IInterpreterService { onDidChangeInterpreter: Event; onDidChangeInterpreterInformation: Event; hasInterpreters: Promise; - getInterpreters(resource?: Uri): Promise; + getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise; getActiveInterpreter(resource?: Uri): Promise; getInterpreterDetails(pythonPath: string, resoure?: Uri): Promise; refresh(resource: Resource): Promise; diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 81d6330d0aaa..6eef287fe8e4 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -23,6 +23,7 @@ import { IServiceContainer } from '../ioc/types'; import { captureTelemetry } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { + GetInterpreterOptions, IInterpreterDisplay, IInterpreterHelper, IInterpreterLocatorService, @@ -112,8 +113,8 @@ export class InterpreterService implements Disposable, IInterpreterService { } @captureTelemetry(EventName.PYTHON_INTERPRETER_DISCOVERY, { locator: 'all' }, true) - public async getInterpreters(resource?: Uri): Promise { - const interpreters = await this.locator.getInterpreters(resource); + public async getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise { + const interpreters = await this.locator.getInterpreters(resource, options); await Promise.all( interpreters .filter((item) => !item.displayName) diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts index 01eb171a96aa..76b3443c4a80 100644 --- a/src/client/interpreter/locators/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -10,6 +10,7 @@ import { CONDA_ENV_FILE_SERVICE, CONDA_ENV_SERVICE, CURRENT_PATH_SERVICE, + GetInterpreterLocatorOptions, GLOBAL_VIRTUAL_ENV_SERVICE, IInterpreterLocatorHelper, IInterpreterLocatorService, @@ -73,8 +74,8 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi * interpreters. */ @traceDecorators.verbose('Get Interpreters') - public async getInterpreters(resource?: Uri): Promise { - const locators = this.getLocators(); + public async getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise { + const locators = this.getLocators(options); const promises = locators.map(async (provider) => provider.getInterpreters(resource)); locators.forEach((locator) => { locator.hasInterpreters @@ -100,23 +101,23 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi * * The locators are pulled from the registry. */ - private getLocators(): IInterpreterLocatorService[] { + private getLocators(options?: GetInterpreterLocatorOptions): IInterpreterLocatorService[] { // The order of the services is important. // The order is important because the data sources at the bottom of the list do not contain all, // the information about the interpreters (e.g. type, environment name, etc). // This way, the items returned from the top of the list will win, when we combine the items returned. - const keys: [string, OSType | undefined][] = [ + const keys: [string | undefined, OSType | undefined][] = [ [WINDOWS_REGISTRY_SERVICE, OSType.Windows], [CONDA_ENV_SERVICE, undefined], [CONDA_ENV_FILE_SERVICE, undefined], - [PIPENV_SERVICE, undefined], + options?.onActivation ? [undefined, undefined] : [PIPENV_SERVICE, undefined], [GLOBAL_VIRTUAL_ENV_SERVICE, undefined], [WORKSPACE_VIRTUAL_ENV_SERVICE, undefined], [KNOWN_PATH_SERVICE, undefined], [CURRENT_PATH_SERVICE, undefined] ]; return keys - .filter((item) => item[1] === undefined || item[1] === this.platform.osType) + .filter((item) => item[0] !== undefined && (item[1] === undefined || item[1] === this.platform.osType)) .map((item) => this.serviceContainer.get(IInterpreterLocatorService, item[0])); } } diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 3071371d5a76..320b4c9a21b9 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -15,7 +15,12 @@ import { StopWatch } from '../../../common/utils/stopWatch'; import { IServiceContainer } from '../../../ioc/types'; import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; -import { IInterpreterLocatorService, IInterpreterWatcher, PythonInterpreter } from '../../contracts'; +import { + GetInterpreterLocatorOptions, + IInterpreterLocatorService, + IInterpreterWatcher, + PythonInterpreter +} from '../../contracts'; /** * This class exists so that the interpreter fetching can be cached in between tests. Normally @@ -82,10 +87,10 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ } public abstract dispose(): void; @traceDecorators.verbose('Get Interpreters in CacheableLocatorService') - public async getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise { + public async getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise { const cacheKey = this.getCacheKey(resource); let deferred = this.promisesPerResource.get(cacheKey); - if (!deferred || ignoreCache) { + if (!deferred || options?.ignoreCache) { deferred = createDeferred(); this.promisesPerResource.set(cacheKey, deferred); @@ -125,7 +130,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ return deferred.promise; } - const cachedInterpreters = ignoreCache ? undefined : this.getCachedInterpreters(resource); + const cachedInterpreters = options?.ignoreCache ? undefined : this.getCachedInterpreters(resource); return Array.isArray(cachedInterpreters) ? cachedInterpreters : deferred.promise; } protected async addHandlersForInterpreterWatchers(cacheKey: string, resource: Uri | undefined): Promise { diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts index c84e78c10837..e5cc8b03c57a 100644 --- a/src/client/startupTelemetry.ts +++ b/src/client/startupTelemetry.ts @@ -129,7 +129,9 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer): .then((ver) => (ver ? ver.raw : '')) .catch(() => ''), interpreterService.getActiveInterpreter().catch(() => undefined), - interpreterService.getInterpreters(mainWorkspaceUri).catch(() => []) + interpreterService + .getInterpreters(mainWorkspaceUri, { onActivation: true }) + .catch(() => []) ]); const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0; const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined; diff --git a/src/test/activation/activationManager.unit.test.ts b/src/test/activation/activationManager.unit.test.ts index 43a14e4978c0..0dff26f9e6ed 100644 --- a/src/test/activation/activationManager.unit.test.ts +++ b/src/test/activation/activationManager.unit.test.ts @@ -198,7 +198,7 @@ suite('Activation Manager', () => { when(workspaceService.getWorkspaceFolder(resource)).thenReturn(folder2); when(activationService1.activate(resource)).thenResolve(); when(activationService2.activate(resource)).thenResolve(); - when(interpreterService.getInterpreters(anything())).thenResolve(); + when(interpreterService.getInterpreters(anything(), anything())).thenResolve(); autoSelection .setup((a) => a.autoSelectInterpreter(resource)) .returns(() => Promise.resolve()) @@ -232,7 +232,7 @@ suite('Activation Manager', () => { const resource = Uri.parse('two'); when(activationService1.activate(resource)).thenResolve(); when(activationService2.activate(resource)).thenResolve(); - when(interpreterService.getInterpreters(anything())).thenResolve(); + when(interpreterService.getInterpreters(anything(), anything())).thenResolve(); autoSelection .setup((a) => a.autoSelectInterpreter(resource)) .returns(() => Promise.resolve()) @@ -252,7 +252,7 @@ suite('Activation Manager', () => { const resource = Uri.parse('two'); when(activationService1.activate(resource)).thenResolve(); when(activationService2.activate(resource)).thenResolve(); - when(interpreterService.getInterpreters(anything())).thenResolve(); + when(interpreterService.getInterpreters(anything(), anything())).thenResolve(); when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); interpreterPathService .setup((i) => i.copyOldInterpreterStorageValuesToNew(resource)) @@ -278,7 +278,7 @@ suite('Activation Manager', () => { const resource = Uri.parse('two'); when(activationService1.activate(resource)).thenResolve(); when(activationService2.activate(resource)).thenResolve(); - when(interpreterService.getInterpreters(anything())).thenResolve(); + when(interpreterService.getInterpreters(anything(), anything())).thenResolve(); autoSelection .setup((a) => a.autoSelectInterpreter(resource)) .returns(() => Promise.resolve()) diff --git a/src/test/interpreters/autoSelection/rules/currentPath.unit.test.ts b/src/test/interpreters/autoSelection/rules/currentPath.unit.test.ts index 1215ca88af70..ca3d3b599d06 100644 --- a/src/test/interpreters/autoSelection/rules/currentPath.unit.test.ts +++ b/src/test/interpreters/autoSelection/rules/currentPath.unit.test.ts @@ -69,7 +69,7 @@ suite('Interpreters - Auto Selection - Current Path Rule', () => { const manager = mock(InterpreterAutoSelectionService); const resource = Uri.file('x'); - when(locator.getInterpreters(resource)).thenResolve([]); + when(locator.getInterpreters(resource, anything())).thenResolve([]); const nextAction = await rule.onAutoSelectInterpreter(resource, manager); diff --git a/src/test/interpreters/autoSelection/rules/system.unit.test.ts b/src/test/interpreters/autoSelection/rules/system.unit.test.ts index d5ccb4173cd4..a18baf5d82cf 100644 --- a/src/test/interpreters/autoSelection/rules/system.unit.test.ts +++ b/src/test/interpreters/autoSelection/rules/system.unit.test.ts @@ -64,8 +64,9 @@ suite('Interpreters - Auto Selection - System Interpreters Rule', () => { test('Invoke next rule if there are no interpreters in the current path', async () => { const manager = mock(InterpreterAutoSelectionService); const resource = Uri.file('x'); + const options = { onActivation: true }; let setGlobalInterpreterInvoked = false; - when(interpreterService.getInterpreters(resource)).thenResolve([]); + when(interpreterService.getInterpreters(resource, deepEqual(options))).thenResolve([]); when(helper.getBestInterpreter(deepEqual([]))).thenReturn(undefined); rule.setGlobalInterpreter = async (res: any) => { setGlobalInterpreterInvoked = true; @@ -75,16 +76,17 @@ suite('Interpreters - Auto Selection - System Interpreters Rule', () => { const nextAction = await rule.onAutoSelectInterpreter(resource, manager); - verify(interpreterService.getInterpreters(resource)).once(); + verify(interpreterService.getInterpreters(resource, deepEqual(options))).once(); expect(nextAction).to.be.equal(NextAction.runNextRule); expect(setGlobalInterpreterInvoked).to.be.equal(true, 'setGlobalInterpreter not invoked'); }); test('Invoke next rule if there interpreters in the current path but update fails', async () => { const manager = mock(InterpreterAutoSelectionService); const resource = Uri.file('x'); + const options = { onActivation: true }; let setGlobalInterpreterInvoked = false; const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - when(interpreterService.getInterpreters(resource)).thenResolve([interpreterInfo]); + when(interpreterService.getInterpreters(resource, deepEqual(options))).thenResolve([interpreterInfo]); when(helper.getBestInterpreter(deepEqual([interpreterInfo]))).thenReturn(interpreterInfo); rule.setGlobalInterpreter = async (res: any) => { setGlobalInterpreterInvoked = true; @@ -94,16 +96,17 @@ suite('Interpreters - Auto Selection - System Interpreters Rule', () => { const nextAction = await rule.onAutoSelectInterpreter(resource, manager); - verify(interpreterService.getInterpreters(resource)).once(); + verify(interpreterService.getInterpreters(resource, deepEqual(options))).once(); expect(nextAction).to.be.equal(NextAction.runNextRule); expect(setGlobalInterpreterInvoked).to.be.equal(true, 'setGlobalInterpreter not invoked'); }); test('Do not Invoke next rule if there interpreters in the current path and update does not fail', async () => { const manager = mock(InterpreterAutoSelectionService); const resource = Uri.file('x'); + const options = { onActivation: true }; let setGlobalInterpreterInvoked = false; const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - when(interpreterService.getInterpreters(resource)).thenResolve([interpreterInfo]); + when(interpreterService.getInterpreters(resource, deepEqual(options))).thenResolve([interpreterInfo]); when(helper.getBestInterpreter(deepEqual([interpreterInfo]))).thenReturn(interpreterInfo); rule.setGlobalInterpreter = async (res: any) => { setGlobalInterpreterInvoked = true; @@ -113,7 +116,7 @@ suite('Interpreters - Auto Selection - System Interpreters Rule', () => { const nextAction = await rule.onAutoSelectInterpreter(resource, manager); - verify(interpreterService.getInterpreters(resource)).once(); + verify(interpreterService.getInterpreters(resource, deepEqual(options))).once(); expect(nextAction).to.be.equal(NextAction.exit); expect(setGlobalInterpreterInvoked).to.be.equal(true, 'setGlobalInterpreter not invoked'); }); diff --git a/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts b/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts index e3780bd261aa..91add8e2b24e 100644 --- a/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts +++ b/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts @@ -48,7 +48,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { let state: PersistentState; let helper: IInterpreterHelper; let platform: IPlatformService; - let pipEnvLocator: IInterpreterLocatorService; let virtualEnvLocator: IInterpreterLocatorService; let workspaceService: IWorkspaceService; let experimentsManager: IExperimentsManager; @@ -76,7 +75,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { fs = mock(FileSystem); helper = mock(InterpreterHelper); platform = mock(PlatformService); - pipEnvLocator = mock(KnownPathsService); workspaceService = mock(WorkspaceService); virtualEnvLocator = mock(KnownPathsService); experimentsManager = mock(ExperimentsManager); @@ -91,7 +89,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { instance(stateFactory), instance(platform), instance(workspaceService), - instance(pipEnvLocator), instance(virtualEnvLocator), instance(experimentsManager), instance(interpreterPathService) @@ -201,8 +198,9 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { const folderUri = Uri.file(folderPath); const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: folderUri }; const resource = Uri.file('x'); + const options = { ignoreCache: true }; - when(virtualEnvLocator.getInterpreters(resource, true)).thenResolve([interpreter1 as any]); + when(virtualEnvLocator.getInterpreters(resource, deepEqual(options))).thenResolve([interpreter1 as any]); when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder); when(platform.osType).thenReturn(OSType.Windows); @@ -217,8 +215,9 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { const folderUri = Uri.file(folderPath); const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: folderUri }; const resource = Uri.file('x'); + const options = { ignoreCache: true }; - when(virtualEnvLocator.getInterpreters(resource, true)).thenResolve([ + when(virtualEnvLocator.getInterpreters(resource, deepEqual(options))).thenResolve([ interpreter1, interpreter2, interpreter3 @@ -236,8 +235,9 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { const folderUri = Uri.file(folderPath); const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: folderUri }; const resource = Uri.file('x'); + const options = { ignoreCache: true }; - when(virtualEnvLocator.getInterpreters(resource, true)).thenResolve([interpreter1 as any]); + when(virtualEnvLocator.getInterpreters(resource, deepEqual(options))).thenResolve([interpreter1 as any]); when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder); when(platform.osType).thenReturn(osType); @@ -252,8 +252,9 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { const folderUri = Uri.file(folderPath); const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: folderUri }; const resource = Uri.file('x'); + const options = { ignoreCache: true }; - when(virtualEnvLocator.getInterpreters(resource, true)).thenResolve([ + when(virtualEnvLocator.getInterpreters(resource, deepEqual(options))).thenResolve([ interpreter1, interpreter2, interpreter3 @@ -292,7 +293,7 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { verify(nextRule.autoSelectInterpreter(undefined, manager)).once(); verify(helper.getActiveWorkspaceUri(undefined)).once(); }); - test('Use pipEnv if that completes first with results', async () => { + test('Use virtualEnv if that completes with results', async () => { const folderUri = Uri.parse('Folder'); const pythonPathInConfig = typemoq.Mock.ofType(); const pythonPath = { inspect: () => pythonPathInConfig.object }; @@ -310,118 +311,14 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { const resource = Uri.file('x'); const manager = mock(InterpreterAutoSelectionService); const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - const virtualEnvPromise = createDeferred(); const nextInvoked = createDeferred(); - rule.next = () => Promise.resolve(nextInvoked.resolve()); - rule.getWorkspaceVirtualEnvInterpreters = () => virtualEnvPromise.promise; - when(pipEnvLocator.getInterpreters(folderUri, true)).thenResolve([interpreterInfo]); - when(helper.getBestInterpreter(deepEqual([interpreterInfo]))).thenReturn(interpreterInfo); - - rule.cacheSelectedInterpreter = () => Promise.resolve(); - - await rule.autoSelectInterpreter(resource, instance(manager)); - virtualEnvPromise.resolve([]); - - expect(nextInvoked.completed).to.be.equal(true, 'Next rule not invoked'); - verify(helper.getActiveWorkspaceUri(resource)).atLeast(1); - verify(manager.setWorkspaceInterpreter(folderUri, interpreterInfo)).once(); - }); - test('Use virtualEnv if that completes first with results', async () => { - const folderUri = Uri.parse('Folder'); - const pythonPathInConfig = typemoq.Mock.ofType(); - const pythonPath = { inspect: () => pythonPathInConfig.object }; - pythonPathInConfig - .setup((p) => p.workspaceFolderValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - pythonPathInConfig - .setup((p) => p.workspaceValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - when(helper.getActiveWorkspaceUri(anything())).thenReturn({ folderUri } as any); - when(workspaceService.getConfiguration('python', folderUri)).thenReturn(pythonPath as any); - const resource = Uri.file('x'); - const manager = mock(InterpreterAutoSelectionService); - const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - const pipEnvPromise = createDeferred(); - const nextInvoked = createDeferred(); rule.next = () => Promise.resolve(nextInvoked.resolve()); rule.getWorkspaceVirtualEnvInterpreters = () => Promise.resolve([interpreterInfo]); - when(pipEnvLocator.getInterpreters(folderUri, true)).thenResolve([interpreterInfo]); when(helper.getBestInterpreter(deepEqual([interpreterInfo]))).thenReturn(interpreterInfo); rule.cacheSelectedInterpreter = () => Promise.resolve(); - await rule.autoSelectInterpreter(resource, instance(manager)); - pipEnvPromise.resolve([]); - - expect(nextInvoked.completed).to.be.equal(true, 'Next rule not invoked'); - verify(helper.getActiveWorkspaceUri(resource)).atLeast(1); - verify(manager.setWorkspaceInterpreter(folderUri, interpreterInfo)).once(); - }); - test('Wait for virtualEnv if pipEnv completes without any interpreters', async () => { - const folderUri = Uri.parse('Folder'); - const pythonPathInConfig = typemoq.Mock.ofType(); - const pythonPath = { inspect: () => pythonPathInConfig.object }; - pythonPathInConfig - .setup((p) => p.workspaceFolderValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - pythonPathInConfig - .setup((p) => p.workspaceValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - when(helper.getActiveWorkspaceUri(anything())).thenReturn({ folderUri } as any); - when(workspaceService.getConfiguration('python', folderUri)).thenReturn(pythonPath as any); - - const manager = mock(InterpreterAutoSelectionService); - const resource = Uri.file('x'); - const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - const virtualEnvPromise = createDeferred(); - const nextInvoked = createDeferred(); - rule.next = () => Promise.resolve(nextInvoked.resolve()); - rule.getWorkspaceVirtualEnvInterpreters = () => virtualEnvPromise.promise; - when(pipEnvLocator.getInterpreters(folderUri, true)).thenResolve([]); - when(helper.getBestInterpreter(deepEqual(anything()))).thenReturn(interpreterInfo); - - rule.cacheSelectedInterpreter = () => Promise.resolve(); - - setTimeout(() => virtualEnvPromise.resolve([interpreterInfo]), 10); - await rule.autoSelectInterpreter(resource, instance(manager)); - - expect(nextInvoked.completed).to.be.equal(true, 'Next rule not invoked'); - verify(helper.getActiveWorkspaceUri(resource)).atLeast(1); - verify(manager.setWorkspaceInterpreter(folderUri, interpreterInfo)).once(); - }); - test('Wait for pipEnv if VirtualEnv completes without any interpreters', async () => { - const folderUri = Uri.parse('Folder'); - const pythonPathInConfig = typemoq.Mock.ofType(); - const pythonPath = { inspect: () => pythonPathInConfig.object }; - pythonPathInConfig - .setup((p) => p.workspaceFolderValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - pythonPathInConfig - .setup((p) => p.workspaceValue) - .returns(() => undefined as any) - .verifiable(typemoq.Times.once()); - when(helper.getActiveWorkspaceUri(anything())).thenReturn({ folderUri } as any); - when(workspaceService.getConfiguration('python', folderUri)).thenReturn(pythonPath as any); - - const manager = mock(InterpreterAutoSelectionService); - const resource = Uri.file('x'); - const interpreterInfo = { path: '1', version: new SemVer('1.0.0') } as any; - const pipEnvPromise = createDeferred(); - const nextInvoked = createDeferred(); - rule.next = () => Promise.resolve(nextInvoked.resolve()); - rule.getWorkspaceVirtualEnvInterpreters = () => Promise.resolve([]); - when(pipEnvLocator.getInterpreters(folderUri, true)).thenResolve([]); - when(helper.getBestInterpreter(deepEqual(anything()))).thenReturn(interpreterInfo); - - rule.cacheSelectedInterpreter = () => Promise.resolve(); - - setTimeout(() => pipEnvPromise.resolve([interpreterInfo]), 10); await rule.autoSelectInterpreter(resource, instance(manager)); expect(nextInvoked.completed).to.be.equal(true, 'Next rule not invoked'); From af5b98d149965cf60b547449145342b1a9fd2c52 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 23 Apr 2020 13:58:08 -0700 Subject: [PATCH 2/8] News file --- news/2 Fixes/11127.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/11127.md diff --git a/news/2 Fixes/11127.md b/news/2 Fixes/11127.md new file mode 100644 index 000000000000..f720bfd0bf14 --- /dev/null +++ b/news/2 Fixes/11127.md @@ -0,0 +1 @@ +Do not perform pipenv interpreter discovery on extension activation. From c2583f155bd9c62f514ec0f2091fe7db571cb6a0 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 23 Apr 2020 14:30:48 -0700 Subject: [PATCH 3/8] Couldn't you have failed compilation earlier --- src/client/datascience/kernel-launcher/kernelFinder.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/client/datascience/kernel-launcher/kernelFinder.ts b/src/client/datascience/kernel-launcher/kernelFinder.ts index 568862481c2a..d240609355cf 100644 --- a/src/client/datascience/kernel-launcher/kernelFinder.ts +++ b/src/client/datascience/kernel-launcher/kernelFinder.ts @@ -83,10 +83,12 @@ export class KernelFinder implements IKernelFinder { } const diskSearch = this.findDiskPath(kernelName); - const interpreterSearch = this.interpreterLocator.getInterpreters(resource, false).then((interpreters) => { - const interpreterPaths = interpreters.map((interp) => interp.sysPrefix); - return this.findInterpreterPath(interpreterPaths, kernelName); - }); + const interpreterSearch = this.interpreterLocator + .getInterpreters(resource, { ignoreCache: false }) + .then((interpreters) => { + const interpreterPaths = interpreters.map((interp) => interp.sysPrefix); + return this.findInterpreterPath(interpreterPaths, kernelName); + }); let result = await Promise.race([diskSearch, interpreterSearch]); if (!result) { From e3d8b9b23da47e38bdde4a4f897c6f54b9003ebe Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 23 Apr 2020 15:12:29 -0700 Subject: [PATCH 4/8] More test fixing --- src/test/interpreters/interpreterService.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index e362068ae95f..48b224666585 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -178,7 +178,7 @@ suite('Interpreters service', () => { test(`get Interpreters uses interpreter locactors to get interpreters ${resourceTestSuffix}`, async () => { locator - .setup((l) => l.getInterpreters(TypeMoq.It.isValue(resource))) + .setup((l) => l.getInterpreters(TypeMoq.It.isValue(resource), TypeMoq.It.isAny())) .returns(() => Promise.resolve([])) .verifiable(TypeMoq.Times.once()); @@ -343,7 +343,7 @@ suite('Interpreters service', () => { const pythonPath = 'SOME VALUE'; const service = new InterpreterService(serviceContainer, hashProviderFactory.object); locator - .setup((l) => l.getInterpreters(TypeMoq.It.isValue(resource))) + .setup((l) => l.getInterpreters(TypeMoq.It.isValue(resource), TypeMoq.It.isAny())) .returns(() => Promise.resolve([])) .verifiable(TypeMoq.Times.once()); helper From 57d08609fe41ed699dfb69b4df9bd1867e5509df Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 24 Apr 2020 11:46:07 -0700 Subject: [PATCH 5/8] Add pipenv-specific test --- .../interpreters/locators/index.unit.test.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/test/interpreters/locators/index.unit.test.ts b/src/test/interpreters/locators/index.unit.test.ts index e3057a95883c..669a2386a205 100644 --- a/src/test/interpreters/locators/index.unit.test.ts +++ b/src/test/interpreters/locators/index.unit.test.ts @@ -183,6 +183,78 @@ suite('Interpreters - Locators Index', () => { expect(interpreters).to.be.lengthOf(locatorsTypes.length); expect(interpreters).to.be.deep.equal(expectedInterpreters); }); + + test(`The pipenv locator service isn't used when the onActivation option is true ${testSuffix}`, async () => { + const locatorsTypes: string[] = []; + if (osType.value === OSType.Windows) { + locatorsTypes.push(WINDOWS_REGISTRY_SERVICE); + } + platformSvc.setup((p) => p.osType).returns(() => osType.value); + platformSvc.setup((p) => p.isWindows).returns(() => osType.value === OSType.Windows); + platformSvc.setup((p) => p.isLinux).returns(() => osType.value === OSType.Linux); + platformSvc.setup((p) => p.isMac).returns(() => osType.value === OSType.OSX); + + locatorsTypes.push(CONDA_ENV_SERVICE); + locatorsTypes.push(CONDA_ENV_FILE_SERVICE); + locatorsTypes.push(PIPENV_SERVICE); + locatorsTypes.push(GLOBAL_VIRTUAL_ENV_SERVICE); + locatorsTypes.push(WORKSPACE_VIRTUAL_ENV_SERVICE); + locatorsTypes.push(KNOWN_PATH_SERVICE); + locatorsTypes.push(CURRENT_PATH_SERVICE); + + const locatorsWithInterpreters = locatorsTypes.map((typeName) => { + const interpreter: PythonInterpreter = { + architecture: Architecture.Unknown, + displayName: typeName, + path: typeName, + sysPrefix: typeName, + sysVersion: typeName, + type: InterpreterType.Unknown, + version: new SemVer('0.0.0-alpha') + }; + + const typeLocator = TypeMoq.Mock.ofType(); + typeLocator + .setup((l) => l.hasInterpreters) + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.once()); + typeLocator + .setup((l) => l.getInterpreters(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve([interpreter])) + .verifiable(TypeMoq.Times.once()); + + serviceContainer + .setup((c) => + c.get(TypeMoq.It.isValue(IInterpreterLocatorService), TypeMoq.It.isValue(typeName)) + ) + .returns(() => typeLocator.object); + + return { + type: typeName, + locator: typeLocator, + interpreters: [interpreter] + }; + }); + + const expectedInterpreters: PythonInterpreter[] = []; + locatorsWithInterpreters.forEach((item) => { + if (item.type !== PIPENV_SERVICE) { + expectedInterpreters.push(item.interpreters[0]); + } + }); + + // const expectedInterpreters = locatorsWithInterpreters.map((item) => item.interpreters[0]); + helper + .setup((h) => h.mergeInterpreters(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(expectedInterpreters)) + .verifiable(TypeMoq.Times.once()); + + const interpreters = await locator.getInterpreters(resource, { onActivation: false }); + + locatorsWithInterpreters.forEach((item) => item.locator.verifyAll()); + helper.verifyAll(); + expect(interpreters).to.be.deep.equal(expectedInterpreters); + }); }); }); }); From c7a50a06909b38a6f959b69a8ac0dd08f731f388 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 28 Apr 2020 15:21:37 -0700 Subject: [PATCH 6/8] Remove unnecessary else condition --- src/client/interpreter/autoSelection/rules/workspaceEnv.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts index 92ec8c067749..2d84729fac76 100644 --- a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts +++ b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts @@ -63,10 +63,6 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe let bestInterpreter: PythonInterpreter | undefined; if (Array.isArray(interpreters) && interpreters.length > 0) { bestInterpreter = this.helper.getBestInterpreter(interpreters); - } else { - const virtualEnv = await virtualEnvPromise.promise; - const virtualEnvList = Array.isArray(virtualEnv) ? virtualEnv : []; - bestInterpreter = this.helper.getBestInterpreter(virtualEnvList); } if (bestInterpreter && manager) { await super.cacheSelectedInterpreter(workspacePath.folderUri, bestInterpreter); From d71790b78c91fda1a8e44a2e2a45657b208ed5e6 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 28 Apr 2020 15:31:11 -0700 Subject: [PATCH 7/8] Update mac interpreter diagnostics --- .../diagnostics/checks/macPythonInterpreter.ts | 2 +- .../diagnostics/checks/macPythonInterpreter.unit.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/application/diagnostics/checks/macPythonInterpreter.ts b/src/client/application/diagnostics/checks/macPythonInterpreter.ts index 26344f41a607..9515c7b76928 100644 --- a/src/client/application/diagnostics/checks/macPythonInterpreter.ts +++ b/src/client/application/diagnostics/checks/macPythonInterpreter.ts @@ -101,7 +101,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService { return []; } - const interpreters = await this.interpreterService.getInterpreters(resource); + const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true }); if (interpreters.filter((i) => !this.helper.isMacDefaultPythonPath(i.path)).length === 0) { return [ new InvalidMacPythonInterpreterDiagnostic( diff --git a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts index eaa9315341d3..43a6c365929d 100644 --- a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts @@ -174,7 +174,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => Promise.resolve(true)) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) .returns(() => Promise.resolve([{} as any])) .verifiable(typemoq.Times.never()); interpreterService @@ -204,7 +204,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => Promise.resolve(true)) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) .returns(() => Promise.resolve([{} as any])) .verifiable(typemoq.Times.never()); interpreterService @@ -235,7 +235,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => false) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) .returns(() => Promise.resolve([{ path: pythonPath } as any, { path: pythonPath } as any])) .verifiable(typemoq.Times.once()); interpreterService @@ -275,7 +275,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => false) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) .returns(() => Promise.resolve([ { path: pythonPath } as any, From f0cd3fbe1575a3c8be4144f968ff3ae8c61b6033 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 29 Apr 2020 14:10:09 -0700 Subject: [PATCH 8/8] Suggestions --- .../interpreter/autoSelection/rules/workspaceEnv.ts | 9 +++++---- .../diagnostics/checks/macPythonInterpreter.unit.test.ts | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts index 2d84729fac76..d033b087c451 100644 --- a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts +++ b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts @@ -60,10 +60,11 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe ); const interpreters = await virtualEnvPromise.promise; - let bestInterpreter: PythonInterpreter | undefined; - if (Array.isArray(interpreters) && interpreters.length > 0) { - bestInterpreter = this.helper.getBestInterpreter(interpreters); - } + const bestInterpreter = + Array.isArray(interpreters) && interpreters.length > 0 + ? this.helper.getBestInterpreter(interpreters) + : undefined; + if (bestInterpreter && manager) { await super.cacheSelectedInterpreter(workspacePath.folderUri, bestInterpreter); await manager.setWorkspaceInterpreter(workspacePath.folderUri!, bestInterpreter); diff --git a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts index 43a6c365929d..dfaad331374f 100644 --- a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts @@ -174,7 +174,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => Promise.resolve(true)) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true })) .returns(() => Promise.resolve([{} as any])) .verifiable(typemoq.Times.never()); interpreterService @@ -204,7 +204,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => Promise.resolve(true)) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true })) .returns(() => Promise.resolve([{} as any])) .verifiable(typemoq.Times.never()); interpreterService @@ -235,7 +235,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => false) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true })) .returns(() => Promise.resolve([{ path: pythonPath } as any, { path: pythonPath } as any])) .verifiable(typemoq.Times.once()); interpreterService @@ -275,7 +275,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => false) .verifiable(typemoq.Times.once()); interpreterService - .setup((i) => i.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny())) + .setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true })) .returns(() => Promise.resolve([ { path: pythonPath } as any,