From d9706fb3365cc617d0303c7b2f09362630f09e0d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Jul 2018 01:58:50 -0700 Subject: [PATCH 01/27] Auto activation of environment in new terminals --- src/client/terminals/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index d47d4b612e51..9d216560954c 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { TextEditor, Uri } from 'vscode'; +import { Terminal, TextEditor, Uri } from 'vscode'; export const ICodeExecutionService = Symbol('ICodeExecutionService'); From 424a8662d62bda5fc4a95aefc52f6a3d3936f4d6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Sep 2018 21:13:23 -0700 Subject: [PATCH 02/27] refactor --- src/client/terminals/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 9d216560954c..d47d4b612e51 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { Terminal, TextEditor, Uri } from 'vscode'; +import { TextEditor, Uri } from 'vscode'; export const ICodeExecutionService = Symbol('ICodeExecutionService'); From f4ed816a6dd22b6148c4ea7bfd371f56e3e601ec Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Sep 2018 11:15:48 -0700 Subject: [PATCH 03/27] xxx --- src/client/common/platform/pathUtils.ts | 7 ++- src/client/common/platform/registry.ts | 2 +- src/client/common/types.ts | 1 + .../configuration/interpreterComparer.ts | 57 +++++++++++++++++++ .../configuration/interpreterSelector.ts | 18 ++++-- src/client/interpreter/configuration/types.ts | 6 ++ src/client/interpreter/contracts.ts | 4 +- src/client/interpreter/display/index.ts | 22 +++---- src/client/interpreter/helpers.ts | 26 ++++++++- src/client/interpreter/interpreterService.ts | 54 +++++++++++++++--- src/client/interpreter/locators/helpers.ts | 4 +- .../services/baseVirtualEnvService.ts | 8 +-- .../services/cacheableLocatorService.ts | 2 +- .../locators/services/currentPathService.ts | 2 +- src/client/interpreter/serviceRegistry.ts | 4 +- src/client/interpreter/virtualEnvs/index.ts | 20 +++++-- src/client/interpreter/virtualEnvs/types.ts | 2 +- 17 files changed, 194 insertions(+), 45 deletions(-) create mode 100644 src/client/interpreter/configuration/interpreterComparer.ts diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 319d523402fb..5758da4e4bea 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -2,10 +2,15 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IPathUtils, IsWindows } from '../types'; import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; +// tslint:disable-next-line:no-var-requires no-require-imports +const untildify = require('untildify'); @injectable() export class PathUtils implements IPathUtils { - constructor(@inject(IsWindows) private isWindows: boolean) { } + public readonly home = ''; + constructor(@inject(IsWindows) private isWindows: boolean) { + this.home = untildify('~'); + } public get delimiter(): string { return path.delimiter; } diff --git a/src/client/common/platform/registry.ts b/src/client/common/platform/registry.ts index 05daee3f8c45..9032f1d42a1c 100644 --- a/src/client/common/platform/registry.ts +++ b/src/client/common/platform/registry.ts @@ -18,7 +18,7 @@ export class RegistryImplementation implements IRegistry { } } -export function getArchitectureDislayName(arch?: Architecture) { +export function getArchitectureDisplayName(arch?: Architecture) { switch (arch) { case Architecture.x64: return '64-bit'; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 9c4e3b9b71f0..3bdc2942d441 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -101,6 +101,7 @@ export const IPathUtils = Symbol('IPathUtils'); export interface IPathUtils { readonly delimiter: string; + readonly home: string; getPathVariableName(): 'Path' | 'PATH'; basename(pathValue: string, ext?: string): string; } diff --git a/src/client/interpreter/configuration/interpreterComparer.ts b/src/client/interpreter/configuration/interpreterComparer.ts new file mode 100644 index 000000000000..f214a7664c19 --- /dev/null +++ b/src/client/interpreter/configuration/interpreterComparer.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { getArchitectureDisplayName } from '../../common/platform/registry'; +import { IServiceContainer } from '../../ioc/types'; +import { IInterpreterHelper, PythonInterpreter } from '../contracts'; +import { IInterpreterComparer } from './types'; + +@injectable() +export class InterpreterComparer implements IInterpreterComparer { + private readonly interpreterHelper: IInterpreterHelper; + + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { + this.interpreterHelper = this.serviceContainer.get(IInterpreterHelper); + } + public compare(a: PythonInterpreter, b: PythonInterpreter): number { + return this.getSortName(a) > this.getSortName(b) ? 1 : -1; + } + private getSortName(info: PythonInterpreter): string { + const sortNameParts: string[] = []; + const envSuffixParts: string[] = []; + + // Sort order for interpreters is: + // * Version + // * Architecture + // * Interpreter Type + // * Environment name + if (info.version_info && info.version_info.length > 0) { + sortNameParts.push(info.version_info.slice(0, 3).join('.')); + } + if (info.version_info) { + sortNameParts.push(getArchitectureDisplayName(info.architecture)); + } + if (info.companyDisplayName && info.companyDisplayName.length > 0) { + sortNameParts.push(info.companyDisplayName.trim()); + } else { + sortNameParts.push('Python'); + } + + if (info.type) { + const name = this.interpreterHelper.getInterpreterTypeDisplayName(info.type); + if (name) { + envSuffixParts.push(name); + } + } + if (info.envName && info.envName.length > 0) { + envSuffixParts.push(info.envName); + } + + const envSuffix = envSuffixParts.length === 0 ? '' : + `(${envSuffixParts.join(': ')})`; + return `${sortNameParts.join(' ')} ${envSuffix}`.trim(); + } +} diff --git a/src/client/interpreter/configuration/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector.ts index 4a490b865a39..dcd3202a2e9c 100644 --- a/src/client/interpreter/configuration/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector.ts @@ -4,9 +4,10 @@ import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri } import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types'; import * as settings from '../../common/configSettings'; import { Commands } from '../../common/constants'; +import { IPathUtils } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts'; -import { IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types'; +import { IInterpreterComparer, IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types'; export interface IInterpreterQuickPickItem extends QuickPickItem { path: string; @@ -19,12 +20,16 @@ export class InterpreterSelector implements IInterpreterSelector { private readonly workspaceService: IWorkspaceService; private readonly applicationShell: IApplicationShell; private readonly documentManager: IDocumentManager; + private readonly pathUtils: IPathUtils; + private readonly interpreterComparer: IInterpreterComparer; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.interpreterManager = serviceContainer.get(IInterpreterService); this.workspaceService = this.serviceContainer.get(IWorkspaceService); this.applicationShell = this.serviceContainer.get(IApplicationShell); this.documentManager = this.serviceContainer.get(IDocumentManager); + this.pathUtils = this.serviceContainer.get(IPathUtils); + this.interpreterComparer = this.serviceContainer.get(IInterpreterComparer); const commandManager = serviceContainer.get(ICommandManager); this.disposables.push(commandManager.registerCommand(Commands.Set_Interpreter, this.setInterpreter.bind(this))); @@ -36,11 +41,9 @@ export class InterpreterSelector implements IInterpreterSelector { public async getSuggestions(resourceUri?: Uri) { const interpreters = await this.interpreterManager.getInterpreters(resourceUri); - // tslint:disable-next-line:no-non-null-assertion - interpreters.sort((a, b) => a.displayName! > b.displayName! ? 1 : -1); + interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer)); return Promise.all(interpreters.map(item => this.suggestionToQuickPickItem(item, resourceUri))); } - private async getWorkspaceToSetPythonPath(): Promise { if (!Array.isArray(this.workspaceService.workspaceFolders) || this.workspaceService.workspaceFolders.length === 0) { return undefined; @@ -57,6 +60,9 @@ export class InterpreterSelector implements IInterpreterSelector { private async suggestionToQuickPickItem(suggestion: PythonInterpreter, workspaceUri?: Uri): Promise { let detail = suggestion.path; + if (suggestion.path.startsWith(this.pathUtils.home)) { + detail = `~${path.sep}${path.relative(this.pathUtils.home, suggestion.path)}`; + } if (workspaceUri && suggestion.path.startsWith(workspaceUri.fsPath)) { detail = `.${path.sep}${path.relative(workspaceUri.fsPath, suggestion.path)}`; } @@ -64,7 +70,6 @@ export class InterpreterSelector implements IInterpreterSelector { return { // tslint:disable-next-line:no-non-null-assertion label: suggestion.displayName!, - description: suggestion.companyDisplayName || '', detail: `${cachedPrefix}${detail}`, path: suggestion.path }; @@ -85,6 +90,9 @@ export class InterpreterSelector implements IInterpreterSelector { const suggestions = await this.getSuggestions(wkspace); let currentPythonPath = settings.PythonSettings.getInstance().pythonPath; + if (currentPythonPath.startsWith(this.pathUtils.home)) { + currentPythonPath = `~${path.sep}${path.relative(this.pathUtils.home, currentPythonPath)}`; + } if (wkspace && currentPythonPath.startsWith(wkspace.fsPath)) { currentPythonPath = `.${path.sep}${path.relative(wkspace.fsPath, currentPythonPath)}`; } diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 43ec286f77f0..6cf2fed4a1bd 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -1,4 +1,5 @@ import { ConfigurationTarget, Disposable, Uri } from 'vscode'; +import { PythonInterpreter } from '../contracts'; export interface IPythonPathUpdaterService { updatePythonPath(pythonPath: string): Promise; @@ -20,3 +21,8 @@ export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { } + +export const IInterpreterComparer = Symbol('IInterpreterComparer'); +export interface IInterpreterComparer { + compare(a: PythonInterpreter, b: PythonInterpreter): number; +} diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index f4dd60ba52a9..5eb799034db6 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -78,9 +78,10 @@ export interface IInterpreterService { getInterpreters(resource?: Uri): Promise; autoSetInterpreter(): Promise; getActiveInterpreter(resource?: Uri): Promise; - getInterpreterDetails(pythonPath: string): Promise>; + getInterpreterDetails(pythonPath: string, resoure?: Uri): Promise; refresh(): Promise; initialize(): void; + getDisplayName(interpreter: Partial): string; } export const IInterpreterDisplay = Symbol('IInterpreterDisplay'); @@ -98,6 +99,7 @@ export interface IInterpreterHelper { getActiveWorkspaceUri(): WorkspacePythonPath | undefined; getInterpreterInformation(pythonPath: string): Promise>; isMacDefaultPythonPath(pythonPath: string): Boolean; + getInterpreterTypeDisplayName(interpreterType: InterpreterType): string|undefined; } export const IPipEnvService = Symbol('IPipEnvService'); diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 14291ea76468..680ff3063b89 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -1,6 +1,4 @@ import { inject, injectable } from 'inversify'; -import { EOL } from 'os'; -import * as path from 'path'; import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; import { IFileSystem } from '../../common/platform/types'; @@ -56,20 +54,18 @@ export class InterpreterDisplay implements IInterpreterDisplay { if (interpreter) { // tslint:disable-next-line:no-non-null-assertion this.statusBar.text = interpreter.displayName!; - if (interpreter.companyDisplayName) { - const toolTipSuffix = `${EOL}${interpreter.companyDisplayName}`; - this.statusBar.tooltip += toolTipSuffix; - } + this.statusBar.tooltip = pythonPath; } else { await Promise.all([ this.fileSystem.fileExists(pythonPath), - this.helper.getInterpreterInformation(pythonPath).catch>(() => undefined), - this.getVirtualEnvironmentName(pythonPath).catch(() => '') + this.helper.getInterpreterInformation(pythonPath).catch | undefined>(() => undefined), + this.getVirtualEnvironmentName(pythonPath, workspaceFolder).catch(() => '') ]) .then(([interpreterExists, details, virtualEnvName]) => { - const defaultDisplayName = `${path.basename(pythonPath)} [Environment]`; - const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : ''; - this.statusBar.text = `${details ? details.version : defaultDisplayName}${dislayNameSuffix}`; + if (details) { + const displayName = this.interpreterService.getDisplayName({ ...details, envName: virtualEnvName }); + this.statusBar.text = displayName; + } if (!interpreterExists && !details && interpreters.length > 0) { this.statusBar.color = 'yellow'; @@ -79,7 +75,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { } this.statusBar.show(); } - private async getVirtualEnvironmentName(pythonPath: string): Promise { - return this.virtualEnvMgr.getEnvironmentName(pythonPath); + private async getVirtualEnvironmentName(pythonPath: string, resource?: Uri): Promise { + return this.virtualEnvMgr.getEnvironmentName(pythonPath, resource); } } diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index ee0278b9ef59..0f85b0b542e7 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -5,7 +5,7 @@ import { IFileSystem } from '../common/platform/types'; import { InterpreterInfomation, IPythonExecutionFactory } from '../common/process/types'; import { IPersistentStateFactory } from '../common/types'; import { IServiceContainer } from '../ioc/types'; -import { IInterpreterHelper, PythonInterpreter, WorkspacePythonPath } from './contracts'; +import { IInterpreterHelper, InterpreterType, PythonInterpreter, WorkspacePythonPath } from './contracts'; const EXPITY_DURATION = 24 * 60 * 60 * 1000; type CachedPythonInterpreter = Partial & { fileHash: string }; @@ -46,7 +46,7 @@ export class InterpreterHelper implements IInterpreterHelper { public async getInterpreterInformation(pythonPath: string): Promise> { let fileHash = await this.fs.getFileHash(pythonPath).catch(() => ''); fileHash = fileHash ? fileHash : ''; - const store = this.persistentFactory.createGlobalPersistentState(pythonPath, undefined, EXPITY_DURATION); + const store = this.persistentFactory.createGlobalPersistentState(`${pythonPath}.1`, undefined, EXPITY_DURATION); if (store.value && (!fileHash || store.value.fileHash === fileHash)) { return store.value; } @@ -71,4 +71,26 @@ export class InterpreterHelper implements IInterpreterHelper { public isMacDefaultPythonPath(pythonPath: string) { return pythonPath === 'python' || pythonPath === '/usr/bin/python'; } + public getInterpreterTypeDisplayName(interpreterType: InterpreterType) { + switch (interpreterType) { + case InterpreterType.Conda: { + return 'conda'; + } + case InterpreterType.PipEnv: { + return 'pipenv'; + } + case InterpreterType.Pyenv: { + return 'pyenv'; + } + case InterpreterType.Venv: { + return 'venv'; + } + case InterpreterType.VirtualEnv: { + return 'virtualEnv'; + } + default: { + return ''; + } + } + } } diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 6eeb4766208a..b59fbcca68ee 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import { ConfigurationTarget, Disposable, Event, EventEmitter, Uri } from 'vscode'; import { IDocumentManager, IWorkspaceService } from '../common/application/types'; import { PythonSettings } from '../common/configSettings'; +import { getArchitectureDisplayName } from '../common/platform/registry'; import { IFileSystem } from '../common/platform/types'; import { IPythonExecutionFactory } from '../common/process/types'; import { IConfigurationService, IDisposableRegistry } from '../common/types'; @@ -41,8 +42,10 @@ export class InterpreterService implements Disposable, IInterpreterService { (configService.getSettings() as PythonSettings).addListener('change', this.onConfigChanged); } - public getInterpreters(resource?: Uri): Promise { - return this.locator.getInterpreters(resource); + public async getInterpreters(resource?: Uri): Promise { + const interpreters = await this.locator.getInterpreters(resource); + interpreters.forEach(item => item.displayName = this.getDisplayName(item)); + return interpreters; } public async autoSetInterpreter(): Promise { @@ -112,24 +115,59 @@ export class InterpreterService implements Disposable, IInterpreterService { } const interpreterHelper = this.serviceContainer.get(IInterpreterHelper); const virtualEnvManager = this.serviceContainer.get(IVirtualEnvironmentManager); - const [details, virtualEnvName, type] = await Promise.all([ + const [info, type] = await Promise.all([ interpreterHelper.getInterpreterInformation(pythonPath), - virtualEnvManager.getEnvironmentName(pythonPath), virtualEnvManager.getEnvironmentType(pythonPath) ]); - if (!details) { + if (!info) { return; } + const details: Partial = { + ...(info as PythonInterpreter), + path: pythonPath, + type: type + }; + + const virtualEnvName = await virtualEnvManager.getEnvironmentName(pythonPath, resource); const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : ''; const displayName = `${details.version!}${dislayNameSuffix}`; return { ...(details as PythonInterpreter), - displayName, - path: pythonPath, envName: virtualEnvName, - type: type + displayName }; } + public getDisplayName(info: Partial): string { + const displayNameParts: string[] = []; + const envSuffixParts: string[] = []; + + if (info.companyDisplayName && info.companyDisplayName.length > 0) { + displayNameParts.push(info.companyDisplayName.trim()); + } else { + displayNameParts.push('Python'); + } + + if (info.version_info && info.version_info.length > 0) { + displayNameParts.push(info.version_info.slice(0, 3).join('.')); + } + if (info.version_info) { + displayNameParts.push(getArchitectureDisplayName(info.architecture)); + } + if (info.envName && info.envName.length > 0) { + envSuffixParts.push(info.envName); + } + if (info.type) { + const interpreterHelper = this.serviceContainer.get(IInterpreterHelper); + const name = interpreterHelper.getInterpreterTypeDisplayName(info.type); + if (name) { + envSuffixParts.push(name); + } + } + + const envSuffix = envSuffixParts.length === 0 ? '' : + `(${envSuffixParts.join(': ')})`; + return `${displayNameParts.join(' ')} ${envSuffix}`.trim(); + } private async shouldAutoSetInterpreter(): Promise { const activeWorkspace = this.helper.getActiveWorkspaceUri(); if (!activeWorkspace) { diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index 54e68d2efa01..9b69736a455d 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -1,7 +1,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { fsReaddirAsync } from '../../../utils/fs'; -import { getArchitectureDislayName } from '../../common/platform/registry'; +import { getArchitectureDisplayName } from '../../common/platform/registry'; import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IS_WINDOWS } from '../../common/util'; import { IServiceContainer } from '../../ioc/types'; @@ -20,7 +20,7 @@ export function lookForInterpretersInDirectory(pathToCheck: string): Promise namePart.length > 0).join(' ').trim(); diff --git a/src/client/interpreter/locators/services/baseVirtualEnvService.ts b/src/client/interpreter/locators/services/baseVirtualEnvService.ts index 8c48d470d9bc..bb7cdd6b00bc 100644 --- a/src/client/interpreter/locators/services/baseVirtualEnvService.ts +++ b/src/client/interpreter/locators/services/baseVirtualEnvService.ts @@ -39,7 +39,7 @@ export class BaseVirtualEnvService extends CacheableLocatorService { .then(dirs => dirs.filter(dir => dir.length > 0)) .then(dirs => Promise.all(dirs.map(lookForInterpretersInDirectory))) .then(pathsWithInterpreters => _.flatten(pathsWithInterpreters)) - .then(interpreters => Promise.all(interpreters.map(interpreter => this.getVirtualEnvDetails(interpreter)))) + .then(interpreters => Promise.all(interpreters.map(interpreter => this.getVirtualEnvDetails(interpreter, resource)))) .then(interpreters => interpreters.filter(interpreter => !!interpreter).map(interpreter => interpreter!)) .catch((err) => { console.error('Python Extension (lookForInterpretersInVenvs):', err); @@ -65,11 +65,11 @@ export class BaseVirtualEnvService extends CacheableLocatorService { return ''; })); } - private async getVirtualEnvDetails(interpreter: string): Promise { + private async getVirtualEnvDetails(interpreter: string, resource?: Uri): Promise { return Promise.all([ this.helper.getInterpreterInformation(interpreter), - this.virtualEnvMgr.getEnvironmentName(interpreter), - this.virtualEnvMgr.getEnvironmentType(interpreter) + this.virtualEnvMgr.getEnvironmentName(interpreter, resource), + this.virtualEnvMgr.getEnvironmentType(interpreter, resource) ]) .then(([details, virtualEnvName, type]) => { if (!details) { diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 930b40d6cc84..7ef1f618396f 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -19,7 +19,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ constructor(@unmanaged() name: string, @unmanaged() protected readonly serviceContainer: IServiceContainer, @unmanaged() private cachePerWorkspace: boolean = false) { - this.cacheKeyPrefix = `INTERPRETERS_CACHE_${name}`; + this.cacheKeyPrefix = `INTERPRETERS_CACHE_1_${name}`; } public abstract dispose(); public async getInterpreters(resource?: Uri): Promise { diff --git a/src/client/interpreter/locators/services/currentPathService.ts b/src/client/interpreter/locators/services/currentPathService.ts index 0ace3b61f48c..2b0c3623e17f 100644 --- a/src/client/interpreter/locators/services/currentPathService.ts +++ b/src/client/interpreter/locators/services/currentPathService.ts @@ -70,7 +70,7 @@ export class CurrentPathService extends CacheableLocatorService { private async getInterpreterDetails(interpreter: string, resource?: Uri): Promise { return Promise.all([ this.helper.getInterpreterInformation(interpreter), - this.virtualEnvMgr.getEnvironmentName(interpreter), + this.virtualEnvMgr.getEnvironmentName(interpreter, resource), this.virtualEnvMgr.getEnvironmentType(interpreter, resource) ]). then(([details, virtualEnvName, type]) => { diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index ddc70750f7a1..af6363e1ddca 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -3,10 +3,11 @@ import { IsWindows } from '../common/types'; import { IServiceManager } from '../ioc/types'; +import { InterpreterComparer } from './configuration/interpreterComparer'; import { InterpreterSelector } from './configuration/interpreterSelector'; import { PythonPathUpdaterService } from './configuration/pythonPathUpdaterService'; import { PythonPathUpdaterServiceFactory } from './configuration/pythonPathUpdaterServiceFactory'; -import { IInterpreterSelector, IPythonPathUpdaterServiceFactory, IPythonPathUpdaterServiceManager } from './configuration/types'; +import { IInterpreterComparer, IInterpreterSelector, IPythonPathUpdaterServiceFactory, IPythonPathUpdaterServiceManager } from './configuration/types'; import { CONDA_ENV_FILE_SERVICE, CONDA_ENV_SERVICE, @@ -82,4 +83,5 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IShebangCodeLensProvider, ShebangCodeLensProvider); serviceManager.addSingleton(IInterpreterHelper, InterpreterHelper); serviceManager.addSingleton(IInterpreterLocatorHelper, InterpreterLocatorHelper); + serviceManager.addSingleton(IInterpreterComparer, InterpreterComparer); } diff --git a/src/client/interpreter/virtualEnvs/index.ts b/src/client/interpreter/virtualEnvs/index.ts index 61f14d7f1c6f..b44efa2b4bd6 100644 --- a/src/client/interpreter/virtualEnvs/index.ts +++ b/src/client/interpreter/virtualEnvs/index.ts @@ -26,7 +26,17 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { this.pipEnvService = serviceContainer.get(IPipEnvService); this.workspaceService = serviceContainer.get(IWorkspaceService); } - public async getEnvironmentName(pythonPath: string): Promise { + public async getEnvironmentName(pythonPath: string, resource?: Uri): Promise { + const defaultWorkspaceUri = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders![0].uri : undefined; + const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; + const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri; + const grandParentDirName = path.basename(path.dirname(path.dirname(pythonPath))); + if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { + // In pipenv, the folder name is suffixed with `-`. + // So, to get the environment name, just drop the suffix starting with `-`. + return grandParentDirName; + } + // https://stackoverflow.com/questions/1871549/determine-if-python-is-running-inside-virtualenv // hasattr(sys, 'real_prefix') works for virtualenv while // '(hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix))' works for venv @@ -34,9 +44,11 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { const processService = await this.processServiceFactory.create(); const code = 'import sys\nif hasattr(sys, "real_prefix"):\n print("virtualenv")\nelif hasattr(sys, "base_prefix") and sys.base_prefix != sys.prefix:\n print("venv")'; const output = await processService.exec(pythonPath, ['-c', code]); - if (output.stdout.length > 0) { - return output.stdout.trim(); + const envName = output.stdout.trim(); + if (envName.length === 0) { + return ''; } + return envName.toUpperCase() === 'VIRTUALENV' ? grandParentDirName : envName; } catch { // do nothing. } @@ -59,7 +71,7 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { const defaultWorkspaceUri = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders![0].uri : undefined; const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri; - if (workspaceUri && this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { + if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { return InterpreterType.PipEnv; } diff --git a/src/client/interpreter/virtualEnvs/types.ts b/src/client/interpreter/virtualEnvs/types.ts index 6096f88d374c..e13336760bf9 100644 --- a/src/client/interpreter/virtualEnvs/types.ts +++ b/src/client/interpreter/virtualEnvs/types.ts @@ -6,6 +6,6 @@ import { Uri } from 'vscode'; import { InterpreterType } from '../contracts'; export const IVirtualEnvironmentManager = Symbol('VirtualEnvironmentManager'); export interface IVirtualEnvironmentManager { - getEnvironmentName(pythonPath: string): Promise; + getEnvironmentName(pythonPath: string, resource?: Uri): Promise; getEnvironmentType(pythonPath: string, resource?: Uri): Promise; } From cbbf1925068819e6ec914f58bc8471c78127d0f7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 10:11:15 -0700 Subject: [PATCH 04/27] Add method to get path display name --- src/client/common/platform/pathUtils.ts | 9 +++++++++ src/client/common/types.ts | 1 + .../configuration/interpreterSelector.ts | 17 ++--------------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 5758da4e4bea..24e93f6fcd92 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -21,5 +21,14 @@ export class PathUtils implements IPathUtils { public basename(pathValue: string, ext?: string): string { return path.basename(pathValue, ext); } + public getDisplayName(pathValue: string, home?: string, cwd?: string): string { + if (home && pathValue.startsWith(home)) { + pathValue = `~${path.sep}${path.relative(home, pathValue)}`; + } + if (cwd && pathValue.startsWith(cwd)) { + pathValue = `.${path.sep}${path.relative(cwd, pathValue)}`; + } + return pathValue; + } } diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 3bdc2942d441..cc9d66ba5367 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -104,6 +104,7 @@ export interface IPathUtils { readonly home: string; getPathVariableName(): 'Path' | 'PATH'; basename(pathValue: string, ext?: string): string; + getDisplayName(pathValue: string, home?: string, cwd?: string): string; } export const IRandom = Symbol('IRandom'); diff --git a/src/client/interpreter/configuration/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector.ts index dcd3202a2e9c..85af26078326 100644 --- a/src/client/interpreter/configuration/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector.ts @@ -1,5 +1,4 @@ import { inject, injectable } from 'inversify'; -import * as path from 'path'; import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri } from 'vscode'; import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types'; import * as settings from '../../common/configSettings'; @@ -59,13 +58,7 @@ export class InterpreterSelector implements IInterpreterSelector { } private async suggestionToQuickPickItem(suggestion: PythonInterpreter, workspaceUri?: Uri): Promise { - let detail = suggestion.path; - if (suggestion.path.startsWith(this.pathUtils.home)) { - detail = `~${path.sep}${path.relative(this.pathUtils.home, suggestion.path)}`; - } - if (workspaceUri && suggestion.path.startsWith(workspaceUri.fsPath)) { - detail = `.${path.sep}${path.relative(workspaceUri.fsPath, suggestion.path)}`; - } + const detail = this.pathUtils.getDisplayName(suggestion.path, this.pathUtils.home, workspaceUri ? workspaceUri.fsPath : undefined); const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : ''; return { // tslint:disable-next-line:no-non-null-assertion @@ -89,13 +82,7 @@ export class InterpreterSelector implements IInterpreterSelector { } const suggestions = await this.getSuggestions(wkspace); - let currentPythonPath = settings.PythonSettings.getInstance().pythonPath; - if (currentPythonPath.startsWith(this.pathUtils.home)) { - currentPythonPath = `~${path.sep}${path.relative(this.pathUtils.home, currentPythonPath)}`; - } - if (wkspace && currentPythonPath.startsWith(wkspace.fsPath)) { - currentPythonPath = `.${path.sep}${path.relative(wkspace.fsPath, currentPythonPath)}`; - } + const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, this.pathUtils.home, wkspace ? wkspace.fsPath : undefined); const quickPickOptions: QuickPickOptions = { matchOnDetail: true, matchOnDescription: true, From 0380347124a8721d9cbc22652da74a6b5a3d78dd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 10:35:25 -0700 Subject: [PATCH 05/27] Refactor --- src/client/common/platform/pathUtils.ts | 6 +++--- src/client/common/types.ts | 2 +- src/client/interpreter/configuration/interpreterSelector.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 24e93f6fcd92..fc6cdb5c0858 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -21,9 +21,9 @@ export class PathUtils implements IPathUtils { public basename(pathValue: string, ext?: string): string { return path.basename(pathValue, ext); } - public getDisplayName(pathValue: string, home?: string, cwd?: string): string { - if (home && pathValue.startsWith(home)) { - pathValue = `~${path.sep}${path.relative(home, pathValue)}`; + public getDisplayName(pathValue: string, cwd?: string): string { + if (pathValue.startsWith(this.home)) { + pathValue = `~${path.sep}${path.relative(this.home, pathValue)}`; } if (cwd && pathValue.startsWith(cwd)) { pathValue = `.${path.sep}${path.relative(cwd, pathValue)}`; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index cc9d66ba5367..c42889e9964e 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -104,7 +104,7 @@ export interface IPathUtils { readonly home: string; getPathVariableName(): 'Path' | 'PATH'; basename(pathValue: string, ext?: string): string; - getDisplayName(pathValue: string, home?: string, cwd?: string): string; + getDisplayName(pathValue: string, cwd?: string): string; } export const IRandom = Symbol('IRandom'); diff --git a/src/client/interpreter/configuration/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector.ts index 85af26078326..78a213d19849 100644 --- a/src/client/interpreter/configuration/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector.ts @@ -58,7 +58,7 @@ export class InterpreterSelector implements IInterpreterSelector { } private async suggestionToQuickPickItem(suggestion: PythonInterpreter, workspaceUri?: Uri): Promise { - const detail = this.pathUtils.getDisplayName(suggestion.path, this.pathUtils.home, workspaceUri ? workspaceUri.fsPath : undefined); + const detail = this.pathUtils.getDisplayName(suggestion.path, workspaceUri ? workspaceUri.fsPath : undefined); const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : ''; return { // tslint:disable-next-line:no-non-null-assertion @@ -82,7 +82,7 @@ export class InterpreterSelector implements IInterpreterSelector { } const suggestions = await this.getSuggestions(wkspace); - const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, this.pathUtils.home, wkspace ? wkspace.fsPath : undefined); + const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, wkspace ? wkspace.fsPath : undefined); const quickPickOptions: QuickPickOptions = { matchOnDetail: true, matchOnDescription: true, From 7347df3b3cf646f8b97ee68b6feb1d4353c35875 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 10:46:37 -0700 Subject: [PATCH 06/27] Misc changes --- src/client/interpreter/helpers.ts | 2 +- src/client/interpreter/interpreterService.ts | 17 ++++++++++------- .../services/cacheableLocatorService.ts | 2 +- src/client/interpreter/virtualEnvs/index.ts | 3 +-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index 0f85b0b542e7..7ef8ffdc873c 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -46,7 +46,7 @@ export class InterpreterHelper implements IInterpreterHelper { public async getInterpreterInformation(pythonPath: string): Promise> { let fileHash = await this.fs.getFileHash(pythonPath).catch(() => ''); fileHash = fileHash ? fileHash : ''; - const store = this.persistentFactory.createGlobalPersistentState(`${pythonPath}.1`, undefined, EXPITY_DURATION); + const store = this.persistentFactory.createGlobalPersistentState(`${pythonPath}.v1`, undefined, EXPITY_DURATION); if (store.value && (!fileHash || store.value.fileHash === fileHash)) { return store.value; } diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index b59fbcca68ee..d29fa4c940bd 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -137,16 +137,19 @@ export class InterpreterService implements Disposable, IInterpreterService { displayName }; } + + /** + * Gets the display name of an interpreter. + * The format is `Python (: )` + * E.g. `Python 3.5.1 32-bit (myenv2: virtualenv)` + * @param {Partial} info + * @returns {string} + * @memberof InterpreterService + */ public getDisplayName(info: Partial): string { - const displayNameParts: string[] = []; + const displayNameParts: string[] = ['Python']; const envSuffixParts: string[] = []; - if (info.companyDisplayName && info.companyDisplayName.length > 0) { - displayNameParts.push(info.companyDisplayName.trim()); - } else { - displayNameParts.push('Python'); - } - if (info.version_info && info.version_info.length > 0) { displayNameParts.push(info.version_info.slice(0, 3).join('.')); } diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 7ef1f618396f..7c7ee00507d3 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -19,7 +19,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ constructor(@unmanaged() name: string, @unmanaged() protected readonly serviceContainer: IServiceContainer, @unmanaged() private cachePerWorkspace: boolean = false) { - this.cacheKeyPrefix = `INTERPRETERS_CACHE_1_${name}`; + this.cacheKeyPrefix = `INTERPRETERS_CACHE_v1_${name}`; } public abstract dispose(); public async getInterpreters(resource?: Uri): Promise { diff --git a/src/client/interpreter/virtualEnvs/index.ts b/src/client/interpreter/virtualEnvs/index.ts index b44efa2b4bd6..ee8b6e2f9b35 100644 --- a/src/client/interpreter/virtualEnvs/index.ts +++ b/src/client/interpreter/virtualEnvs/index.ts @@ -32,8 +32,7 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri; const grandParentDirName = path.basename(path.dirname(path.dirname(pythonPath))); if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { - // In pipenv, the folder name is suffixed with `-`. - // So, to get the environment name, just drop the suffix starting with `-`. + // In pipenv, return the folder name of the environment. return grandParentDirName; } From f5eb35883d30d58101bc0771e4b4f6ae7a99dada Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 11:47:38 -0700 Subject: [PATCH 07/27] Minor refactorings --- .../common/installer/pythonInstallation.ts | 8 +++- src/client/common/platform/pathUtils.ts | 6 +-- src/client/interpreter/contracts.ts | 2 +- src/client/interpreter/display/index.ts | 38 ++++++++++--------- src/client/interpreter/interpreterService.ts | 14 +++++-- .../locators/services/currentPathService.ts | 2 +- .../services/globalVirtualEnvService.ts | 4 +- src/client/interpreter/virtualEnvs/index.ts | 8 ++-- 8 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index d70b1e96bcb7..6cfe6b40da46 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -4,24 +4,28 @@ import { IInterpreterHelper, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; -import { IApplicationShell } from '../application/types'; +import { IApplicationShell, IWorkspaceService } from '../application/types'; import { IPlatformService } from '../platform/types'; import { IPythonSettings } from '../types'; export class PythonInstaller { private locator: IInterpreterLocatorService; private shell: IApplicationShell; + private workspaceService: IWorkspaceService; constructor(private serviceContainer: IServiceContainer) { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); this.shell = serviceContainer.get(IApplicationShell); + this.workspaceService = serviceContainer.get(IWorkspaceService); } public async checkPythonInstallation(settings: IPythonSettings): Promise { if (settings.disableInstallationChecks === true) { return true; } - const interpreters = await this.locator.getInterpreters(); + + const workspaceUri = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders![0].uri : undefined; + const interpreters = await this.locator.getInterpreters(workspaceUri); if (interpreters.length > 0) { const platform = this.serviceContainer.get(IPlatformService); const helper = this.serviceContainer.get(IInterpreterHelper); diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index fc6cdb5c0858..4a6d6c6ddd3a 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -22,12 +22,12 @@ export class PathUtils implements IPathUtils { return path.basename(pathValue, ext); } public getDisplayName(pathValue: string, cwd?: string): string { - if (pathValue.startsWith(this.home)) { - pathValue = `~${path.sep}${path.relative(this.home, pathValue)}`; - } if (cwd && pathValue.startsWith(cwd)) { pathValue = `.${path.sep}${path.relative(cwd, pathValue)}`; } + if (pathValue.startsWith(this.home)) { + pathValue = `~${path.sep}${path.relative(this.home, pathValue)}`; + } return pathValue; } diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index 5eb799034db6..2934f9c22d0b 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -81,7 +81,7 @@ export interface IInterpreterService { getInterpreterDetails(pythonPath: string, resoure?: Uri): Promise; refresh(): Promise; initialize(): void; - getDisplayName(interpreter: Partial): string; + getDisplayName(interpreter: Partial): Promise; } export const IInterpreterDisplay = Symbol('IInterpreterDisplay'); diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 680ff3063b89..34440614843a 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -2,7 +2,7 @@ import { inject, injectable } from 'inversify'; import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IDisposableRegistry } from '../../common/types'; +import { IConfigurationService, IDisposableRegistry, IPathUtils } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService, PythonInterpreter } from '../contracts'; import { IVirtualEnvironmentManager } from '../virtualEnvs/types'; @@ -17,6 +17,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { private readonly configurationService: IConfigurationService; private readonly helper: IInterpreterHelper; private readonly workspaceService: IWorkspaceService; + private readonly pathUtils: IPathUtils; constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { this.interpreterService = serviceContainer.get(IInterpreterService); @@ -25,6 +26,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { this.configurationService = serviceContainer.get(IConfigurationService); this.helper = serviceContainer.get(IInterpreterHelper); this.workspaceService = serviceContainer.get(IWorkspaceService); + this.pathUtils = serviceContainer.get(IPathUtils); const application = serviceContainer.get(IApplicationShell); const disposableRegistry = serviceContainer.get(IDisposableRegistry); @@ -54,24 +56,24 @@ export class InterpreterDisplay implements IInterpreterDisplay { if (interpreter) { // tslint:disable-next-line:no-non-null-assertion this.statusBar.text = interpreter.displayName!; - this.statusBar.tooltip = pythonPath; - } else { - await Promise.all([ - this.fileSystem.fileExists(pythonPath), - this.helper.getInterpreterInformation(pythonPath).catch | undefined>(() => undefined), - this.getVirtualEnvironmentName(pythonPath, workspaceFolder).catch(() => '') - ]) - .then(([interpreterExists, details, virtualEnvName]) => { - if (details) { - const displayName = this.interpreterService.getDisplayName({ ...details, envName: virtualEnvName }); - this.statusBar.text = displayName; - } + this.statusBar.tooltip = this.pathUtils.getDisplayName(pythonPath, workspaceFolder ? workspaceFolder.fsPath : undefined); + this.statusBar.show(); + return; + } + + const [interpreterExists, details, virtualEnvName] = await Promise.all([ + this.fileSystem.fileExists(pythonPath), + this.helper.getInterpreterInformation(pythonPath).catch | undefined>(() => undefined), + this.getVirtualEnvironmentName(pythonPath, workspaceFolder).catch(() => '') + ]); + if (details) { + const displayName = await this.interpreterService.getDisplayName({ ...details, envName: virtualEnvName }); + this.statusBar.text = displayName; + } - if (!interpreterExists && !details && interpreters.length > 0) { - this.statusBar.color = 'yellow'; - this.statusBar.text = '$(alert) Select Python Environment'; - } - }); + if (!interpreterExists && !details && interpreters.length > 0) { + this.statusBar.color = 'yellow'; + this.statusBar.text = '$(alert) Select Python Environment'; } this.statusBar.show(); } diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index d29fa4c940bd..c5f42cf082ac 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -12,7 +12,7 @@ import { IPythonPathUpdaterServiceManager } from './configuration/types'; import { IInterpreterDisplay, IInterpreterHelper, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, - PIPENV_SERVICE, PythonInterpreter, WORKSPACE_VIRTUAL_ENV_SERVICE + InterpreterType, PIPENV_SERVICE, PythonInterpreter, WORKSPACE_VIRTUAL_ENV_SERVICE } from './contracts'; import { IVirtualEnvironmentManager } from './virtualEnvs/types'; @@ -22,11 +22,13 @@ export class InterpreterService implements Disposable, IInterpreterService { private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager; private readonly helper: IInterpreterHelper; private readonly didChangeInterpreterEmitter = new EventEmitter(); + private readonly virtualEnvMgr: IVirtualEnvironmentManager; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); this.helper = serviceContainer.get(IInterpreterHelper); this.pythonPathUpdaterService = this.serviceContainer.get(IPythonPathUpdaterServiceManager); + this.virtualEnvMgr = this.serviceContainer.get(IVirtualEnvironmentManager); } public async refresh(resource?: Uri) { @@ -44,7 +46,7 @@ export class InterpreterService implements Disposable, IInterpreterService { public async getInterpreters(resource?: Uri): Promise { const interpreters = await this.locator.getInterpreters(resource); - interpreters.forEach(item => item.displayName = this.getDisplayName(item)); + await Promise.all(interpreters.map(async item => item.displayName = await this.getDisplayName(item, resource))); return interpreters; } @@ -146,7 +148,7 @@ export class InterpreterService implements Disposable, IInterpreterService { * @returns {string} * @memberof InterpreterService */ - public getDisplayName(info: Partial): string { + public async getDisplayName(info: Partial, resource?: Uri): Promise { const displayNameParts: string[] = ['Python']; const envSuffixParts: string[] = []; @@ -156,6 +158,12 @@ export class InterpreterService implements Disposable, IInterpreterService { if (info.version_info) { displayNameParts.push(getArchitectureDisplayName(info.architecture)); } + if (!info.envName && info.path && info.type && info.type === InterpreterType.PipEnv) { + // If we do not have the name of the environment, then try to get it again. + // This can happen based on the context (i.e. resource). + // I.e. we can determine if an environment is PipEnv only when giving it the right workspacec path (i.e. resource). + info.envName = await this.virtualEnvMgr.getEnvironmentName(info.path, resource); + } if (info.envName && info.envName.length > 0) { envSuffixParts.push(info.envName); } diff --git a/src/client/interpreter/locators/services/currentPathService.ts b/src/client/interpreter/locators/services/currentPathService.ts index 2b0c3623e17f..c7c862eb8736 100644 --- a/src/client/interpreter/locators/services/currentPathService.ts +++ b/src/client/interpreter/locators/services/currentPathService.ts @@ -43,7 +43,7 @@ export class CurrentPathService extends CacheableLocatorService { * This is used by CacheableLocatorService.getInterpreters(). */ protected getInterpretersImplementation(resource?: Uri): Promise { - return this.suggestionsFromKnownPaths(); + return this.suggestionsFromKnownPaths(resource); } /** diff --git a/src/client/interpreter/locators/services/globalVirtualEnvService.ts b/src/client/interpreter/locators/services/globalVirtualEnvService.ts index 12a03d006fac..9812cc72bed1 100644 --- a/src/client/interpreter/locators/services/globalVirtualEnvService.ts +++ b/src/client/interpreter/locators/services/globalVirtualEnvService.ts @@ -31,9 +31,9 @@ export class GlobalVirtualEnvironmentsSearchPathProvider implements IVirtualEnvi this.config = serviceContainer.get(IConfigurationService); } - public getSearchPaths(_resource?: Uri): string[] { + public getSearchPaths(resource?: Uri): string[] { const homedir = os.homedir(); - const venvFolders = this.config.getSettings(_resource).venvFolders; + const venvFolders = this.config.getSettings(resource).venvFolders; const folders = venvFolders.map(item => path.join(homedir, item)); // tslint:disable-next-line:no-string-literal diff --git a/src/client/interpreter/virtualEnvs/index.ts b/src/client/interpreter/virtualEnvs/index.ts index ee8b6e2f9b35..44d99854aa7a 100644 --- a/src/client/interpreter/virtualEnvs/index.ts +++ b/src/client/interpreter/virtualEnvs/index.ts @@ -31,9 +31,9 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri; const grandParentDirName = path.basename(path.dirname(path.dirname(pythonPath))); - if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { - // In pipenv, return the folder name of the environment. - return grandParentDirName; + if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(workspaceUri.fsPath, pythonPath)) { + // In pipenv, return the folder name of the workspace. + return path.basename(workspaceUri.fsPath); } // https://stackoverflow.com/questions/1871549/determine-if-python-is-running-inside-virtualenv @@ -70,7 +70,7 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { const defaultWorkspaceUri = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders![0].uri : undefined; const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri; - if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(pythonPath, workspaceUri.fsPath)) { + if (workspaceUri && await this.pipEnvService.isRelatedPipEnvironment(workspaceUri.fsPath, pythonPath)) { return InterpreterType.PipEnv; } From aa7cba48510be9eb36a276a72c979cdc27901337 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 11:49:32 -0700 Subject: [PATCH 08/27] refactor --- src/client/common/platform/pathUtils.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 4a6d6c6ddd3a..16fc16c530de 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -23,12 +23,11 @@ export class PathUtils implements IPathUtils { } public getDisplayName(pathValue: string, cwd?: string): string { if (cwd && pathValue.startsWith(cwd)) { - pathValue = `.${path.sep}${path.relative(cwd, pathValue)}`; + return `.${path.sep}${path.relative(cwd, pathValue)}`; + } else if (pathValue.startsWith(this.home)) { + return `~${path.sep}${path.relative(this.home, pathValue)}`; + } else { + return pathValue; } - if (pathValue.startsWith(this.home)) { - pathValue = `~${path.sep}${path.relative(this.home, pathValue)}`; - } - return pathValue; } - } From f9196d3808b25f45ad4bd21fae34918942556446 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 15:57:16 -0700 Subject: [PATCH 09/27] Fixed tests --- .../interpreterSelector.unit.test.ts | 43 +++++++++---------- .../install/pythonInstallation.unit.test.ts | 9 ++-- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/test/configuration/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector.unit.test.ts index 2cc72701dc09..49b33cfc9bed 100644 --- a/src/test/configuration/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector.unit.test.ts @@ -2,17 +2,15 @@ // Licensed under the MIT License. import * as assert from 'assert'; -import { Container } from 'inversify'; import * as TypeMoq from 'typemoq'; import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; import { PathUtils } from '../../client/common/platform/pathUtils'; import { IFileSystem } from '../../client/common/platform/types'; import { IPathUtils } from '../../client/common/types'; import { IInterpreterQuickPickItem, InterpreterSelector } from '../../client/interpreter/configuration/interpreterSelector'; +import { IInterpreterComparer } from '../../client/interpreter/configuration/types'; import { IInterpreterService, InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts'; -import { ServiceContainer } from '../../client/ioc/container'; -import { ServiceManager } from '../../client/ioc/serviceManager'; -import { IServiceContainer, IServiceManager } from '../../client/ioc/types'; +import { IServiceContainer } from '../../client/ioc/types'; import { Architecture } from '../../utils/platform'; const info: PythonInterpreter = { @@ -41,30 +39,21 @@ class InterpreterQuickPickItem implements IInterpreterQuickPickItem { // tslint:disable-next-line:max-func-body-length suite('Interpreters - selector', () => { - let serviceContainer: IServiceContainer; + let serviceContainer: TypeMoq.IMock; let workspace: TypeMoq.IMock; let appShell: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let documentManager: TypeMoq.IMock; let fileSystem: TypeMoq.IMock; - let serviceManager: IServiceManager; setup(() => { - const cont = new Container(); - serviceManager = new ServiceManager(cont); - serviceContainer = new ServiceContainer(cont); - - workspace = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(IWorkspaceService, workspace.object); - + const commandManager = TypeMoq.Mock.ofType(); + const comparer = TypeMoq.Mock.ofType(); + serviceContainer = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(IApplicationShell, appShell.object); - interpreterService = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(IInterpreterService, interpreterService.object); - documentManager = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); + workspace = TypeMoq.Mock.ofType(); fileSystem = TypeMoq.Mock.ofType(); fileSystem .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) @@ -73,15 +62,23 @@ suite('Interpreters - selector', () => { .setup(x => x.getRealPath(TypeMoq.It.isAnyString())) .returns((a: string) => new Promise(resolve => resolve(a))); - serviceManager.addSingletonInstance(IFileSystem, fileSystem.object); + comparer.setup(c => c.compare(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => 0); - const commandManager = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(ICommandManager, commandManager.object); + serviceContainer.setup(c => c.get(IWorkspaceService)).returns(() => workspace.object); + serviceContainer.setup(c => c.get(IApplicationShell)).returns(() => appShell.object); + serviceContainer.setup(c => c.get(IInterpreterService)).returns(() => interpreterService.object); + serviceContainer.setup(c => c.get(IDocumentManager)).returns(() => documentManager.object); + serviceContainer.setup(c => c.get(IFileSystem)).returns(() => fileSystem.object); + serviceContainer.setup(c => c.get(IInterpreterComparer)).returns(() => comparer.object); + serviceContainer.setup(c => c.get(ICommandManager)).returns(() => commandManager.object); }); [true, false].forEach(isWindows => { test(`Suggestions (${isWindows} ? 'Windows' : 'Non-Windows')`, async () => { - serviceManager.addSingletonInstance(IPathUtils, new PathUtils(isWindows)); + serviceContainer + .setup(c => c.get(IPathUtils)) + .returns(() => new PathUtils(isWindows)); + const initial: PythonInterpreter[] = [ { displayName: '1', path: 'c:/path1/path1', type: InterpreterType.Unknown }, { displayName: '2', path: 'c:/path1/path1', type: InterpreterType.Unknown }, @@ -94,7 +91,7 @@ suite('Interpreters - selector', () => { .setup(x => x.getInterpreters(TypeMoq.It.isAny())) .returns(() => new Promise((resolve) => resolve(initial))); - const selector = new InterpreterSelector(serviceContainer); + const selector = new InterpreterSelector(serviceContainer.object); const actual = await selector.getSuggestions(); const expected: InterpreterQuickPickItem[] = [ diff --git a/src/test/install/pythonInstallation.unit.test.ts b/src/test/install/pythonInstallation.unit.test.ts index 0bb3eda0d8d2..6ed960483c3d 100644 --- a/src/test/install/pythonInstallation.unit.test.ts +++ b/src/test/install/pythonInstallation.unit.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { Container } from 'inversify'; import * as TypeMoq from 'typemoq'; -import { IApplicationShell } from '../../client/common/application/types'; +import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { PythonInstaller } from '../../client/common/installer/pythonInstallation'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IPersistentStateFactory, IPythonSettings } from '../../client/common/types'; @@ -53,6 +53,7 @@ class TestContext { type: InterpreterType.Unknown, path: '' }; + const workspaceService = TypeMoq.Mock.ofType(); const interpreterService = TypeMoq.Mock.ofType(); interpreterService .setup(x => x.getActiveInterpreter(TypeMoq.It.isAny())) @@ -65,10 +66,12 @@ class TestContext { this.serviceManager.addSingletonInstance(IApplicationShell, this.appShell.object); this.serviceManager.addSingletonInstance(IInterpreterLocatorService, this.locator.object); this.serviceManager.addSingletonInstance(IInterpreterService, interpreterService.object); + this.serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); this.pythonInstaller = new PythonInstaller(this.serviceContainer); this.platform.setup(x => x.isMac).returns(() => isMac); this.platform.setup(x => x.isWindows).returns(() => !isMac); + workspaceService.setup(w => w.hasWorkspaceFolders).returns(() => false); } } @@ -100,7 +103,7 @@ suite('Installation', () => { openUrlCalled = true; url = s; }); - c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); + c.locator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); assert.equal(passed, false, 'Python reported as present'); @@ -130,7 +133,7 @@ suite('Installation', () => { path: 'python', type: InterpreterType.Unknown }; - c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([interpreter])); + c.locator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve([interpreter])); const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); assert.equal(passed, true, 'Default MacOS Python not accepted'); From 5ef3621776b3d25bf77ea22a27586eeb3de2fa6f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Sep 2018 16:22:19 -0700 Subject: [PATCH 10/27] Renamed test files and fixed some tests --- ...ileService.test.ts => condaEnvFileService.unit.test.ts} | 5 +---- ...ondaEnvService.test.ts => condaEnvService.unit.test.ts} | 7 ++----- .../{condaHelper.test.ts => condaHelper.unit.test.ts} | 3 --- .../{condaService.test.ts => condaService.unit.test.ts} | 0 ...PathService.test.ts => currentPathService.unit.test.ts} | 2 +- .../interpreters/{display.test.ts => display.unit.test.ts} | 0 .../interpreters/{helper.test.ts => helper.unit.test.ts} | 0 ...eterService.test.ts => interpreterService.unit.test.ts} | 0 ...eterVersion.test.ts => interpreterVersion.unit.test.ts} | 0 src/test/interpreters/{venv.test.ts => venv.unit.test.ts} | 0 ...alEnvManager.test.ts => virtualEnvManager.unit.test.ts} | 0 11 files changed, 4 insertions(+), 13 deletions(-) rename src/test/interpreters/{condaEnvFileService.test.ts => condaEnvFileService.unit.test.ts} (98%) rename src/test/interpreters/{condaEnvService.test.ts => condaEnvService.unit.test.ts} (99%) rename src/test/interpreters/{condaHelper.test.ts => condaHelper.unit.test.ts} (95%) rename src/test/interpreters/{condaService.test.ts => condaService.unit.test.ts} (100%) rename src/test/interpreters/{currentPathService.test.ts => currentPathService.unit.test.ts} (98%) rename src/test/interpreters/{display.test.ts => display.unit.test.ts} (100%) rename src/test/interpreters/{helper.test.ts => helper.unit.test.ts} (100%) rename src/test/interpreters/{interpreterService.test.ts => interpreterService.unit.test.ts} (100%) rename src/test/interpreters/{interpreterVersion.test.ts => interpreterVersion.unit.test.ts} (100%) rename src/test/interpreters/{venv.test.ts => venv.unit.test.ts} (100%) rename src/test/interpreters/{virtualEnvManager.test.ts => virtualEnvManager.unit.test.ts} (100%) diff --git a/src/test/interpreters/condaEnvFileService.test.ts b/src/test/interpreters/condaEnvFileService.unit.test.ts similarity index 98% rename from src/test/interpreters/condaEnvFileService.test.ts rename to src/test/interpreters/condaEnvFileService.unit.test.ts index 062722354c90..47d6b21beb79 100644 --- a/src/test/interpreters/condaEnvFileService.test.ts +++ b/src/test/interpreters/condaEnvFileService.unit.test.ts @@ -8,7 +8,6 @@ import { ICondaService, IInterpreterHelper, IInterpreterLocatorService, Interpre import { AnacondaCompanyName, AnacondaCompanyNames, AnacondaDisplayName } from '../../client/interpreter/locators/services/conda'; import { CondaEnvFileService } from '../../client/interpreter/locators/services/condaEnvFileService'; import { IServiceContainer } from '../../client/ioc/types'; -import { initialize, initializeTest } from '../initialize'; import { MockState } from './mocks'; const environmentsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'environments'); @@ -21,9 +20,7 @@ suite('Interpreters from Conda Environments Text File', () => { let interpreterHelper: TypeMoq.IMock; let condaFileProvider: IInterpreterLocatorService; let fileSystem: TypeMoq.IMock; - suiteSetup(initialize); - setup(async () => { - await initializeTest(); + setup(() => { const serviceContainer = TypeMoq.Mock.ofType(); const stateFactory = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPersistentStateFactory))).returns(() => stateFactory.object); diff --git a/src/test/interpreters/condaEnvService.test.ts b/src/test/interpreters/condaEnvService.unit.test.ts similarity index 99% rename from src/test/interpreters/condaEnvService.test.ts rename to src/test/interpreters/condaEnvService.unit.test.ts index ac9ec03efecd..314bcc39f0cf 100644 --- a/src/test/interpreters/condaEnvService.test.ts +++ b/src/test/interpreters/condaEnvService.unit.test.ts @@ -8,23 +8,20 @@ import { InterpreterHelper } from '../../client/interpreter/helpers'; import { AnacondaCompanyName, AnacondaDisplayName } from '../../client/interpreter/locators/services/conda'; import { CondaEnvService, parseCondaInfo } from '../../client/interpreter/locators/services/condaEnvService'; import { IServiceContainer } from '../../client/ioc/types'; -import { initialize, initializeTest } from '../initialize'; import { UnitTestIocContainer } from '../unittests/serviceRegistry'; import { MockState } from './mocks'; const environmentsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'environments'); // tslint:disable-next-line:max-func-body-length -suite('Interpreters from Conda Environmentsx', () => { +suite('Interpreters from Conda Environments', () => { let ioc: UnitTestIocContainer; let logger: TypeMoq.IMock; let condaProvider: CondaEnvService; let condaService: TypeMoq.IMock; let interpreterHelper: TypeMoq.IMock; let fileSystem: TypeMoq.IMock; - suiteSetup(initialize); - setup(async () => { - await initializeTest(); + setup(() => { initializeDI(); const serviceContainer = TypeMoq.Mock.ofType(); const stateFactory = TypeMoq.Mock.ofType(); diff --git a/src/test/interpreters/condaHelper.test.ts b/src/test/interpreters/condaHelper.unit.test.ts similarity index 95% rename from src/test/interpreters/condaHelper.test.ts rename to src/test/interpreters/condaHelper.unit.test.ts index 322a0fed9ffd..318ae6fa0685 100644 --- a/src/test/interpreters/condaHelper.test.ts +++ b/src/test/interpreters/condaHelper.unit.test.ts @@ -2,13 +2,10 @@ import * as assert from 'assert'; import { CondaInfo } from '../../client/interpreter/contracts'; import { AnacondaDisplayName } from '../../client/interpreter/locators/services/conda'; import { CondaHelper } from '../../client/interpreter/locators/services/condaHelper'; -import { initialize, initializeTest } from '../initialize'; // tslint:disable-next-line:max-func-body-length suite('Interpreters display name from Conda Environments', () => { const condaHelper = new CondaHelper(); - suiteSetup(initialize); - setup(initializeTest); test('Must return default display name for invalid Conda Info', () => { assert.equal(condaHelper.getDisplayName(), AnacondaDisplayName, 'Incorrect display name'); assert.equal(condaHelper.getDisplayName({}), AnacondaDisplayName, 'Incorrect display name'); diff --git a/src/test/interpreters/condaService.test.ts b/src/test/interpreters/condaService.unit.test.ts similarity index 100% rename from src/test/interpreters/condaService.test.ts rename to src/test/interpreters/condaService.unit.test.ts diff --git a/src/test/interpreters/currentPathService.test.ts b/src/test/interpreters/currentPathService.unit.test.ts similarity index 98% rename from src/test/interpreters/currentPathService.test.ts rename to src/test/interpreters/currentPathService.unit.test.ts index 87949998b5e6..ce932c39cac4 100644 --- a/src/test/interpreters/currentPathService.test.ts +++ b/src/test/interpreters/currentPathService.unit.test.ts @@ -57,7 +57,7 @@ suite('Interpreters CurrentPath Service', () => { const version = 'mockVersion'; const envName = 'mockEnvName'; interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ version })); - virtualEnvironmentManager.setup(v => v.getEnvironmentName(TypeMoq.It.isAny())).returns(() => Promise.resolve(envName)); + virtualEnvironmentManager.setup(v => v.getEnvironmentName(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(envName)); virtualEnvironmentManager.setup(v => v.getEnvironmentType(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(InterpreterType.VirtualEnv)); const execArgs = ['-c', 'import sys;print(sys.executable)']; diff --git a/src/test/interpreters/display.test.ts b/src/test/interpreters/display.unit.test.ts similarity index 100% rename from src/test/interpreters/display.test.ts rename to src/test/interpreters/display.unit.test.ts diff --git a/src/test/interpreters/helper.test.ts b/src/test/interpreters/helper.unit.test.ts similarity index 100% rename from src/test/interpreters/helper.test.ts rename to src/test/interpreters/helper.unit.test.ts diff --git a/src/test/interpreters/interpreterService.test.ts b/src/test/interpreters/interpreterService.unit.test.ts similarity index 100% rename from src/test/interpreters/interpreterService.test.ts rename to src/test/interpreters/interpreterService.unit.test.ts diff --git a/src/test/interpreters/interpreterVersion.test.ts b/src/test/interpreters/interpreterVersion.unit.test.ts similarity index 100% rename from src/test/interpreters/interpreterVersion.test.ts rename to src/test/interpreters/interpreterVersion.unit.test.ts diff --git a/src/test/interpreters/venv.test.ts b/src/test/interpreters/venv.unit.test.ts similarity index 100% rename from src/test/interpreters/venv.test.ts rename to src/test/interpreters/venv.unit.test.ts diff --git a/src/test/interpreters/virtualEnvManager.test.ts b/src/test/interpreters/virtualEnvManager.unit.test.ts similarity index 100% rename from src/test/interpreters/virtualEnvManager.test.ts rename to src/test/interpreters/virtualEnvManager.unit.test.ts From 9ba547017cb2199f3f5b3d5a4b3e315612eb19e8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 09:32:53 -0700 Subject: [PATCH 11/27] Fixed tests --- src/client/interpreter/display/index.ts | 4 +- src/test/interpreters/display.unit.test.ts | 121 +++++---------------- 2 files changed, 32 insertions(+), 93 deletions(-) diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 34440614843a..0ac73d8af537 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -52,11 +52,10 @@ export class InterpreterDisplay implements IInterpreterDisplay { const pythonPath = interpreter ? interpreter.path : this.configurationService.getSettings(workspaceFolder).pythonPath; this.statusBar.color = ''; - this.statusBar.tooltip = pythonPath; + this.statusBar.tooltip = this.pathUtils.getDisplayName(pythonPath, workspaceFolder ? workspaceFolder.fsPath : undefined); if (interpreter) { // tslint:disable-next-line:no-non-null-assertion this.statusBar.text = interpreter.displayName!; - this.statusBar.tooltip = this.pathUtils.getDisplayName(pythonPath, workspaceFolder ? workspaceFolder.fsPath : undefined); this.statusBar.show(); return; } @@ -72,6 +71,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { } if (!interpreterExists && !details && interpreters.length > 0) { + this.statusBar.tooltip = ''; this.statusBar.color = 'yellow'; this.statusBar.text = '$(alert) Select Python Environment'; } diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index 6499684079c0..9b71aac720b3 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -1,11 +1,10 @@ import { expect } from 'chai'; -import { EOL } from 'os'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Disposable, StatusBarAlignment, StatusBarItem, Uri, WorkspaceFolder } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; -import { IConfigurationService, IDisposableRegistry, IPythonSettings } from '../../client/common/types'; +import { IConfigurationService, IDisposableRegistry, IPathUtils, IPythonSettings } from '../../client/common/types'; import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService, InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts'; import { InterpreterDisplay } from '../../client/interpreter/display'; import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; @@ -39,6 +38,7 @@ suite('Interpreters Display', () => { let configurationService: TypeMoq.IMock; let interpreterDisplay: IInterpreterDisplay; let interpreterHelper: TypeMoq.IMock; + let pathUtils: TypeMoq.IMock; setup(() => { serviceContainer = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); @@ -51,6 +51,7 @@ suite('Interpreters Display', () => { statusBar = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); configurationService = TypeMoq.Mock.ofType(); + pathUtils = TypeMoq.Mock.ofType(); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService))).returns(() => workspaceService.object); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => applicationShell.object); @@ -60,8 +61,10 @@ suite('Interpreters Display', () => { serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => disposableRegistry); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IConfigurationService))).returns(() => configurationService.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); applicationShell.setup(a => a.createStatusBarItem(TypeMoq.It.isValue(StatusBarAlignment.Left), TypeMoq.It.isValue(undefined))).returns(() => statusBar.object); + pathUtils.setup(p => p.getDisplayName(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(p => p); interpreterDisplay = new InterpreterDisplay(serviceContainer.object); }); @@ -97,60 +100,25 @@ suite('Interpreters Display', () => { 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.once()); }); - test('Display name and tooltip must include company display name from interpreter info', async () => { - const resource = Uri.file('x'); - const workspaceFolder = Uri.file('workspace'); - const activeInterpreter: PythonInterpreter = { - ...info, - displayName: 'Dummy_Display_Name', - type: InterpreterType.Unknown, - companyDisplayName: 'Company Name', - path: path.join('user', 'development', 'env', 'bin', 'python') - }; - setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(activeInterpreter)); - const expectedTooltip = `${activeInterpreter.path}${EOL}${activeInterpreter.companyDisplayName}`; - - await interpreterDisplay.refresh(resource); - - statusBar.verify(s => s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!, TypeMoq.Times.once()); - statusBar.verify(s => s.tooltip = TypeMoq.It.isValue(expectedTooltip)!, TypeMoq.Times.once()); - }); - test('If interpreter is not identified then tooltip should point to python Path and text containing the folder name', async () => { + test('If interpreter is not identified then tooltip should point to python Path', async () => { const resource = Uri.file('x'); const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); const workspaceFolder = Uri.file('workspace'); - setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(undefined)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - pythonSettings.setup(p => p.pythonPath).returns(() => pythonPath); - virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); - interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(undefined)); - - await interpreterDisplay.refresh(resource); + const displayName = 'This is the display name'; - statusBar.verify(s => s.tooltip = TypeMoq.It.isValue(pythonPath), TypeMoq.Times.once()); - statusBar.verify(s => s.text = TypeMoq.It.isValue(`${path.basename(pythonPath)} [Environment]`), TypeMoq.Times.once()); - }); - test('If virtual environment interpreter is not identified then text should contain the type of virtual environment', async () => { - const resource = Uri.file('x'); - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const workspaceFolder = Uri.file('workspace'); setupWorkspaceFolder(resource, workspaceFolder); interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([])); interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(undefined)); + interpreterService.setup(i => i.getDisplayName(TypeMoq.It.isAny())).returns(() => Promise.resolve(displayName)); configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); pythonSettings.setup(p => p.pythonPath).returns(() => pythonPath); - // tslint:disable-next-line:no-any - virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('Mock Name')); - interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(undefined)); + virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); + interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve({})); await interpreterDisplay.refresh(resource); statusBar.verify(s => s.tooltip = TypeMoq.It.isValue(pythonPath), TypeMoq.Times.once()); - statusBar.verify(s => s.text = TypeMoq.It.isValue(`${path.basename(pythonPath)} [Environment] (Mock Name)`), TypeMoq.Times.once()); + statusBar.verify(s => s.text = TypeMoq.It.isValue(displayName), TypeMoq.Times.once()); }); test('If interpreter file does not exist then update status bar accordingly', async () => { const resource = Uri.file('x'); @@ -171,66 +139,37 @@ suite('Interpreters Display', () => { statusBar.verify(s => s.color = TypeMoq.It.isValue('yellow'), TypeMoq.Times.once()); statusBar.verify(s => s.text = TypeMoq.It.isValue('$(alert) Select Python Environment'), TypeMoq.Times.once()); }); - test('Suffix display name with the virtual env name', async () => { - const resource = Uri.file('x'); - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const workspaceFolder = Uri.file('workspace'); - setupWorkspaceFolder(resource, workspaceFolder); - // tslint:disable-next-line:no-any - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([{} as any])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(undefined)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - pythonSettings.setup(p => p.pythonPath).returns(() => pythonPath); - fileSystem.setup(f => f.fileExists(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(true)); - const defaultDisplayName = `${path.basename(pythonPath)} [Environment]`; - interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(undefined)); - // tslint:disable-next-line:no-any - virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('Mock Env Name')); - const expectedText = `${defaultDisplayName} (Mock Env Name)`; - - await interpreterDisplay.refresh(resource); - - statusBar.verify(s => s.text = TypeMoq.It.isValue(expectedText), TypeMoq.Times.once()); - }); - test('Use version of interpreter instead of a default interpreter name', async () => { - const resource = Uri.file('x'); - const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); - const workspaceFolder = Uri.file('workspace'); - setupWorkspaceFolder(resource, workspaceFolder); - // tslint:disable-next-line:no-any - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([{} as any])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(undefined)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - pythonSettings.setup(p => p.pythonPath).returns(() => pythonPath); - fileSystem.setup(f => f.fileExists(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(true)); - const displayName = 'Version from Interperter'; - interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve({ version: displayName })); - // tslint:disable-next-line:no-any - virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); - - await interpreterDisplay.refresh(resource); - - statusBar.verify(s => s.text = TypeMoq.It.isValue(displayName), TypeMoq.Times.once()); - }); test('Ensure we try to identify the active workspace when a resource is not provided ', async () => { const workspaceFolder = Uri.file('x'); const resource = workspaceFolder; + const pythonPath = path.join('user', 'development', 'env', 'bin', 'python'); const activeInterpreter: PythonInterpreter = { ...info, displayName: 'Dummy_Display_Name', type: InterpreterType.Unknown, companyDisplayName: 'Company Name', - path: path.join('user', 'development', 'env', 'bin', 'python') + path: pythonPath }; - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(resource))).returns(() => Promise.resolve([])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(resource))).returns(() => Promise.resolve(activeInterpreter)); - interpreterHelper.setup(i => i.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); - const expectedTooltip = `${activeInterpreter.path}${EOL}${activeInterpreter.companyDisplayName}`; - interpreterHelper.setup(i => i.getActiveWorkspaceUri()).returns(() => { return { folderUri: workspaceFolder, configTarget: ConfigurationTarget.Workspace }; }); + fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); + virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); + interpreterService + .setup(i => i.getInterpreters(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve([])) + .verifiable(TypeMoq.Times.once()); + interpreterService + .setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve(activeInterpreter)) + .verifiable(TypeMoq.Times.once()); + interpreterHelper + .setup(i => i.getActiveWorkspaceUri()) + .returns(() => { return { folderUri: workspaceFolder, configTarget: ConfigurationTarget.Workspace }; }) + .verifiable(TypeMoq.Times.once()); await interpreterDisplay.refresh(); + interpreterHelper.verifyAll(); + interpreterService.verifyAll(); statusBar.verify(s => s.text = TypeMoq.It.isValue(activeInterpreter.displayName)!, TypeMoq.Times.once()); - statusBar.verify(s => s.tooltip = TypeMoq.It.isValue(expectedTooltip)!, TypeMoq.Times.once()); + statusBar.verify(s => s.tooltip = TypeMoq.It.isValue(pythonPath)!, TypeMoq.Times.once()); }); }); From fe1379a8d53dfceafbc5e146e96aa5c5371976ed Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 09:47:58 -0700 Subject: [PATCH 12/27] Fixed more tests --- src/client/interpreter/interpreterService.ts | 5 ++--- src/test/interpreters/interpreterService.unit.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index c5f42cf082ac..c24747b5a8f1 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -22,13 +22,11 @@ export class InterpreterService implements Disposable, IInterpreterService { private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager; private readonly helper: IInterpreterHelper; private readonly didChangeInterpreterEmitter = new EventEmitter(); - private readonly virtualEnvMgr: IVirtualEnvironmentManager; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); this.helper = serviceContainer.get(IInterpreterHelper); this.pythonPathUpdaterService = this.serviceContainer.get(IPythonPathUpdaterServiceManager); - this.virtualEnvMgr = this.serviceContainer.get(IVirtualEnvironmentManager); } public async refresh(resource?: Uri) { @@ -162,7 +160,8 @@ export class InterpreterService implements Disposable, IInterpreterService { // If we do not have the name of the environment, then try to get it again. // This can happen based on the context (i.e. resource). // I.e. we can determine if an environment is PipEnv only when giving it the right workspacec path (i.e. resource). - info.envName = await this.virtualEnvMgr.getEnvironmentName(info.path, resource); + const virtualEnvMgr = this.serviceContainer.get(IVirtualEnvironmentManager); + info.envName = await virtualEnvMgr.getEnvironmentName(info.path, resource); } if (info.envName && info.envName.length > 0) { envSuffixParts.push(info.envName); diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 7449d111ce9d..121bf7e0557c 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -101,7 +101,7 @@ suite('Interpreters service', () => { }); const interpreter: PythonInterpreter = { ...info, - path: path.join(path.sep, 'folder', 'py1', 'bin', 'python.exe'), + path: path.join('folder', 'py1', 'bin', 'python.exe'), type: InterpreterType.Unknown }; setupLocators([interpreter], []); @@ -151,7 +151,7 @@ suite('Interpreters service', () => { config.setup(x => x.inspect('pythonPath')).returns(() => { return { key: 'python' }; }); - const intPath = path.join(path.sep, 'root', 'under', 'bin', 'python.exe'); + const intPath = path.join('root', 'under', 'bin', 'python.exe'); const interpreter: PythonInterpreter = { ...info, path: intPath, @@ -192,7 +192,7 @@ suite('Interpreters service', () => { expect(pp!).to.be.equal(pythonPath, 'invalid Python path'); expect(confTarget).to.be.equal(target, 'invalid configuration target'); expect(trigger).to.be.equal('load', 'invalid trigger'); - expect(wks.fsPath).to.be.equal(`${path.sep}${wksFolder}`, 'invalid workspace Uri'); + expect(wks.fsPath).to.be.equal(wksFolder, 'invalid workspace Uri'); } function setupWorkspace(folder: string) { From 979eed726cec924fc63617ede87b4eae7955ff5e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 10:12:05 -0700 Subject: [PATCH 13/27] Fix tests --- .../virtualEnvManager.unit.test.ts | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/test/interpreters/virtualEnvManager.unit.test.ts b/src/test/interpreters/virtualEnvManager.unit.test.ts index de16e18c03bd..4b3d38e94fa8 100644 --- a/src/test/interpreters/virtualEnvManager.unit.test.ts +++ b/src/test/interpreters/virtualEnvManager.unit.test.ts @@ -5,7 +5,9 @@ import { expect } from 'chai'; import { Container } from 'inversify'; +import * as path from 'path'; import * as TypeMoq from 'typemoq'; +import { Uri, WorkspaceFolder } from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; import { FileSystem } from '../../client/common/platform/fileSystem'; import { PlatformService } from '../../client/common/platform/platformService'; @@ -17,21 +19,23 @@ import { IPipEnvService } from '../../client/interpreter/contracts'; import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; import { ServiceContainer } from '../../client/ioc/container'; import { ServiceManager } from '../../client/ioc/serviceManager'; -import { PYTHON_PATH } from '../common'; suite('Virtual environment manager', () => { let serviceManager: ServiceManager; let serviceContainer: ServiceContainer; - + const virtualEnvFolderName = 'virtual Env Folder Name'; + const pythonPath = path.join('a', 'b', virtualEnvFolderName, 'd', 'python'); setup(async () => { const cont = new Container(); serviceManager = new ServiceManager(cont); serviceContainer = new ServiceContainer(cont); }); - test('Plain Python environment suffix', async () => testSuffix('')); - test('Venv environment suffix', async () => testSuffix('venv')); - test('Virtualenv Python environment suffix', async () => testSuffix('virtualenv')); + test('Plain Python environment suffix', async () => testSuffix('', '')); + test('Plain Python environment suffix with workspace Uri', async () => testSuffix('', '', false, Uri.file(path.join('1', '2', '3', '4')))); + test('Plain Python environment suffix with PipEnv', async () => testSuffix('', 'workspaceName', true, Uri.file(path.join('1', '2', '3', 'workspaceName')))); + test('Venv environment suffix', async () => testSuffix('venv', 'venv')); + test('Virtualenv Python environment suffix', async () => testSuffix('virtualenv', virtualEnvFolderName)); test('Run actual virtual env detection code', async () => { const processServiceFactory = TypeMoq.Mock.ofType(); @@ -41,32 +45,43 @@ suite('Virtual environment manager', () => { serviceManager.addSingleton(IFileSystem, FileSystem); serviceManager.addSingleton(IPlatformService, PlatformService); serviceManager.addSingletonInstance(IPipEnvService, TypeMoq.Mock.ofType().object); - serviceManager.addSingletonInstance(IWorkspaceService, TypeMoq.Mock.ofType().object); + const workspaceService = TypeMoq.Mock.ofType(); + workspaceService.setup(w => w.hasWorkspaceFolders).returns(() => false); + serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); const venvManager = new VirtualEnvironmentManager(serviceContainer); - const name = await venvManager.getEnvironmentName(PYTHON_PATH); + const name = await venvManager.getEnvironmentName(pythonPath); const result = name === '' || name === 'venv' || name === 'virtualenv'; expect(result).to.be.equal(true, 'Running venv detection code failed.'); }); - async function testSuffix(expectedName: string) { + async function testSuffix(virtualEnvProcOutput: string, expectedEnvName: string, isPipEnvironment: boolean = false, resource?: Uri) { const processService = TypeMoq.Mock.ofType(); const processServiceFactory = TypeMoq.Mock.ofType(); processService.setup((x: any) => x.then).returns(() => undefined); processServiceFactory.setup(f => f.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(processService.object)); serviceManager.addSingletonInstance(IProcessServiceFactory, processServiceFactory.object); serviceManager.addSingletonInstance(IFileSystem, TypeMoq.Mock.ofType().object); - serviceManager.addSingletonInstance(IPipEnvService, TypeMoq.Mock.ofType().object); - serviceManager.addSingletonInstance(IWorkspaceService, TypeMoq.Mock.ofType().object); + const pipEnvService = TypeMoq.Mock.ofType(); + pipEnvService.setup(w => w.isRelatedPipEnvironment(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(isPipEnvironment)); + serviceManager.addSingletonInstance(IPipEnvService, pipEnvService.object); + const workspaceService = TypeMoq.Mock.ofType(); + workspaceService.setup(w => w.hasWorkspaceFolders).returns(() => false); + if (resource) { + const workspaceFolder = TypeMoq.Mock.ofType(); + workspaceFolder.setup(w => w.uri).returns(() => resource); + workspaceService.setup(w => w.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => workspaceFolder.object); + } + serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); const venvManager = new VirtualEnvironmentManager(serviceContainer); processService - .setup(x => x.exec(PYTHON_PATH, TypeMoq.It.isAny())) + .setup(x => x.exec(pythonPath, TypeMoq.It.isAny())) .returns(() => Promise.resolve({ - stdout: expectedName, + stdout: virtualEnvProcOutput, stderr: '' })); - const name = await venvManager.getEnvironmentName(PYTHON_PATH); - expect(name).to.be.equal(expectedName, 'Virtual envrironment name suffix is incorrect.'); + const name = await venvManager.getEnvironmentName(pythonPath, resource); + expect(name).to.be.equal(expectedEnvName, 'Virtual envrironment name suffix is incorrect.'); } }); From 6ee554def398bb3850778a4f85c1a498202d7b50 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 11:13:20 -0700 Subject: [PATCH 14/27] Use Uri to build paths when comparing Uri paths --- src/client/interpreter/interpreterService.ts | 2 +- src/test/interpreters/interpreterService.unit.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index c24747b5a8f1..22247ac2515c 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -68,7 +68,7 @@ export class InterpreterService implements Disposable, IInterpreterService { interpreters = await virtualEnvInterpreterProvider.getInterpreters(activeWorkspace.folderUri); const workspacePathUpper = activeWorkspace.folderUri.fsPath.toUpperCase(); - const interpretersInWorkspace = interpreters.filter(interpreter => interpreter.path.toUpperCase().startsWith(workspacePathUpper)); + const interpretersInWorkspace = interpreters.filter(interpreter => Uri.file(interpreter.path).fsPath.toUpperCase().startsWith(workspacePathUpper)); if (interpretersInWorkspace.length === 0) { return; } diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 121bf7e0557c..6c293b494db7 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -192,7 +192,7 @@ suite('Interpreters service', () => { expect(pp!).to.be.equal(pythonPath, 'invalid Python path'); expect(confTarget).to.be.equal(target, 'invalid configuration target'); expect(trigger).to.be.equal('load', 'invalid trigger'); - expect(wks.fsPath).to.be.equal(wksFolder, 'invalid workspace Uri'); + expect(wks.fsPath).to.be.equal(Uri.file(wksFolder).fsPath, 'invalid workspace Uri'); } function setupWorkspace(folder: string) { From bf13254b11f74f749db9e1204e58d5a46d0d2289 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 12:48:16 -0700 Subject: [PATCH 15/27] Fixed tests --- src/test/interpreters/interpreterVersion.unit.test.ts | 7 +------ src/test/interpreters/venv.unit.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/test/interpreters/interpreterVersion.unit.test.ts b/src/test/interpreters/interpreterVersion.unit.test.ts index 63b0dc45b3b9..31710ca4726a 100644 --- a/src/test/interpreters/interpreterVersion.unit.test.ts +++ b/src/test/interpreters/interpreterVersion.unit.test.ts @@ -8,18 +8,13 @@ import { IProcessServiceFactory } from '../../client/common/process/types'; import { IInterpreterVersionService } from '../../client/interpreter/contracts'; import { PIP_VERSION_REGEX } from '../../client/interpreter/interpreterVersion'; import { PYTHON_PATH } from '../common'; -import { initialize, initializeTest } from '../initialize'; import { UnitTestIocContainer } from '../unittests/serviceRegistry'; use(chaiAsPromised); suite('Interpreters display version', () => { let ioc: UnitTestIocContainer; - suiteSetup(initialize); - setup(async () => { - initializeDI(); - await initializeTest(); - }); + setup(initializeDI); teardown(() => ioc.dispose()); function initializeDI() { diff --git a/src/test/interpreters/venv.unit.test.ts b/src/test/interpreters/venv.unit.test.ts index 13f064bcbc4a..1e78b3bc1dcd 100644 --- a/src/test/interpreters/venv.unit.test.ts +++ b/src/test/interpreters/venv.unit.test.ts @@ -23,7 +23,7 @@ suite('Virtual environments', () => { let workspace: TypeMoq.IMock; let process: TypeMoq.IMock; - setup(async () => { + setup(() => { const cont = new Container(); serviceManager = new ServiceManager(cont); serviceContainer = new ServiceContainer(cont); @@ -68,7 +68,7 @@ suite('Virtual environments', () => { }); test('Workspace search paths', async () => { - settings.setup(x => x.venvPath).returns(() => `~${path.sep}foo`); + settings.setup(x => x.venvPath).returns(() => path.join('~', 'foo')); const wsRoot = TypeMoq.Mock.ofType(); wsRoot.setup(x => x.uri).returns(() => Uri.file('root')); @@ -83,7 +83,7 @@ suite('Virtual environments', () => { const paths = pathProvider.getSearchPaths(Uri.file('')); const homedir = os.homedir(); - const expected = [path.join(homedir, 'foo'), `${path.sep}root`, `${path.sep}root${path.sep}.direnv`]; + const expected = [path.join(homedir, 'foo'), 'root', path.join('root', '.direnv')]; expect(paths).to.deep.equal(expected, 'Workspace venv folder search list does not match.'); }); }); From 27fe89915fa2cd37217d1b7c20f15b61caf6669d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 15:36:46 -0700 Subject: [PATCH 16/27] Added tests --- src/client/interpreter/contracts.ts | 3 +- src/client/interpreter/interpreterService.ts | 4 +- src/client/unittests/display/main.ts | 26 +- .../interpreterService.unit.test.ts | 651 +++++++++++++----- 4 files changed, 475 insertions(+), 209 deletions(-) diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index 2934f9c22d0b..df99184498e6 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -82,6 +82,7 @@ export interface IInterpreterService { refresh(): Promise; initialize(): void; getDisplayName(interpreter: Partial): Promise; + shouldAutoSetInterpreter(): Promise; } export const IInterpreterDisplay = Symbol('IInterpreterDisplay'); @@ -99,7 +100,7 @@ export interface IInterpreterHelper { getActiveWorkspaceUri(): WorkspacePythonPath | undefined; getInterpreterInformation(pythonPath: string): Promise>; isMacDefaultPythonPath(pythonPath: string): Boolean; - getInterpreterTypeDisplayName(interpreterType: InterpreterType): string|undefined; + getInterpreterTypeDisplayName(interpreterType: InterpreterType): string | undefined; } export const IPipEnvService = Symbol('IPipEnvService'); diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 22247ac2515c..525e0db387c8 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -153,7 +153,7 @@ export class InterpreterService implements Disposable, IInterpreterService { if (info.version_info && info.version_info.length > 0) { displayNameParts.push(info.version_info.slice(0, 3).join('.')); } - if (info.version_info) { + if (info.architecture) { displayNameParts.push(getArchitectureDisplayName(info.architecture)); } if (!info.envName && info.path && info.type && info.type === InterpreterType.PipEnv) { @@ -178,7 +178,7 @@ export class InterpreterService implements Disposable, IInterpreterService { `(${envSuffixParts.join(': ')})`; return `${displayNameParts.join(' ')} ${envSuffix}`.trim(); } - private async shouldAutoSetInterpreter(): Promise { + public async shouldAutoSetInterpreter(): Promise { const activeWorkspace = this.helper.getActiveWorkspaceUri(); if (!activeWorkspace) { return false; diff --git a/src/client/unittests/display/main.ts b/src/client/unittests/display/main.ts index 5812183ddee2..df58776d6dae 100644 --- a/src/client/unittests/display/main.ts +++ b/src/client/unittests/display/main.ts @@ -14,11 +14,7 @@ import { ITestResultDisplay } from '../types'; @injectable() export class TestResultDisplay implements ITestResultDisplay { private statusBar: StatusBarItem; - private discoverCounter = 0; - private ticker = ['|', '/', '-', '|', '/', '-', '\\']; - private progressTimeout; private _enabled: boolean = false; - private progressPrefix!: string; private readonly didChange = new EventEmitter(); private readonly appShell: IApplicationShell; private readonly testsHelper: ITestsHelper; @@ -33,7 +29,6 @@ export class TestResultDisplay implements ITestResultDisplay { this.testsHelper = serviceContainer.get(ITestsHelper); } public dispose() { - this.clearProgressTicker(); this.statusBar.dispose(); } public get enabled() { @@ -67,8 +62,6 @@ export class TestResultDisplay implements ITestResultDisplay { } private updateTestRunWithSuccess(tests: Tests, debug: boolean = false): Tests { - this.clearProgressTicker(); - // Treat errors as a special case, as we generally wouldn't have any errors const statusText: string[] = []; const toolTip: string[] = []; @@ -107,7 +100,6 @@ export class TestResultDisplay implements ITestResultDisplay { // tslint:disable-next-line:no-any private updateTestRunWithFailure(reason: any): Promise { - this.clearProgressTicker(); this.statusBar.command = constants.Commands.Tests_View_UI; if (reason === CANCELLATION_REASON) { this.statusBar.text = '$(zap) Run Tests'; @@ -121,24 +113,10 @@ export class TestResultDisplay implements ITestResultDisplay { } private displayProgress(message: string, tooltip: string, command: string) { - this.progressPrefix = this.statusBar.text = `$(stop) ${message}`; + this.statusBar.text = `$(stop) ${message} '$(icon~spin)'`; this.statusBar.command = command; this.statusBar.tooltip = tooltip; this.statusBar.show(); - this.clearProgressTicker(); - this.progressTimeout = setInterval(() => this.updateProgressTicker(), 150); - } - private updateProgressTicker() { - const text = `${this.progressPrefix} ${this.ticker[this.discoverCounter % 7]}`; - this.discoverCounter += 1; - this.statusBar.text = text; - } - private clearProgressTicker() { - if (this.progressTimeout) { - clearInterval(this.progressTimeout); - } - this.progressTimeout = null; - this.discoverCounter = 0; } // tslint:disable-next-line:no-any @@ -153,7 +131,6 @@ export class TestResultDisplay implements ITestResultDisplay { } private updateWithDiscoverSuccess(tests: Tests, quietMode: boolean = false) { - this.clearProgressTicker(); const haveTests = tests && (tests.testFunctions.length > 0); this.statusBar.text = '$(zap) Run Tests'; this.statusBar.tooltip = 'Run Tests'; @@ -175,7 +152,6 @@ export class TestResultDisplay implements ITestResultDisplay { // tslint:disable-next-line:no-any private updateWithDiscoverFailure(reason: any) { - this.clearProgressTicker(); this.statusBar.text = '$(zap) Discover Tests'; this.statusBar.tooltip = 'Discover Tests'; this.statusBar.command = constants.Commands.Tests_Discover; diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 6c293b494db7..dde4f45bbea1 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -1,13 +1,19 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { expect } from 'chai'; +'use strict'; + +// tslint:disable:max-func-body-length no-any + +import { expect, use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; import { EventEmitter } from 'events'; import { Container } from 'inversify'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Disposable, TextDocument, TextEditor, Uri, WorkspaceConfiguration } from 'vscode'; import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; +import { getArchitectureDisplayName } from '../../client/common/platform/registry'; import { IFileSystem } from '../../client/common/platform/types'; import { IConfigurationService, IDisposableRegistry } from '../../client/common/types'; import { IPythonPathUpdaterServiceManager } from '../../client/interpreter/configuration/types'; @@ -26,9 +32,12 @@ import { InterpreterService } from '../../client/interpreter/interpreterService' import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; import { ServiceContainer } from '../../client/ioc/container'; import { ServiceManager } from '../../client/ioc/serviceManager'; +import * as EnumEx from '../../utils/enum'; import { noop } from '../../utils/misc'; import { Architecture } from '../../utils/platform'; +use(chaiAsPromised); + const info: PythonInterpreter = { architecture: Architecture.Unknown, companyDisplayName: '', @@ -42,7 +51,6 @@ const info: PythonInterpreter = { sysVersion: '' }; -// tslint:disable-next-line:max-func-body-length suite('Interpreters service', () => { let serviceManager: ServiceManager; let serviceContainer: ServiceContainer; @@ -55,8 +63,11 @@ suite('Interpreters service', () => { let wksLocator: TypeMoq.IMock; let fileSystem: TypeMoq.IMock; let interpreterDisplay: TypeMoq.IMock; + let workspacePythonPath: TypeMoq.IMock; + let virtualEnvMgr: TypeMoq.IMock; + type ConfigValue = { key: string; defaultValue?: T; globalValue?: T; workspaceValue?: T; workspaceFolderValue?: T }; - setup(async () => { + function setupSuite() { const cont = new Container(); serviceManager = new ServiceManager(cont); serviceContainer = new ServiceContainer(cont); @@ -68,6 +79,8 @@ suite('Interpreters service', () => { config = TypeMoq.Mock.ofType(); fileSystem = TypeMoq.Mock.ofType(); interpreterDisplay = TypeMoq.Mock.ofType(); + workspacePythonPath = TypeMoq.Mock.ofType(); + virtualEnvMgr = TypeMoq.Mock.ofType(); workspace.setup(x => x.getConfiguration('python', TypeMoq.It.isAny())).returns(() => config.object); serviceManager.addSingletonInstance(IDisposableRegistry, []); @@ -77,215 +90,491 @@ suite('Interpreters service', () => { serviceManager.addSingletonInstance(IInterpreterLocatorService, locator.object, INTERPRETER_LOCATOR_SERVICE); serviceManager.addSingletonInstance(IFileSystem, fileSystem.object); serviceManager.addSingletonInstance(IInterpreterDisplay, interpreterDisplay.object); + serviceManager.addSingletonInstance(IVirtualEnvironmentManager, virtualEnvMgr.object); pipenvLocator = TypeMoq.Mock.ofType(); wksLocator = TypeMoq.Mock.ofType(); - }); - test('autoset interpreter - no workspace', async () => { - await verifyUpdateCalled(TypeMoq.Times.never()); - }); + } + suite('Misc', () => { + setup(setupSuite); + [undefined, Uri.file('xyz')] + .forEach(resource => { + const resourceTestSuffix = `(${resource ? 'with' : 'without'} a resource)`; + + test(`Refresh invokes refresh of display ${resourceTestSuffix}`, async () => { + interpreterDisplay + .setup(i => i.refresh(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.once()); + + const service = new InterpreterService(serviceContainer); + await service.refresh(resource); + + interpreterDisplay.verifyAll(); + }); + + test(`get Interpreters uses interpreter locactors to get interpreters ${resourceTestSuffix}`, async () => { + locator + .setup(l => l.getInterpreters(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve([])) + .verifiable(TypeMoq.Times.once()); + + const service = new InterpreterService(serviceContainer); + await service.getInterpreters(resource); + + locator.verifyAll(); + }); + }); - test('autoset interpreter - global pythonPath in config', async () => { - setupWorkspace('folder'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python', globalValue: 'global' }; - }); - await verifyUpdateCalled(TypeMoq.Times.never()); - }); + test('Changes to active document should invoke intrepreter.refresh method', async () => { + const service = new InterpreterService(serviceContainer); + const configService = TypeMoq.Mock.ofType(); + const documentManager = TypeMoq.Mock.ofType(); - test('autoset interpreter - workspace has no pythonPath in config', async () => { - setupWorkspace('folder'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python' }; + let activeTextEditorChangeHandler: Function | undefined; + documentManager.setup(d => d.onDidChangeActiveTextEditor(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(handler => { + activeTextEditorChangeHandler = handler; + return { dispose: noop }; + }); + serviceManager.addSingletonInstance(IConfigurationService, configService.object); + serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); + + // tslint:disable-next-line:no-any + configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => new EventEmitter() as any); + service.initialize(); + const textEditor = TypeMoq.Mock.ofType(); + const uri = Uri.file(path.join('usr', 'file.py')); + const document = TypeMoq.Mock.ofType(); + textEditor.setup(t => t.document).returns(() => document.object); + document.setup(d => d.uri).returns(() => uri); + activeTextEditorChangeHandler!(textEditor.object); + + interpreterDisplay.verify(i => i.refresh(TypeMoq.It.isValue(uri)), TypeMoq.Times.once()); }); - const interpreter: PythonInterpreter = { - ...info, - path: path.join('folder', 'py1', 'bin', 'python.exe'), - type: InterpreterType.Unknown - }; - setupLocators([interpreter], []); - await verifyUpdateCalled(TypeMoq.Times.once()); - }); - test('autoset interpreter - workspace has default pythonPath in config', async () => { - setupWorkspace('folder'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python', workspaceValue: 'python' }; + test('If there is no active document then intrepreter.refresh should not be invoked', async () => { + const service = new InterpreterService(serviceContainer); + const configService = TypeMoq.Mock.ofType(); + const documentManager = TypeMoq.Mock.ofType(); + + let activeTextEditorChangeHandler: Function | undefined; + documentManager.setup(d => d.onDidChangeActiveTextEditor(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(handler => { + activeTextEditorChangeHandler = handler; + return { dispose: noop }; + }); + serviceManager.addSingletonInstance(IConfigurationService, configService.object); + serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); + + // tslint:disable-next-line:no-any + configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => new EventEmitter() as any); + service.initialize(); + activeTextEditorChangeHandler!(); + + interpreterDisplay.verify(i => i.refresh(TypeMoq.It.isValue(undefined)), TypeMoq.Times.never()); }); - setupLocators([], []); - await verifyUpdateCalled(TypeMoq.Times.never()); + }); - test('autoset interpreter - pipenv workspace', async () => { - setupWorkspace('folder'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python', workspaceValue: 'python' }; - }); - const interpreter: PythonInterpreter = { - ...info, - path: 'python', - type: InterpreterType.VirtualEnv - }; - setupLocators([], [interpreter]); - await verifyUpdateCallData('python', ConfigurationTarget.Workspace, 'folder'); + suite('Get Interpreter Details', () => { + setup(setupSuite); + [undefined, Uri.file('some workspace')] + .forEach(resource => { + test(`Ensure undefined is returned if we're unable to retrieve interpreter info (Resource is ${resource})`, async () => { + const pythonPath = 'SOME VALUE'; + const service = new InterpreterService(serviceContainer); + locator + .setup(l => l.getInterpreters(TypeMoq.It.isValue(resource))) + .returns(() => Promise.resolve([])) + .verifiable(TypeMoq.Times.once()); + helper + .setup(h => h.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.once()); + virtualEnvMgr + .setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve('')) + .verifiable(TypeMoq.Times.once()); + virtualEnvMgr + .setup(v => v.getEnvironmentType(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve(InterpreterType.Unknown)) + .verifiable(TypeMoq.Times.once()); + + const details = await service.getInterpreterDetails(pythonPath, resource); + + locator.verifyAll(); + helper.verifyAll(); + expect(details).to.be.equal(undefined, 'Not undefined'); + }); + }); }); - test('autoset interpreter - workspace without interpreter', async () => { - setupWorkspace('root'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python', workspaceValue: 'elsewhere' }; + suite('Should Auto Set Interpreter', () => { + setup(setupSuite); + test('Should not auto set interpreter if there is no workspace', async () => { + const service = new InterpreterService(serviceContainer); + helper + .setup(h => h.getActiveWorkspaceUri()) + .returns(() => undefined) + .verifiable(TypeMoq.Times.once()); + + await expect(service.shouldAutoSetInterpreter()).to.eventually.equal(false, 'not false'); + + helper.verifyAll(); }); - const interpreter: PythonInterpreter = { - ...info, - path: 'elsewhere', - type: InterpreterType.Unknown - }; - - setupLocators([interpreter], []); - await verifyUpdateCalled(TypeMoq.Times.never()); - }); - test('autoset interpreter - workspace with interpreter', async () => { - setupWorkspace('root'); - config.setup(x => x.inspect('pythonPath')).returns(() => { - return { key: 'python' }; + test('Should not auto set interpreter if there is a value in global user settings (global value is not \'python\')', async () => { + const service = new InterpreterService(serviceContainer); + workspacePythonPath + .setup(w => w.folderUri) + .returns(() => Uri.file('w')) + .verifiable(TypeMoq.Times.once()); + helper + .setup(h => h.getActiveWorkspaceUri()) + .returns(() => workspacePythonPath.object) + .verifiable(TypeMoq.Times.once()); + const pythonPathConfigValue = TypeMoq.Mock.ofType>(); + config + .setup(w => w.inspect(TypeMoq.It.isAny())) + .returns(() => pythonPathConfigValue.object) + .verifiable(TypeMoq.Times.once()); + pythonPathConfigValue + .setup(p => p.globalValue) + .returns(() => path.join('a', 'bin', 'python')) + .verifiable(TypeMoq.Times.atLeastOnce()); + + await expect(service.shouldAutoSetInterpreter()).to.eventually.equal(false, 'not false'); + + helper.verifyAll(); + workspace.verifyAll(); + config.verifyAll(); + pythonPathConfigValue.verifyAll(); + }); + test('Should not auto set interpreter if there is a value in workspace settings (& value is not \'python\')', async () => { + const service = new InterpreterService(serviceContainer); + workspacePythonPath + .setup(w => w.configTarget) + .returns(() => ConfigurationTarget.Workspace) + .verifiable(TypeMoq.Times.once()); + helper + .setup(h => h.getActiveWorkspaceUri()) + .returns(() => workspacePythonPath.object) + .verifiable(TypeMoq.Times.once()); + const pythonPathConfigValue = TypeMoq.Mock.ofType>(); + config + .setup(w => w.inspect(TypeMoq.It.isValue('pythonPath'))) + .returns(() => pythonPathConfigValue.object) + .verifiable(TypeMoq.Times.once()); + pythonPathConfigValue + .setup(p => p.globalValue) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeastOnce()); + pythonPathConfigValue + .setup(p => p.workspaceValue) + .returns(() => path.join('a', 'bin', 'python')) + .verifiable(TypeMoq.Times.atLeastOnce()); + + await expect(service.shouldAutoSetInterpreter()).to.eventually.equal(false, 'not false'); + + helper.verifyAll(); + workspace.verifyAll(); + config.verifyAll(); + pythonPathConfigValue.verifyAll(); }); - const intPath = path.join('root', 'under', 'bin', 'python.exe'); - const interpreter: PythonInterpreter = { - ...info, - path: intPath, - type: InterpreterType.Unknown - }; - - setupLocators([interpreter], []); - await verifyUpdateCallData(intPath, ConfigurationTarget.Workspace, 'root'); - }); - async function verifyUpdateCalled(times: TypeMoq.Times) { - const service = new InterpreterService(serviceContainer); - await service.autoSetInterpreter(); - updater - .verify(x => x.updatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), times); - } + [ + { configTarget: ConfigurationTarget.Workspace, label: 'Workspace' }, + { configTarget: ConfigurationTarget.WorkspaceFolder, label: 'Workspace Folder' } + ].forEach(item => { + const testSuffix = `(${item.label})`; + const cfgTarget = item.configTarget as (ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder); + test(`Should auto set interpreter if there is no value in workspace settings ${testSuffix}`, async () => { + const service = new InterpreterService(serviceContainer); + workspacePythonPath + .setup(w => w.configTarget) + .returns(() => cfgTarget) + .verifiable(TypeMoq.Times.once()); + helper + .setup(h => h.getActiveWorkspaceUri()) + .returns(() => workspacePythonPath.object) + .verifiable(TypeMoq.Times.once()); + const pythonPathConfigValue = TypeMoq.Mock.ofType>(); + config + .setup(w => w.inspect(TypeMoq.It.isValue('pythonPath'))) + .returns(() => pythonPathConfigValue.object) + .verifiable(TypeMoq.Times.once()); + pythonPathConfigValue + .setup(p => p.globalValue) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeastOnce()); + if (cfgTarget === ConfigurationTarget.Workspace) { + pythonPathConfigValue + .setup(p => p.workspaceValue) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeastOnce()); + } else { + pythonPathConfigValue + .setup(p => p.workspaceFolderValue) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeastOnce()); + } + + await expect(service.shouldAutoSetInterpreter()).to.eventually.equal(true, 'not true'); - async function verifyUpdateCallData(pythonPath: string, target: ConfigurationTarget, wksFolder: string) { - let pp: string | undefined; - let confTarget: ConfigurationTarget | undefined; - let trigger; - let wks; - updater - .setup(x => x.updatePythonPath(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - // tslint:disable-next-line:no-any - .callback((p: string, c: ConfigurationTarget, t: any, w: any) => { - pp = p; - confTarget = c; - trigger = t; - wks = w; - }) - .returns(() => Promise.resolve()); - - const service = new InterpreterService(serviceContainer); - await service.autoSetInterpreter(); - - expect(pp).not.to.be.equal(undefined, 'updatePythonPath not called'); - expect(pp!).to.be.equal(pythonPath, 'invalid Python path'); - expect(confTarget).to.be.equal(target, 'invalid configuration target'); - expect(trigger).to.be.equal('load', 'invalid trigger'); - expect(wks.fsPath).to.be.equal(Uri.file(wksFolder).fsPath, 'invalid workspace Uri'); - } + helper.verifyAll(); + workspace.verifyAll(); + config.verifyAll(); + pythonPathConfigValue.verifyAll(); + }); - function setupWorkspace(folder: string) { - const wsPath: WorkspacePythonPath = { - folderUri: Uri.file(folder), - configTarget: ConfigurationTarget.Workspace - }; - helper.setup(x => x.getActiveWorkspaceUri()).returns(() => wsPath); - } + test(`Should auto set interpreter if there is no value in workspace settings and value is \'python\' ${testSuffix}`, async () => { + const service = new InterpreterService(serviceContainer); + workspacePythonPath + .setup(w => w.configTarget) + .returns(() => ConfigurationTarget.Workspace) + .verifiable(TypeMoq.Times.once()); + helper + .setup(h => h.getActiveWorkspaceUri()) + .returns(() => workspacePythonPath.object) + .verifiable(TypeMoq.Times.once()); + const pythonPathConfigValue = TypeMoq.Mock.ofType>(); + config + .setup(w => w.inspect(TypeMoq.It.isValue('pythonPath'))) + .returns(() => pythonPathConfigValue.object) + .verifiable(TypeMoq.Times.once()); + pythonPathConfigValue + .setup(p => p.globalValue) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeastOnce()); + pythonPathConfigValue + .setup(p => p.workspaceValue) + .returns(() => 'python') + .verifiable(TypeMoq.Times.atLeastOnce()); - function setupLocators(wks: PythonInterpreter[], pipenv: PythonInterpreter[]) { - pipenvLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(pipenv)); - serviceManager.addSingletonInstance(IInterpreterLocatorService, pipenvLocator.object, PIPENV_SERVICE); - wksLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(wks)); - serviceManager.addSingletonInstance(IInterpreterLocatorService, wksLocator.object, WORKSPACE_VIRTUAL_ENV_SERVICE); + await expect(service.shouldAutoSetInterpreter()).to.eventually.equal(true, 'not true'); - } + helper.verifyAll(); + workspace.verifyAll(); + config.verifyAll(); + pythonPathConfigValue.verifyAll(); + }); + }); + }); - test('Changes to active document should invoke intrepreter.refresh method', async () => { - const service = new InterpreterService(serviceContainer); - const configService = TypeMoq.Mock.ofType(); - const documentManager = TypeMoq.Mock.ofType(); + suite('Auto Set Interpreter', () => { + setup(setupSuite); + test('autoset interpreter - no workspace', async () => { + await verifyUpdateCalled(TypeMoq.Times.never()); + }); - let activeTextEditorChangeHandler: Function | undefined; - documentManager.setup(d => d.onDidChangeActiveTextEditor(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(handler => { - activeTextEditorChangeHandler = handler; - return { dispose: noop }; + test('autoset interpreter - global pythonPath in config', async () => { + setupWorkspace('folder'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python', globalValue: 'global' }; + }); + await verifyUpdateCalled(TypeMoq.Times.never()); }); - serviceManager.addSingletonInstance(IConfigurationService, configService.object); - serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); - - // tslint:disable-next-line:no-any - configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => new EventEmitter() as any); - service.initialize(); - const textEditor = TypeMoq.Mock.ofType(); - const uri = Uri.file(path.join('usr', 'file.py')); - const document = TypeMoq.Mock.ofType(); - textEditor.setup(t => t.document).returns(() => document.object); - document.setup(d => d.uri).returns(() => uri); - activeTextEditorChangeHandler!(textEditor.object); - - interpreterDisplay.verify(i => i.refresh(TypeMoq.It.isValue(uri)), TypeMoq.Times.once()); - }); - test('If there is no active document then intrepreter.refresh should not be invoked', async () => { - const service = new InterpreterService(serviceContainer); - const configService = TypeMoq.Mock.ofType(); - const documentManager = TypeMoq.Mock.ofType(); + test('autoset interpreter - workspace has no pythonPath in config', async () => { + setupWorkspace('folder'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python' }; + }); + const interpreter: PythonInterpreter = { + ...info, + path: path.join('folder', 'py1', 'bin', 'python.exe'), + type: InterpreterType.Unknown + }; + setupLocators([interpreter], []); + await verifyUpdateCalled(TypeMoq.Times.once()); + }); - let activeTextEditorChangeHandler: Function | undefined; - documentManager.setup(d => d.onDidChangeActiveTextEditor(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(handler => { - activeTextEditorChangeHandler = handler; - return { dispose: noop }; + test('autoset interpreter - workspace has default pythonPath in config', async () => { + setupWorkspace('folder'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python', workspaceValue: 'python' }; + }); + setupLocators([], []); + await verifyUpdateCalled(TypeMoq.Times.never()); }); - serviceManager.addSingletonInstance(IConfigurationService, configService.object); - serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); - // tslint:disable-next-line:no-any - configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => new EventEmitter() as any); - service.initialize(); - activeTextEditorChangeHandler!(); + test('autoset interpreter - pipenv workspace', async () => { + setupWorkspace('folder'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python', workspaceValue: 'python' }; + }); + const interpreter: PythonInterpreter = { + ...info, + path: 'python', + type: InterpreterType.VirtualEnv + }; + setupLocators([], [interpreter]); + await verifyUpdateCallData('python', ConfigurationTarget.Workspace, 'folder'); + }); - interpreterDisplay.verify(i => i.refresh(TypeMoq.It.isValue(undefined)), TypeMoq.Times.never()); - }); - [undefined, Uri.file('some workspace')] - .forEach(resource => { - test(`Ensure undefined is returned if we're unable to retrieve interpreter info (Resource is ${resource})`, async () => { - const pythonPath = 'SOME VALUE'; - const service = new InterpreterService(serviceContainer); - locator - .setup(l => l.getInterpreters(TypeMoq.It.isValue(resource))) - .returns(() => Promise.resolve([])) - .verifiable(TypeMoq.Times.once()); - helper - .setup(h => h.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve(undefined)) - .verifiable(TypeMoq.Times.once()); - const virtualEnvMgr = TypeMoq.Mock.ofType(); - serviceManager.addSingletonInstance(IVirtualEnvironmentManager, virtualEnvMgr.object); - virtualEnvMgr - .setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve('')) - .verifiable(TypeMoq.Times.once()); - virtualEnvMgr - .setup(v => v.getEnvironmentType(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve(InterpreterType.Unknown)) - .verifiable(TypeMoq.Times.once()); + test('autoset interpreter - workspace without interpreter', async () => { + setupWorkspace('root'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python', workspaceValue: 'elsewhere' }; + }); + const interpreter: PythonInterpreter = { + ...info, + path: 'elsewhere', + type: InterpreterType.Unknown + }; + + setupLocators([interpreter], []); + await verifyUpdateCalled(TypeMoq.Times.never()); + }); + + test('autoset interpreter - workspace with interpreter', async () => { + setupWorkspace('root'); + config.setup(x => x.inspect('pythonPath')).returns(() => { + return { key: 'python' }; + }); + const intPath = path.join('root', 'under', 'bin', 'python.exe'); + const interpreter: PythonInterpreter = { + ...info, + path: intPath, + type: InterpreterType.Unknown + }; + + setupLocators([interpreter], []); + await verifyUpdateCallData(intPath, ConfigurationTarget.Workspace, 'root'); + }); - const details = await service.getInterpreterDetails(pythonPath, resource); + async function verifyUpdateCalled(times: TypeMoq.Times) { + const service = new InterpreterService(serviceContainer); + await service.autoSetInterpreter(); + updater + .verify(x => x.updatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), times); + } + + async function verifyUpdateCallData(pythonPath: string, target: ConfigurationTarget, wksFolder: string) { + let pp: string | undefined; + let confTarget: ConfigurationTarget | undefined; + let trigger; + let wks; + updater + .setup(x => x.updatePythonPath(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + // tslint:disable-next-line:no-any + .callback((p: string, c: ConfigurationTarget, t: any, w: any) => { + pp = p; + confTarget = c; + trigger = t; + wks = w; + }) + .returns(() => Promise.resolve()); + + const service = new InterpreterService(serviceContainer); + await service.autoSetInterpreter(); + + expect(pp).not.to.be.equal(undefined, 'updatePythonPath not called'); + expect(pp!).to.be.equal(pythonPath, 'invalid Python path'); + expect(confTarget).to.be.equal(target, 'invalid configuration target'); + expect(trigger).to.be.equal('load', 'invalid trigger'); + expect(wks.fsPath).to.be.equal(Uri.file(wksFolder).fsPath, 'invalid workspace Uri'); + } + + function setupWorkspace(folder: string) { + const wsPath: WorkspacePythonPath = { + folderUri: Uri.file(folder), + configTarget: ConfigurationTarget.Workspace + }; + helper.setup(x => x.getActiveWorkspaceUri()).returns(() => wsPath); + } + + function setupLocators(wks: PythonInterpreter[], pipenv: PythonInterpreter[]) { + pipenvLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(pipenv)); + serviceManager.addSingletonInstance(IInterpreterLocatorService, pipenvLocator.object, PIPENV_SERVICE); + wksLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(wks)); + serviceManager.addSingletonInstance(IInterpreterLocatorService, wksLocator.object, WORKSPACE_VIRTUAL_ENV_SERVICE); + + } + }); - locator.verifyAll(); - helper.verifyAll(); - expect(details).to.be.equal(undefined, 'Not undefined'); + // This is kind of a verbose test, but we need to ensure we have covered all permutations. + // Also we have special handling for certain types of interpreters. + suite('Display Format (with all permutations)', () => { + setup(setupSuite); + [undefined, Uri.file('xyz')].forEach(resource => { + [undefined, [1, 2, 3, 'alpha']].forEach(versionInfo => { + // Forced cast to ignore TS warnings. + (EnumEx.getNamesAndValues(Architecture) as ({ name: string; value: Architecture } | undefined)[]).concat(undefined).forEach(arch => { + [undefined, path.join('a', 'b', 'c', 'd', 'bin', 'python')].forEach(pythonPath => { + // Forced cast to ignore TS warnings. + (EnumEx.getNamesAndValues(InterpreterType) as ({ name: string; value: InterpreterType } | undefined)[]).concat(undefined).forEach(interpreterType => { + [undefined, 'my env name'].forEach(envName => { + ['', 'my pipenv name'].forEach(pipEnvName => { + const testName = [`${resource ? 'With' : 'Without'} a workspace`, + `${versionInfo ? 'with' : 'without'} version information`, + `${arch ? arch.name : 'without'} architecture`, + `${pythonPath ? 'with' : 'without'} python Path`, + `${interpreterType ? `${interpreterType.name} interpreter type` : 'without interpreter type'}`, + `${envName ? 'with' : 'without'} environment name`, + `${pipEnvName ? 'with' : 'without'} pip environment` + ].join(', '); + + test(testName, async () => { + const interpreterInfo: Partial = { + version_info: versionInfo as any, + architecture: arch ? arch.value : undefined, + envName, + type: interpreterType ? interpreterType.value : undefined, + path: pythonPath + }; + + if (interpreterInfo.path && interpreterType && interpreterType.value === InterpreterType.PipEnv) { + virtualEnvMgr + .setup(v => v.getEnvironmentName(TypeMoq.It.isValue(interpreterInfo.path!), TypeMoq.It.isAny())) + .returns(() => Promise.resolve(pipEnvName)); + } + if (interpreterType) { + helper + .setup(h => h.getInterpreterTypeDisplayName(TypeMoq.It.isValue(interpreterType.value))) + .returns(() => `${interpreterType!.name}_display`); + } + + const service = new InterpreterService(serviceContainer); + const expectedDisplayName = buildDisplayName(interpreterInfo); + + const displayName = await service.getDisplayName(interpreterInfo, resource); + expect(displayName).to.equal(expectedDisplayName); + }); + + function buildDisplayName(interpreterInfo: Partial) { + const displayNameParts: string[] = ['Python']; + const envSuffixParts: string[] = []; + + if (interpreterInfo.version_info && interpreterInfo.version_info.length > 0) { + displayNameParts.push(interpreterInfo.version_info.slice(0, 3).join('.')); + } + if (interpreterInfo.architecture) { + displayNameParts.push(getArchitectureDisplayName(interpreterInfo.architecture)); + } + if (!interpreterInfo.envName && interpreterInfo.path && interpreterInfo.type && interpreterInfo.type === InterpreterType.PipEnv && pipEnvName) { + // If we do not have the name of the environment, then try to get it again. + // This can happen based on the context (i.e. resource). + // I.e. we can determine if an environment is PipEnv only when giving it the right workspacec path (i.e. resource). + interpreterInfo.envName = pipEnvName; + } + if (interpreterInfo.envName && interpreterInfo.envName.length > 0) { + envSuffixParts.push(interpreterInfo.envName); + } + if (interpreterInfo.type) { + envSuffixParts.push(`${interpreterType!.name}_display`); + } + + const envSuffix = envSuffixParts.length === 0 ? '' : + `(${envSuffixParts.join(': ')})`; + return `${displayNameParts.join(' ')} ${envSuffix}`.trim(); + } + }); + }); + }); + }); + }); }); }); + }); }); From 71f19aa45363ab02e217becea1ea831d6cfec061 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 15:38:00 -0700 Subject: [PATCH 17/27] Add news entry --- news/2 Fixes/1352.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/1352.md diff --git a/news/2 Fixes/1352.md b/news/2 Fixes/1352.md new file mode 100644 index 000000000000..3f246199823d --- /dev/null +++ b/news/2 Fixes/1352.md @@ -0,0 +1 @@ +Improvements to the display format of interpreter information in the list of interpreters. From 58c6b0d8435f674fd8d7d2c7606d49475dde126a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 15:43:30 -0700 Subject: [PATCH 18/27] Revert changes --- src/client/unittests/display/main.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/client/unittests/display/main.ts b/src/client/unittests/display/main.ts index df58776d6dae..5812183ddee2 100644 --- a/src/client/unittests/display/main.ts +++ b/src/client/unittests/display/main.ts @@ -14,7 +14,11 @@ import { ITestResultDisplay } from '../types'; @injectable() export class TestResultDisplay implements ITestResultDisplay { private statusBar: StatusBarItem; + private discoverCounter = 0; + private ticker = ['|', '/', '-', '|', '/', '-', '\\']; + private progressTimeout; private _enabled: boolean = false; + private progressPrefix!: string; private readonly didChange = new EventEmitter(); private readonly appShell: IApplicationShell; private readonly testsHelper: ITestsHelper; @@ -29,6 +33,7 @@ export class TestResultDisplay implements ITestResultDisplay { this.testsHelper = serviceContainer.get(ITestsHelper); } public dispose() { + this.clearProgressTicker(); this.statusBar.dispose(); } public get enabled() { @@ -62,6 +67,8 @@ export class TestResultDisplay implements ITestResultDisplay { } private updateTestRunWithSuccess(tests: Tests, debug: boolean = false): Tests { + this.clearProgressTicker(); + // Treat errors as a special case, as we generally wouldn't have any errors const statusText: string[] = []; const toolTip: string[] = []; @@ -100,6 +107,7 @@ export class TestResultDisplay implements ITestResultDisplay { // tslint:disable-next-line:no-any private updateTestRunWithFailure(reason: any): Promise { + this.clearProgressTicker(); this.statusBar.command = constants.Commands.Tests_View_UI; if (reason === CANCELLATION_REASON) { this.statusBar.text = '$(zap) Run Tests'; @@ -113,10 +121,24 @@ export class TestResultDisplay implements ITestResultDisplay { } private displayProgress(message: string, tooltip: string, command: string) { - this.statusBar.text = `$(stop) ${message} '$(icon~spin)'`; + this.progressPrefix = this.statusBar.text = `$(stop) ${message}`; this.statusBar.command = command; this.statusBar.tooltip = tooltip; this.statusBar.show(); + this.clearProgressTicker(); + this.progressTimeout = setInterval(() => this.updateProgressTicker(), 150); + } + private updateProgressTicker() { + const text = `${this.progressPrefix} ${this.ticker[this.discoverCounter % 7]}`; + this.discoverCounter += 1; + this.statusBar.text = text; + } + private clearProgressTicker() { + if (this.progressTimeout) { + clearInterval(this.progressTimeout); + } + this.progressTimeout = null; + this.discoverCounter = 0; } // tslint:disable-next-line:no-any @@ -131,6 +153,7 @@ export class TestResultDisplay implements ITestResultDisplay { } private updateWithDiscoverSuccess(tests: Tests, quietMode: boolean = false) { + this.clearProgressTicker(); const haveTests = tests && (tests.testFunctions.length > 0); this.statusBar.text = '$(zap) Run Tests'; this.statusBar.tooltip = 'Run Tests'; @@ -152,6 +175,7 @@ export class TestResultDisplay implements ITestResultDisplay { // tslint:disable-next-line:no-any private updateWithDiscoverFailure(reason: any) { + this.clearProgressTicker(); this.statusBar.text = '$(zap) Discover Tests'; this.statusBar.tooltip = 'Discover Tests'; this.statusBar.command = constants.Commands.Tests_Discover; From 8ef5ccb5c3f26a5c6def236c5898c04acf20cf99 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 15:50:53 -0700 Subject: [PATCH 19/27] Use Uri.fspath to build paths --- src/test/interpreters/venv.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/interpreters/venv.unit.test.ts b/src/test/interpreters/venv.unit.test.ts index 1e78b3bc1dcd..cd158b9fe79e 100644 --- a/src/test/interpreters/venv.unit.test.ts +++ b/src/test/interpreters/venv.unit.test.ts @@ -83,7 +83,7 @@ suite('Virtual environments', () => { const paths = pathProvider.getSearchPaths(Uri.file('')); const homedir = os.homedir(); - const expected = [path.join(homedir, 'foo'), 'root', path.join('root', '.direnv')]; + const expected = [path.join(homedir, 'foo'), 'root', path.join('root', '.direnv')].map(item => Uri.file(item).fsPath); expect(paths).to.deep.equal(expected, 'Workspace venv folder search list does not match.'); }); }); From bfeea9c8a901ce970f604c37c323f02624ec4f10 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 16:23:32 -0700 Subject: [PATCH 20/27] Fixed tests --- .../interpreterVersion.unit.test.ts | 92 ++++++++++--------- 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/src/test/interpreters/interpreterVersion.unit.test.ts b/src/test/interpreters/interpreterVersion.unit.test.ts index 31710ca4726a..d0b9161ec5e2 100644 --- a/src/test/interpreters/interpreterVersion.unit.test.ts +++ b/src/test/interpreters/interpreterVersion.unit.test.ts @@ -1,63 +1,67 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { assert, expect, use } from 'chai'; -import * as chaiAsPromised from 'chai-as-promised'; +import { assert, expect } from 'chai'; +import * as path from 'path'; +import * as typeMoq from 'typemoq'; import '../../client/common/extensions'; -import { IProcessServiceFactory } from '../../client/common/process/types'; +import { IProcessService, IProcessServiceFactory } from '../../client/common/process/types'; import { IInterpreterVersionService } from '../../client/interpreter/contracts'; -import { PIP_VERSION_REGEX } from '../../client/interpreter/interpreterVersion'; -import { PYTHON_PATH } from '../common'; -import { UnitTestIocContainer } from '../unittests/serviceRegistry'; - -use(chaiAsPromised); +import { InterpreterVersionService } from '../../client/interpreter/interpreterVersion'; suite('Interpreters display version', () => { - let ioc: UnitTestIocContainer; - setup(initializeDI); - teardown(() => ioc.dispose()); - - function initializeDI() { - ioc = new UnitTestIocContainer(); - ioc.registerCommonTypes(); - ioc.registerProcessTypes(); - ioc.registerVariableTypes(); - ioc.registerInterpreterTypes(); - } + let processService: typeMoq.IMock; + let interpreterVersionService: IInterpreterVersionService; + + setup(() => { + const processFactory = typeMoq.Mock.ofType(); + processService = typeMoq.Mock.ofType(); + // tslint:disable-next-line:no-any + processService.setup((p: any) => p.then).returns(() => undefined); + processFactory.setup(p => p.create()).returns(() => Promise.resolve(processService.object)); + interpreterVersionService = new InterpreterVersionService(processFactory.object); + }); test('Must return the Python Version', async () => { - const pythonProcess = await ioc.serviceContainer.get(IProcessServiceFactory).create(); - const output = await pythonProcess.exec(PYTHON_PATH, ['--version'], { cwd: __dirname, mergeStdOutErr: true }); - const version = output.stdout.splitLines()[0]; - const interpreterVersion = ioc.serviceContainer.get(IInterpreterVersionService); - const pyVersion = await interpreterVersion.getVersion(PYTHON_PATH, 'DEFAULT_TEST_VALUE'); - assert.equal(pyVersion, version, 'Incorrect version'); + const pythonPath = path.join('a', 'b', 'python'); + const pythonVersion = 'Output from the Procecss'; + processService + .setup(p => p.exec(typeMoq.It.isValue(pythonPath), typeMoq.It.isValue(['--version']), typeMoq.It.isAny())) + .returns(() => Promise.resolve({ stdout: pythonVersion })) + .verifiable(typeMoq.Times.once()); + + const pyVersion = await interpreterVersionService.getVersion(pythonPath, 'DEFAULT_TEST_VALUE'); + assert.equal(pyVersion, pythonVersion, 'Incorrect version'); }); test('Must return the default value when Python path is invalid', async () => { - const interpreterVersion = ioc.serviceContainer.get(IInterpreterVersionService); - const pyVersion = await interpreterVersion.getVersion('INVALID_INTERPRETER', 'DEFAULT_TEST_VALUE'); + const pythonPath = path.join('a', 'b', 'python'); + processService + .setup(p => p.exec(typeMoq.It.isValue(pythonPath), typeMoq.It.isValue(['--version']), typeMoq.It.isAny())) + .returns(() => Promise.reject({})) + .verifiable(typeMoq.Times.once()); + + const pyVersion = await interpreterVersionService.getVersion(pythonPath, 'DEFAULT_TEST_VALUE'); assert.equal(pyVersion, 'DEFAULT_TEST_VALUE', 'Incorrect version'); }); test('Must return the pip Version.', async () => { - const pythonProcess = await ioc.serviceContainer.get(IProcessServiceFactory).create(); - const result = await pythonProcess.exec(PYTHON_PATH, ['-m', 'pip', '--version'], { cwd: __dirname, mergeStdOutErr: true }); - const output = result.stdout.splitLines()[0]; - // Take the second part, see below example. - // pip 9.0.1 from /Users/donjayamanne/anaconda3/lib/python3.6/site-packages (python 3.6). - const re = new RegExp(PIP_VERSION_REGEX, 'g'); - const matches = re.exec(output); - assert.isNotNull(matches, 'No matches for version found'); - // tslint:disable-next-line:no-non-null-assertion - assert.isAtLeast(matches!.length, 1, 'Version number not found'); - - const interpreterVersion = ioc.serviceContainer.get(IInterpreterVersionService); - const pipVersionPromise = interpreterVersion.getPipVersion(PYTHON_PATH); - // tslint:disable-next-line:no-non-null-assertion - await expect(pipVersionPromise).to.eventually.equal(matches![0].trim()); + const pythonPath = path.join('a', 'b', 'python'); + const pipVersion = '1.2.3'; + processService + .setup(p => p.exec(typeMoq.It.isValue(pythonPath), typeMoq.It.isValue(['-m', 'pip', '--version']), typeMoq.It.isAny())) + .returns(() => Promise.resolve({ stdout: pipVersion })) + .verifiable(typeMoq.Times.once()); + + const pyVersion = await interpreterVersionService.getPipVersion(pythonPath); + assert.equal(pyVersion, pipVersion, 'Incorrect version'); }); test('Must throw an exception when pip version cannot be determined', async () => { - const interpreterVersion = ioc.serviceContainer.get(IInterpreterVersionService); - const pipVersionPromise = interpreterVersion.getPipVersion('INVALID_INTERPRETER'); + const pythonPath = path.join('a', 'b', 'python'); + processService + .setup(p => p.exec(typeMoq.It.isValue(pythonPath), typeMoq.It.isValue(['-m', 'pip', '--version']), typeMoq.It.isAny())) + .returns(() => Promise.reject()) + .verifiable(typeMoq.Times.once()); + + const pipVersionPromise = interpreterVersionService.getPipVersion(pythonPath); await expect(pipVersionPromise).to.be.rejectedWith(); }); }); From bda2ca0d8c61e490819c6a7bb32d998469b06847 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 16:51:31 -0700 Subject: [PATCH 21/27] Address code review comments --- .../configuration/interpreterComparer.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterComparer.ts b/src/client/interpreter/configuration/interpreterComparer.ts index f214a7664c19..a2a0fd3e78c7 100644 --- a/src/client/interpreter/configuration/interpreterComparer.ts +++ b/src/client/interpreter/configuration/interpreterComparer.ts @@ -5,19 +5,20 @@ import { inject, injectable } from 'inversify'; import { getArchitectureDisplayName } from '../../common/platform/registry'; -import { IServiceContainer } from '../../ioc/types'; import { IInterpreterHelper, PythonInterpreter } from '../contracts'; import { IInterpreterComparer } from './types'; @injectable() export class InterpreterComparer implements IInterpreterComparer { - private readonly interpreterHelper: IInterpreterHelper; - - constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { - this.interpreterHelper = this.serviceContainer.get(IInterpreterHelper); + constructor(@inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper) { } public compare(a: PythonInterpreter, b: PythonInterpreter): number { - return this.getSortName(a) > this.getSortName(b) ? 1 : -1; + const nameA = this.getSortName(a); + const nameB = this.getSortName(b); + if (nameA === nameB) { + return 0; + } + return nameA > nameB ? 1 : -1; } private getSortName(info: PythonInterpreter): string { const sortNameParts: string[] = []; @@ -31,7 +32,7 @@ export class InterpreterComparer implements IInterpreterComparer { if (info.version_info && info.version_info.length > 0) { sortNameParts.push(info.version_info.slice(0, 3).join('.')); } - if (info.version_info) { + if (info.architecture) { sortNameParts.push(getArchitectureDisplayName(info.architecture)); } if (info.companyDisplayName && info.companyDisplayName.length > 0) { From 12ce72be2b682f8438160809b048d690ea170625 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 20:58:20 -0700 Subject: [PATCH 22/27] Oops --- src/client/interpreter/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index 7ef8ffdc873c..acc53aad836b 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -65,7 +65,7 @@ export class InterpreterHelper implements IInterpreterHelper { return details; } catch (ex) { console.error(`Failed to get interpreter information for '${pythonPath}'`, ex); - return {}; + return; } } public isMacDefaultPythonPath(pythonPath: string) { From 8c61e7c49eb63ef2d4e996dda507c6335bd0c047 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 22:05:07 -0700 Subject: [PATCH 23/27] Improvements --- src/client/interpreter/interpreterService.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 525e0db387c8..41adf2cd311f 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -129,13 +129,13 @@ export class InterpreterService implements Disposable, IInterpreterService { }; const virtualEnvName = await virtualEnvManager.getEnvironmentName(pythonPath, resource); - const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : ''; - const displayName = `${details.version!}${dislayNameSuffix}`; - return { + const interpreterInfo = { ...(details as PythonInterpreter), - envName: virtualEnvName, - displayName + envName: virtualEnvName }; + + interpreterInfo.displayName = await this.getDisplayName(interpreterInfo, resource); + return interpreterInfo; } /** From 21847069047fda9314773164236d1683d4986ac1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Sep 2018 23:32:34 -0700 Subject: [PATCH 24/27] disable cache if file has cannot be calculated --- src/client/interpreter/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index acc53aad836b..f37102e20994 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -47,7 +47,7 @@ export class InterpreterHelper implements IInterpreterHelper { let fileHash = await this.fs.getFileHash(pythonPath).catch(() => ''); fileHash = fileHash ? fileHash : ''; const store = this.persistentFactory.createGlobalPersistentState(`${pythonPath}.v1`, undefined, EXPITY_DURATION); - if (store.value && (!fileHash || store.value.fileHash === fileHash)) { + if (store.value && fileHash && store.value.fileHash === fileHash) { return store.value; } const processService = await this.serviceContainer.get(IPythonExecutionFactory).create({ pythonPath }); From e02b39fb3166e07f3cd96bd13f4f5581863caf0f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 11 Sep 2018 09:01:22 -0700 Subject: [PATCH 25/27] Perf improvements --- src/client/debugger/PythonProcess.ts | 4 +- src/client/interpreter/display/index.ts | 43 ++------- src/client/interpreter/interpreterService.ts | 93 +++++++++++++------ src/client/interpreter/locators/helpers.ts | 11 --- .../services/baseVirtualEnvService.ts | 6 -- .../locators/services/condaEnvFileService.ts | 20 +--- .../locators/services/condaEnvService.ts | 50 +--------- .../locators/services/currentPathService.ts | 2 - .../locators/services/pipEnvService.ts | 1 - .../services/windowsRegistryService.ts | 14 +-- src/test/interpreters/helper.unit.test.ts | 28 ------ 11 files changed, 79 insertions(+), 193 deletions(-) diff --git a/src/client/debugger/PythonProcess.ts b/src/client/debugger/PythonProcess.ts index 31316c5db3a7..fcf5415a69e4 100644 --- a/src/client/debugger/PythonProcess.ts +++ b/src/client/debugger/PythonProcess.ts @@ -4,9 +4,7 @@ import * as net from "net"; import { ChildProcess } from 'child_process'; import { EventEmitter } from "events"; -import { IPythonProcess, IPythonThread, IPythonEvaluationResult, IPythonStackFrame } from "./Common/Contracts"; -import { IPythonBreakpoint, IBreakpointCommand, IChildEnumCommand } from "./Common/Contracts"; -import { PythonEvaluationResultReprKind, IExecutionCommand, enum_EXCEPTION_STATE } from "./Common/Contracts"; +import { IPythonProcess, IPythonThread, IPythonEvaluationResult, IPythonStackFrame, IPythonBreakpoint, IBreakpointCommand, IChildEnumCommand, PythonEvaluationResultReprKind, IExecutionCommand, enum_EXCEPTION_STATE } from "./Common/Contracts"; import { Commands } from "./ProxyCommands"; import { IdDispenser } from "../common/idDispenser"; import { PythonProcessCallbackHandler } from "./PythonProcessCallbackHandler"; diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index 0ac73d8af537..49f63647294c 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -1,32 +1,24 @@ import { inject, injectable } from 'inversify'; import { Disposable, StatusBarAlignment, StatusBarItem, Uri } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; -import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IDisposableRegistry, IPathUtils } from '../../common/types'; +import { IDisposableRegistry, IPathUtils } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; -import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService, PythonInterpreter } from '../contracts'; -import { IVirtualEnvironmentManager } from '../virtualEnvs/types'; +import { IInterpreterDisplay, IInterpreterHelper, IInterpreterService } from '../contracts'; // tslint:disable-next-line:completed-docs @injectable() export class InterpreterDisplay implements IInterpreterDisplay { private readonly statusBar: StatusBarItem; - private readonly interpreterService: IInterpreterService; - private readonly virtualEnvMgr: IVirtualEnvironmentManager; - private readonly fileSystem: IFileSystem; - private readonly configurationService: IConfigurationService; private readonly helper: IInterpreterHelper; private readonly workspaceService: IWorkspaceService; private readonly pathUtils: IPathUtils; + private readonly interpreterService: IInterpreterService; constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { - this.interpreterService = serviceContainer.get(IInterpreterService); - this.virtualEnvMgr = serviceContainer.get(IVirtualEnvironmentManager); - this.fileSystem = serviceContainer.get(IFileSystem); - this.configurationService = serviceContainer.get(IConfigurationService); this.helper = serviceContainer.get(IInterpreterHelper); this.workspaceService = serviceContainer.get(IWorkspaceService); this.pathUtils = serviceContainer.get(IPathUtils); + this.interpreterService = serviceContainer.get(IInterpreterService); const application = serviceContainer.get(IApplicationShell); const disposableRegistry = serviceContainer.get(IDisposableRegistry); @@ -47,37 +39,16 @@ export class InterpreterDisplay implements IInterpreterDisplay { await this.updateDisplay(resource); } private async updateDisplay(workspaceFolder?: Uri) { - const interpreters = await this.interpreterService.getInterpreters(workspaceFolder); const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder); - const pythonPath = interpreter ? interpreter.path : this.configurationService.getSettings(workspaceFolder).pythonPath; - - this.statusBar.color = ''; - this.statusBar.tooltip = this.pathUtils.getDisplayName(pythonPath, workspaceFolder ? workspaceFolder.fsPath : undefined); if (interpreter) { - // tslint:disable-next-line:no-non-null-assertion + this.statusBar.color = ''; + this.statusBar.tooltip = this.pathUtils.getDisplayName(interpreter.path, workspaceFolder ? workspaceFolder.fsPath : undefined); this.statusBar.text = interpreter.displayName!; - this.statusBar.show(); - return; - } - - const [interpreterExists, details, virtualEnvName] = await Promise.all([ - this.fileSystem.fileExists(pythonPath), - this.helper.getInterpreterInformation(pythonPath).catch | undefined>(() => undefined), - this.getVirtualEnvironmentName(pythonPath, workspaceFolder).catch(() => '') - ]); - if (details) { - const displayName = await this.interpreterService.getDisplayName({ ...details, envName: virtualEnvName }); - this.statusBar.text = displayName; - } - - if (!interpreterExists && !details && interpreters.length > 0) { + } else { this.statusBar.tooltip = ''; this.statusBar.color = 'yellow'; this.statusBar.text = '$(alert) Select Python Environment'; } this.statusBar.show(); } - private async getVirtualEnvironmentName(pythonPath: string, resource?: Uri): Promise { - return this.virtualEnvMgr.getEnvironmentName(pythonPath, resource); - } } diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 41adf2cd311f..d3ffe773fd7b 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -6,7 +6,7 @@ import { PythonSettings } from '../common/configSettings'; import { getArchitectureDisplayName } from '../common/platform/registry'; import { IFileSystem } from '../common/platform/types'; import { IPythonExecutionFactory } from '../common/process/types'; -import { IConfigurationService, IDisposableRegistry } from '../common/types'; +import { IConfigurationService, IDisposableRegistry, IPersistentStateFactory } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { IPythonPathUpdaterServiceManager } from './configuration/types'; import { @@ -16,10 +16,14 @@ import { } from './contracts'; import { IVirtualEnvironmentManager } from './virtualEnvs/types'; +const EXPITY_DURATION = 24 * 60 * 60 * 1000; + @injectable() export class InterpreterService implements Disposable, IInterpreterService { private readonly locator: IInterpreterLocatorService; private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager; + private readonly fs: IFileSystem; + private readonly persistentStateFactory: IPersistentStateFactory; private readonly helper: IInterpreterHelper; private readonly didChangeInterpreterEmitter = new EventEmitter(); @@ -27,6 +31,8 @@ export class InterpreterService implements Disposable, IInterpreterService { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); this.helper = serviceContainer.get(IInterpreterHelper); this.pythonPathUpdaterService = this.serviceContainer.get(IPythonPathUpdaterServiceManager); + this.fs = this.serviceContainer.get(IFileSystem); + this.persistentStateFactory = this.serviceContainer.get(IPersistentStateFactory); } public async refresh(resource?: Uri) { @@ -44,7 +50,9 @@ export class InterpreterService implements Disposable, IInterpreterService { public async getInterpreters(resource?: Uri): Promise { const interpreters = await this.locator.getInterpreters(resource); - await Promise.all(interpreters.map(async item => item.displayName = await this.getDisplayName(item, resource))); + await Promise.all(interpreters + .filter(item => !item.displayName) + .map(async item => item.displayName = await this.getDisplayName(item, resource))); return interpreters; } @@ -106,35 +114,52 @@ export class InterpreterService implements Disposable, IInterpreterService { return this.getInterpreterDetails(fullyQualifiedPath, resource); } public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise { + // If we don't have the fully qualified path, then get it. + if (path.basename(pythonPath) === pythonPath) { + const pythonExecutionFactory = this.serviceContainer.get(IPythonExecutionFactory); + const pythonExecutionService = await pythonExecutionFactory.create({ resource }); + pythonPath = await pythonExecutionService.getExecutablePath().catch(() => ''); + // Python path is invalid or python isn't installed. + if (!pythonPath) { + return; + } + } + + let fileHash = await this.fs.getFileHash(pythonPath).catch(() => ''); + fileHash = fileHash ? fileHash : ''; + const store = this.persistentStateFactory.createGlobalPersistentState(`${pythonPath}.interpreter.details.v1`, undefined, EXPITY_DURATION); + if (store.value && fileHash && store.value.fileHash === fileHash) { + return store.value; + } + const fs = this.serviceContainer.get(IFileSystem); const interpreters = await this.getInterpreters(resource); - const interpreter = interpreters.find(i => fs.arePathsSame(i.path, pythonPath)); - - if (interpreter) { - return interpreter; - } - const interpreterHelper = this.serviceContainer.get(IInterpreterHelper); - const virtualEnvManager = this.serviceContainer.get(IVirtualEnvironmentManager); - const [info, type] = await Promise.all([ - interpreterHelper.getInterpreterInformation(pythonPath), - virtualEnvManager.getEnvironmentType(pythonPath) - ]); - if (!info) { - return; - } - const details: Partial = { - ...(info as PythonInterpreter), - path: pythonPath, - type: type - }; + let interpreterInfo = interpreters.find(i => fs.arePathsSame(i.path, pythonPath)); + if (!interpreterInfo) { + const interpreterHelper = this.serviceContainer.get(IInterpreterHelper); + const virtualEnvManager = this.serviceContainer.get(IVirtualEnvironmentManager); + const [info, type] = await Promise.all([ + interpreterHelper.getInterpreterInformation(pythonPath), + virtualEnvManager.getEnvironmentType(pythonPath) + ]); + if (!info) { + return; + } + const details: Partial = { + ...(info as PythonInterpreter), + path: pythonPath, + type: type + }; - const virtualEnvName = await virtualEnvManager.getEnvironmentName(pythonPath, resource); - const interpreterInfo = { - ...(details as PythonInterpreter), - envName: virtualEnvName - }; + const virtualEnvName = await virtualEnvManager.getEnvironmentName(pythonPath, resource); + interpreterInfo = { + ...(details as PythonInterpreter), + envName: virtualEnvName + }; + interpreterInfo.displayName = await this.getDisplayName(interpreterInfo, resource); + } - interpreterInfo.displayName = await this.getDisplayName(interpreterInfo, resource); + await store.updateValue({ ...interpreterInfo, path: pythonPath, fileHash }); return interpreterInfo; } @@ -147,6 +172,11 @@ export class InterpreterService implements Disposable, IInterpreterService { * @memberof InterpreterService */ public async getDisplayName(info: Partial, resource?: Uri): Promise { + const store = this.persistentStateFactory.createGlobalPersistentState(`${info.path}.interpreter.displayName.v1`, undefined, EXPITY_DURATION); + if (store.value) { + return store.value; + } + const displayNameParts: string[] = ['Python']; const envSuffixParts: string[] = []; @@ -176,7 +206,14 @@ export class InterpreterService implements Disposable, IInterpreterService { const envSuffix = envSuffixParts.length === 0 ? '' : `(${envSuffixParts.join(': ')})`; - return `${displayNameParts.join(' ')} ${envSuffix}`.trim(); + const displayName = `${displayNameParts.join(' ')} ${envSuffix}`.trim(); + + // If dealing with cached entry, then do not store the display name in cache. + if (!info.cachedEntry) { + await store.updateValue(displayName); + } + + return displayName; } public async shouldAutoSetInterpreter(): Promise { const activeWorkspace = this.helper.getActiveWorkspaceUri(); diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index 9b69736a455d..d06ab6fedc1f 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -1,7 +1,6 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { fsReaddirAsync } from '../../../utils/fs'; -import { getArchitectureDisplayName } from '../../common/platform/registry'; import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IS_WINDOWS } from '../../common/util'; import { IServiceContainer } from '../../ioc/types'; @@ -18,15 +17,6 @@ export function lookForInterpretersInDirectory(pathToCheck: string): Promise namePart.length > 0).join(' ').trim(); - } - return item; -} @injectable() export class InterpreterLocatorHelper implements IInterpreterLocatorHelper { private readonly platform: IPlatformService; @@ -41,7 +31,6 @@ export class InterpreterLocatorHelper implements IInterpreterLocatorHelper { public mergeInterpreters(interpreters: PythonInterpreter[]) { return interpreters .map(item => { return { ...item }; }) - .map(fixInterpreterDisplayName) .map(item => { item.path = path.normalize(item.path); return item; }) .reduce((accumulator, current) => { if (this.platform.isMac && this.helper.isMacDefaultPythonPath(current.path)) { diff --git a/src/client/interpreter/locators/services/baseVirtualEnvService.ts b/src/client/interpreter/locators/services/baseVirtualEnvService.ts index bb7cdd6b00bc..dca6823df3c1 100644 --- a/src/client/interpreter/locators/services/baseVirtualEnvService.ts +++ b/src/client/interpreter/locators/services/baseVirtualEnvService.ts @@ -75,17 +75,11 @@ export class BaseVirtualEnvService extends CacheableLocatorService { if (!details) { return; } - const virtualEnvSuffix = virtualEnvName.length ? virtualEnvName : this.getVirtualEnvironmentRootDirectory(interpreter); return { ...(details as PythonInterpreter), - displayName: `${details.version!} (${virtualEnvSuffix})`.trim(), envName: virtualEnvName, type: type }; }); } - private getVirtualEnvironmentRootDirectory(interpreter: string) { - // Python interperters are always in a subdirectory of the environment folder. - return path.basename(path.dirname(path.dirname(interpreter))); - } } diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts index 4527ac9bda3b..9a0c38f84ebc 100644 --- a/src/client/interpreter/locators/services/condaEnvFileService.ts +++ b/src/client/interpreter/locators/services/condaEnvFileService.ts @@ -10,7 +10,7 @@ import { PythonInterpreter } from '../../contracts'; import { CacheableLocatorService } from './cacheableLocatorService'; -import { AnacondaCompanyName, AnacondaCompanyNames, AnacondaDisplayName } from './conda'; +import { AnacondaCompanyName } from './conda'; /** * Locate conda env interpreters based on the "conda environments file". @@ -77,7 +77,6 @@ export class CondaEnvFileService extends CacheableLocatorService { const environment = environments.find(item => this.fileSystem.arePathsSame(item.path, interpreter!.envPath!)); if (environment) { interpreter.envName = environment!.name; - interpreter.displayName = `${interpreter.displayName} (${environment!.name})`; } }); } @@ -102,9 +101,7 @@ export class CondaEnvFileService extends CacheableLocatorService { if (!details) { return; } - const versionWithoutCompanyName = this.stripCompanyName(details.version!); return { - displayName: `${AnacondaDisplayName} ${versionWithoutCompanyName}`, ...(details as PythonInterpreter), path: interpreter, companyDisplayName: AnacondaCompanyName, @@ -112,19 +109,4 @@ export class CondaEnvFileService extends CacheableLocatorService { envPath: environmentPath }; } - - /** - * Remove the Anaconda company name from the given string. - */ - private stripCompanyName(content: string) { - // Strip company name from version. - const startOfCompanyName = AnacondaCompanyNames.reduce((index, companyName) => { - if (index > 0) { - return index; - } - return content.indexOf(`:: ${companyName}`); - }, -1); - - return startOfCompanyName > 0 ? content.substring(0, startOfCompanyName).trim() : content; - } } diff --git a/src/client/interpreter/locators/services/condaEnvService.ts b/src/client/interpreter/locators/services/condaEnvService.ts index 6a7afdc55478..25655269f8c6 100644 --- a/src/client/interpreter/locators/services/condaEnvService.ts +++ b/src/client/interpreter/locators/services/condaEnvService.ts @@ -8,15 +8,13 @@ import { ILogger } from '../../../common/types'; import { IServiceContainer } from '../../../ioc/types'; import { CondaInfo, ICondaService, IInterpreterHelper, InterpreterType, PythonInterpreter } from '../../contracts'; import { CacheableLocatorService } from './cacheableLocatorService'; -import { AnacondaCompanyName, AnacondaCompanyNames } from './conda'; -import { CondaHelper } from './condaHelper'; +import { AnacondaCompanyName } from './conda'; /** * Locates conda env interpreters based on the conda service's info. */ @injectable() export class CondaEnvService extends CacheableLocatorService { - private readonly condaHelper = new CondaHelper(); constructor( @inject(ICondaService) private condaService: ICondaService, @@ -58,8 +56,7 @@ export class CondaEnvService extends CacheableLocatorService { info, this.condaService, this.fileSystem, - this.helper, - this.condaHelper + this.helper ); const environments = await this.condaService.getCondaEnvironments(true); if (Array.isArray(environments) && environments.length > 0) { @@ -68,7 +65,6 @@ export class CondaEnvService extends CacheableLocatorService { const environment = environments.find(item => this.fileSystem.arePathsSame(item.path, interpreter!.envPath!)); if (environment) { interpreter.envName = environment!.name; - interpreter.displayName = `${interpreter.displayName} (${environment!.name})`; } }); } @@ -93,11 +89,8 @@ export async function parseCondaInfo( info: CondaInfo, condaService: ICondaService, fileSystem: IFileSystem, - helper: IInterpreterHelper, - condaHelper: CondaHelper = new CondaHelper() + helper: IInterpreterHelper ) { - const condaDisplayName = condaHelper.getDisplayName(info); - // The root of the conda environment is itself a Python interpreter // envs reported as e.g.: /Users/bob/miniconda3/envs/someEnv. const envs = Array.isArray(info.envs) ? info.envs : []; @@ -117,15 +110,9 @@ export async function parseCondaInfo( return; } - const versionWithoutCompanyName = stripCondaDisplayName( - stripCompanyName(details.version!), - condaDisplayName - ); - const displayName = `${condaDisplayName} ${versionWithoutCompanyName}`.trim(); return { ...(details as PythonInterpreter), path: pythonPath, - displayName, companyDisplayName: AnacondaCompanyName, type: InterpreterType.Conda, envPath @@ -137,34 +124,3 @@ export async function parseCondaInfo( // tslint:disable-next-line:no-non-null-assertion .then(interpreters => interpreters.map(interpreter => interpreter!)); } - -/** - * Remove the Anaconda company name from the given string. - */ -function stripCompanyName(content: string) { - // Strip company name from version. - const startOfCompanyName = AnacondaCompanyNames.reduce((index, companyName) => { - if (index > 0) { - return index; - } - return content.indexOf(`:: ${companyName}`); - }, -1); - - return startOfCompanyName > 0 ? content.substring(0, startOfCompanyName).trim() : content; -} - -/** - * Remove the Anaconda display name from the given string. - */ -function stripCondaDisplayName(content: string, condaDisplayName: string) { - // Strip company name from version. - if (content.endsWith(condaDisplayName)) { - let updatedContent = content.substr(0, content.indexOf(condaDisplayName)).trim(); - if (updatedContent.endsWith('::')) { - updatedContent = updatedContent.substr(0, content.indexOf('::')).trim(); - } - return updatedContent; - } else { - return content; - } -} diff --git a/src/client/interpreter/locators/services/currentPathService.ts b/src/client/interpreter/locators/services/currentPathService.ts index c7c862eb8736..32599fbdf4c0 100644 --- a/src/client/interpreter/locators/services/currentPathService.ts +++ b/src/client/interpreter/locators/services/currentPathService.ts @@ -77,10 +77,8 @@ export class CurrentPathService extends CacheableLocatorService { if (!details) { return; } - const displayName = `${details.version ? details.version : ''}${virtualEnvName.length > 0 ? ` (${virtualEnvName})` : ''}`; return { ...(details as PythonInterpreter), - displayName, envName: virtualEnvName, path: interpreter, type: type ? type : InterpreterType.Unknown diff --git a/src/client/interpreter/locators/services/pipEnvService.ts b/src/client/interpreter/locators/services/pipEnvService.ts index 708cc544a45a..4d6459392342 100644 --- a/src/client/interpreter/locators/services/pipEnvService.ts +++ b/src/client/interpreter/locators/services/pipEnvService.ts @@ -64,7 +64,6 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer } return { ...(details as PythonInterpreter), - displayName: `${details.version} (${execName})`, path: interpreterPath, type: InterpreterType.PipEnv }; diff --git a/src/client/interpreter/locators/services/windowsRegistryService.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts index cb0dafb04b67..4da481316fba 100644 --- a/src/client/interpreter/locators/services/windowsRegistryService.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -105,15 +105,13 @@ export class WindowsRegistryService extends CacheableLocatorService { return Promise.all([ Promise.resolve(installPath), this.registry.getValue(key, hive, arch, 'ExecutablePath'), - // tslint:disable-next-line:no-non-null-assertion - this.getInterpreterDisplayName(tagKey, companyKey, hive, arch), this.registry.getValue(tagKey, hive, arch, 'SysVersion'), this.getCompanyDisplayName(companyKey, hive, arch) ]) - .then(([installedPath, executablePath, displayName, version, companyDisplayName]) => { + .then(([installedPath, executablePath, version, companyDisplayName]) => { companyDisplayName = AnacondaCompanyNames.indexOf(companyDisplayName) === -1 ? companyDisplayName : AnacondaCompanyName; // tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion - return { installPath: installedPath, executablePath, displayName, version, companyDisplayName } as InterpreterInformation; + return { installPath: installedPath, executablePath, version, companyDisplayName } as InterpreterInformation; }); }) .then(async (interpreterInfo?: InterpreterInformation) => { @@ -122,7 +120,6 @@ export class WindowsRegistryService extends CacheableLocatorService { } const executablePath = interpreterInfo.executablePath && interpreterInfo.executablePath.length > 0 ? interpreterInfo.executablePath : path.join(interpreterInfo.installPath, DefaultPythonExecutable); - const displayName = interpreterInfo.displayName; const helper = this.serviceContainer.get(IInterpreterHelper); const details = await helper.getInterpreterInformation(executablePath); if (!details) { @@ -133,7 +130,6 @@ export class WindowsRegistryService extends CacheableLocatorService { return { ...(details as PythonInterpreter), architecture: arch, - displayName, path: executablePath, version, companyDisplayName: interpreterInfo.companyDisplayName, @@ -147,12 +143,6 @@ export class WindowsRegistryService extends CacheableLocatorService { return null; }); } - private async getInterpreterDisplayName(tagKey: string, companyKey: string, hive: RegistryHive, arch?: Architecture) { - const displayName = await this.registry.getValue(tagKey, hive, arch, 'DisplayName'); - if (displayName && displayName.length > 0) { - return displayName; - } - } private async getCompanyDisplayName(companyKey: string, hive: RegistryHive, arch?: Architecture) { const displayName = await this.registry.getValue(companyKey, hive, arch, 'DisplayName'); if (displayName && displayName.length > 0) { diff --git a/src/test/interpreters/helper.unit.test.ts b/src/test/interpreters/helper.unit.test.ts index 616df83b7d82..7ff97624c9d0 100644 --- a/src/test/interpreters/helper.unit.test.ts +++ b/src/test/interpreters/helper.unit.test.ts @@ -7,11 +7,8 @@ import { expect } from 'chai'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, TextDocument, TextEditor, Uri } from 'vscode'; import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; -import { InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts'; import { InterpreterHelper } from '../../client/interpreter/helpers'; -import { fixInterpreterDisplayName } from '../../client/interpreter/locators/helpers'; import { IServiceContainer } from '../../client/ioc/types'; -import { Architecture } from '../../utils/platform'; // tslint:disable-next-line:max-func-body-length suite('Interpreters Display Helper', () => { @@ -88,29 +85,4 @@ suite('Interpreters Display Helper', () => { expect(workspace!.folderUri).to.be.equal(documentWorkspaceFolderUri); expect(workspace!.configTarget).to.be.equal(ConfigurationTarget.WorkspaceFolder); }); - test('Ensure Python prefix is added to displayName', () => { - const interpreter: PythonInterpreter = { - path: '', - type: InterpreterType.Unknown, - version: 'Something', - sysPrefix: '', - architecture: Architecture.Unknown, - sysVersion: '', - version_info: [0, 0, 0, 'alpha'] - }; - const expectedDisplayName = `Python ${interpreter.version!}`; - expect(fixInterpreterDisplayName(interpreter)).to.have.property('displayName', expectedDisplayName); - }); - test('Ensure Python prefix is not added to displayName', () => { - const interpreter: PythonInterpreter = { - path: '', - type: InterpreterType.Unknown, - version: 'Python Something', - sysPrefix: '', - architecture: Architecture.Unknown, - sysVersion: '', - version_info: [0, 0, 0, 'alpha'] - }; - expect(fixInterpreterDisplayName(interpreter)).to.have.property('displayName', interpreter.version); - }); }); From f47451fe5a72a53d0afca02ec89119e02f5e5478 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 11 Sep 2018 09:55:50 -0700 Subject: [PATCH 26/27] Fixed tests --- .../locators/services/condaService.ts | 6 ++-- .../condaEnvFileService.unit.test.ts | 4 +-- .../interpreters/condaEnvService.unit.test.ts | 11 +------ .../interpreters/condaService.unit.test.ts | 11 +++---- .../currentPathService.unit.test.ts | 4 +-- src/test/interpreters/display.unit.test.ts | 19 +++++------- .../interpreterService.unit.test.ts | 29 ++++++++++++++++++- .../windowsRegistryService.unit.test.ts | 11 ------- 8 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/client/interpreter/locators/services/condaService.ts b/src/client/interpreter/locators/services/condaService.ts index f455e2ee9e33..aed0d3384093 100644 --- a/src/client/interpreter/locators/services/condaService.ts +++ b/src/client/interpreter/locators/services/condaService.ts @@ -5,7 +5,7 @@ import { IFileSystem, IPlatformService } from '../../../common/platform/types'; import { IProcessServiceFactory } from '../../../common/process/types'; import { ILogger, IPersistentStateFactory } from '../../../common/types'; import { IServiceContainer } from '../../../ioc/types'; -import { CondaInfo, ICondaService, IInterpreterLocatorService, PythonInterpreter, WINDOWS_REGISTRY_SERVICE } from '../../contracts'; +import { CondaInfo, ICondaService, IInterpreterLocatorService, InterpreterType, PythonInterpreter, WINDOWS_REGISTRY_SERVICE } from '../../contracts'; import { CondaHelper } from './condaHelper'; // tslint:disable-next-line:no-require-imports no-var-requires @@ -201,7 +201,9 @@ export class CondaService implements ICondaService { * Is the given interpreter from conda? */ private detectCondaEnvironment(interpreter: PythonInterpreter) { - return (interpreter.displayName ? interpreter.displayName : '').toUpperCase().indexOf('ANACONDA') >= 0 || + return interpreter.type === InterpreterType.Conda || + (interpreter.displayName ? interpreter.displayName : '').toUpperCase().indexOf('ANACONDA') >= 0 || + (interpreter.companyDisplayName ? interpreter.companyDisplayName : '').toUpperCase().indexOf('ANACONDA') >= 0 || (interpreter.companyDisplayName ? interpreter.companyDisplayName : '').toUpperCase().indexOf('CONTINUUM') >= 0; } diff --git a/src/test/interpreters/condaEnvFileService.unit.test.ts b/src/test/interpreters/condaEnvFileService.unit.test.ts index 47d6b21beb79..60fcd1d42dc5 100644 --- a/src/test/interpreters/condaEnvFileService.unit.test.ts +++ b/src/test/interpreters/condaEnvFileService.unit.test.ts @@ -5,7 +5,7 @@ import * as TypeMoq from 'typemoq'; import { IFileSystem } from '../../client/common/platform/types'; import { ILogger, IPersistentStateFactory } from '../../client/common/types'; import { ICondaService, IInterpreterHelper, IInterpreterLocatorService, InterpreterType } from '../../client/interpreter/contracts'; -import { AnacondaCompanyName, AnacondaCompanyNames, AnacondaDisplayName } from '../../client/interpreter/locators/services/conda'; +import { AnacondaCompanyName, AnacondaCompanyNames } from '../../client/interpreter/locators/services/conda'; import { CondaEnvFileService } from '../../client/interpreter/locators/services/condaEnvFileService'; import { IServiceContainer } from '../../client/ioc/types'; import { MockState } from './mocks'; @@ -84,7 +84,6 @@ suite('Interpreters from Conda Environments Text File', () => { const expectedPythonPath = isWindows ? path.join(validPaths[0], 'python.exe') : path.join(validPaths[0], 'bin', 'python'); assert.equal(interpreters.length, 2, 'Incorrect number of entries'); - assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Name (numpy)`, 'Incorrect display name'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect display name'); assert.equal(interpreters[0].path, expectedPythonPath, 'Incorrect path'); assert.equal(interpreters[0].envPath, validPaths[0], 'Incorrect envpath'); @@ -114,7 +113,6 @@ suite('Interpreters from Conda Environments Text File', () => { const interpreters = await condaFileProvider.getInterpreters(); assert.equal(interpreters.length, 1, 'Incorrect number of entries'); - assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version`, 'Incorrect display name'); } }); }); diff --git a/src/test/interpreters/condaEnvService.unit.test.ts b/src/test/interpreters/condaEnvService.unit.test.ts index 314bcc39f0cf..b3ec2de6caae 100644 --- a/src/test/interpreters/condaEnvService.unit.test.ts +++ b/src/test/interpreters/condaEnvService.unit.test.ts @@ -5,7 +5,7 @@ import { IFileSystem } from '../../client/common/platform/types'; import { ILogger, IPersistentStateFactory } from '../../client/common/types'; import { ICondaService, InterpreterType } from '../../client/interpreter/contracts'; import { InterpreterHelper } from '../../client/interpreter/helpers'; -import { AnacondaCompanyName, AnacondaDisplayName } from '../../client/interpreter/locators/services/conda'; +import { AnacondaCompanyName } from '../../client/interpreter/locators/services/conda'; import { CondaEnvService, parseCondaInfo } from '../../client/interpreter/locators/services/condaEnvService'; import { IServiceContainer } from '../../client/ioc/types'; import { UnitTestIocContainer } from '../unittests/serviceRegistry'; @@ -80,13 +80,11 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, 'Anaconda 4.4.0 (64-bit)', 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); const path2 = path.join(info.envs[1], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[1].path, path2, 'Incorrect path for first env'); - assert.equal(interpreters[1].displayName, 'Anaconda 4.4.0 (64-bit)', 'Incorrect display name for first env'); assert.equal(interpreters[1].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[1].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -125,13 +123,11 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, 'Anaconda 4.4.0 (64-bit) (numpy)', 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); const path2 = path.join(info.envs[1], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[1].path, path2, 'Incorrect path for first env'); - assert.equal(interpreters[1].displayName, 'Anaconda 4.4.0 (64-bit) (scipy)', 'Incorrect display name for first env'); assert.equal(interpreters[1].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[1].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -167,7 +163,6 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, `Anaonda 4.4.0 (64-bit) : ${AnacondaDisplayName}`, 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -206,7 +201,6 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, `Anaonda 4.4.0 (64-bit) : ${AnacondaDisplayName} (numpy)`, 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -240,7 +234,6 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, `${AnacondaDisplayName}`, 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -278,7 +271,6 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} (numpy)`, 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } @@ -310,7 +302,6 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.default_prefix, isWindows ? 'python.exe' : path.join('bin', 'python')); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); - assert.equal(interpreters[0].displayName, AnacondaDisplayName, 'Incorrect display name for first env'); assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); assert.equal(interpreters[0].type, InterpreterType.Conda, 'Environment not detected as a conda environment'); } diff --git a/src/test/interpreters/condaService.unit.test.ts b/src/test/interpreters/condaService.unit.test.ts index cc7b460897a3..bf4fa4bc1d25 100644 --- a/src/test/interpreters/condaService.unit.test.ts +++ b/src/test/interpreters/condaService.unit.test.ts @@ -270,7 +270,7 @@ suite('Interpreters Conda Service', () => { const condaPythonExePath = path.join('dumyPath', 'environments', 'conda', 'Scripts', 'python.exe'); const registryInterpreters: PythonInterpreter[] = [ { displayName: 'One', path: path.join(environmentsPath, 'path1', 'one.exe'), companyDisplayName: 'One 1', version: '1', type: InterpreterType.Unknown }, - { displayName: 'Anaconda', path: condaPythonExePath, companyDisplayName: 'Two 2', version: '1.11.0', enyTpe: InterpreterType.Unknown }, + { displayName: 'Anaconda', path: condaPythonExePath, companyDisplayName: 'Two 2', version: '1.11.0', type: InterpreterType.Conda }, { displayName: 'Three', path: path.join(environmentsPath, 'path2', 'one.exe'), companyDisplayName: 'Three 3', version: '2.10.1', type: InterpreterType.Unknown }, { displayName: 'Seven', path: path.join(environmentsPath, 'conda', 'envs', 'numpy'), companyDisplayName: 'Continuum Analytics, Inc.', type: InterpreterType.Unknown } ].map(item => { @@ -291,10 +291,10 @@ suite('Interpreters Conda Service', () => { const condaPythonExePath = path.join('dumyPath', 'environments'); const registryInterpreters: PythonInterpreter[] = [ { displayName: 'One', path: path.join(environmentsPath, 'path1', 'one.exe'), companyDisplayName: 'One 1', version: '1', type: InterpreterType.Unknown }, - { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda1', 'Scripts', 'python.exe'), companyDisplayName: 'Two 1', version: '1.11.0', type: InterpreterType.Unknown }, - { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda211', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.11', version: '2.11.0', type: InterpreterType.Unknown }, - { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda231', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.31', version: '2.31.0', type: InterpreterType.Unknown }, - { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda221', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.21', version: '2.21.0', type: InterpreterType.Unknown }, + { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda1', 'Scripts', 'python.exe'), companyDisplayName: 'Two 1', version: '1.11.0', type: InterpreterType.Conda }, + { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda211', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.11', version: '2.11.0', type: InterpreterType.Conda }, + { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda231', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.31', version: '2.31.0', type: InterpreterType.Conda }, + { displayName: 'Anaconda', path: path.join(condaPythonExePath, 'conda221', 'Scripts', 'python.exe'), companyDisplayName: 'Two 2.21', version: '2.21.0', type: InterpreterType.Conda }, { displayName: 'Three', path: path.join(environmentsPath, 'path2', 'one.exe'), companyDisplayName: 'Three 3', version: '2.10.1', type: InterpreterType.Unknown }, { displayName: 'Seven', path: path.join(environmentsPath, 'conda', 'envs', 'numpy'), companyDisplayName: 'Continuum Analytics, Inc.', type: InterpreterType.Unknown } ].map(item => { @@ -522,6 +522,7 @@ suite('Interpreters Conda Service', () => { test('isAvailable will return false if conda is not available', async () => { processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.reject(new Error('not found'))); fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); + fileSystem.setup(fs => fs.search(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); platformService.setup(p => p.isWindows).returns(() => false); const isAvailable = await condaService.isCondaAvailable(); diff --git a/src/test/interpreters/currentPathService.unit.test.ts b/src/test/interpreters/currentPathService.unit.test.ts index ce932c39cac4..d3060072cb39 100644 --- a/src/test/interpreters/currentPathService.unit.test.ts +++ b/src/test/interpreters/currentPathService.unit.test.ts @@ -76,7 +76,7 @@ suite('Interpreters CurrentPath Service', () => { processService.verifyAll(); fileSystem.verifyAll(); expect(interpreters).to.be.of.length(2); - expect(interpreters).to.deep.include({ version, envName, displayName: `${version} (${envName})`, path: 'c:/root:python', type: InterpreterType.VirtualEnv }); - expect(interpreters).to.deep.include({ version, envName, displayName: `${version} (${envName})`, path: 'c:/python3', type: InterpreterType.VirtualEnv }); + expect(interpreters).to.deep.include({ version, envName, path: 'c:/root:python', type: InterpreterType.VirtualEnv }); + expect(interpreters).to.deep.include({ version, envName, path: 'c:/python3', type: InterpreterType.VirtualEnv }); }); }); diff --git a/src/test/interpreters/display.unit.test.ts b/src/test/interpreters/display.unit.test.ts index 9b71aac720b3..483ade45763e 100644 --- a/src/test/interpreters/display.unit.test.ts +++ b/src/test/interpreters/display.unit.test.ts @@ -11,6 +11,8 @@ import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs import { IServiceContainer } from '../../client/ioc/types'; import { Architecture } from '../../utils/platform'; +// tslint:disable:no-any max-func-body-length + const info: PythonInterpreter = { architecture: Architecture.Unknown, companyDisplayName: '', @@ -24,7 +26,6 @@ const info: PythonInterpreter = { sysVersion: '' }; -// tslint:disable-next-line:max-func-body-length suite('Interpreters Display', () => { let applicationShell: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; @@ -107,13 +108,11 @@ suite('Interpreters Display', () => { const displayName = 'This is the display name'; setupWorkspaceFolder(resource, workspaceFolder); - interpreterService.setup(i => i.getInterpreters(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve([])); - interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(undefined)); - interpreterService.setup(i => i.getDisplayName(TypeMoq.It.isAny())).returns(() => Promise.resolve(displayName)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - pythonSettings.setup(p => p.pythonPath).returns(() => pythonPath); - virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); - interpreterHelper.setup(v => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve({})); + const pythonInterpreter: PythonInterpreter = { + displayName, + path: pythonPath + } as any as PythonInterpreter; + interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder))).returns(() => Promise.resolve(pythonInterpreter)); await interpreterDisplay.refresh(resource); @@ -152,10 +151,6 @@ suite('Interpreters Display', () => { }; fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); virtualEnvMgr.setup(v => v.getEnvironmentName(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve('')); - interpreterService - .setup(i => i.getInterpreters(TypeMoq.It.isValue(resource))) - .returns(() => Promise.resolve([])) - .verifiable(TypeMoq.Times.once()); interpreterService .setup(i => i.getActiveInterpreter(TypeMoq.It.isValue(resource))) .returns(() => Promise.resolve(activeInterpreter)) diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index dde4f45bbea1..f411f73128a4 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -15,7 +15,8 @@ import { ConfigurationTarget, Disposable, TextDocument, TextEditor, Uri, Workspa import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; import { getArchitectureDisplayName } from '../../client/common/platform/registry'; import { IFileSystem } from '../../client/common/platform/types'; -import { IConfigurationService, IDisposableRegistry } from '../../client/common/types'; +import { IPythonExecutionFactory, IPythonExecutionService } from '../../client/common/process/types'; +import { IConfigurationService, IDisposableRegistry, IPersistentStateFactory } from '../../client/common/types'; import { IPythonPathUpdaterServiceManager } from '../../client/interpreter/configuration/types'; import { IInterpreterDisplay, @@ -65,6 +66,9 @@ suite('Interpreters service', () => { let interpreterDisplay: TypeMoq.IMock; let workspacePythonPath: TypeMoq.IMock; let virtualEnvMgr: TypeMoq.IMock; + let persistentStateFactory: TypeMoq.IMock; + let pythonExecutionFactory: TypeMoq.IMock; + let pythonExecutionService: TypeMoq.IMock; type ConfigValue = { key: string; defaultValue?: T; globalValue?: T; workspaceValue?: T; workspaceFolderValue?: T }; function setupSuite() { @@ -81,8 +85,23 @@ suite('Interpreters service', () => { interpreterDisplay = TypeMoq.Mock.ofType(); workspacePythonPath = TypeMoq.Mock.ofType(); virtualEnvMgr = TypeMoq.Mock.ofType(); + persistentStateFactory = TypeMoq.Mock.ofType(); + pythonExecutionFactory = TypeMoq.Mock.ofType(); + pythonExecutionService = TypeMoq.Mock.ofType(); + pythonExecutionService.setup((p: any) => p.then).returns(() => undefined); workspace.setup(x => x.getConfiguration('python', TypeMoq.It.isAny())).returns(() => config.object); + pythonExecutionFactory.setup(f => f.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object)); + fileSystem.setup(fs => fs.getFileHash(TypeMoq.It.isAny())).returns(() => Promise.resolve('')); + persistentStateFactory + .setup(p => p.createGlobalPersistentState(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => { + const state = { + updateValue: () => Promise.resolve() + }; + return state as any; + }); + serviceManager.addSingletonInstance(IDisposableRegistry, []); serviceManager.addSingletonInstance(IInterpreterHelper, helper.object); serviceManager.addSingletonInstance(IPythonPathUpdaterServiceManager, updater.object); @@ -91,6 +110,9 @@ suite('Interpreters service', () => { serviceManager.addSingletonInstance(IFileSystem, fileSystem.object); serviceManager.addSingletonInstance(IInterpreterDisplay, interpreterDisplay.object); serviceManager.addSingletonInstance(IVirtualEnvironmentManager, virtualEnvMgr.object); + serviceManager.addSingletonInstance(IPersistentStateFactory, persistentStateFactory.object); + serviceManager.addSingletonInstance(IPythonExecutionFactory, pythonExecutionFactory.object); + serviceManager.addSingletonInstance(IPythonExecutionService, pythonExecutionService.object); pipenvLocator = TypeMoq.Mock.ofType(); wksLocator = TypeMoq.Mock.ofType(); @@ -199,10 +221,15 @@ suite('Interpreters service', () => { .setup(v => v.getEnvironmentType(TypeMoq.It.isValue(pythonPath))) .returns(() => Promise.resolve(InterpreterType.Unknown)) .verifiable(TypeMoq.Times.once()); + pythonExecutionService + .setup(p => p.getExecutablePath()) + .returns(() => Promise.resolve(pythonPath)) + .verifiable(TypeMoq.Times.once()); const details = await service.getInterpreterDetails(pythonPath, resource); locator.verifyAll(); + pythonExecutionService.verifyAll(); helper.verifyAll(); expect(details).to.be.equal(undefined, 'Not undefined'); }); diff --git a/src/test/interpreters/windowsRegistryService.unit.test.ts b/src/test/interpreters/windowsRegistryService.unit.test.ts index fe32e5752e14..8fcdd674fbf5 100644 --- a/src/test/interpreters/windowsRegistryService.unit.test.ts +++ b/src/test/interpreters/windowsRegistryService.unit.test.ts @@ -62,7 +62,6 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters.length, 1, 'Incorrect number of entries'); assert.equal(interpreters[0].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Display Name for Company One', 'Incorrect company name'); - assert.equal(interpreters[0].displayName, 'DisplayName.Tag1', 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'path1', 'one.exe'), 'Incorrect executable path'); assert.equal(interpreters[0].version, 'Version.Tag1', 'Incorrect version'); }); @@ -82,7 +81,6 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters.length, 1, 'Incorrect number of entries'); assert.equal(interpreters[0].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Python Software Foundation', 'Incorrect company name'); - assert.equal(interpreters[0].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'path1', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[0].version, 'Tag1', 'Incorrect version'); }); @@ -117,7 +115,6 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters.length, 1, 'Incorrect number of entries'); assert.equal(interpreters[0].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Company One', 'Incorrect company name'); - assert.equal(interpreters[0].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'path1', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[0].version, 'Tag1', 'Incorrect version'); }); @@ -159,19 +156,16 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters.length, 4, 'Incorrect number of entries'); assert.equal(interpreters[0].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Display Name for Company One', 'Incorrect company name'); - assert.equal(interpreters[0].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'path1', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[0].version, 'Tag1', 'Incorrect version'); assert.equal(interpreters[1].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[1].companyDisplayName, 'Display Name for Company One', 'Incorrect company name'); - assert.equal(interpreters[1].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[1].path, path.join(environmentsPath, 'path2', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[1].version, 'Tag2', 'Incorrect version'); assert.equal(interpreters[2].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[2].companyDisplayName, 'Company Two', 'Incorrect company name'); - assert.equal(interpreters[2].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[2].path, path.join(environmentsPath, 'conda', 'envs', 'numpy', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[2].version, 'Tag B', 'Incorrect version'); }); @@ -218,19 +212,16 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters.length, 4, 'Incorrect number of entries'); assert.equal(interpreters[0].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Display Name for Company One', 'Incorrect company name'); - assert.equal(interpreters[0].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'conda', 'envs', 'numpy', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[0].version, 'Tag1', 'Incorrect version'); assert.equal(interpreters[1].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[1].companyDisplayName, 'Display Name for Company One', 'Incorrect company name'); - assert.equal(interpreters[1].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[1].path, path.join(environmentsPath, 'conda', 'envs', 'scipy', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[1].version, 'Tag2', 'Incorrect version'); assert.equal(interpreters[2].architecture, Architecture.x86, 'Incorrect arhictecture'); assert.equal(interpreters[2].companyDisplayName, 'Company Two', 'Incorrect company name'); - assert.equal(interpreters[2].displayName, undefined, 'Incorrect display name'); assert.equal(interpreters[2].path, path.join(environmentsPath, 'path1', 'python.exe'), 'Incorrect path'); assert.equal(interpreters[2].version, 'Tag A', 'Incorrect version'); }); @@ -278,13 +269,11 @@ suite('Interpreters from Windows Registry (unit)', () => { assert.equal(interpreters[0].architecture, Architecture.x86, '1. Incorrect arhictecture'); assert.equal(interpreters[0].companyDisplayName, 'Display Name for Company One', '1. Incorrect company name'); - assert.equal(interpreters[0].displayName, undefined, '1. Incorrect display name'); assert.equal(interpreters[0].path, path.join(environmentsPath, 'conda', 'envs', 'numpy', 'python.exe'), '1. Incorrect path'); assert.equal(interpreters[0].version, 'Tag1', '1. Incorrect version'); assert.equal(interpreters[1].architecture, Architecture.x86, '2. Incorrect arhictecture'); assert.equal(interpreters[1].companyDisplayName, 'Company Two', '2. Incorrect company name'); - assert.equal(interpreters[1].displayName, undefined, '2. Incorrect display name'); assert.equal(interpreters[1].path, path.join(environmentsPath, 'path2', 'python.exe'), '2. Incorrect path'); assert.equal(interpreters[1].version, 'Tag B', '2. Incorrect version'); }); From 7f7379ce6903e655b63639fbf45563518a3b7b09 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 11 Sep 2018 13:06:13 -0700 Subject: [PATCH 27/27] Fixed tests --- src/test/interpreters/venv.unit.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/test/interpreters/venv.unit.test.ts b/src/test/interpreters/venv.unit.test.ts index cd158b9fe79e..75ac86b8a615 100644 --- a/src/test/interpreters/venv.unit.test.ts +++ b/src/test/interpreters/venv.unit.test.ts @@ -8,6 +8,7 @@ import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { Uri, WorkspaceFolder } from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; +import { PlatformService } from '../../client/common/platform/platformService'; import { IConfigurationService, ICurrentProcess, IPythonSettings } from '../../client/common/types'; import { EnvironmentVariables } from '../../client/common/variables/types'; import { GlobalVirtualEnvironmentsSearchPathProvider } from '../../client/interpreter/locators/services/globalVirtualEnvService'; @@ -83,7 +84,11 @@ suite('Virtual environments', () => { const paths = pathProvider.getSearchPaths(Uri.file('')); const homedir = os.homedir(); - const expected = [path.join(homedir, 'foo'), 'root', path.join('root', '.direnv')].map(item => Uri.file(item).fsPath); - expect(paths).to.deep.equal(expected, 'Workspace venv folder search list does not match.'); + const isWindows = new PlatformService(); + const fixCase = (item: string) => isWindows ? item.toUpperCase() : item; + const expected = [path.join(homedir, 'foo'), 'root', path.join('root', '.direnv')] + .map(item => Uri.file(item).fsPath) + .map(fixCase); + expect(paths.map(fixCase)).to.deep.equal(expected, 'Workspace venv folder search list does not match.'); }); });