From 68a566df52eb26a911ba9f410060e189fe840763 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 1 Jan 2019 21:01:55 -0800 Subject: [PATCH 01/49] Misc --- CHANGELOG.md | 1 + gulpfile.js | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8180e9b0bb1..be86887336c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes + 1. Lowering threshold for Language Server support on a platform. ([#3693](https://github.com/Microsoft/vscode-python/issues/3693)) 1. Fix bug affecting multiple linters used in a workspace. diff --git a/gulpfile.js b/gulpfile.js index 69848ab1a4f0..191c50d0562a 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -704,6 +704,7 @@ function getFilesToProcess(fileList) { * @param {hygieneOptions} options */ function getFileListToProcess(options) { + return []; const mode = options ? options.mode : 'all'; const gulpSrcOptions = { base: '.' }; From fa9c397010b4dd06f7b2cdd49fd42b5026b40593 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Jan 2019 15:22:41 -0800 Subject: [PATCH 02/49] Run python code using env variables from activated envs --- src/client/common/logger.ts | 6 +- src/client/common/process/proc.ts | 4 +- .../common/process/pythonExecutionFactory.ts | 20 ++++- src/client/common/process/types.ts | 3 +- src/client/common/serviceRegistry.ts | 15 +++- .../condaActivationProvider.ts | 4 +- .../pipEnvActivationProvider.ts | 28 +++++++ src/client/common/terminal/helper.ts | 41 ++++++++-- src/client/common/terminal/types.ts | 11 ++- src/client/common/utils/decorators.ts | 79 +++++++++++++++++- src/client/interpreter/activation/service.ts | 82 +++++++++++++++++++ src/client/interpreter/activation/types.ts | 11 +++ src/client/interpreter/serviceRegistry.ts | 4 + src/client/unittests/common/runner.ts | 28 ++++--- .../terminals/activation.conda.unit.test.ts | 11 ++- .../terminals/helper.activation.unit.test.ts | 17 +++- src/test/common/terminals/helper.unit.test.ts | 12 ++- src/test/refactor/rename.test.ts | 4 +- 18 files changed, 335 insertions(+), 45 deletions(-) create mode 100644 src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts create mode 100644 src/client/interpreter/activation/service.ts create mode 100644 src/client/interpreter/activation/types.ts diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index 1390ba63edb0..9023dec2e37c 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -46,7 +46,7 @@ export class Logger implements ILogger { } } -enum LogOptions { +export enum LogOptions { None = 0, Arguments = 1, ReturnValue = 2 @@ -91,8 +91,8 @@ export function traceInfo(message: string) { } export namespace traceDecorators { - export function verbose(message: string) { - return trace(message, LogOptions.Arguments | LogOptions.ReturnValue); + export function verbose(message: string, options: LogOptions = LogOptions.Arguments | LogOptions.ReturnValue) { + return trace(message, options); } export function error(message: string, ex?: Error) { return trace(message, LogOptions.Arguments | LogOptions.ReturnValue, LogLevel.Error); diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index b1180cf9003b..e3ae918251a2 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -4,7 +4,7 @@ import { exec, spawn } from 'child_process'; import { Observable } from 'rxjs/Observable'; import * as tk from 'tree-kill'; import { Disposable } from 'vscode'; - +import { LogOptions, traceDecorators } from '../logger'; import { createDeferred } from '../utils/async'; import { EnvironmentVariables } from '../variables/types'; import { DEFAULT_ENCODING } from './constants'; @@ -149,7 +149,7 @@ export class ProcessService implements IProcessService { return deferred.promise; } - + @traceDecorators.verbose('Invokeing shellExec', LogOptions.None) public shellExec(command: string, options: ShellOptions = {}): Promise> { const shellOptions = this.getDefaultOptions(options); return new Promise((resolve, reject) => { diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 3e9551905cec..04dd9cfabde2 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -2,17 +2,20 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; +import { IEnvironmentActivationService } from '../../interpreter/activation/types'; import { IServiceContainer } from '../../ioc/types'; -import { IConfigurationService } from '../types'; +import { IConfigurationService, Resource } from '../types'; +import { ProcessService } from './proc'; import { PythonExecutionService } from './pythonProcess'; -import { ExecutionFactoryCreationOptions, IProcessServiceFactory, IPythonExecutionFactory, IPythonExecutionService } from './types'; +import { ExecutionFactoryCreationOptions, IBufferDecoder, IProcessServiceFactory, IPythonExecutionFactory, IPythonExecutionService } from './types'; @injectable() export class PythonExecutionFactory implements IPythonExecutionFactory { private readonly configService: IConfigurationService; private processServiceFactory: IProcessServiceFactory; - constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, + @inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService, + @inject(IBufferDecoder) private readonly decoder: IBufferDecoder) { this.processServiceFactory = serviceContainer.get(IProcessServiceFactory); this.configService = serviceContainer.get(IConfigurationService); } @@ -21,4 +24,13 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { const processService = await this.processServiceFactory.create(options.resource); return new PythonExecutionService(this.serviceContainer, processService, pythonPath); } + public async createActivatedEnvironment(resource: Resource): Promise { + const envVars = await this.activationHelper.getActivatedEnvironmentVariables(resource); + if (!envVars || Object.keys(envVars).length === 0) { + return this.create({ resource }); + } + const pythonPath = this.configService.getSettings(resource).pythonPath; + const processService = new ProcessService(this.decoder, envVars); + return new PythonExecutionService(this.serviceContainer, processService, pythonPath); + } } diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index 84d42cd0e375..fc519a4ac151 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -3,7 +3,7 @@ import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process'; import { Observable } from 'rxjs/Observable'; import { CancellationToken, Uri } from 'vscode'; -import { ExecutionInfo, Version } from '../types'; +import { ExecutionInfo, Resource, Version } from '../types'; import { Architecture } from '../utils/platform'; import { EnvironmentVariables } from '../variables/types'; @@ -57,6 +57,7 @@ export type ExecutionFactoryCreationOptions = { }; export interface IPythonExecutionFactory { create(options: ExecutionFactoryCreationOptions): Promise; + createActivatedEnvironment(resource: Resource): Promise; } export type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown'; export type PythonVersionInfo = [number, number, number, ReleaseLevel]; diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index fea49c7b2c24..7852ed829f4d 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -37,6 +37,8 @@ import { TerminalActivator } from './terminal/activator'; import { PowershellTerminalActivationFailedHandler } from './terminal/activator/powershellFailedHandler'; import { Bash } from './terminal/environmentActivationProviders/bash'; import { CommandPromptAndPowerShell } from './terminal/environmentActivationProviders/commandPrompt'; +import { CondaActivationCommandProvider } from './terminal/environmentActivationProviders/condaActivationProvider'; +import { PipEnvActivationCommandProvider } from './terminal/environmentActivationProviders/pipEnvActivationProvider'; import { PyEnvActivationCommandProvider } from './terminal/environmentActivationProviders/pyenvActivationProvider'; import { TerminalServiceFactory } from './terminal/factory'; import { TerminalHelper } from './terminal/helper'; @@ -45,7 +47,8 @@ import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, - ITerminalServiceFactory + ITerminalServiceFactory, + TerminalActivationProviders } from './terminal/types'; import { IAsyncDisposableRegistry, @@ -93,11 +96,15 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ITerminalHelper, TerminalHelper); serviceManager.addSingleton( - ITerminalActivationCommandProvider, Bash, 'bashCShellFish'); + ITerminalActivationCommandProvider, Bash, TerminalActivationProviders.bashCShellFish); serviceManager.addSingleton( - ITerminalActivationCommandProvider, CommandPromptAndPowerShell, 'commandPromptAndPowerShell'); + ITerminalActivationCommandProvider, CommandPromptAndPowerShell, TerminalActivationProviders.commandPromptAndPowerShell); serviceManager.addSingleton( - ITerminalActivationCommandProvider, PyEnvActivationCommandProvider, 'pyenv'); + ITerminalActivationCommandProvider, PyEnvActivationCommandProvider, TerminalActivationProviders.pyenv); + serviceManager.addSingleton( + ITerminalActivationCommandProvider, CondaActivationCommandProvider, TerminalActivationProviders.conda); + serviceManager.addSingleton( + ITerminalActivationCommandProvider, PipEnvActivationCommandProvider, TerminalActivationProviders.pipenv); serviceManager.addSingleton(IFeatureDeprecationManager, FeatureDeprecationManager); serviceManager.addSingleton(IAsyncDisposableRegistry, AsyncDisposableRegistry); diff --git a/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts index a2087507f72c..28784b396acb 100644 --- a/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts @@ -3,7 +3,7 @@ 'use strict'; -import { injectable } from 'inversify'; +import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; import { ICondaService } from '../../../interpreter/contracts'; @@ -19,7 +19,7 @@ import { ITerminalActivationCommandProvider, TerminalShellType } from '../types' @injectable() export class CondaActivationCommandProvider implements ITerminalActivationCommandProvider { constructor( - private readonly serviceContainer: IServiceContainer + @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer ) { } /** diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts new file mode 100644 index 000000000000..9f66d676d84c --- /dev/null +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { Uri } from 'vscode'; +import { IInterpreterService, InterpreterType } from '../../../interpreter/contracts'; +import { IServiceContainer } from '../../../ioc/types'; +import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; + +@injectable() +export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider { + constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { } + + public isShellSupported(_targetShell: TerminalShellType): boolean { + return true; + } + + public async getActivationCommands(resource: Uri | undefined, _: TerminalShellType): Promise { + const interpreter = await this.serviceContainer.get(IInterpreterService).getActiveInterpreter(resource); + if (!interpreter || interpreter.type !== InterpreterType.PipEnv) { + return; + } + + return ['pipenv shell']; + } +} diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index f119ed80cf85..acb23c31570b 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -1,16 +1,16 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { Terminal, Uri } from 'vscode'; import { ICondaService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { ITerminalManager, IWorkspaceService } from '../application/types'; import '../extensions'; import { IPlatformService } from '../platform/types'; -import { IConfigurationService } from '../types'; -import { CondaActivationCommandProvider } from './environmentActivationProviders/condaActivationProvider'; -import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalShellType } from './types'; +import { IConfigurationService, Resource } from '../types'; +import { OSType } from '../utils/platform'; +import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalActivationProviders, TerminalShellType } from './types'; // Types of shells can be found here: // 1. https://wiki.ubuntu.com/ChangingShells @@ -25,11 +25,25 @@ const IS_POWERSHELL_CORE = /(pwsh.exe$|pwsh$)/i; const IS_FISH = /(fish$)/i; const IS_CSHELL = /(csh$)/i; const IS_TCSHELL = /(tcsh$)/i; +const IS_XONSH = /(xonsh$)/i; + +const defaultOSShells = { + [OSType.Linux]: TerminalShellType.bash, + [OSType.OSX]: TerminalShellType.bash, + [OSType.Windows]: TerminalShellType.commandPrompt +}; @injectable() export class TerminalHelper implements ITerminalHelper { private readonly detectableShells: Map; - constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, + @inject(IPlatformService) private readonly platform: IPlatformService, + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.conda) private readonly conda: ITerminalActivationCommandProvider, + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.bashCShellFish) private readonly bashCShellFish: ITerminalActivationCommandProvider, + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.commandPromptAndPowerShell) private readonly commandPromptAndPowerShell: ITerminalActivationCommandProvider, + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.pyenv) private readonly pyenv: ITerminalActivationCommandProvider, + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.pipenv) private readonly pipenv: ITerminalActivationCommandProvider + ) { this.detectableShells = new Map(); this.detectableShells.set(TerminalShellType.powershell, IS_POWERSHELL); this.detectableShells.set(TerminalShellType.gitbash, IS_GITBASH); @@ -42,6 +56,7 @@ export class TerminalHelper implements ITerminalHelper { this.detectableShells.set(TerminalShellType.tcshell, IS_TCSHELL); this.detectableShells.set(TerminalShellType.cshell, IS_CSHELL); this.detectableShells.set(TerminalShellType.powershellCore, IS_POWERSHELL_CORE); + this.detectableShells.set(TerminalShellType.xonsh, IS_XONSH); } public createTerminal(title?: string): Terminal { const terminalManager = this.serviceContainer.get(ITerminalManager); @@ -79,6 +94,18 @@ export class TerminalHelper implements ITerminalHelper { return `${commandPrefix}${command.fileToCommandArgument()} ${args.join(' ')}`.trim(); } public async getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise { + const providers = [this.bashCShellFish, this.commandPromptAndPowerShell, this.pyenv, this.pipenv]; + return this.getActivationCommands(resource || undefined, terminalShellType, providers); + } + public async getEnvironmentActivationShellCommands(resource: Resource): Promise { + const shell = defaultOSShells[this.platform.osType]; + if (!shell) { + return; + } + const providers = [this.bashCShellFish, this.commandPromptAndPowerShell]; + return this.getActivationCommands(resource || undefined, shell, providers); + } + protected async getActivationCommands(resource: Resource, terminalShellType: TerminalShellType, providers: ITerminalActivationCommandProvider[]): Promise { const settings = this.serviceContainer.get(IConfigurationService).getSettings(resource); const activateEnvironment = settings.terminal.activateEnvironment; if (!activateEnvironment) { @@ -88,15 +115,13 @@ export class TerminalHelper implements ITerminalHelper { // If we have a conda environment, then use that. const isCondaEnvironment = await this.serviceContainer.get(ICondaService).isCondaEnvironment(settings.pythonPath); if (isCondaEnvironment) { - const condaActivationProvider = new CondaActivationCommandProvider(this.serviceContainer); - const activationCommands = await condaActivationProvider.getActivationCommands(resource, terminalShellType); + const activationCommands = await this.conda.getActivationCommands(resource, terminalShellType); if (Array.isArray(activationCommands)) { return activationCommands; } } // Search from the list of providers. - const providers = this.serviceContainer.getAll(ITerminalActivationCommandProvider); const supportedProviders = providers.filter(provider => provider.isShellSupported(terminalShellType)); for (const provider of supportedProviders) { diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index aebf8758b7fc..040d588d044a 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -1,9 +1,16 @@ - // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { Event, Terminal, Uri } from 'vscode'; +import { Resource } from '../types'; +export enum TerminalActivationProviders { + bashCShellFish = 'bashCShellFish', + commandPromptAndPowerShell = 'commandPromptAndPowerShell', + pyenv = 'pyenv', + conda = 'conda', + pipenv = 'pipenv' +} export enum TerminalShellType { powershell = 'powershell', powershellCore = 'powershellCore', @@ -16,6 +23,7 @@ export enum TerminalShellType { cshell = 'cshell', tcshell = 'tshell', wsl = 'wsl', + xonsh = 'xonsh', other = 'other' } @@ -49,6 +57,7 @@ export interface ITerminalHelper { getTerminalShellPath(): string; buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]): string; getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise; + getEnvironmentActivationShellCommands(resource: Resource): Promise; } export const ITerminalActivator = Symbol('ITerminalActivator'); diff --git a/src/client/common/utils/decorators.ts b/src/client/common/utils/decorators.ts index 7e4a81acf07e..6fd5cf3cc647 100644 --- a/src/client/common/utils/decorators.ts +++ b/src/client/common/utils/decorators.ts @@ -1,5 +1,10 @@ -import { ProgressLocation, ProgressOptions, window } from 'vscode'; +// tslint:disable:no-any + +import { ProgressLocation, ProgressOptions, Uri, window } from 'vscode'; +import '../../common/extensions'; import { isTestExecution } from '../constants'; +import { traceError, traceVerbose } from '../logger'; + // tslint:disable-next-line:no-require-imports no-var-requires const _debounce = require('lodash/debounce') as typeof import('lodash/debounce'); @@ -20,8 +25,76 @@ export function debounce(wait?: number) { }; } +type vsCodeType = typeof import('vscode'); +type cacheData = { + value: any; + expiry: number; +}; +const simpleCache = new Map(); + +function getCachedData(key: string): undefined | cacheData { + const data = simpleCache.get(key); + if (!data) { + return; + } + return data.expiry < Date.now() ? undefined : data; +} +function storeCachedData(key: string, data: any, expiryDurationMs: number): void { + simpleCache.set(key, { + value: data, + expiry: Date.now() + expiryDurationMs + }); +} +// tslint:disable-next-line:no-any no-require-imports +export function getCacheKeyFromFunctionArgs(fnArgs: any[], vscode: vsCodeType = require('vscode')): string { + const keys: string[] = []; + fnArgs.forEach((arg, index) => { + if (index === 0) { + // get workspace related to this resource + if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { + return keys.push('undefined'); + } + const folder = vscode.workspace.getWorkspaceFolder(arg as Uri); + if (folder) { + const pythonPath = vscode.workspace.getConfiguration('python', arg as Uri).get('pythonPath'); + keys.push(`${folder.uri.fsPath}-${pythonPath}`); + } else { + keys.push('undefined'); + } + } else { + keys.push(`${arg}`); + } + }); + + return keys.join('-Arg-Separator-'); +} + +// tslint:disable-next-line:no-any +type PromiseFunctionWithFirstArgOfResource = (...any: [Uri | undefined, ...any[]]) => Promise; + +// tslint:disable-next-line:no-require-imports +export function cacheResourceSpecificIngterpreterData(expiryDurationMs: number, vscode: vsCodeType = require('vscode')) { + return function (_target: Object, _propertyName: string, descriptor: TypedPropertyDescriptor) { + const originalMethod = descriptor.value!; + // tslint:disable-next-line:no-any no-function-expression + descriptor.value = async function (...args: [Uri | undefined, ...any[]]) { + const cacheKey = getCacheKeyFromFunctionArgs(args, vscode); + const data = getCachedData(cacheKey); + if (data) { + traceVerbose(`Cached data exists ${cacheKey}`); + return Promise.resolve(data.value); + } + // tslint:disable-next-line:no-invalid-this + const promise = originalMethod.apply(this, args) as Promise; + promise.then(result => storeCachedData(cacheKey, result, expiryDurationMs)).ignoreErrors(); + return promise; + }; + }; +} + /** * Swallows exceptions thrown by a function. Function must return either a void or a promise that resolves to a void. + * When exceptions (including in promises) are caught, this will return `undefined` to calling code. * @export * @param {string} [scopeName] Scope for the error message to be logged along with the error. * @returns void @@ -43,14 +116,14 @@ export function swallowExceptions(scopeName: string) { if (isTestExecution()) { return; } - console.error(errorMessage, error); + traceError(errorMessage, error); }); } } catch (error) { if (isTestExecution()) { return; } - console.error(errorMessage, error); + traceError(errorMessage, error); } }; }; diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts new file mode 100644 index 000000000000..610eaab14359 --- /dev/null +++ b/src/client/interpreter/activation/service.ts @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { LogOptions, traceDecorators, traceVerbose } from '../../common/logger'; +import { IPlatformService } from '../../common/platform/types'; +import { IProcessServiceFactory } from '../../common/process/types'; +import { ITerminalHelper } from '../../common/terminal/types'; +import { ICurrentProcess, Resource } from '../../common/types'; +import { cacheResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; +import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { IEnvironmentActivationService } from './types'; + +const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; + +// Regex for splitting environment strings +const environmentSplitRegex = /^\s*([^=]+)\s*=\s*(.+)\s*$/; +const cacheDuration = 60 * 60 * 1000; + +@injectable() +export class EnvironmentActivationService implements IEnvironmentActivationService { + constructor(@inject(ITerminalHelper) private readonly helper: ITerminalHelper, + @inject(IPlatformService) private readonly platform: IPlatformService, + @inject(IProcessServiceFactory) private processServiceFactory: IProcessServiceFactory, + @inject(ICurrentProcess) private currentProcess: ICurrentProcess, + @inject(IEnvironmentVariablesProvider) private readonly envVarsService: IEnvironmentVariablesProvider) { + } + @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments) + @swallowExceptions('getActivatedEnvironmentVariables') + @cacheResourceSpecificIngterpreterData(cacheDuration) + public async getActivatedEnvironmentVariables(resource: Resource): Promise { + const activationCommands = await this.helper.getEnvironmentActivationShellCommands(resource); + traceVerbose(`Activation Commands received ${activationCommands}`); + if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) { + return; + } + + // Run the activate command collect the environment from it. + const listEnv = this.platform.isWindows ? 'set' : 'printenv'; + const activationCommand = this.fixActivationCommands(activationCommands).join(' && '); + const processService = await this.processServiceFactory.create(resource); + const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource); + const hasCustomEnvVars = Object.keys(customEnvVars).length; + const env = hasCustomEnvVars ? this.currentProcess.env : customEnvVars; + traceVerbose(`${hasCustomEnvVars ? 'Has' : 'No'} Custom Env Vars`); + + // In order to make sure we know where the environment output is, + // put in a dummy echo we can look for + const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && ${listEnv}`; + const result = await processService.shellExec(command, { env }); + if (result.stderr && result.stderr.length > 0) { + throw new Error(`StdErr from ShellExec, ${result.stderr}`); + } + return this.parseEnvironmentOutput(result.stdout); + } + protected fixActivationCommands(commands: string[]): string[] { + // Replace 'source ' with '. ' as that works in shell exec + return commands.map(cmd => cmd.replace(/^source\s+/, '. ')); + } + @traceDecorators.verbose('parseEnvironmentOutput', LogOptions.None) + protected parseEnvironmentOutput(output: string): NodeJS.ProcessEnv | undefined { + const result = {}; + const lines = output.splitLines({ trim: true, removeEmptyEntries: true }); + let foundDummyOutput = false; + for (let i = 0; i < lines.length; i += 1) { + if (foundDummyOutput) { + // Split on equal + const match = environmentSplitRegex.exec(lines[i]); + if (match && match !== null && match.length > 2) { + result[match[1]] = match[2]; + } + } else { + // See if we found the dummy output or not yet + foundDummyOutput = lines[i].includes(getEnvironmentPrefix); + } + } + + return Object.keys(result).length === 0 ? undefined : result; + } +} diff --git a/src/client/interpreter/activation/types.ts b/src/client/interpreter/activation/types.ts new file mode 100644 index 000000000000..e1b703480989 --- /dev/null +++ b/src/client/interpreter/activation/types.ts @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { Resource } from '../../common/types'; + +export const IEnvironmentActivationService = Symbol('IEnvironmentActivationService'); +export interface IEnvironmentActivationService { + getActivatedEnvironmentVariables(resource: Resource): Promise; +} diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 13cfc74065f0..27b9173e21e8 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -2,6 +2,8 @@ // Licensed under the MIT License. import { IServiceManager } from '../ioc/types'; +import { EnvironmentActivationService } from './activation/service'; +import { IEnvironmentActivationService } from './activation/types'; import { InterpreterAutoSelectionService } from './autoSelection/index'; import { InterpreterAutoSeletionProxyService } from './autoSelection/proxy'; import { CachedInterpretersAutoSelectionRule } from './autoSelection/rules/cached'; @@ -113,4 +115,6 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IInterpreterAutoSelectionRule, SettingsInterpretersAutoSelectionRule, AutoSelectionRule.settings); serviceManager.addSingleton(IInterpreterAutoSeletionProxyService, InterpreterAutoSeletionProxyService); serviceManager.addSingleton(IInterpreterAutoSelectionService, InterpreterAutoSelectionService); + + serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); } diff --git a/src/client/unittests/common/runner.ts b/src/client/unittests/common/runner.ts index 3345ff72996d..f38f69dc457b 100644 --- a/src/client/unittests/common/runner.ts +++ b/src/client/unittests/common/runner.ts @@ -32,20 +32,24 @@ export async function run(serviceContainer: IServiceContainer, testProvider: Tes let promise: Promise>; - if (!testExecutablePath && testProvider === UNITTEST_PROVIDER) { - // Unit tests have a special way of being executed - const pythonServiceFactory = serviceContainer.get(IPythonExecutionFactory); - pythonExecutionServicePromise = pythonServiceFactory.create({ resource: options.workspaceFolder }); - promise = pythonExecutionServicePromise.then(executionService => executionService.execObservable(options.args, { ...spawnOptions })); + // Since conda 4.4.0 we have found that running python code needs the environment activated. + // So if running an executable, there's no way we can activate, if its a module, then activate and run the module. + const testHelper = serviceContainer.get(ITestsHelper); + const executionInfo: ExecutionInfo = { + execPath: testExecutablePath, + args: options.args, + moduleName: testExecutablePath && testExecutablePath.length > 0 ? undefined : moduleName, + product: testHelper.parseProduct(testProvider) + }; + + if (testProvider === UNITTEST_PROVIDER) { + promise = serviceContainer.get(IPythonExecutionFactory).createActivatedEnvironment(options.workspaceFolder) + .then(executionService => executionService.execObservable(options.args, { ...spawnOptions })); + } else if (typeof executionInfo.moduleName === 'string' && executionInfo.moduleName.length > 0) { + pythonExecutionServicePromise = serviceContainer.get(IPythonExecutionFactory).createActivatedEnvironment(options.workspaceFolder); + promise = pythonExecutionServicePromise.then(executionService => executionService.execModuleObservable(executionInfo.moduleName!, executionInfo.args, options)); } else { const pythonToolsExecutionService = serviceContainer.get(IPythonToolExecutionService); - const testHelper = serviceContainer.get(ITestsHelper); - const executionInfo: ExecutionInfo = { - execPath: testExecutablePath, - args: options.args, - moduleName: testExecutablePath && testExecutablePath.length > 0 ? undefined : moduleName, - product: testHelper.parseProduct(testProvider) - }; promise = pythonToolsExecutionService.execObservable(executionInfo, spawnOptions, options.workspaceFolder); } diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts index 3a906d1b79c2..c985d9a2d968 100644 --- a/src/test/common/terminals/activation.conda.unit.test.ts +++ b/src/test/common/terminals/activation.conda.unit.test.ts @@ -6,6 +6,7 @@ import { expect } from 'chai'; import * as path from 'path'; import { parse } from 'semver'; +import { instance, mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable } from 'vscode'; import '../../../client/common/extensions'; @@ -15,9 +16,13 @@ import { import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types'; +import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; +import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; import { CondaActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/condaActivationProvider'; +import { PipEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; +import { PyEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pyenvActivationProvider'; import { TerminalHelper } from '../../../client/common/terminal/helper'; import { ITerminalActivationCommandProvider, TerminalShellType @@ -72,7 +77,11 @@ suite('Terminal Environment Activation conda', () => { terminalSettings = TypeMoq.Mock.ofType(); pythonSettings.setup(s => s.terminal).returns(() => terminalSettings.object); - terminalHelper = new TerminalHelper(serviceContainer.object); + terminalHelper = new TerminalHelper(serviceContainer.object, platformService.object, + instance(mock(CondaActivationCommandProvider)), + instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), + instance(mock(PyEnvActivationCommandProvider)), + instance(mock(PipEnvActivationCommandProvider))); }); teardown(() => { disposables.forEach(disposable => { diff --git a/src/test/common/terminals/helper.activation.unit.test.ts b/src/test/common/terminals/helper.activation.unit.test.ts index 380085b05f50..74f5861990da 100644 --- a/src/test/common/terminals/helper.activation.unit.test.ts +++ b/src/test/common/terminals/helper.activation.unit.test.ts @@ -2,12 +2,16 @@ // Licensed under the MIT License. import { expect } from 'chai'; +import { instance, mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable } from 'vscode'; import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types'; import { IPlatformService } from '../../../client/common/platform/types'; import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; +import { CondaActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/condaActivationProvider'; +import { PipEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; +import { PyEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pyenvActivationProvider'; import { TerminalHelper } from '../../../client/common/terminal/helper'; import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; import { IConfigurationService, IDisposableRegistry, IPythonSettings, ITerminalSettings } from '../../../client/common/types'; @@ -51,7 +55,12 @@ suite('Terminal Service helpers', () => { condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICondaService))).returns(() => condaService.object); - helper = new TerminalHelper(serviceContainer.object); + helper = new TerminalHelper(serviceContainer.object, platformService.object, + instance(mock(CondaActivationCommandProvider)), + instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), + instance(mock(PyEnvActivationCommandProvider)), + instance(mock(PipEnvActivationCommandProvider))); + }); teardown(() => { disposables.filter(item => !!item).forEach(item => item.dispose()); @@ -112,7 +121,11 @@ getNamesAndValues(TerminalShellType).forEach(terminalShell => condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICondaService))).returns(() => condaService.object); - helper = new TerminalHelper(serviceContainer.object); + helper = new TerminalHelper(serviceContainer.object, platformService.object, + instance(mock(CondaActivationCommandProvider)), + instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), + instance(mock(PyEnvActivationCommandProvider)), + instance(mock(PipEnvActivationCommandProvider))); }); teardown(() => { disposables.filter(disposable => !!disposable).forEach(disposable => disposable.dispose()); diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts index b06011efa2d3..c068965c59aa 100644 --- a/src/test/common/terminals/helper.unit.test.ts +++ b/src/test/common/terminals/helper.unit.test.ts @@ -2,10 +2,16 @@ // Licensed under the MIT License. import { expect } from 'chai'; +import { instance, mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, WorkspaceConfiguration } from 'vscode'; import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types'; import { IPlatformService } from '../../../client/common/platform/types'; +import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; +import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; +import { CondaActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/condaActivationProvider'; +import { PipEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; +import { PyEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pyenvActivationProvider'; import { TerminalHelper } from '../../../client/common/terminal/helper'; import { ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; import { IDisposableRegistry } from '../../../client/common/types'; @@ -37,7 +43,11 @@ suite('Terminal Service helpers', () => { serviceContainer.setup(c => c.get(IWorkspaceService)).returns(() => workspaceService.object); serviceContainer.setup(c => c.get(IInterpreterService)).returns(() => interpreterService.object); - helper = new TerminalHelper(serviceContainer.object); + helper = new TerminalHelper(serviceContainer.object, platformService.object, + instance(mock(CondaActivationCommandProvider)), + instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), + instance(mock(PyEnvActivationCommandProvider)), + instance(mock(PipEnvActivationCommandProvider))); }); teardown(() => { disposables.filter(item => !!item).forEach(item => item.dispose()); diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index 03bffd245899..65080d3ccdaa 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -20,6 +20,8 @@ import { RefactorProxy } from '../../client/refactor/proxy'; import { PYTHON_PATH } from '../common'; import { closeActiveWindows, initialize, initializeTest } from './../initialize'; +// tslint:disable:no-any + type RenameResponse = { results: [{ diff: string }]; }; @@ -40,7 +42,7 @@ suite('Refactor Rename', () => { serviceContainer = typeMoq.Mock.ofType(); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IConfigurationService), typeMoq.It.isAny())).returns(() => configService.object); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); - serviceContainer.setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())).returns(() => new PythonExecutionFactory(serviceContainer.object)); + serviceContainer.setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())).returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any)); await initializeTest(); }); teardown(closeActiveWindows); From bfc634a98c5f3e07cbc2eec47da5d2f80b082dc1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Jan 2019 15:30:20 -0800 Subject: [PATCH 03/49] Improve readability --- src/client/common/utils/decorators.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/client/common/utils/decorators.ts b/src/client/common/utils/decorators.ts index 6fd5cf3cc647..4a66db27cafe 100644 --- a/src/client/common/utils/decorators.ts +++ b/src/client/common/utils/decorators.ts @@ -49,21 +49,19 @@ function storeCachedData(key: string, data: any, expiryDurationMs: number): void export function getCacheKeyFromFunctionArgs(fnArgs: any[], vscode: vsCodeType = require('vscode')): string { const keys: string[] = []; fnArgs.forEach((arg, index) => { - if (index === 0) { - // get workspace related to this resource - if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { - return keys.push('undefined'); - } - const folder = vscode.workspace.getWorkspaceFolder(arg as Uri); - if (folder) { - const pythonPath = vscode.workspace.getConfiguration('python', arg as Uri).get('pythonPath'); - keys.push(`${folder.uri.fsPath}-${pythonPath}`); - } else { - keys.push('undefined'); - } - } else { - keys.push(`${arg}`); + if (index > 0) { + return keys.push(`${arg}`); + } + // get workspace related to this resource + if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { + return keys.push('undefined'); + } + const folder = vscode.workspace.getWorkspaceFolder(arg as Uri); + if (!folder) { + return keys.push('undefined'); } + const pythonPath = vscode.workspace.getConfiguration('python', arg as Uri).get('pythonPath'); + keys.push(`${folder.uri.fsPath}-${pythonPath}`); }); return keys.join('-Arg-Separator-'); From cf1535ae2751f205a06e9222cfc15e3afb0796ae Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Jan 2019 16:36:00 -0800 Subject: [PATCH 04/49] Give preference to version returned by python process --- .../interpreter/locators/services/windowsRegistryService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/locators/services/windowsRegistryService.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts index 0c6e842f4f3b..c368eaa4e484 100644 --- a/src/client/interpreter/locators/services/windowsRegistryService.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -131,7 +131,9 @@ export class WindowsRegistryService extends CacheableLocatorService { return { ...(details as PythonInterpreter), path: executablePath, - version: parsePythonVersion(version), + // Do not use version info from registry, this doesn't contain the release level. + // Give preference to what we have retrieved from getInterpreterInformation. + version: details.version || parsePythonVersion(version), companyDisplayName: interpreterInfo.companyDisplayName, type: InterpreterType.Unknown } as PythonInterpreter; From eb69a758a8e76f2dd390b9690ef7f67309023b36 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Jan 2019 16:39:34 -0800 Subject: [PATCH 05/49] Fix trace message --- src/client/interpreter/locators/progressService.ts | 2 +- .../interpreter/locators/services/cacheableLocatorService.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/locators/progressService.ts b/src/client/interpreter/locators/progressService.ts index 2e68b2194ccf..58f917cbf0d8 100644 --- a/src/client/interpreter/locators/progressService.ts +++ b/src/client/interpreter/locators/progressService.ts @@ -48,7 +48,6 @@ export class InterpreterLocatorProgressService implements IInterpreterLocatorPro private notifyRefreshing() { this.refreshing.fire(); } - @traceDecorators.verbose('Checking whether locactors have completed locating') private checkProgress() { if (this.areAllItemsCcomplete()) { return this.notifyCompleted(); @@ -58,6 +57,7 @@ export class InterpreterLocatorProgressService implements IInterpreterLocatorPro .then(() => this.checkProgress()) .ignoreErrors(); } + @traceDecorators.verbose('Checking whether locactors have completed locating') private areAllItemsCcomplete() { this.deferreds = this.deferreds.filter(item => !item.completed); return this.deferreds.length === 0; diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 395d3e0929da..872b16637941 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -8,7 +8,7 @@ import * as md5 from 'md5'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../../common/application/types'; import '../../../common/extensions'; -import { Logger } from '../../../common/logger'; +import { Logger, traceVerbose } from '../../../common/logger'; import { IDisposableRegistry, IPersistentStateFactory } from '../../../common/types'; import { createDeferred, Deferred } from '../../../common/utils/async'; import { IServiceContainer } from '../../../ioc/types'; @@ -50,6 +50,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ const promise = this.getInterpretersImplementation(resource) .then(async items => { await this.cacheInterpreters(items, resource); + traceVerbose(`Interpreters returned by ${this.name} are ${JSON.stringify(items)}`); deferred!.resolve(items); }) .catch(ex => deferred!.reject(ex)); From a5ae72dbf51bf48c923b576bb454ac5eaa30607b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Jan 2019 16:42:32 -0800 Subject: [PATCH 06/49] Fix typo --- src/client/interpreter/locators/progressService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/locators/progressService.ts b/src/client/interpreter/locators/progressService.ts index 58f917cbf0d8..4f37a56a7e73 100644 --- a/src/client/interpreter/locators/progressService.ts +++ b/src/client/interpreter/locators/progressService.ts @@ -49,7 +49,7 @@ export class InterpreterLocatorProgressService implements IInterpreterLocatorPro this.refreshing.fire(); } private checkProgress() { - if (this.areAllItemsCcomplete()) { + if (this.areAllItemsComplete()) { return this.notifyCompleted(); } Promise.all(this.deferreds.map(item => item.promise)) @@ -58,7 +58,7 @@ export class InterpreterLocatorProgressService implements IInterpreterLocatorPro .ignoreErrors(); } @traceDecorators.verbose('Checking whether locactors have completed locating') - private areAllItemsCcomplete() { + private areAllItemsComplete() { this.deferreds = this.deferreds.filter(item => !item.completed); return this.deferreds.length === 0; } From 8bec99431e71afe63f48930331264deaecc76191 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 09:37:24 -0800 Subject: [PATCH 07/49] Fixed errors, ensure vsc, console does not leak into debug adapter code --- gulpfile.js | 4 - .../common/installer/productInstaller.ts | 4 +- src/client/common/logger.ts | 22 ++-- src/client/common/platform/fileSystem.ts | 2 - src/client/common/platform/platformService.ts | 3 - src/client/common/process/proc.ts | 12 +- .../common/process/pythonExecutionFactory.ts | 2 +- src/client/common/utils/cacheUtils.ts | 120 ++++++++++++++++++ src/client/common/utils/decorators.ts | 68 ++-------- .../variables/environmentVariablesProvider.ts | 40 +++--- src/client/interpreter/activation/service.ts | 19 ++- .../interpreter/locators/progressService.ts | 3 + .../services/cacheableLocatorService.ts | 2 +- src/client/unittests/common/runner.ts | 2 +- src/client/unittests/configuration.ts | 2 +- src/client/unittests/nosetest/runner.ts | 2 +- src/test/refactor/rename.test.ts | 4 +- src/test/unittests/argsService.test.ts | 2 +- 18 files changed, 201 insertions(+), 112 deletions(-) create mode 100644 src/client/common/utils/cacheUtils.ts diff --git a/gulpfile.js b/gulpfile.js index ad091382b721..21e45228c4fb 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -154,10 +154,6 @@ gulp.task("compile", () => { gulp.task('compile-webviews', async () => spawnAsync('npx', ['webpack', '--config', 'webpack.datascience-ui.config.js', '--mode', 'production'])); -gulp.task('webpack', async () => { - await spawnAsync('npx', ['webpack', '--mode', 'production', '--inline', '--progress']); - await spawnAsync('npx', ['webpack', '--config', './build/webpack/webpack.extension.config.js', '--mode', 'production', '--inline', '--progress']); -}); gulp.task('webpack', async () => { await spawnAsync('npx', ['webpack', '--mode', 'production']); diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 8514e978c059..a0ed70fd058f 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -317,9 +317,7 @@ export class ProductInstaller implements IInstaller { function translateProductToModule(product: Product, purpose: ModuleNamePurpose): string { switch (product) { case Product.mypy: return 'mypy'; - case Product.nosetest: { - return purpose === ModuleNamePurpose.install ? 'nose' : 'nosetests'; - } + case Product.nosetest: return 'nose2'; case Product.pylama: return 'pylama'; case Product.prospector: return 'prospector'; case Product.pylint: return 'pylint'; diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index 9023dec2e37c..7a34cc59ffc3 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -57,6 +57,9 @@ function argsToLogString(args: any[]): string { try { return (args || []).map((item, index) => { try { + if (item.fsPath) { + return `Arg ${index + 1}: `; + } return `Arg ${index + 1}: ${JSON.stringify(item)}`; } catch { return `Arg ${index + 1}: UNABLE TO DETERMINE VALUE`; @@ -69,15 +72,18 @@ function argsToLogString(args: any[]): string { // tslint:disable-next-line:no-any function returnValueToLogString(returnValue: any): string { - let returnValueMessage = 'Return Value: '; - if (returnValue) { - try { - returnValueMessage += `${JSON.stringify(returnValue)}`; - } catch { - returnValueMessage += 'UNABLE TO DETERMINE VALUE'; - } + const returnValueMessage = 'Return Value: '; + if (returnValue === undefined) { + return `${returnValueMessage}undefined`; + } + if (returnValue === null) { + return `${returnValueMessage}null`; + } + try { + return `${returnValueMessage}${JSON.stringify(returnValue)}`; + } catch { + return `${returnValueMessage}`; } - return returnValueMessage; } export function traceVerbose(message: string) { diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index b30950d9cfbf..2e18b045d467 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -9,7 +9,6 @@ import * as glob from 'glob'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; -import { traceDecorators } from '../logger'; import { createDeferred } from '../utils/async'; import { IFileSystem, IPlatformService, TemporaryFile } from './types'; @@ -144,7 +143,6 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - @traceDecorators.error('Failed to get FileHash') public getFileHash(filePath: string): Promise { return new Promise(resolve => { fs.lstat(filePath, (err, stats) => { diff --git a/src/client/common/platform/platformService.ts b/src/client/common/platform/platformService.ts index 42a42bab8468..8f86b1f36fa4 100644 --- a/src/client/common/platform/platformService.ts +++ b/src/client/common/platform/platformService.ts @@ -7,7 +7,6 @@ import * as os from 'os'; import { coerce, SemVer } from 'semver'; import { sendTelemetryEvent } from '../../telemetry'; import { PLATFORM_INFO, PlatformErrors } from '../../telemetry/constants'; -import { traceDecorators, traceError } from '../logger'; import { OSType } from '../utils/platform'; import { parseVersion } from '../utils/version'; import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; @@ -23,7 +22,6 @@ export class PlatformService implements IPlatformService { public get virtualEnvBinName() { return this.isWindows ? 'Scripts' : 'bin'; } - @traceDecorators.verbose('Get Platform Version') public async getVersion(): Promise { if (this.version) { return this.version; @@ -43,7 +41,6 @@ export class PlatformService implements IPlatformService { throw new Error('Unable to parse version'); } catch (ex) { sendTelemetryEvent(PLATFORM_INFO, undefined, { failureType: PlatformErrors.FailedToParseVersion }); - traceError(`Failed to parse Version ${os.release()}`, ex); return parseVersion(os.release()); } default: diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index e3ae918251a2..0c4fbdb1ecff 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -3,8 +3,7 @@ import { exec, spawn } from 'child_process'; import { Observable } from 'rxjs/Observable'; import * as tk from 'tree-kill'; -import { Disposable } from 'vscode'; -import { LogOptions, traceDecorators } from '../logger'; +import { IDisposable } from '../types'; import { createDeferred } from '../utils/async'; import { EnvironmentVariables } from '../variables/types'; import { DEFAULT_ENCODING } from './constants'; @@ -47,11 +46,11 @@ export class ProcessService implements IProcessService { let procExited = false; const output = new Observable>(subscriber => { - const disposables: Disposable[] = []; + const disposables: IDisposable[] = []; const on = (ee: NodeJS.EventEmitter, name: string, fn: Function) => { ee.on(name, fn as any); - disposables.push({ dispose: () => ee.removeListener(name, fn as any) }); + disposables.push({ dispose: () => ee.removeListener(name, fn as any) as any }); }; if (options.token) { @@ -102,11 +101,11 @@ export class ProcessService implements IProcessService { const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; const proc = spawn(file, args, spawnOptions); const deferred = createDeferred>(); - const disposables: Disposable[] = []; + const disposables: IDisposable[] = []; const on = (ee: NodeJS.EventEmitter, name: string, fn: Function) => { ee.on(name, fn as any); - disposables.push({ dispose: () => ee.removeListener(name, fn as any) }); + disposables.push({ dispose: () => ee.removeListener(name, fn as any) as any}); }; if (options.token) { @@ -149,7 +148,6 @@ export class ProcessService implements IProcessService { return deferred.promise; } - @traceDecorators.verbose('Invokeing shellExec', LogOptions.None) public shellExec(command: string, options: ShellOptions = {}): Promise> { const shellOptions = this.getDefaultOptions(options); return new Promise((resolve, reject) => { diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 04dd9cfabde2..12afc73a8c5a 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -30,7 +30,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { return this.create({ resource }); } const pythonPath = this.configService.getSettings(resource).pythonPath; - const processService = new ProcessService(this.decoder, envVars); + const processService = new ProcessService(this.decoder, { ...envVars }); return new PythonExecutionService(this.serviceContainer, processService, pythonPath); } } diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts new file mode 100644 index 000000000000..a28f26b265b4 --- /dev/null +++ b/src/client/common/utils/cacheUtils.ts @@ -0,0 +1,120 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:no-any no-require-imports + +import { Uri } from 'vscode'; +import '../../common/extensions'; +import { Resource } from '../types'; + +type VSCodeType = typeof import('vscode'); +type CacheData = { + value: unknown; + expiry: number; +}; +// const simpleCache = new Map(); +const resourceSpecificCacheStores = new Map>(); + +/** + * Get a cache key specific to a resource (i.e. workspace) + * This key will be used to cache interpreter related data, hence the Python Path + * used in a workspace will affect the cache key. + * @param {String} keyPrefix + * @param {Resource} resource + * @param {VSCodeType} [vscode=require('vscode')] + * @returns + */ +function getCacheKey(keyPrefix: string, resource: Resource, vscode: VSCodeType = require('vscode')) { + const globalPythonPath = vscode.workspace.getConfiguration('python', null as any).get('pythonPath'); + // get workspace related to this resource + if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { + return `${keyPrefix}-${globalPythonPath}`; + } + const folder = resource ? vscode.workspace.getWorkspaceFolder(resource) : vscode.workspace.workspaceFolders[0]; + if (!folder) { + return `${keyPrefix}-${globalPythonPath}`; + } + const workspacePythonPath = vscode.workspace.getConfiguration('python', resource).get('pythonPath'); + return `${keyPrefix}-${folder.uri.fsPath}-${workspacePythonPath}`; +} +/** + * Gets the cache store for a resource that's specific to the interpreter as well. + * @param {string} keyPrefix + * @param {Resource} resource + * @param {VSCodeType} [vscode=require('vscode')] + * @returns + */ +function getCacheStore(keyPrefix: string, resource: Resource, vscode: VSCodeType = require('vscode')) { + const key = getCacheKey(keyPrefix, resource, vscode); + if (!resourceSpecificCacheStores.has(key)) { + resourceSpecificCacheStores.set(key, new Map()); + } + return resourceSpecificCacheStores.get(key)!; +} + +function getCacheKeyFromFunctionArgs(fnArgs: any[]): string { + return fnArgs.map(arg => `${arg}`).join('-Arg-Separator-'); +} + +export function clearCache() { + resourceSpecificCacheStores.clear(); +} + +export class InMemoryInterpreterSpecificCache { + private readonly resource: Resource; + private readonly args: any[]; + constructor(private readonly keyPrefix: string, + private readonly expiryDurationMs: number, + args: [Uri | undefined, ...any[]], + private readonly vscode: VSCodeType = require('vscode')) { + this.resource = args[0]; + this.args = args.splice(1); + } + public get hasData() { + const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.args); + const data = store.get(key); + if (!store.has(key) || !data) { + return false; + } + if (data.expiry < Date.now()) { + store.delete(key); + return false; + } + return true; + } + /** + * Returns undefined if there is no data. + * Use of `hasData` to determine whether cached data exists. + * + * @type {(T | undefined)} + * @memberof InMemoryInterpreterSpecificCache + */ + public get data(): T | undefined { + if (!this.hasData) { + return; + } + const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.args); + const data = store.get(key); + if (!store.has(key) || !data) { + return; + } + return data.value as T; + } + public set data(value: T | undefined) { + const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.args); + store.set(key, { + expiry: Date.now() + this.expiryDurationMs, + value + }); + } + public clear() { + const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.args); + store.delete(key); + } +} diff --git a/src/client/common/utils/decorators.ts b/src/client/common/utils/decorators.ts index 4a66db27cafe..f53819e72267 100644 --- a/src/client/common/utils/decorators.ts +++ b/src/client/common/utils/decorators.ts @@ -1,9 +1,11 @@ -// tslint:disable:no-any +// tslint:disable:no-any no-require-imports no-function-expression no-invalid-this import { ProgressLocation, ProgressOptions, Uri, window } from 'vscode'; import '../../common/extensions'; import { isTestExecution } from '../constants'; import { traceError, traceVerbose } from '../logger'; +import { Resource } from '../types'; +import { InMemoryInterpreterSpecificCache } from './cacheUtils'; // tslint:disable-next-line:no-require-imports no-var-requires const _debounce = require('lodash/debounce') as typeof import('lodash/debounce'); @@ -25,66 +27,24 @@ export function debounce(wait?: number) { }; } -type vsCodeType = typeof import('vscode'); -type cacheData = { - value: any; - expiry: number; -}; -const simpleCache = new Map(); - -function getCachedData(key: string): undefined | cacheData { - const data = simpleCache.get(key); - if (!data) { - return; - } - return data.expiry < Date.now() ? undefined : data; -} -function storeCachedData(key: string, data: any, expiryDurationMs: number): void { - simpleCache.set(key, { - value: data, - expiry: Date.now() + expiryDurationMs - }); -} -// tslint:disable-next-line:no-any no-require-imports -export function getCacheKeyFromFunctionArgs(fnArgs: any[], vscode: vsCodeType = require('vscode')): string { - const keys: string[] = []; - fnArgs.forEach((arg, index) => { - if (index > 0) { - return keys.push(`${arg}`); - } - // get workspace related to this resource - if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { - return keys.push('undefined'); - } - const folder = vscode.workspace.getWorkspaceFolder(arg as Uri); - if (!folder) { - return keys.push('undefined'); - } - const pythonPath = vscode.workspace.getConfiguration('python', arg as Uri).get('pythonPath'); - keys.push(`${folder.uri.fsPath}-${pythonPath}`); - }); - - return keys.join('-Arg-Separator-'); -} - -// tslint:disable-next-line:no-any +type VSCodeType = typeof import('vscode'); type PromiseFunctionWithFirstArgOfResource = (...any: [Uri | undefined, ...any[]]) => Promise; -// tslint:disable-next-line:no-require-imports -export function cacheResourceSpecificIngterpreterData(expiryDurationMs: number, vscode: vsCodeType = require('vscode')) { +export function clearCachedResourceSpecificIngterpreterData(key: string, resource: Resource, vscode: VSCodeType = require('vscode')) { + const cache = new InMemoryInterpreterSpecificCache(key, 0, [resource], vscode); + cache.clear(); +} +export function cacheResourceSpecificIngterpreterData(key: string, expiryDurationMs: number, vscode: VSCodeType = require('vscode')) { return function (_target: Object, _propertyName: string, descriptor: TypedPropertyDescriptor) { const originalMethod = descriptor.value!; - // tslint:disable-next-line:no-any no-function-expression descriptor.value = async function (...args: [Uri | undefined, ...any[]]) { - const cacheKey = getCacheKeyFromFunctionArgs(args, vscode); - const data = getCachedData(cacheKey); - if (data) { - traceVerbose(`Cached data exists ${cacheKey}`); - return Promise.resolve(data.value); + const cache = new InMemoryInterpreterSpecificCache(key, expiryDurationMs, args, vscode); + if (cache.hasData) { + traceVerbose(`Cached data exists ${key}, ${args[0] ? args[0].fsPath : ''}`); + return Promise.resolve(cache.data); } - // tslint:disable-next-line:no-invalid-this const promise = originalMethod.apply(this, args) as Promise; - promise.then(result => storeCachedData(cacheKey, result, expiryDurationMs)).ignoreErrors(); + promise.then(result => cache.data = result).ignoreErrors(); return promise; }; }; diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index 046927903e32..e5a3fb1c2ad3 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -5,11 +5,12 @@ import { inject, injectable } from 'inversify'; import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode'; import { IPlatformService } from '../platform/types'; import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types'; +import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData } from '../utils/decorators'; import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; +const cacheDuration = 60 * 60 * 1000; @injectable() export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvider, Disposable { - private cache = new Map(); private fileWatchers = new Map(); private disposables: Disposable[] = []; private changeEventEmitter: EventEmitter; @@ -32,27 +33,25 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid watcher.dispose(); }); } + @cacheResourceSpecificIngterpreterData('getEnvironmentVariables', cacheDuration) public async getEnvironmentVariables(resource?: Uri): Promise { const settings = this.configurationService.getSettings(resource); - if (!this.cache.has(settings.envFile)) { - const workspaceFolderUri = this.getWorkspaceFolderUri(resource); - this.createFileWatcher(settings.envFile, workspaceFolderUri); - let mergedVars = await this.envVarsService.parseFile(settings.envFile); - if (!mergedVars) { - mergedVars = {}; - } - this.envVarsService.mergeVariables(this.process.env, mergedVars!); - const pathVariable = this.platformService.pathVariableName; - const pathValue = this.process.env[pathVariable]; - if (pathValue) { - this.envVarsService.appendPath(mergedVars!, pathValue); - } - if (this.process.env.PYTHONPATH) { - this.envVarsService.appendPythonPath(mergedVars!, this.process.env.PYTHONPATH); - } - this.cache.set(settings.envFile, mergedVars); + const workspaceFolderUri = this.getWorkspaceFolderUri(resource); + this.createFileWatcher(settings.envFile, workspaceFolderUri); + let mergedVars = await this.envVarsService.parseFile(settings.envFile); + if (!mergedVars) { + mergedVars = {}; } - return this.cache.get(settings.envFile)!; + this.envVarsService.mergeVariables(this.process.env, mergedVars!); + const pathVariable = this.platformService.pathVariableName; + const pathValue = this.process.env[pathVariable]; + if (pathValue) { + this.envVarsService.appendPath(mergedVars!, pathValue); + } + if (this.process.env.PYTHONPATH) { + this.envVarsService.appendPythonPath(mergedVars!, this.process.env.PYTHONPATH); + } + return mergedVars; } private getWorkspaceFolderUri(resource?: Uri): Uri | undefined { if (!resource) { @@ -74,7 +73,8 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid } } private onEnvironmentFileChanged(envFile, workspaceFolderUri?: Uri) { - this.cache.delete(envFile); + clearCachedResourceSpecificIngterpreterData('getEnvironmentVariables', workspaceFolderUri); + clearCachedResourceSpecificIngterpreterData('CustomEnvironmentVariables', workspaceFolderUri); this.changeEventEmitter.fire(workspaceFolderUri); } } diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 610eaab14359..7b673a9be9ef 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -8,8 +8,8 @@ import { LogOptions, traceDecorators, traceVerbose } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; import { IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper } from '../../common/terminal/types'; -import { ICurrentProcess, Resource } from '../../common/types'; -import { cacheResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; +import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; +import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { IEnvironmentActivationService } from './types'; @@ -20,16 +20,23 @@ const environmentSplitRegex = /^\s*([^=]+)\s*=\s*(.+)\s*$/; const cacheDuration = 60 * 60 * 1000; @injectable() -export class EnvironmentActivationService implements IEnvironmentActivationService { +export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable { + private readonly disposables: IDisposable[] = []; constructor(@inject(ITerminalHelper) private readonly helper: ITerminalHelper, @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IProcessServiceFactory) private processServiceFactory: IProcessServiceFactory, @inject(ICurrentProcess) private currentProcess: ICurrentProcess, @inject(IEnvironmentVariablesProvider) private readonly envVarsService: IEnvironmentVariablesProvider) { + + this.envVarsService.onDidEnvironmentVariablesChange(this.onDidEnvironmentVariablesChange, this, this.disposables); + } + + public dispose(): void | Promise { + this.disposables.forEach(d => d.dispose()); } @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments) @swallowExceptions('getActivatedEnvironmentVariables') - @cacheResourceSpecificIngterpreterData(cacheDuration) + @cacheResourceSpecificIngterpreterData('ActivatedEnvironmentVariables', cacheDuration) public async getActivatedEnvironmentVariables(resource: Resource): Promise { const activationCommands = await this.helper.getEnvironmentActivationShellCommands(resource); traceVerbose(`Activation Commands received ${activationCommands}`); @@ -41,6 +48,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const listEnv = this.platform.isWindows ? 'set' : 'printenv'; const activationCommand = this.fixActivationCommands(activationCommands).join(' && '); const processService = await this.processServiceFactory.create(resource); + const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource); const hasCustomEnvVars = Object.keys(customEnvVars).length; const env = hasCustomEnvVars ? this.currentProcess.env : customEnvVars; @@ -55,6 +63,9 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } return this.parseEnvironmentOutput(result.stdout); } + protected onDidEnvironmentVariablesChange(affectedResource: Resource) { + clearCachedResourceSpecificIngterpreterData('ActivatedEnvironmentVariables', affectedResource); + } protected fixActivationCommands(commands: string[]): string[] { // Replace 'source ' with '. ' as that works in shell exec return commands.map(cmd => cmd.replace(/^source\s+/, '. ')); diff --git a/src/client/interpreter/locators/progressService.ts b/src/client/interpreter/locators/progressService.ts index 4f37a56a7e73..20aaa74cc537 100644 --- a/src/client/interpreter/locators/progressService.ts +++ b/src/client/interpreter/locators/progressService.ts @@ -49,6 +49,9 @@ export class InterpreterLocatorProgressService implements IInterpreterLocatorPro this.refreshing.fire(); } private checkProgress() { + if (this.deferreds.length === 0) { + return; + } if (this.areAllItemsComplete()) { return this.notifyCompleted(); } diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 872b16637941..ea5c004eb9fd 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -50,7 +50,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ const promise = this.getInterpretersImplementation(resource) .then(async items => { await this.cacheInterpreters(items, resource); - traceVerbose(`Interpreters returned by ${this.name} are ${JSON.stringify(items)}`); + traceVerbose(`Interpreters returned by ${this.name} are of count ${items.length}`); deferred!.resolve(items); }) .catch(ex => deferred!.reject(ex)); diff --git a/src/client/unittests/common/runner.ts b/src/client/unittests/common/runner.ts index f38f69dc457b..445370dc140a 100644 --- a/src/client/unittests/common/runner.ts +++ b/src/client/unittests/common/runner.ts @@ -102,7 +102,7 @@ function getExecutablePath(testProvider: TestProvider, settings: IPythonSettings function getTestModuleName(testProvider: TestProvider) { switch (testProvider) { case NOSETEST_PROVIDER: { - return 'nose'; + return 'nose2'; } case PYTEST_PROVIDER: { return 'pytest'; diff --git a/src/client/unittests/configuration.ts b/src/client/unittests/configuration.ts index 3772a90241a3..afdc56bdf898 100644 --- a/src/client/unittests/configuration.ts +++ b/src/client/unittests/configuration.ts @@ -54,7 +54,7 @@ export class UnitTestConfigurationService implements IUnitTestConfigurationServi detail: 'http://docs.pytest.org/' }, { - label: 'nose', + label: 'nose2', product: Product.nosetest, description: 'nose framework', detail: 'https://nose.readthedocs.io/' diff --git a/src/client/unittests/nosetest/runner.ts b/src/client/unittests/nosetest/runner.ts index 692e038aca8b..0faef98909fc 100644 --- a/src/client/unittests/nosetest/runner.ts +++ b/src/client/unittests/nosetest/runner.ts @@ -61,7 +61,7 @@ export class TestManagerRunner implements ITestManagerRunner { if (options.debug === true) { const debugLauncher = this.serviceContainer.get(ITestDebugLauncher); - const debuggerArgs = [options.cwd, 'nose'].concat(testArgs); + const debuggerArgs = [options.cwd, 'nose2'].concat(testArgs); const launchOptions: LaunchOptions = { cwd: options.cwd, args: debuggerArgs, token: options.token, outChannel: options.outChannel, testProvider: NOSETEST_PROVIDER }; await debugLauncher.launchDebugger(launchOptions); } else { diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index 65080d3ccdaa..f19b9e0222f8 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -42,7 +42,9 @@ suite('Refactor Rename', () => { serviceContainer = typeMoq.Mock.ofType(); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IConfigurationService), typeMoq.It.isAny())).returns(() => configService.object); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); - serviceContainer.setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())).returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any)); + serviceContainer + .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) + .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any)); await initializeTest(); }); teardown(closeActiveWindows); diff --git a/src/test/unittests/argsService.test.ts b/src/test/unittests/argsService.test.ts index 3be5899ce634..f08a206fe29a 100644 --- a/src/test/unittests/argsService.test.ts +++ b/src/test/unittests/argsService.test.ts @@ -55,7 +55,7 @@ suite('ArgsService: Common', () => { } case Product.nosetest: { argumentsService = new NoseTestArgumentsService(serviceContainer.object); - moduleName = 'nose'; + moduleName = 'noses'; break; } case Product.pytest: { From f5e2b978eb8370dc23fc93f94ca5714d699f094f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 09:41:46 -0800 Subject: [PATCH 08/49] Fixes --- src/client/common/installer/productInstaller.ts | 4 +++- src/client/unittests/common/runner.ts | 2 +- src/client/unittests/configuration.ts | 2 +- src/client/unittests/nosetest/runner.ts | 2 +- src/test/unittests/argsService.test.ts | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index a0ed70fd058f..8514e978c059 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -317,7 +317,9 @@ export class ProductInstaller implements IInstaller { function translateProductToModule(product: Product, purpose: ModuleNamePurpose): string { switch (product) { case Product.mypy: return 'mypy'; - case Product.nosetest: return 'nose2'; + case Product.nosetest: { + return purpose === ModuleNamePurpose.install ? 'nose' : 'nosetests'; + } case Product.pylama: return 'pylama'; case Product.prospector: return 'prospector'; case Product.pylint: return 'pylint'; diff --git a/src/client/unittests/common/runner.ts b/src/client/unittests/common/runner.ts index 445370dc140a..f38f69dc457b 100644 --- a/src/client/unittests/common/runner.ts +++ b/src/client/unittests/common/runner.ts @@ -102,7 +102,7 @@ function getExecutablePath(testProvider: TestProvider, settings: IPythonSettings function getTestModuleName(testProvider: TestProvider) { switch (testProvider) { case NOSETEST_PROVIDER: { - return 'nose2'; + return 'nose'; } case PYTEST_PROVIDER: { return 'pytest'; diff --git a/src/client/unittests/configuration.ts b/src/client/unittests/configuration.ts index afdc56bdf898..3772a90241a3 100644 --- a/src/client/unittests/configuration.ts +++ b/src/client/unittests/configuration.ts @@ -54,7 +54,7 @@ export class UnitTestConfigurationService implements IUnitTestConfigurationServi detail: 'http://docs.pytest.org/' }, { - label: 'nose2', + label: 'nose', product: Product.nosetest, description: 'nose framework', detail: 'https://nose.readthedocs.io/' diff --git a/src/client/unittests/nosetest/runner.ts b/src/client/unittests/nosetest/runner.ts index 0faef98909fc..692e038aca8b 100644 --- a/src/client/unittests/nosetest/runner.ts +++ b/src/client/unittests/nosetest/runner.ts @@ -61,7 +61,7 @@ export class TestManagerRunner implements ITestManagerRunner { if (options.debug === true) { const debugLauncher = this.serviceContainer.get(ITestDebugLauncher); - const debuggerArgs = [options.cwd, 'nose2'].concat(testArgs); + const debuggerArgs = [options.cwd, 'nose'].concat(testArgs); const launchOptions: LaunchOptions = { cwd: options.cwd, args: debuggerArgs, token: options.token, outChannel: options.outChannel, testProvider: NOSETEST_PROVIDER }; await debugLauncher.launchDebugger(launchOptions); } else { diff --git a/src/test/unittests/argsService.test.ts b/src/test/unittests/argsService.test.ts index f08a206fe29a..3be5899ce634 100644 --- a/src/test/unittests/argsService.test.ts +++ b/src/test/unittests/argsService.test.ts @@ -55,7 +55,7 @@ suite('ArgsService: Common', () => { } case Product.nosetest: { argumentsService = new NoseTestArgumentsService(serviceContainer.object); - moduleName = 'noses'; + moduleName = 'nose'; break; } case Product.pytest: { From 85f77d939691108bb4ab0ce98361108805b71b75 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 12:50:12 -0800 Subject: [PATCH 09/49] Fixed tests --- src/client/common/process/proc.ts | 1 + .../common/process/pythonExecutionFactory.ts | 6 +- .../pipEnvActivationProvider.ts | 5 +- src/client/common/terminal/helper.ts | 49 +- .../pythonExecutionFactory.unit.test.ts | 108 +++++ .../pipEnvActivationProvider.unit.test.ts | 60 +++ .../terminals/helper.activation.unit.test.ts | 181 ------- src/test/common/terminals/helper.unit.test.ts | 457 +++++++++++++----- 8 files changed, 544 insertions(+), 323 deletions(-) create mode 100644 src/test/common/process/pythonExecutionFactory.unit.test.ts create mode 100644 src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts delete mode 100644 src/test/common/terminals/helper.activation.unit.test.ts diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 0c4fbdb1ecff..e613ba672758 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -148,6 +148,7 @@ export class ProcessService implements IProcessService { return deferred.promise; } + public shellExec(command: string, options: ShellOptions = {}): Promise> { const shellOptions = this.getDefaultOptions(options); return new Promise((resolve, reject) => { diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 12afc73a8c5a..e2972bb649e8 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -11,13 +11,11 @@ import { ExecutionFactoryCreationOptions, IBufferDecoder, IProcessServiceFactory @injectable() export class PythonExecutionFactory implements IPythonExecutionFactory { - private readonly configService: IConfigurationService; - private processServiceFactory: IProcessServiceFactory; constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService, + @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, + @inject(IConfigurationService) private readonly configService: IConfigurationService, @inject(IBufferDecoder) private readonly decoder: IBufferDecoder) { - this.processServiceFactory = serviceContainer.get(IProcessServiceFactory); - this.configService = serviceContainer.get(IConfigurationService); } public async create(options: ExecutionFactoryCreationOptions): Promise { const pythonPath = options.pythonPath ? options.pythonPath : this.configService.getSettings(options.resource).pythonPath; diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts index 9f66d676d84c..0ff4f288ff07 100644 --- a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -6,19 +6,18 @@ import { inject, injectable } from 'inversify'; import { Uri } from 'vscode'; import { IInterpreterService, InterpreterType } from '../../../interpreter/contracts'; -import { IServiceContainer } from '../../../ioc/types'; import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; @injectable() export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider { - constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { } + constructor(@inject(IInterpreterService) private readonly interpreterService: IInterpreterService) { } public isShellSupported(_targetShell: TerminalShellType): boolean { return true; } public async getActivationCommands(resource: Uri | undefined, _: TerminalShellType): Promise { - const interpreter = await this.serviceContainer.get(IInterpreterService).getActiveInterpreter(resource); + const interpreter = await this.interpreterService.getActiveInterpreter(resource); if (!interpreter || interpreter.type !== InterpreterType.PipEnv) { return; } diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index acb23c31570b..774092bd8c6c 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -4,7 +4,6 @@ import { inject, injectable, named } from 'inversify'; import { Terminal, Uri } from 'vscode'; import { ICondaService } from '../../interpreter/contracts'; -import { IServiceContainer } from '../../ioc/types'; import { ITerminalManager, IWorkspaceService } from '../application/types'; import '../extensions'; import { IPlatformService } from '../platform/types'; @@ -36,8 +35,12 @@ const defaultOSShells = { @injectable() export class TerminalHelper implements ITerminalHelper { private readonly detectableShells: Map; - constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IPlatformService) private readonly platform: IPlatformService, + constructor(@inject(IPlatformService) private readonly platform: IPlatformService, + @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, + @inject(IPlatformService) private readonly platformService: IPlatformService, + @inject(ICondaService) private readonly condaService: ICondaService, + @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.conda) private readonly conda: ITerminalActivationCommandProvider, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.bashCShellFish) private readonly bashCShellFish: ITerminalActivationCommandProvider, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.commandPromptAndPowerShell) private readonly commandPromptAndPowerShell: ITerminalActivationCommandProvider, @@ -59,8 +62,7 @@ export class TerminalHelper implements ITerminalHelper { this.detectableShells.set(TerminalShellType.xonsh, IS_XONSH); } public createTerminal(title?: string): Terminal { - const terminalManager = this.serviceContainer.get(ITerminalManager); - return terminalManager.createTerminal({ name: title }); + return this.terminalManager.createTerminal({ name: title }); } public identifyTerminalShell(shellPath: string): TerminalShellType { return Array.from(this.detectableShells.keys()) @@ -72,19 +74,24 @@ export class TerminalHelper implements ITerminalHelper { }, TerminalShellType.other); } public getTerminalShellPath(): string { - const workspace = this.serviceContainer.get(IWorkspaceService); - const shellConfig = workspace.getConfiguration('terminal.integrated.shell'); - const platformService = this.serviceContainer.get(IPlatformService); + const shellConfig = this.workspace.getConfiguration('terminal.integrated.shell'); let osSection = ''; - if (platformService.isWindows) { - osSection = 'windows'; - } else if (platformService.isMac) { - osSection = 'osx'; - } else if (platformService.isLinux) { - osSection = 'linux'; - } - if (osSection.length === 0) { - return ''; + switch (this.platformService.osType) { + case OSType.Windows: { + osSection = 'windows'; + break; + } + case OSType.OSX: { + osSection = 'osx'; + break; + } + case OSType.Linux: { + osSection = 'linux'; + break; + } + default: { + return ''; + } } return shellConfig.get(osSection)!; } @@ -103,17 +110,17 @@ export class TerminalHelper implements ITerminalHelper { return; } const providers = [this.bashCShellFish, this.commandPromptAndPowerShell]; - return this.getActivationCommands(resource || undefined, shell, providers); + return this.getActivationCommands(resource, shell, providers); } protected async getActivationCommands(resource: Resource, terminalShellType: TerminalShellType, providers: ITerminalActivationCommandProvider[]): Promise { - const settings = this.serviceContainer.get(IConfigurationService).getSettings(resource); + const settings = this.configurationService.getSettings(resource); const activateEnvironment = settings.terminal.activateEnvironment; if (!activateEnvironment) { return; } // If we have a conda environment, then use that. - const isCondaEnvironment = await this.serviceContainer.get(ICondaService).isCondaEnvironment(settings.pythonPath); + const isCondaEnvironment = await this.condaService.isCondaEnvironment(settings.pythonPath); if (isCondaEnvironment) { const activationCommands = await this.conda.getActivationCommands(resource, terminalShellType); if (Array.isArray(activationCommands)) { @@ -126,7 +133,7 @@ export class TerminalHelper implements ITerminalHelper { for (const provider of supportedProviders) { const activationCommands = await provider.getActivationCommands(resource, terminalShellType); - if (Array.isArray(activationCommands)) { + if (Array.isArray(activationCommands) && activationCommands.length > 0) { return activationCommands; } } diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts new file mode 100644 index 000000000000..750c573ccb57 --- /dev/null +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -0,0 +1,108 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; +// tslint:disable:no-any + +import * as assert from 'assert'; +import { expect } from 'chai'; +import { instance, mock, verify, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { PythonSettings } from '../../../client/common/configSettings'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { BufferDecoder } from '../../../client/common/process/decoder'; +import { ProcessService } from '../../../client/common/process/proc'; +import { ProcessServiceFactory } from '../../../client/common/process/processFactory'; +import { PythonExecutionFactory } from '../../../client/common/process/pythonExecutionFactory'; +import { PythonExecutionService } from '../../../client/common/process/pythonProcess'; +import { ExecutionFactoryCreationOptions, IBufferDecoder, IProcessServiceFactory, IPythonExecutionService } from '../../../client/common/process/types'; +import { IConfigurationService } from '../../../client/common/types'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; +import { ServiceContainer } from '../../../client/ioc/container'; + +suite('Process - PythonExecutionFactory', () => { + [undefined, Uri.parse('x')].forEach(resource => { + suite(resource ? 'With a resource' : 'Without a resource', () => { + let factory: PythonExecutionFactory; + let activationHelper: IEnvironmentActivationService; + let bufferDecoder: IBufferDecoder; + let procecssFactory: IProcessServiceFactory; + let configService: IConfigurationService; + setup(() => { + bufferDecoder = mock(BufferDecoder); + activationHelper = mock(EnvironmentActivationService); + procecssFactory = mock(ProcessServiceFactory); + configService = mock(ConfigurationService); + factory = new PythonExecutionFactory(instance(mock(ServiceContainer)), + instance(activationHelper), instance(procecssFactory), + instance(configService), instance(bufferDecoder)); + }); + + test('Ensure PythonExecutionService is created', async () => { + const pythonSettings = mock(PythonSettings); + when(procecssFactory.create(resource)).thenResolve(instance(mock(ProcessService))); + when(activationHelper.getActivatedEnvironmentVariables(resource)).thenResolve({ x: '1' }); + when(pythonSettings.pythonPath).thenReturn('HELLO'); + when(configService.getSettings(resource)).thenReturn(instance(pythonSettings)); + + const service = await factory.create({ resource }); + + verify(procecssFactory.create(resource)).once(); + verify(pythonSettings.pythonPath).once(); + expect(service).instanceOf(PythonExecutionService); + }); + test('Ensure we use existing create method if there are no environment variables for activated env', async () => { + let createInvoked = false; + const mockExecService = 'something'; + factory.create = async (_options: ExecutionFactoryCreationOptions) => { + createInvoked = true; + return Promise.resolve(mockExecService as any as IPythonExecutionService); + }; + + when(activationHelper.getActivatedEnvironmentVariables(resource)).thenResolve(); + + const service = await factory.createActivatedEnvironment(resource); + + verify(activationHelper.getActivatedEnvironmentVariables(resource)).once(); + assert.deepEqual(service, mockExecService); + assert.equal(createInvoked, true); + }); + test('Ensure we use existing create method if there are no environment variables (0 length) for activated env', async () => { + let createInvoked = false; + const mockExecService = 'something'; + factory.create = async (_options: ExecutionFactoryCreationOptions) => { + createInvoked = true; + return Promise.resolve(mockExecService as any as IPythonExecutionService); + }; + + when(activationHelper.getActivatedEnvironmentVariables(resource)).thenResolve({}); + + const service = await factory.createActivatedEnvironment(resource); + + verify(activationHelper.getActivatedEnvironmentVariables(resource)).once(); + assert.deepEqual(service, mockExecService); + assert.equal(createInvoked, true); + }); + test('PythonExecutionService is created', async () => { + let createInvoked = false; + const mockExecService = 'something'; + factory.create = async (_options: ExecutionFactoryCreationOptions) => { + createInvoked = true; + return Promise.resolve(mockExecService as any as IPythonExecutionService); + }; + + const pythonSettings = mock(PythonSettings); + when(activationHelper.getActivatedEnvironmentVariables(resource)).thenResolve({ x: '1' }); + when(pythonSettings.pythonPath).thenReturn('HELLO'); + when(configService.getSettings(resource)).thenReturn(instance(pythonSettings)); + const service = await factory.createActivatedEnvironment(resource); + + verify(activationHelper.getActivatedEnvironmentVariables(resource)).once(); + verify(pythonSettings.pythonPath).once(); + expect(service).instanceOf(PythonExecutionService); + assert.equal(createInvoked, false); + }); + }); + }); +}); diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts new file mode 100644 index 000000000000..24efe0928d54 --- /dev/null +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as assert from 'assert'; +import { instance, mock, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { PipEnvActivationCommandProvider } from '../../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; +import { ITerminalActivationCommandProvider, TerminalShellType } from '../../../../client/common/terminal/types'; +import { getNamesAndValues } from '../../../../client/common/utils/enum'; +import { IInterpreterService, InterpreterType } from '../../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../../client/interpreter/interpreterService'; + +// tslint:disable:no-any + +suite('Terminals Activation - PipEnv', () => { + [undefined, Uri.parse('x')].forEach(resource => { + suite(resource ? 'With a resource' : 'Without a resource', () => { + let activationProvider: ITerminalActivationCommandProvider; + let interpreterService: IInterpreterService; + setup(() => { + interpreterService = mock(InterpreterService); + activationProvider = new PipEnvActivationCommandProvider(instance(interpreterService)); + }); + + test('No commands for no interpreter', async () => { + when(interpreterService.getActiveInterpreter(resource)).thenResolve(); + + for (const shell of getNamesAndValues(TerminalShellType)) { + const cmd = await activationProvider.getActivationCommands(resource, shell.value); + + assert.equal(cmd, undefined); + } + }); + test('No commands for an interpreter that is not pipEnv', async () => { + const nonPipInterpreterTypes = getNamesAndValues(InterpreterType) + .filter(t => t.value !== InterpreterType.PipEnv); + for (const interpreterType of nonPipInterpreterTypes) { + when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: interpreterType } as any); + + for (const shell of getNamesAndValues(TerminalShellType)) { + const cmd = await activationProvider.getActivationCommands(resource, shell.value); + + assert.equal(cmd, undefined); + } + } + }); + test('pipenv shell is returned for pipenv interpeter', async () => { + when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: InterpreterType.PipEnv } as any); + + for (const shell of getNamesAndValues(TerminalShellType)) { + const cmd = await activationProvider.getActivationCommands(resource, shell.value); + + assert.deepEqual(cmd, ['pipenv shell']); + } + }); + }); + }); +}); diff --git a/src/test/common/terminals/helper.activation.unit.test.ts b/src/test/common/terminals/helper.activation.unit.test.ts deleted file mode 100644 index 74f5861990da..000000000000 --- a/src/test/common/terminals/helper.activation.unit.test.ts +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { expect } from 'chai'; -import { instance, mock } from 'ts-mockito'; -import * as TypeMoq from 'typemoq'; -import { Disposable } from 'vscode'; -import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types'; -import { IPlatformService } from '../../../client/common/platform/types'; -import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; -import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; -import { CondaActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/condaActivationProvider'; -import { PipEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; -import { PyEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pyenvActivationProvider'; -import { TerminalHelper } from '../../../client/common/terminal/helper'; -import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; -import { IConfigurationService, IDisposableRegistry, IPythonSettings, ITerminalSettings } from '../../../client/common/types'; -import { getNamesAndValues } from '../../../client/common/utils/enum'; -import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; -import { IServiceContainer } from '../../../client/ioc/types'; - -// tslint:disable-next-line:max-func-body-length -suite('Terminal Service helpers', () => { - let helper: ITerminalHelper; - let terminalManager: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let disposables: Disposable[] = []; - let serviceContainer: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - let terminalSettings: TypeMoq.IMock; - - setup(() => { - terminalManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - interpreterService = TypeMoq.Mock.ofType(); - terminalSettings = TypeMoq.Mock.ofType(); - disposables = []; - - serviceContainer = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(ITerminalManager)).returns(() => terminalManager.object); - serviceContainer.setup(c => c.get(IPlatformService)).returns(() => platformService.object); - serviceContainer.setup(c => c.get(IDisposableRegistry)).returns(() => disposables); - serviceContainer.setup(c => c.get(IWorkspaceService)).returns(() => workspaceService.object); - serviceContainer.setup(c => c.get(IInterpreterService)).returns(() => interpreterService.object); - - const configService = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(IConfigurationService)).returns(() => configService.object); - const settings = TypeMoq.Mock.ofType(); - configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); - settings.setup(s => s.terminal).returns(() => terminalSettings.object); - - const condaService = TypeMoq.Mock.ofType(); - condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICondaService))).returns(() => condaService.object); - - helper = new TerminalHelper(serviceContainer.object, platformService.object, - instance(mock(CondaActivationCommandProvider)), - instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), - instance(mock(PyEnvActivationCommandProvider)), - instance(mock(PipEnvActivationCommandProvider))); - - }); - teardown(() => { - disposables.filter(item => !!item).forEach(item => item.dispose()); - }); - - test('Activation command is undefined when terminal activation is disabled', async () => { - terminalSettings.setup(t => t.activateEnvironment).returns(() => false); - const commands = await helper.getEnvironmentActivationCommands(TerminalShellType.other); - - expect(commands).to.equal(undefined, 'Activation command should be undefined if terminal type cannot be determined'); - }); - - test('Activation command is undefined for unknown terminal', async () => { - terminalSettings.setup(t => t.activateEnvironment).returns(() => true); - - const bashActivation = new Bash(serviceContainer.object); - const commandPromptActivation = new CommandPromptAndPowerShell(serviceContainer.object); - serviceContainer.setup(c => c.getAll(ITerminalActivationCommandProvider)).returns(() => [bashActivation, commandPromptActivation]); - const commands = await helper.getEnvironmentActivationCommands(TerminalShellType.other); - - expect(commands).to.equal(undefined, 'Activation command should be undefined if terminal type cannot be determined'); - }); -}); - -getNamesAndValues(TerminalShellType).forEach(terminalShell => { - suite(`Terminal Service helpers (${terminalShell.name})`, () => { - let helper: ITerminalHelper; - let terminalManager: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let disposables: Disposable[] = []; - let serviceContainer: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - - setup(() => { - terminalManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - interpreterService = TypeMoq.Mock.ofType(); - disposables = []; - - serviceContainer = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(ITerminalManager)).returns(() => terminalManager.object); - serviceContainer.setup(c => c.get(IPlatformService)).returns(() => platformService.object); - serviceContainer.setup(c => c.get(IDisposableRegistry)).returns(() => disposables); - serviceContainer.setup(c => c.get(IWorkspaceService)).returns(() => workspaceService.object); - serviceContainer.setup(c => c.get(IInterpreterService)).returns(() => interpreterService.object); - - const configService = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(IConfigurationService)).returns(() => configService.object); - const settings = TypeMoq.Mock.ofType(); - configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); - const terminalSettings = TypeMoq.Mock.ofType(); - settings.setup(s => s.terminal).returns(() => terminalSettings.object); - terminalSettings.setup(t => t.activateEnvironment).returns(() => true); - - const condaService = TypeMoq.Mock.ofType(); - condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); - serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICondaService))).returns(() => condaService.object); - - helper = new TerminalHelper(serviceContainer.object, platformService.object, - instance(mock(CondaActivationCommandProvider)), - instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), - instance(mock(PyEnvActivationCommandProvider)), - instance(mock(PipEnvActivationCommandProvider))); - }); - teardown(() => { - disposables.filter(disposable => !!disposable).forEach(disposable => disposable.dispose()); - }); - - async function activationCommandShouldReturnCorrectly(shellType: TerminalShellType, expectedActivationCommand?: string[]) { - // This will only work for the current shell type. - const validProvider = TypeMoq.Mock.ofType(); - validProvider.setup(p => p.isShellSupported(TypeMoq.It.isValue(shellType))).returns(() => true); - validProvider.setup(p => p.getActivationCommands(TypeMoq.It.isValue(undefined), TypeMoq.It.isValue(shellType))).returns(() => Promise.resolve(expectedActivationCommand)); - - // This will support other providers. - const invalidProvider = TypeMoq.Mock.ofType(); - invalidProvider.setup(p => p.isShellSupported(TypeMoq.It.isAny())).returns(item => shellType !== shellType); - - serviceContainer.setup(c => c.getAll(ITerminalActivationCommandProvider)).returns(() => [validProvider.object, invalidProvider.object]); - const commands = await helper.getEnvironmentActivationCommands(shellType); - - validProvider.verify(p => p.getActivationCommands(TypeMoq.It.isValue(undefined), TypeMoq.It.isValue(shellType)), TypeMoq.Times.once()); - validProvider.verify(p => p.isShellSupported(TypeMoq.It.isValue(shellType)), TypeMoq.Times.once()); - invalidProvider.verify(p => p.getActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.never()); - invalidProvider.verify(p => p.isShellSupported(TypeMoq.It.isValue(shellType)), TypeMoq.Times.once()); - - expect(commands).to.deep.equal(expectedActivationCommand, 'Incorrect activation command'); - } - - test(`Activation command should be correctly identified for ${terminalShell.name} (command array)`, async () => { - await activationCommandShouldReturnCorrectly(terminalShell.value, ['a', 'b']); - }); - test(`Activation command should be correctly identified for ${terminalShell.name} (command string)`, async () => { - await activationCommandShouldReturnCorrectly(terminalShell.value, ['command to be executed']); - }); - test(`Activation command should be correctly identified for ${terminalShell.name} (undefined)`, async () => { - await activationCommandShouldReturnCorrectly(terminalShell.value); - }); - - async function activationCommandShouldReturnUndefined(shellType: TerminalShellType) { - // This will support other providers. - const invalidProvider = TypeMoq.Mock.ofType(); - invalidProvider.setup(p => p.isShellSupported(TypeMoq.It.isAny())).returns(item => shellType !== shellType); - - serviceContainer.setup(c => c.getAll(ITerminalActivationCommandProvider)).returns(() => [invalidProvider.object]); - const commands = await helper.getEnvironmentActivationCommands(shellType); - - invalidProvider.verify(p => p.getActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.never()); - expect(commands).to.deep.equal(undefined, 'Incorrect activation command'); - } - - test(`Activation command should return undefined ${terminalShell.name} (no matching providers)`, async () => { - await activationCommandShouldReturnUndefined(terminalShell.value); - }); - }); -}); diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts index c068965c59aa..130dfe672338 100644 --- a/src/test/common/terminals/helper.unit.test.ts +++ b/src/test/common/terminals/helper.unit.test.ts @@ -2,10 +2,15 @@ // Licensed under the MIT License. import { expect } from 'chai'; -import { instance, mock } from 'ts-mockito'; +import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; -import { Disposable, WorkspaceConfiguration } from 'vscode'; +import { Uri, WorkspaceConfiguration } from 'vscode'; +import { TerminalManager } from '../../../client/common/application/terminalManager'; import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types'; +import { WorkspaceService } from '../../../client/common/application/workspace'; +import { PythonSettings } from '../../../client/common/configSettings'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { PlatformService } from '../../../client/common/platform/platformService'; import { IPlatformService } from '../../../client/common/platform/types'; import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; @@ -13,144 +18,368 @@ import { CondaActivationCommandProvider } from '../../../client/common/terminal/ import { PipEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; import { PyEnvActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/pyenvActivationProvider'; import { TerminalHelper } from '../../../client/common/terminal/helper'; -import { ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; -import { IDisposableRegistry } from '../../../client/common/types'; +import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; +import { IConfigurationService } from '../../../client/common/types'; import { getNamesAndValues } from '../../../client/common/utils/enum'; -import { IInterpreterService } from '../../../client/interpreter/contracts'; -import { IServiceContainer } from '../../../client/ioc/types'; +import { OSType } from '../../../client/common/utils/platform'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; + +// tslint:disable:max-func-body-length no-any -// tslint:disable-next-line:max-func-body-length suite('Terminal Service helpers', () => { let helper: ITerminalHelper; - let terminalManager: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let disposables: Disposable[] = []; - let serviceContainer: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - - setup(() => { - terminalManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - interpreterService = TypeMoq.Mock.ofType(); - disposables = []; - - serviceContainer = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.get(ITerminalManager)).returns(() => terminalManager.object); - serviceContainer.setup(c => c.get(IPlatformService)).returns(() => platformService.object); - serviceContainer.setup(c => c.get(IDisposableRegistry)).returns(() => disposables); - serviceContainer.setup(c => c.get(IWorkspaceService)).returns(() => workspaceService.object); - serviceContainer.setup(c => c.get(IInterpreterService)).returns(() => interpreterService.object); - - helper = new TerminalHelper(serviceContainer.object, platformService.object, - instance(mock(CondaActivationCommandProvider)), - instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), - instance(mock(PyEnvActivationCommandProvider)), - instance(mock(PipEnvActivationCommandProvider))); - }); - teardown(() => { - disposables.filter(item => !!item).forEach(item => item.dispose()); - }); + let terminalManager: ITerminalManager; + let platformService: IPlatformService; + let workspaceService: IWorkspaceService; + let condaService: ICondaService; + let configurationService: IConfigurationService; + let condaActivationProvider: ITerminalActivationCommandProvider; + let bashActivationProvider: ITerminalActivationCommandProvider; + let cmdActivationProvider: ITerminalActivationCommandProvider; + let pyenvActivationProvider: ITerminalActivationCommandProvider; + let pipenvActivationProvider: ITerminalActivationCommandProvider; + let pythonSettings: PythonSettings; - test('Test identification of Terminal Shells', async () => { - const shellPathsAndIdentification = new Map(); - shellPathsAndIdentification.set('c:\\windows\\system32\\cmd.exe', TerminalShellType.commandPrompt); + function doSetup() { + terminalManager = mock(TerminalManager); + platformService = mock(PlatformService); + workspaceService = mock(WorkspaceService); + condaService = mock(CondaService); + configurationService = mock(ConfigurationService); + condaActivationProvider = mock(CondaActivationCommandProvider); + bashActivationProvider = mock(Bash); + cmdActivationProvider = mock(CommandPromptAndPowerShell); + pyenvActivationProvider = mock(PyEnvActivationCommandProvider); + pipenvActivationProvider = mock(PipEnvActivationCommandProvider); + pythonSettings = mock(PythonSettings); - shellPathsAndIdentification.set('c:\\windows\\system32\\bash.exe', TerminalShellType.bash); - shellPathsAndIdentification.set('c:\\windows\\system32\\wsl.exe', TerminalShellType.wsl); - shellPathsAndIdentification.set('c:\\windows\\system32\\gitbash.exe', TerminalShellType.gitbash); - shellPathsAndIdentification.set('/usr/bin/bash', TerminalShellType.bash); - shellPathsAndIdentification.set('/usr/bin/zsh', TerminalShellType.zsh); - shellPathsAndIdentification.set('/usr/bin/ksh', TerminalShellType.ksh); + helper = new TerminalHelper(instance(platformService), instance(terminalManager), + instance(workspaceService), instance(platformService), + instance(condaService), instance(configurationService), + instance(condaActivationProvider), + instance(bashActivationProvider), + instance(cmdActivationProvider), + instance(pyenvActivationProvider), + instance(pipenvActivationProvider)); + } + suite('Misc', () => { + setup(doSetup); - shellPathsAndIdentification.set('c:\\windows\\system32\\powershell.exe', TerminalShellType.powershell); - shellPathsAndIdentification.set('c:\\windows\\system32\\pwsh.exe', TerminalShellType.powershellCore); - shellPathsAndIdentification.set('/usr/microsoft/xxx/powershell/powershell', TerminalShellType.powershell); - shellPathsAndIdentification.set('/usr/microsoft/xxx/powershell/pwsh', TerminalShellType.powershellCore); + test('Create terminal without a title', () => { + const terminal = 'Terminal Created'; + when(terminalManager.createTerminal(anything())).thenReturn(terminal as any); - shellPathsAndIdentification.set('/usr/bin/fish', TerminalShellType.fish); + const term = helper.createTerminal(); - shellPathsAndIdentification.set('c:\\windows\\system32\\shell.exe', TerminalShellType.other); - shellPathsAndIdentification.set('/usr/bin/shell', TerminalShellType.other); + verify(terminalManager.createTerminal(anything())).once(); + const args = capture(terminalManager.createTerminal).first()[0]; + expect(term).to.be.deep.equal(terminal); + expect(args.name).to.be.deep.equal(undefined, 'name should be undefined'); + }); + test('Create terminal with a title', () => { + const title = 'Hello'; + const terminal = 'Terminal Created'; + when(terminalManager.createTerminal(anything())).thenReturn(terminal as any); - shellPathsAndIdentification.set('/usr/bin/csh', TerminalShellType.cshell); - shellPathsAndIdentification.set('/usr/bin/tcsh', TerminalShellType.tcshell); + const term = helper.createTerminal(title); - shellPathsAndIdentification.forEach((shellType, shellPath) => { - expect(helper.identifyTerminalShell(shellPath)).to.equal(shellType, `Incorrect Shell Type for path '${shellPath}'`); + verify(terminalManager.createTerminal(anything())).once(); + const args = capture(terminalManager.createTerminal).first()[0]; + expect(term).to.be.deep.equal(terminal); + expect(args.name).to.be.deep.equal(title); }); - }); + test('Test identification of Terminal Shells', async () => { + const shellPathsAndIdentification = new Map(); + shellPathsAndIdentification.set('c:\\windows\\system32\\cmd.exe', TerminalShellType.commandPrompt); - async function ensurePathForShellIsCorrectlyRetrievedFromSettings(os: 'windows' | 'osx' | 'linux', expectedShellPat: string) { - const shellPath = 'abcd'; - workspaceService.setup(w => w.getConfiguration(TypeMoq.It.isValue('terminal.integrated.shell'))).returns(() => { - const workspaceConfig = TypeMoq.Mock.ofType(); - workspaceConfig.setup(c => c.get(os)).returns(() => shellPath); - return workspaceConfig.object; + shellPathsAndIdentification.set('c:\\windows\\system32\\bash.exe', TerminalShellType.bash); + shellPathsAndIdentification.set('c:\\windows\\system32\\wsl.exe', TerminalShellType.wsl); + shellPathsAndIdentification.set('c:\\windows\\system32\\gitbash.exe', TerminalShellType.gitbash); + shellPathsAndIdentification.set('/usr/bin/bash', TerminalShellType.bash); + shellPathsAndIdentification.set('/usr/bin/zsh', TerminalShellType.zsh); + shellPathsAndIdentification.set('/usr/bin/ksh', TerminalShellType.ksh); + + shellPathsAndIdentification.set('c:\\windows\\system32\\powershell.exe', TerminalShellType.powershell); + shellPathsAndIdentification.set('c:\\windows\\system32\\pwsh.exe', TerminalShellType.powershellCore); + shellPathsAndIdentification.set('/usr/microsoft/xxx/powershell/powershell', TerminalShellType.powershell); + shellPathsAndIdentification.set('/usr/microsoft/xxx/powershell/pwsh', TerminalShellType.powershellCore); + + shellPathsAndIdentification.set('/usr/bin/fish', TerminalShellType.fish); + + shellPathsAndIdentification.set('c:\\windows\\system32\\shell.exe', TerminalShellType.other); + shellPathsAndIdentification.set('/usr/bin/shell', TerminalShellType.other); + + shellPathsAndIdentification.set('/usr/bin/csh', TerminalShellType.cshell); + shellPathsAndIdentification.set('/usr/bin/tcsh', TerminalShellType.tcshell); + + shellPathsAndIdentification.set('/usr/bin/xonsh', TerminalShellType.xonsh); + shellPathsAndIdentification.set('/usr/bin/xonshx', TerminalShellType.other); + + shellPathsAndIdentification.forEach((shellType, shellPath) => { + expect(helper.identifyTerminalShell(shellPath)).to.equal(shellType, `Incorrect Shell Type for path '${shellPath}'`); + }); }); + async function ensurePathForShellIsCorrectlyRetrievedFromSettings(osType: OSType, expectedShellPath: string) { + when(platformService.osType).thenReturn(osType); + const cfgSetting = osType === OSType.Windows ? 'windows' : (osType === OSType.OSX ? 'osx' : 'linux'); + const workspaceConfig = TypeMoq.Mock.ofType(); + const invocationCount = osType === OSType.Unknown ? 0 : 1; + workspaceConfig + .setup(w => w.get(TypeMoq.It.isValue(cfgSetting))) + .returns(() => expectedShellPath) + .verifiable(TypeMoq.Times.exactly(invocationCount)); + when(workspaceService.getConfiguration('terminal.integrated.shell')).thenReturn(workspaceConfig.object); - platformService.setup(p => p.isWindows).returns(() => os === 'windows'); - platformService.setup(p => p.isLinux).returns(() => os === 'linux'); - platformService.setup(p => p.isMac).returns(() => os === 'osx'); - expect(helper.getTerminalShellPath()).to.equal(shellPath, 'Incorrect path for Osx'); - } - test('Ensure path for shell is correctly retrieved from settings (osx)', async () => { - await ensurePathForShellIsCorrectlyRetrievedFromSettings('osx', 'abcd'); - }); - test('Ensure path for shell is correctly retrieved from settings (linux)', async () => { - await ensurePathForShellIsCorrectlyRetrievedFromSettings('linux', 'abcd'); - }); - test('Ensure path for shell is correctly retrieved from settings (windows)', async () => { - await ensurePathForShellIsCorrectlyRetrievedFromSettings('windows', 'abcd'); - }); - test('Ensure path for shell is correctly retrieved from settings (unknown os)', async () => { - await ensurePathForShellIsCorrectlyRetrievedFromSettings('windows', ''); - }); + const shellPath = helper.getTerminalShellPath(); - test('Ensure spaces in command is quoted', async () => { - getNamesAndValues(TerminalShellType).forEach(item => { - const command = 'c:\\python 3.7.exe'; - const args = ['1', '2']; - const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; - const expectedTerminalCommand = `${commandPrefix}${command.fileToCommandArgument()} 1 2`; + workspaceConfig.verifyAll(); + expect(shellPath).to.equal(expectedShellPath, 'Incorrect path for Osx'); + } + test('Ensure path for shell is correctly retrieved from settings (osx)', async () => { + await ensurePathForShellIsCorrectlyRetrievedFromSettings(OSType.OSX, 'abcd'); + }); + test('Ensure path for shell is correctly retrieved from settings (linux)', async () => { + await ensurePathForShellIsCorrectlyRetrievedFromSettings(OSType.Linux, 'abcd'); + }); + test('Ensure path for shell is correctly retrieved from settings (windows)', async () => { + await ensurePathForShellIsCorrectlyRetrievedFromSettings(OSType.Windows, 'abcd'); + }); + test('Ensure path for shell is correctly retrieved from settings (unknown os)', async () => { + await ensurePathForShellIsCorrectlyRetrievedFromSettings(OSType.Unknown, ''); + }); + test('Ensure spaces in command is quoted', async () => { + getNamesAndValues(TerminalShellType).forEach(item => { + const command = 'c:\\python 3.7.exe'; + const args = ['1', '2']; + const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; + const expectedTerminalCommand = `${commandPrefix}${command.fileToCommandArgument()} 1 2`; - const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); - expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`); + const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); + expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`); + }); }); - }); - test('Ensure empty args are ignored', async () => { - getNamesAndValues(TerminalShellType).forEach(item => { - const command = 'python3.7.exe'; - const args = []; - const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; - const expectedTerminalCommand = `${commandPrefix}${command}`; + test('Ensure empty args are ignored', async () => { + getNamesAndValues(TerminalShellType).forEach(item => { + const command = 'python3.7.exe'; + const args = []; + const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; + const expectedTerminalCommand = `${commandPrefix}${command}`; - const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); - expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell '${item.name}'`); + const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); + expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell '${item.name}'`); + }); }); - }); - test('Ensure empty args are ignored with s in command', async () => { - getNamesAndValues(TerminalShellType).forEach(item => { - const command = 'c:\\python 3.7.exe'; - const args = []; - const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; - const expectedTerminalCommand = `${commandPrefix}${command.fileToCommandArgument()}`; + test('Ensure empty args are ignored with s in command', async () => { + getNamesAndValues(TerminalShellType).forEach(item => { + const command = 'c:\\python 3.7.exe'; + const args = []; + const commandPrefix = (item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore) ? '& ' : ''; + const expectedTerminalCommand = `${commandPrefix}${command.fileToCommandArgument()}`; - const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); - expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`); + const terminalCommand = helper.buildCommandForTerminal(item.value, command, args); + expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`); + }); }); }); - test('Ensure a terminal is created (without a title)', () => { - helper.createTerminal(); - terminalManager.verify(t => t.createTerminal(TypeMoq.It.isValue({ name: undefined })), TypeMoq.Times.once()); - }); + suite('Activation', () => { + [undefined, Uri.parse('x')].forEach(resource => { + suite(resource ? 'With a resource' : 'Without a resource', () => { + setup(() => { + doSetup(); + when(configurationService.getSettings(resource)).thenReturn(instance(pythonSettings)); + }); + test('Activation command must be empty if activation of terminals is disabled', async () => { + when(pythonSettings.terminal).thenReturn({ activateEnvironment: false } as any); + + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.equal(undefined, 'Command must be undefined'); + verify(pythonSettings.terminal).once(); + }); + function ensureCondaIsSupported(isSupported: boolean, pythonPath: string, condaActivationCommands: string[]) { + when(pythonSettings.pythonPath).thenReturn(pythonPath); + when(pythonSettings.terminal).thenReturn({ activateEnvironment: true } as any); + when(condaService.isCondaEnvironment(pythonPath)).thenResolve(isSupported); + when(condaActivationProvider.getActivationCommands(resource, anything())).thenResolve(condaActivationCommands); + } + test('Activation command must return conda activation command if interpreter is conda', async () => { + const pythonPath = 'some python Path value'; + const condaActivationCommands = ['Hello', '1']; + ensureCondaIsSupported(true, pythonPath, condaActivationCommands); - test('Ensure a terminal is created with the provided title', () => { - helper.createTerminal('1234'); - terminalManager.verify(t => t.createTerminal(TypeMoq.It.isValue({ name: '1234' })), TypeMoq.Times.once()); + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.equal(condaActivationCommands); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(condaActivationProvider.getActivationCommands(resource, anything())).once(); + }); + test('Activation command must return undefined if none of the proivders support the shell', async () => { + const pythonPath = 'some python Path value'; + ensureCondaIsSupported(false, pythonPath, []); + + when(bashActivationProvider.isShellSupported(anything())).thenReturn(false); + when(cmdActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pyenvActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pipenvActivationProvider.isShellSupported(anything())).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationCommands('someShell' as any as TerminalShellType, resource); + + expect(cmd).to.equal(undefined, 'Command must be undefined'); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pyenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pipenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.isShellSupported(anything())).atLeast(1); + }); + test('Activation command must return command from bash if that is supported and others are not', async () => { + const pythonPath = 'some python Path value'; + const expectCommand = ['one', 'two']; + ensureCondaIsSupported(false, pythonPath, []); + + when(bashActivationProvider.getActivationCommands(resource, anything())).thenResolve(expectCommand); + + when(bashActivationProvider.isShellSupported(anything())).thenReturn(true); + when(cmdActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pyenvActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pipenvActivationProvider.isShellSupported(anything())).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.deep.equal(expectCommand); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(anything())).atLeast(1); + verify(bashActivationProvider.getActivationCommands(resource, anything())).once(); + verify(pyenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pipenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.isShellSupported(anything())).atLeast(1); + }); + test('Activation command must return command from bash if that is supported and even if others are supported', async () => { + const pythonPath = 'some python Path value'; + const expectCommand = ['one', 'two']; + ensureCondaIsSupported(false, pythonPath, []); + + when(bashActivationProvider.getActivationCommands(resource, anything())).thenResolve(expectCommand); + when(bashActivationProvider.isShellSupported(anything())).thenReturn(true); + + [pipenvActivationProvider, cmdActivationProvider, pyenvActivationProvider].forEach(provider => { + when(provider.getActivationCommands(resource, anything())).thenResolve(['Something']); + when(provider.isShellSupported(anything())).thenReturn(true); + }); + + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.deep.equal(expectCommand); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(anything())).atLeast(1); + verify(bashActivationProvider.getActivationCommands(resource, anything())).once(); + verify(pyenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pipenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.isShellSupported(anything())).atLeast(1); + }); + test('Activation command must return command from Command Prompt if that is supported and others are not', async () => { + const pythonPath = 'some python Path value'; + const expectCommand = ['one', 'two']; + ensureCondaIsSupported(false, pythonPath, []); + + when(cmdActivationProvider.getActivationCommands(resource, anything())).thenResolve(expectCommand); + + when(bashActivationProvider.isShellSupported(anything())).thenReturn(false); + when(cmdActivationProvider.isShellSupported(anything())).thenReturn(true); + when(pyenvActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pipenvActivationProvider.isShellSupported(anything())).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.deep.equal(expectCommand); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.getActivationCommands(resource, anything())).once(); + verify(pyenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pipenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.isShellSupported(anything())).atLeast(1); + }); + test('Activation command must return command from Command Prompt if that is supported, and so is bash but no commands are returned', async () => { + const pythonPath = 'some python Path value'; + const expectCommand = ['one', 'two']; + ensureCondaIsSupported(false, pythonPath, []); + + when(cmdActivationProvider.getActivationCommands(resource, anything())).thenResolve(expectCommand); + when(bashActivationProvider.getActivationCommands(resource, anything())).thenResolve([]); + + when(bashActivationProvider.isShellSupported(anything())).thenReturn(true); + when(cmdActivationProvider.isShellSupported(anything())).thenReturn(true); + when(pyenvActivationProvider.isShellSupported(anything())).thenReturn(false); + when(pipenvActivationProvider.isShellSupported(anything())).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationCommands(anything(), resource); + + expect(cmd).to.deep.equal(expectCommand); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(anything())).atLeast(1); + verify(bashActivationProvider.getActivationCommands(resource, anything())).once(); + verify(cmdActivationProvider.getActivationCommands(resource, anything())).once(); + verify(pyenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(pipenvActivationProvider.isShellSupported(anything())).atLeast(1); + verify(cmdActivationProvider.isShellSupported(anything())).atLeast(1); + }); + test('Activation command for Shell must be empty for unknown os', async () => { + const pythonPath = 'some python Path value'; + ensureCondaIsSupported(false, pythonPath, []); + + when(platformService.osType).thenReturn(OSType.Unknown); + when(bashActivationProvider.isShellSupported(anything())).thenReturn(false); + when(cmdActivationProvider.isShellSupported(anything())).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationShellCommands(resource); + + expect(cmd).to.equal(undefined, 'Command must be undefined'); + verify(pythonSettings.terminal).never(); + verify(pythonSettings.pythonPath).never(); + verify(condaService.isCondaEnvironment(pythonPath)).never(); + verify(bashActivationProvider.isShellSupported(anything())).never(); + verify(pyenvActivationProvider.isShellSupported(anything())).never(); + verify(pipenvActivationProvider.isShellSupported(anything())).never(); + verify(cmdActivationProvider.isShellSupported(anything())).never(); + }); + [OSType.Linux, OSType.OSX, OSType.Windows].forEach(osType => { + test(`Activation command for Shell must never use pipenv nor pyenv (${osType})`, async () => { + const pythonPath = 'some python Path value'; + const shellToExpect = osType === OSType.Windows ? TerminalShellType.commandPrompt : TerminalShellType.bash; + ensureCondaIsSupported(false, pythonPath, []); + + when(platformService.osType).thenReturn(osType); + when(bashActivationProvider.isShellSupported(shellToExpect)).thenReturn(false); + when(cmdActivationProvider.isShellSupported(shellToExpect)).thenReturn(false); + + const cmd = await helper.getEnvironmentActivationShellCommands(resource); + + expect(cmd).to.equal(undefined, 'Command must be undefined'); + verify(pythonSettings.terminal).once(); + verify(pythonSettings.pythonPath).once(); + verify(condaService.isCondaEnvironment(pythonPath)).once(); + verify(bashActivationProvider.isShellSupported(shellToExpect)).atLeast(1); + verify(pyenvActivationProvider.isShellSupported(anything())).never(); + verify(pipenvActivationProvider.isShellSupported(anything())).never(); + verify(cmdActivationProvider.isShellSupported(shellToExpect)).atLeast(1); + }); + }); + }); + }); }); }); From 817a4df8c037b4a7419d078f9abfeb91620e62ff Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 13:38:10 -0800 Subject: [PATCH 10/49] More tests for caching --- src/client/common/utils/cacheUtils.ts | 39 ++-- src/test/common/utils/cacheUtils.unit.test.ts | 190 ++++++++++++++++++ src/test/common/utils/decorators.unit.test.ts | 98 +++++++++ 3 files changed, 308 insertions(+), 19 deletions(-) create mode 100644 src/test/common/utils/cacheUtils.unit.test.ts create mode 100644 src/test/common/utils/decorators.unit.test.ts diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index a28f26b265b4..4fe28a433b5e 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -26,18 +26,18 @@ const resourceSpecificCacheStores = new Map>(); * @param {VSCodeType} [vscode=require('vscode')] * @returns */ -function getCacheKey(keyPrefix: string, resource: Resource, vscode: VSCodeType = require('vscode')) { - const globalPythonPath = vscode.workspace.getConfiguration('python', null as any).get('pythonPath'); +function getCacheKey(resource: Resource, vscode: VSCodeType = require('vscode')) { + const globalPythonPath = vscode.workspace.getConfiguration('python', null as any).get('pythonPath') || 'python'; // get workspace related to this resource if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { - return `${keyPrefix}-${globalPythonPath}`; + return globalPythonPath; } const folder = resource ? vscode.workspace.getWorkspaceFolder(resource) : vscode.workspace.workspaceFolders[0]; if (!folder) { - return `${keyPrefix}-${globalPythonPath}`; + return globalPythonPath; } - const workspacePythonPath = vscode.workspace.getConfiguration('python', resource).get('pythonPath'); - return `${keyPrefix}-${folder.uri.fsPath}-${workspacePythonPath}`; + const workspacePythonPath = vscode.workspace.getConfiguration('python', resource).get('pythonPath') || 'python'; + return `${folder.uri.fsPath}-${workspacePythonPath}`; } /** * Gets the cache store for a resource that's specific to the interpreter as well. @@ -46,16 +46,17 @@ function getCacheKey(keyPrefix: string, resource: Resource, vscode: VSCodeType = * @param {VSCodeType} [vscode=require('vscode')] * @returns */ -function getCacheStore(keyPrefix: string, resource: Resource, vscode: VSCodeType = require('vscode')) { - const key = getCacheKey(keyPrefix, resource, vscode); +function getCacheStore(resource: Resource, vscode: VSCodeType = require('vscode')) { + const key = getCacheKey(resource, vscode); if (!resourceSpecificCacheStores.has(key)) { resourceSpecificCacheStores.set(key, new Map()); } return resourceSpecificCacheStores.get(key)!; } -function getCacheKeyFromFunctionArgs(fnArgs: any[]): string { - return fnArgs.map(arg => `${arg}`).join('-Arg-Separator-'); +function getCacheKeyFromFunctionArgs(keyPrefix: string, fnArgs: any[]): string { + const argsKey = fnArgs.map(arg => `${arg}`).join('-Arg-Separator-'); + return `KeyPrefix=${keyPrefix}-Args=${argsKey}`; } export function clearCache() { @@ -70,11 +71,11 @@ export class InMemoryInterpreterSpecificCache { args: [Uri | undefined, ...any[]], private readonly vscode: VSCodeType = require('vscode')) { this.resource = args[0]; - this.args = args.splice(1); + this.args = args.slice(1); } public get hasData() { - const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); - const key = getCacheKeyFromFunctionArgs(this.args); + const store = getCacheStore(this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.keyPrefix, this.args); const data = store.get(key); if (!store.has(key) || !data) { return false; @@ -96,8 +97,8 @@ export class InMemoryInterpreterSpecificCache { if (!this.hasData) { return; } - const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); - const key = getCacheKeyFromFunctionArgs(this.args); + const store = getCacheStore(this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.keyPrefix, this.args); const data = store.get(key); if (!store.has(key) || !data) { return; @@ -105,16 +106,16 @@ export class InMemoryInterpreterSpecificCache { return data.value as T; } public set data(value: T | undefined) { - const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); - const key = getCacheKeyFromFunctionArgs(this.args); + const store = getCacheStore(this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.keyPrefix, this.args); store.set(key, { expiry: Date.now() + this.expiryDurationMs, value }); } public clear() { - const store = getCacheStore(this.keyPrefix, this.resource, this.vscode); - const key = getCacheKeyFromFunctionArgs(this.args); + const store = getCacheStore(this.resource, this.vscode); + const key = getCacheKeyFromFunctionArgs(this.keyPrefix, this.args); store.delete(key); } } diff --git a/src/test/common/utils/cacheUtils.unit.test.ts b/src/test/common/utils/cacheUtils.unit.test.ts new file mode 100644 index 000000000000..8d2116abf261 --- /dev/null +++ b/src/test/common/utils/cacheUtils.unit.test.ts @@ -0,0 +1,190 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import { Uri } from 'vscode'; +import { clearCache, InMemoryInterpreterSpecificCache } from '../../../client/common/utils/cacheUtils'; +import { sleep } from '../../core'; + +// tslint:disable:no-any max-func-body-length +suite('Common Utils - CacheUtils', () => { + teardown(() => { + clearCache(); + }); + function createMockVSC(pythonPath: string): typeof import('vscode') { + return { + workspace: { + getConfiguration: () => { + return { + get: () => { + return pythonPath; + } + }; + }, + getWorkspaceFolder: () => { + return; + } + } + } as any; + } + ['hello', undefined, { date: new Date(), hello: 1234 }].forEach(dataToStore => { + test('Data is stored in cache (without workspaces)', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + }); + test('Data is stored in cache must be cleared when clearing globally', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + + clearCache(); + expect(cache.hasData).to.be.equal(false, 'Must not have data'); + expect(cache.data).to.be.deep.equal(undefined, 'Must not have data'); + }); + test('Data is stored in cache must be cleared', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + + cache.clear(); + expect(cache.hasData).to.be.equal(false, 'Must not have data'); + expect(cache.data).to.be.deep.equal(undefined, 'Must not have data'); + }); + test('Data is stored in cache and expired data is not returned', async () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const cache = new InMemoryInterpreterSpecificCache('Something', 100, [resource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + cache.data = dataToStore; + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + + await sleep(10); + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + + await sleep(50); + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + + await sleep(110); + expect(cache.hasData).to.be.equal(false, 'Must not have data'); + expect(cache.data).to.be.deep.equal(undefined, 'Must not have data'); + }); + test('Data is stored in cache (with workspaces)', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + vsc.workspace.workspaceFolders = [{ index: 0, name: '1', uri: Uri.parse('wkfolder') }]; + vsc.workspace.getWorkspaceFolder = () => vsc.workspace.workspaceFolders![0]; + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + }); + test('Data is stored in cache and different resources point to same storage location (without workspaces)', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const anotherResource = Uri.parse('b'); + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + const cache2 = new InMemoryInterpreterSpecificCache('Something', 10000, [anotherResource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + expect(cache2.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache2.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + expect(cache2.data).to.be.deep.equal(dataToStore); + }); + test('Data is stored in cache and different resources point to same storage location (with workspaces)', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const anotherResource = Uri.parse('b'); + vsc.workspace.workspaceFolders = [{ index: 0, name: '1', uri: Uri.parse('wkfolder') }]; + vsc.workspace.getWorkspaceFolder = () => vsc.workspace.workspaceFolders![0]; + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + const cache2 = new InMemoryInterpreterSpecificCache('Something', 10000, [anotherResource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + expect(cache2.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache2.hasData).to.be.equal(true, 'Must have data'); + expect(cache.data).to.be.deep.equal(dataToStore); + expect(cache2.data).to.be.deep.equal(dataToStore); + }); + test('Data is stored in cache and different resources do not point to same storage location (with multiple workspaces)', () => { + const pythonPath = 'Some Python Path'; + const vsc = createMockVSC(pythonPath); + const resource = Uri.parse('a'); + const anotherResource = Uri.parse('b'); + vsc.workspace.workspaceFolders = [ + { index: 0, name: '1', uri: Uri.parse('wkfolder1') }, + { index: 1, name: '2', uri: Uri.parse('wkfolder2') } + ]; + vsc.workspace.getWorkspaceFolder = (res) => { + const index = res.fsPath === resource.fsPath ? 0 : 1; + return vsc.workspace.workspaceFolders![index]; + }; + const cache = new InMemoryInterpreterSpecificCache('Something', 10000, [resource], vsc); + const cache2 = new InMemoryInterpreterSpecificCache('Something', 10000, [anotherResource], vsc); + + expect(cache.hasData).to.be.equal(false, 'Must not have any data'); + expect(cache2.hasData).to.be.equal(false, 'Must not have any data'); + + cache.data = dataToStore; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache2.hasData).to.be.equal(false, 'Must not have any data'); + expect(cache.data).to.be.deep.equal(dataToStore); + expect(cache2.data).to.be.deep.equal(undefined, 'Must not have any data'); + + cache2.data = 'Store some other data'; + + expect(cache.hasData).to.be.equal(true, 'Must have data'); + expect(cache2.hasData).to.be.equal(true, 'Must have'); + expect(cache.data).to.be.deep.equal(dataToStore); + expect(cache2.data).to.be.deep.equal('Store some other data', 'Must have data'); + }); + }); +}); diff --git a/src/test/common/utils/decorators.unit.test.ts b/src/test/common/utils/decorators.unit.test.ts new file mode 100644 index 000000000000..f4821dfe23ec --- /dev/null +++ b/src/test/common/utils/decorators.unit.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import { Uri } from 'vscode'; +import { Resource } from '../../../client/common/types'; +import { clearCache } from '../../../client/common/utils/cacheUtils'; +import { cacheResourceSpecificIngterpreterData } from '../../../client/common/utils/decorators'; +import { sleep } from '../../core'; + +// tslint:disable:no-any max-func-body-length no-unnecessary-class +suite('Common Utils - Decorators', () => { + teardown(() => { + clearCache(); + }); + function createMockVSC(pythonPath: string): typeof import('vscode') { + return { + workspace: { + getConfiguration: () => { + return { + get: () => { + return pythonPath; + } + }; + }, + getWorkspaceFolder: () => { + return; + } + } + } as any; + } + test('Result must be cached when using cache decorator', async () => { + const vsc = createMockVSC(''); + class TestClass { + public invoked = false; + @cacheResourceSpecificIngterpreterData('Something', 100000, vsc) + public async doSomething(_resource: Resource, a: number, b: number): Promise { + this.invoked = true; + return a + b; + } + } + + const cls = new TestClass(); + const uri = Uri.parse('a'); + const uri2 = Uri.parse('b'); + + let result = await cls.doSomething(uri, 1, 2); + expect(result).to.equal(3); + expect(cls.invoked).to.equal(true, 'Must be invoked'); + + cls.invoked = false; + let result2 = await cls.doSomething(uri2, 2, 3); + expect(result2).to.equal(5); + expect(cls.invoked).to.equal(true, 'Must be invoked'); + + cls.invoked = false; + result = await cls.doSomething(uri, 1, 2); + result2 = await cls.doSomething(uri2, 2, 3); + expect(result).to.equal(3); + expect(result2).to.equal(5); + expect(cls.invoked).to.equal(false, 'Must not be invoked'); + }); + test('Cache result must be cleared when cache expires', async () => { + const vsc = createMockVSC(''); + class TestClass { + public invoked = false; + @cacheResourceSpecificIngterpreterData('Something', 100, vsc) + public async doSomething(_resource: Resource, a: number, b: number): Promise { + this.invoked = true; + return a + b; + } + } + + const cls = new TestClass(); + const uri = Uri.parse('a'); + let result = await cls.doSomething(uri, 1, 2); + + expect(result).to.equal(3); + expect(cls.invoked).to.equal(true, 'Must be invoked'); + + cls.invoked = false; + result = await cls.doSomething(uri, 1, 2); + + expect(result).to.equal(3); + expect(cls.invoked).to.equal(false, 'Must not be invoked'); + + await sleep(100); + + cls.invoked = false; + result = await cls.doSomething(uri, 1, 2); + + expect(result).to.equal(3); + expect(cls.invoked).to.equal(true, 'Must be invoked'); + + }); +}); From 917fb3666e4a8e17eb7865e949e8945d225734e4 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:02 -0800 Subject: [PATCH 11/49] Update src/client/common/utils/cacheUtils.ts Co-Authored-By: DonJayamanne --- src/client/common/utils/cacheUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index 4fe28a433b5e..41945daf4d6f 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -40,7 +40,7 @@ function getCacheKey(resource: Resource, vscode: VSCodeType = require('vscode')) return `${folder.uri.fsPath}-${workspacePythonPath}`; } /** - * Gets the cache store for a resource that's specific to the interpreter as well. + * Gets the cache store for a resource that's specific to the interpreter. * @param {string} keyPrefix * @param {Resource} resource * @param {VSCodeType} [vscode=require('vscode')] From 14c8e939200d8f3c5f67ff58fd2568026d80742d Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:10 -0800 Subject: [PATCH 12/49] Update src/client/common/utils/cacheUtils.ts Co-Authored-By: DonJayamanne --- src/client/common/utils/cacheUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index 41945daf4d6f..b9de3be73165 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -28,7 +28,7 @@ const resourceSpecificCacheStores = new Map>(); */ function getCacheKey(resource: Resource, vscode: VSCodeType = require('vscode')) { const globalPythonPath = vscode.workspace.getConfiguration('python', null as any).get('pythonPath') || 'python'; - // get workspace related to this resource + // Get the workspace related to this resource. if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { return globalPythonPath; } From 3f581618cae846181302f5e26a462ff9a1f5f74d Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:19 -0800 Subject: [PATCH 13/49] Update src/test/common/process/pythonExecutionFactory.unit.test.ts Co-Authored-By: DonJayamanne --- src/test/common/process/pythonExecutionFactory.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index 750c573ccb57..314865fb380a 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -52,7 +52,7 @@ suite('Process - PythonExecutionFactory', () => { verify(pythonSettings.pythonPath).once(); expect(service).instanceOf(PythonExecutionService); }); - test('Ensure we use existing create method if there are no environment variables for activated env', async () => { + test('Ensure we use an existing `create` method if there are no environment variables for the activated env', async () => { let createInvoked = false; const mockExecService = 'something'; factory.create = async (_options: ExecutionFactoryCreationOptions) => { From d3dc620f9b3ba32b42fbb56012289af7b0ab47d5 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:27 -0800 Subject: [PATCH 14/49] Update src/test/common/process/pythonExecutionFactory.unit.test.ts Co-Authored-By: DonJayamanne --- src/test/common/process/pythonExecutionFactory.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index 314865fb380a..17664da2cb2d 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -68,7 +68,7 @@ suite('Process - PythonExecutionFactory', () => { assert.deepEqual(service, mockExecService); assert.equal(createInvoked, true); }); - test('Ensure we use existing create method if there are no environment variables (0 length) for activated env', async () => { + test('Ensure we use an existing `create` method if there are no environment variables (0 length) for the activated env', async () => { let createInvoked = false; const mockExecService = 'something'; factory.create = async (_options: ExecutionFactoryCreationOptions) => { From 79316af5cff56bd55159297d0c2558390c1a81da Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:36 -0800 Subject: [PATCH 15/49] Update src/client/common/utils/cacheUtils.ts Co-Authored-By: DonJayamanne --- src/client/common/utils/cacheUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index b9de3be73165..5aa1bb055f2b 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -88,7 +88,7 @@ export class InMemoryInterpreterSpecificCache { } /** * Returns undefined if there is no data. - * Use of `hasData` to determine whether cached data exists. + * Uses `hasData` to determine whether any cached data exists. * * @type {(T | undefined)} * @memberof InMemoryInterpreterSpecificCache From 3b4dbcfdbdcabb383c0402878a6df8b02be0f538 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:46 -0800 Subject: [PATCH 16/49] Update src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts Co-Authored-By: DonJayamanne --- .../pipEnvActivationProvider.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index 24efe0928d54..de09b6fe1922 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -14,7 +14,7 @@ import { InterpreterService } from '../../../../client/interpreter/interpreterSe // tslint:disable:no-any -suite('Terminals Activation - PipEnv', () => { +suite('Terminals Activation - Pipenv', () => { [undefined, Uri.parse('x')].forEach(resource => { suite(resource ? 'With a resource' : 'Without a resource', () => { let activationProvider: ITerminalActivationCommandProvider; From d341e01cf5bb558b0e72843847735e3729ea004e Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 9 Jan 2019 14:46:53 -0800 Subject: [PATCH 17/49] Update src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts Co-Authored-By: DonJayamanne --- .../pipEnvActivationProvider.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index de09b6fe1922..332ba477891f 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -33,7 +33,7 @@ suite('Terminals Activation - Pipenv', () => { assert.equal(cmd, undefined); } }); - test('No commands for an interpreter that is not pipEnv', async () => { + test('No commands for an interpreter that is not Pipenv', async () => { const nonPipInterpreterTypes = getNamesAndValues(InterpreterType) .filter(t => t.value !== InterpreterType.PipEnv); for (const interpreterType of nonPipInterpreterTypes) { From 1777227a9a58cd1c083fb7bc2394ba4f6fd4643a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 15:23:59 -0800 Subject: [PATCH 18/49] Use default shells --- src/client/interpreter/activation/service.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 7b673a9be9ef..2df8291ce71c 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -10,6 +10,7 @@ import { IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper } from '../../common/terminal/types'; import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; +import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { IEnvironmentActivationService } from './types'; @@ -19,6 +20,14 @@ const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; const environmentSplitRegex = /^\s*([^=]+)\s*=\s*(.+)\s*$/; const cacheDuration = 60 * 60 * 1000; +// The shell under which we'll execute activation scripts. +const defaultShells = { + [OSType.Windows]: 'cmd', + [OSType.OSX]: 'bash', + [OSType.Linux]: 'bash', + [OSType.Unknown]: undefined +}; + @injectable() export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable { private readonly disposables: IDisposable[] = []; @@ -38,6 +47,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi @swallowExceptions('getActivatedEnvironmentVariables') @cacheResourceSpecificIngterpreterData('ActivatedEnvironmentVariables', cacheDuration) public async getActivatedEnvironmentVariables(resource: Resource): Promise { + const shell = defaultShells[this.platform.osType]; + if (!shell) { + return; + } + const activationCommands = await this.helper.getEnvironmentActivationShellCommands(resource); traceVerbose(`Activation Commands received ${activationCommands}`); if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) { @@ -57,7 +71,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // In order to make sure we know where the environment output is, // put in a dummy echo we can look for const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && ${listEnv}`; - const result = await processService.shellExec(command, { env }); + const result = await processService.shellExec(command, { env, shell }); if (result.stderr && result.stderr.length > 0) { throw new Error(`StdErr from ShellExec, ${result.stderr}`); } From bd863c6bc04e14e879b4d7c323713a77543b6c32 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 15:30:23 -0800 Subject: [PATCH 19/49] Change time --- src/client/interpreter/activation/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 2df8291ce71c..2aebeae65099 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -18,7 +18,7 @@ const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; // Regex for splitting environment strings const environmentSplitRegex = /^\s*([^=]+)\s*=\s*(.+)\s*$/; -const cacheDuration = 60 * 60 * 1000; +const cacheDuration = 10 * 60 * 1000; // The shell under which we'll execute activation scripts. const defaultShells = { From b5948f2310b27ab0105409021b94edf810db05b2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 15:46:48 -0800 Subject: [PATCH 20/49] Log command --- src/client/interpreter/activation/service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 2aebeae65099..f85af054a4f4 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -71,6 +71,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // In order to make sure we know where the environment output is, // put in a dummy echo we can look for const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && ${listEnv}`; + traceVerbose(`Activating Environment to capture Environment variables, ${command}`); const result = await processService.shellExec(command, { env, shell }); if (result.stderr && result.stderr.length > 0) { throw new Error(`StdErr from ShellExec, ${result.stderr}`); From dc5f730557c3ef39afd6e38f9d3612823bebf7de Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 17:39:40 -0800 Subject: [PATCH 21/49] Rename PipEnv to Pipenv --- .../pipEnvActivationProvider.ts | 2 +- src/client/interpreter/autoSelection/rules/system.ts | 2 +- src/client/interpreter/contracts.ts | 2 +- src/client/interpreter/helpers.ts | 2 +- src/client/interpreter/interpreterService.ts | 2 +- src/client/interpreter/locators/services/pipEnvService.ts | 2 +- src/client/interpreter/virtualEnvs/index.ts | 2 +- .../pipEnvActivationProvider.unit.test.ts | 4 ++-- src/test/interpreters/interpreterService.unit.test.ts | 4 ++-- src/test/interpreters/locators/helpers.unit.test.ts | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts index 0ff4f288ff07..b279c00de109 100644 --- a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -18,7 +18,7 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma public async getActivationCommands(resource: Uri | undefined, _: TerminalShellType): Promise { const interpreter = await this.interpreterService.getActiveInterpreter(resource); - if (!interpreter || interpreter.type !== InterpreterType.PipEnv) { + if (!interpreter || interpreter.type !== InterpreterType.Pipenv) { return; } diff --git a/src/client/interpreter/autoSelection/rules/system.ts b/src/client/interpreter/autoSelection/rules/system.ts index 15bee26fd16d..60c2d5c1b04a 100644 --- a/src/client/interpreter/autoSelection/rules/system.ts +++ b/src/client/interpreter/autoSelection/rules/system.ts @@ -25,7 +25,7 @@ export class SystemWideInterpretersAutoSelectionRule extends BaseRuleService { // Exclude non-local interpreters. const filteredInterpreters = interpreters.filter(int => int.type !== InterpreterType.VirtualEnv && int.type !== InterpreterType.Venv && - int.type !== InterpreterType.PipEnv); + int.type !== InterpreterType.Pipenv); const bestInterpreter = this.helper.getBestInterpreter(filteredInterpreters); return await this.setGlobalInterpreter(bestInterpreter, manager) ? NextAction.exit : NextAction.runNextRule; } diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index 846375450f12..f8db9ad82ba9 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -62,7 +62,7 @@ export enum InterpreterType { Unknown = 'Unknown', Conda = 'Conda', VirtualEnv = 'VirtualEnv', - PipEnv = 'PipEnv', + Pipenv = 'PipEnv', Pyenv = 'Pyenv', Venv = 'Venv' } diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index 9607cc499901..eeff9b796eea 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -84,7 +84,7 @@ export class InterpreterHelper implements IInterpreterHelper { case InterpreterType.Conda: { return 'conda'; } - case InterpreterType.PipEnv: { + case InterpreterType.Pipenv: { return 'pipenv'; } case InterpreterType.Pyenv: { diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 95d0fb57654d..644616d90143 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -170,7 +170,7 @@ export class InterpreterService implements Disposable, IInterpreterService { if (info.architecture) { displayNameParts.push(getArchitectureDisplayName(info.architecture)); } - if (!info.envName && info.path && info.type && info.type === InterpreterType.PipEnv) { + 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). diff --git a/src/client/interpreter/locators/services/pipEnvService.ts b/src/client/interpreter/locators/services/pipEnvService.ts index ffd55ff4f1b7..fde8f6f69746 100644 --- a/src/client/interpreter/locators/services/pipEnvService.ts +++ b/src/client/interpreter/locators/services/pipEnvService.ts @@ -67,7 +67,7 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer return { ...(details as PythonInterpreter), path: interpreterPath, - type: InterpreterType.PipEnv + type: InterpreterType.Pipenv }; } diff --git a/src/client/interpreter/virtualEnvs/index.ts b/src/client/interpreter/virtualEnvs/index.ts index 2c9224aeaeac..fd707fade848 100644 --- a/src/client/interpreter/virtualEnvs/index.ts +++ b/src/client/interpreter/virtualEnvs/index.ts @@ -52,7 +52,7 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager { } if (await this.isPipEnvironment(pythonPath, resource)) { - return InterpreterType.PipEnv; + return InterpreterType.Pipenv; } if (await this.isVirtualEnvironment(pythonPath)) { diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index 332ba477891f..de6ceba62116 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -35,7 +35,7 @@ suite('Terminals Activation - Pipenv', () => { }); test('No commands for an interpreter that is not Pipenv', async () => { const nonPipInterpreterTypes = getNamesAndValues(InterpreterType) - .filter(t => t.value !== InterpreterType.PipEnv); + .filter(t => t.value !== InterpreterType.Pipenv); for (const interpreterType of nonPipInterpreterTypes) { when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: interpreterType } as any); @@ -47,7 +47,7 @@ suite('Terminals Activation - Pipenv', () => { } }); test('pipenv shell is returned for pipenv interpeter', async () => { - when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: InterpreterType.PipEnv } as any); + when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: InterpreterType.Pipenv } as any); for (const shell of getNamesAndValues(TerminalShellType)) { const cmd = await activationProvider.getActivationCommands(resource, shell.value); diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 21bf8cfbcc68..6b09b7970164 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -311,7 +311,7 @@ suite('Interpreters service', () => { path: pythonPath }; - if (interpreterInfo.path && interpreterType && interpreterType.value === InterpreterType.PipEnv) { + 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)); @@ -339,7 +339,7 @@ suite('Interpreters service', () => { if (interpreterInfo.architecture) { displayNameParts.push(getArchitectureDisplayName(interpreterInfo.architecture)); } - if (!interpreterInfo.envName && interpreterInfo.path && interpreterInfo.type && interpreterInfo.type === InterpreterType.PipEnv && pipEnvName) { + 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). diff --git a/src/test/interpreters/locators/helpers.unit.test.ts b/src/test/interpreters/locators/helpers.unit.test.ts index f94d8b93752b..b059f9d15af9 100644 --- a/src/test/interpreters/locators/helpers.unit.test.ts +++ b/src/test/interpreters/locators/helpers.unit.test.ts @@ -165,7 +165,7 @@ suite('Interpreters - Locators Helper', () => { ['3.6', '3.6'].forEach((name, index) => { // Ensure the type in the first item is 'Unknown', // and type in second item is known (e.g. Conda). - const type = index === 0 ? InterpreterType.Unknown : InterpreterType.PipEnv; + const type = index === 0 ? InterpreterType.Unknown : InterpreterType.Pipenv; const interpreter = { architecture: Architecture.Unknown, displayName: name, From 10ec5d4702b99418cde2b7d20886f2ea51f4e79c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 20:27:49 -0800 Subject: [PATCH 22/49] Write tests --- pythonFiles/printEnvVariables.py | 11 + src/client/common/logger.ts | 2 +- src/client/common/terminal/helper.ts | 3 +- src/client/interpreter/activation/service.ts | 34 +-- .../terminals/activation.conda.unit.test.ts | 24 +- .../activation/service.unit.test.ts | 215 ++++++++++++++++++ src/test/vscode-mock.ts | 2 +- 7 files changed, 257 insertions(+), 34 deletions(-) create mode 100644 pythonFiles/printEnvVariables.py create mode 100644 src/test/interpreters/activation/service.unit.test.ts diff --git a/pythonFiles/printEnvVariables.py b/pythonFiles/printEnvVariables.py new file mode 100644 index 000000000000..fd9af2c5dced --- /dev/null +++ b/pythonFiles/printEnvVariables.py @@ -0,0 +1,11 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import os +import json + +data = {} +for item, value in os.environ.items(): + data[item] = value + +print(json.dumps(data)) diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index 7a34cc59ffc3..3843da84a42c 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -100,7 +100,7 @@ export namespace traceDecorators { export function verbose(message: string, options: LogOptions = LogOptions.Arguments | LogOptions.ReturnValue) { return trace(message, options); } - export function error(message: string, ex?: Error) { + export function error(message: string) { return trace(message, LogOptions.Arguments | LogOptions.ReturnValue, LogLevel.Error); } export function info(message: string) { diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index 774092bd8c6c..6d73ee813295 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -38,7 +38,6 @@ export class TerminalHelper implements ITerminalHelper { constructor(@inject(IPlatformService) private readonly platform: IPlatformService, @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, - @inject(IPlatformService) private readonly platformService: IPlatformService, @inject(ICondaService) private readonly condaService: ICondaService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.conda) private readonly conda: ITerminalActivationCommandProvider, @@ -76,7 +75,7 @@ export class TerminalHelper implements ITerminalHelper { public getTerminalShellPath(): string { const shellConfig = this.workspace.getConfiguration('terminal.integrated.shell'); let osSection = ''; - switch (this.platformService.osType) { + switch (this.platform.osType) { case OSType.Windows: { osSection = 'windows'; break; diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index f85af054a4f4..be89a5091e00 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -4,6 +4,8 @@ 'use strict'; import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import '../../common/extensions'; import { LogOptions, traceDecorators, traceVerbose } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; import { IProcessServiceFactory } from '../../common/process/types'; @@ -12,12 +14,10 @@ import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { EXTENSION_ROOT_DIR } from '../../constants'; import { IEnvironmentActivationService } from './types'; const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; - -// Regex for splitting environment strings -const environmentSplitRegex = /^\s*([^=]+)\s*=\s*(.+)\s*$/; const cacheDuration = 10 * 60 * 1000; // The shell under which we'll execute activation scripts. @@ -59,18 +59,17 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } // Run the activate command collect the environment from it. - const listEnv = this.platform.isWindows ? 'set' : 'printenv'; const activationCommand = this.fixActivationCommands(activationCommands).join(' && '); const processService = await this.processServiceFactory.create(resource); - const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource); const hasCustomEnvVars = Object.keys(customEnvVars).length; - const env = hasCustomEnvVars ? this.currentProcess.env : customEnvVars; + const env = hasCustomEnvVars ? customEnvVars : this.currentProcess.env; traceVerbose(`${hasCustomEnvVars ? 'Has' : 'No'} Custom Env Vars`); // In order to make sure we know where the environment output is, // put in a dummy echo we can look for - const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && ${listEnv}`; + const printEnvPyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariables.py'); + const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && python ${printEnvPyFile.fileToCommandArgument()}`; traceVerbose(`Activating Environment to capture Environment variables, ${command}`); const result = await processService.shellExec(command, { env, shell }); if (result.stderr && result.stderr.length > 0) { @@ -85,24 +84,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // Replace 'source ' with '. ' as that works in shell exec return commands.map(cmd => cmd.replace(/^source\s+/, '. ')); } + @traceDecorators.error('Failed to parse Environment variables') @traceDecorators.verbose('parseEnvironmentOutput', LogOptions.None) protected parseEnvironmentOutput(output: string): NodeJS.ProcessEnv | undefined { - const result = {}; - const lines = output.splitLines({ trim: true, removeEmptyEntries: true }); - let foundDummyOutput = false; - for (let i = 0; i < lines.length; i += 1) { - if (foundDummyOutput) { - // Split on equal - const match = environmentSplitRegex.exec(lines[i]); - if (match && match !== null && match.length > 2) { - result[match[1]] = match[2]; - } - } else { - // See if we found the dummy output or not yet - foundDummyOutput = lines[i].includes(getEnvironmentPrefix); - } - } - - return Object.keys(result).length === 0 ? undefined : result; + output = output.substring(output.indexOf(getEnvironmentPrefix) + getEnvironmentPrefix.length); + const js = output.substring(output.indexOf('{')).trim(); + return JSON.parse(js); } } diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts index c985d9a2d968..3411740707fd 100644 --- a/src/test/common/terminals/activation.conda.unit.test.ts +++ b/src/test/common/terminals/activation.conda.unit.test.ts @@ -6,9 +6,11 @@ import { expect } from 'chai'; import * as path from 'path'; import { parse } from 'semver'; -import { instance, mock } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable } from 'vscode'; +import { TerminalManager } from '../../../client/common/application/terminalManager'; +import { WorkspaceService } from '../../../client/common/application/workspace'; import '../../../client/common/extensions'; import { IFileSystem, IPlatformService @@ -47,6 +49,7 @@ suite('Terminal Environment Activation conda', () => { let procServiceFactory: TypeMoq.IMock; let condaService: TypeMoq.IMock; let conda: string; + let bash: ITerminalActivationCommandProvider; setup(() => { conda = 'conda'; @@ -59,6 +62,7 @@ suite('Terminal Environment Activation conda', () => { processService = TypeMoq.Mock.ofType(); condaService = TypeMoq.Mock.ofType(); condaService.setup(c => c.getCondaFile()).returns(() => Promise.resolve(conda)); + bash = mock(Bash); processService.setup((x: any) => x.then).returns(() => undefined); procServiceFactory = TypeMoq.Mock.ofType(); @@ -77,11 +81,15 @@ suite('Terminal Environment Activation conda', () => { terminalSettings = TypeMoq.Mock.ofType(); pythonSettings.setup(s => s.terminal).returns(() => terminalSettings.object); - terminalHelper = new TerminalHelper(serviceContainer.object, platformService.object, - instance(mock(CondaActivationCommandProvider)), - instance(mock(Bash)), instance(mock(CommandPromptAndPowerShell)), - instance(mock(PyEnvActivationCommandProvider)), - instance(mock(PipEnvActivationCommandProvider))); + terminalHelper = new TerminalHelper(platformService.object, + instance(mock(TerminalManager)), instance(mock(WorkspaceService)), + condaService.object, configService.object, + new CondaActivationCommandProvider(serviceContainer.object), + instance(bash), + mock(CommandPromptAndPowerShell), + mock(PyEnvActivationCommandProvider), + mock(PipEnvActivationCommandProvider)); + }); teardown(() => { disposables.forEach(disposable => { @@ -267,7 +275,11 @@ suite('Terminal Environment Activation conda', () => { mockProvider.setup(p => p.getActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(['mock command'])); const expectedActivationCommand = ['mock command']; + when(bash.isShellSupported(anything())).thenReturn(true); + when(bash.getActivationCommands(anything(), TerminalShellType.bash)).thenResolve(expectedActivationCommand); + const activationCommands = await terminalHelper.getEnvironmentActivationCommands(TerminalShellType.bash, undefined); + expect(activationCommands).to.deep.equal(expectedActivationCommand, 'Incorrect Activation command'); }); async function expectActivationCommandIfCondaDetectionFails(isWindows: boolean, isOsx: boolean, isLinux: boolean, pythonPath: string, condaEnvsPath: string) { diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts new file mode 100644 index 000000000000..9dbdf317270a --- /dev/null +++ b/src/test/interpreters/activation/service.unit.test.ts @@ -0,0 +1,215 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import { EOL } from 'os'; +import * as path from 'path'; +import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; +import * as typemoq from 'typemoq'; +import { Uri, workspace as workspaceType } from 'vscode'; +import { PlatformService } from '../../../client/common/platform/platformService'; +import { IPlatformService } from '../../../client/common/platform/types'; +import { CurrentProcess } from '../../../client/common/process/currentProcess'; +import { ProcessService } from '../../../client/common/process/proc'; +import { ProcessServiceFactory } from '../../../client/common/process/processFactory'; +import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types'; +import { TerminalHelper } from '../../../client/common/terminal/helper'; +import { ITerminalHelper } from '../../../client/common/terminal/types'; +import { ICurrentProcess } from '../../../client/common/types'; +import { clearCache } from '../../../client/common/utils/cacheUtils'; +import { getNamesAndValues } from '../../../client/common/utils/enum'; +import { OSType } from '../../../client/common/utils/platform'; +import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; +import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; +import { EXTENSION_ROOT_DIR } from '../../../client/constants'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { noop } from '../../core'; +import { mockedVSCodeNamespaces } from '../../vscode-mock'; + +const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; +const defaultShells = { + [OSType.Windows]: 'cmd', + [OSType.OSX]: 'bash', + [OSType.Linux]: 'bash', + [OSType.Unknown]: undefined +}; + +// tslint:disable:no-unnecessary-override no-any max-func-body-length +suite('Interprters Activation - Python Environment Variables', () => { + let service: EnvironmentActivationService; + let helper: ITerminalHelper; + let platform: IPlatformService; + let processServiceFactory: IProcessServiceFactory; + let processService: IProcessService; + let currentProcess: ICurrentProcess; + let envVarsService: IEnvironmentVariablesProvider; + let workspace: typemoq.IMock; + function initSetup() { + helper = mock(TerminalHelper); + platform = mock(PlatformService); + processServiceFactory = mock(ProcessServiceFactory); + processService = mock(ProcessService); + currentProcess = mock(CurrentProcess); + envVarsService = mock(EnvironmentVariablesProvider); + workspace = mockedVSCodeNamespaces['workspace']!; + when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(noop as any); + service = new EnvironmentActivationService( + instance(helper), instance(platform), + instance(processServiceFactory), instance(currentProcess), + instance(envVarsService) + ); + + const cfg = { get: () => 'xyz' }; + workspace.setup(w => w.getConfiguration(typemoq.It.isValue('python'), typemoq.It.isAny())) + .returns(() => cfg as any); + workspace.setup(w => w.workspaceFolders).returns(() => []); + clearCache(); + + verify(envVarsService.onDidEnvironmentVariablesChange).once(); + } + teardown(() => { + mockedVSCodeNamespaces['workspace']!.reset(); + }); + + [undefined, Uri.parse('a')].forEach(resource => { + suite(resource ? 'With a resource' : 'Without a resource', () => { + setup(initSetup); + test('Unknown os will return empty variables', async () => { + when(platform.osType).thenReturn(OSType.Unknown); + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + }); + + const osTypes = getNamesAndValues(OSType) + .filter(osType => osType.value !== OSType.Unknown); + + osTypes.forEach(osType => { + suite(osType.name, () => { + setup(initSetup); + test('getEnvironmentActivationShellCommands will be invoked', async () => { + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + }); + test('Validate command used to activation and printing env vars', async () => { + const cmd = ['1', '2']; + const envVars = { one: '1', two: '2' }; + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(cmd); + when(processServiceFactory.create(resource)).thenResolve(instance(processService)); + when(envVarsService.getEnvironmentVariables(resource)).thenResolve(envVars); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + verify(processServiceFactory.create(resource)).once(); + verify(envVarsService.getEnvironmentVariables(resource)).once(); + verify(processService.shellExec(anything(), anything())).once(); + + const shellCmd = capture(processService.shellExec).first()[0]; + + const printEnvPyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariables.py'); + const expectedCommand = `${cmd.join(' && ')} && echo '${getEnvironmentPrefix}' && python ${printEnvPyFile.fileToCommandArgument()}`; + + expect(shellCmd).to.equal(expectedCommand); + }); + test('Validate env Vars used to activation and printing env vars', async () => { + const cmd = ['1', '2']; + const envVars = { one: '1', two: '2' }; + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(cmd); + when(processServiceFactory.create(resource)).thenResolve(instance(processService)); + when(envVarsService.getEnvironmentVariables(resource)).thenResolve(envVars); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + verify(processServiceFactory.create(resource)).once(); + verify(envVarsService.getEnvironmentVariables(resource)).once(); + verify(processService.shellExec(anything(), anything())).once(); + + const options = capture(processService.shellExec).first()[1]; + + const expectedShell = defaultShells[osType.value]; + expect(options).to.deep.equal({ shell: expectedShell, env: envVars }); + }); + test('Use current process variables if there are no custom variables', async () => { + const cmd = ['1', '2']; + const envVars = { one: '1', two: '2' }; + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(cmd); + when(processServiceFactory.create(resource)).thenResolve(instance(processService)); + when(envVarsService.getEnvironmentVariables(resource)).thenResolve({}); + when(currentProcess.env).thenReturn(envVars); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + verify(processServiceFactory.create(resource)).once(); + verify(envVarsService.getEnvironmentVariables(resource)).once(); + verify(processService.shellExec(anything(), anything())).once(); + verify(currentProcess.env).once(); + + const options = capture(processService.shellExec).first()[1]; + + const expectedShell = defaultShells[osType.value]; + expect(options).to.deep.equal({ shell: expectedShell, env: envVars }); + }); + test('Error must be swallowed when activation fails', async () => { + const cmd = ['1', '2']; + const envVars = { one: '1', two: '2' }; + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(cmd); + when(processServiceFactory.create(resource)).thenResolve(instance(processService)); + when(envVarsService.getEnvironmentVariables(resource)).thenResolve(envVars); + when(processService.shellExec(anything(), anything())).thenReject(new Error('kaboom')); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.equal(undefined, 'Should not have any variables'); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + verify(processServiceFactory.create(resource)).once(); + verify(envVarsService.getEnvironmentVariables(resource)).once(); + verify(processService.shellExec(anything(), anything())).once(); + }); + test('Return parsed variables', async () => { + const cmd = ['1', '2']; + const envVars = { one: '1', two: '2' }; + const varsFromEnv = { one: '11', two: '22', HELLO: 'xxx' }; + const stdout = `${getEnvironmentPrefix}${EOL}${JSON.stringify(varsFromEnv)}`; + when(platform.osType).thenReturn(osType.value); + when(helper.getEnvironmentActivationShellCommands(resource)).thenResolve(cmd); + when(processServiceFactory.create(resource)).thenResolve(instance(processService)); + when(envVarsService.getEnvironmentVariables(resource)).thenResolve(envVars); + when(processService.shellExec(anything(), anything())).thenResolve({ stdout: stdout }); + + const env = await service.getActivatedEnvironmentVariables(resource); + + verify(platform.osType).once(); + expect(env).to.deep.equal(varsFromEnv); + verify(helper.getEnvironmentActivationShellCommands(resource)).once(); + verify(processServiceFactory.create(resource)).once(); + verify(envVarsService.getEnvironmentVariables(resource)).once(); + verify(processService.shellExec(anything(), anything())).once(); + }); + }); + }); + }); + }); +}); diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index de32f703b227..df498233fa09 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -14,7 +14,7 @@ const Module = require('module'); type VSCode = typeof vscode; const mockedVSCode: Partial = {}; -const mockedVSCodeNamespaces: { [P in keyof VSCode]?: TypeMoq.IMock } = {}; +export const mockedVSCodeNamespaces: { [P in keyof VSCode]?: TypeMoq.IMock } = {}; const originalLoad = Module._load; function generateMock(name: K): void { From 901465b61bd5e4589536646b42018415b4442b20 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 20:31:41 -0800 Subject: [PATCH 23/49] Fixed tests --- src/test/common/terminals/activation.conda.unit.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts index 3411740707fd..b44ac223986a 100644 --- a/src/test/common/terminals/activation.conda.unit.test.ts +++ b/src/test/common/terminals/activation.conda.unit.test.ts @@ -291,10 +291,8 @@ suite('Terminal Environment Activation conda', () => { condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); pythonSettings.setup(s => s.pythonPath).returns(() => pythonPath); - const mockProvider = TypeMoq.Mock.ofType(); - serviceContainer.setup(c => c.getAll(TypeMoq.It.isValue(ITerminalActivationCommandProvider), TypeMoq.It.isAny())).returns(() => [mockProvider.object]); - mockProvider.setup(p => p.isShellSupported(TypeMoq.It.isAny())).returns(() => true); - mockProvider.setup(p => p.getActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(['mock command'])); + when(bash.isShellSupported(anything())).thenReturn(true); + when(bash.getActivationCommands(anything(), TerminalShellType.bash)).thenResolve(['mock command']); const expectedActivationCommand = ['mock command']; const activationCommands = await terminalHelper.getEnvironmentActivationCommands(TerminalShellType.bash, undefined); From 135fbf385ca63418e3b56b936ce191472961ddf3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 21:27:09 -0800 Subject: [PATCH 24/49] Fix tests --- src/test/common/terminals/helper.unit.test.ts | 2 +- src/test/refactor/rename.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts index 130dfe672338..f8fb85ea070c 100644 --- a/src/test/common/terminals/helper.unit.test.ts +++ b/src/test/common/terminals/helper.unit.test.ts @@ -55,7 +55,7 @@ suite('Terminal Service helpers', () => { pythonSettings = mock(PythonSettings); helper = new TerminalHelper(instance(platformService), instance(terminalManager), - instance(workspaceService), instance(platformService), + instance(workspaceService), instance(condaService), instance(configurationService), instance(condaActivationProvider), instance(bashActivationProvider), diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index f19b9e0222f8..6774da6066c1 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -44,7 +44,8 @@ suite('Refactor Rename', () => { serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); serviceContainer .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) - .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any)); + .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any, + undefined as any, undefined as any)); await initializeTest(); }); teardown(closeActiveWindows); From cffda9a2b8ee8bc5ec7a69cf3dd74dadbe381e1a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 23:06:37 -0800 Subject: [PATCH 25/49] Fix test --- .../interpreter/locators/services/cacheableLocatorService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index ea5c004eb9fd..1d69dfa32817 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -50,7 +50,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ const promise = this.getInterpretersImplementation(resource) .then(async items => { await this.cacheInterpreters(items, resource); - traceVerbose(`Interpreters returned by ${this.name} are of count ${items.length}`); + traceVerbose(`Interpreters returned by ${this.name} are of count ${Array.isArray(items) ? items.length : 0}`); deferred!.resolve(items); }) .catch(ex => deferred!.reject(ex)); From 30c40b59661a3ef1a8ecdfe789b2461c31e8f4c1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 23:07:20 -0800 Subject: [PATCH 26/49] Fix tests --- src/test/common/utils/decorators.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/utils/decorators.unit.test.ts b/src/test/common/utils/decorators.unit.test.ts index f4821dfe23ec..a90cc9f210b2 100644 --- a/src/test/common/utils/decorators.unit.test.ts +++ b/src/test/common/utils/decorators.unit.test.ts @@ -86,7 +86,7 @@ suite('Common Utils - Decorators', () => { expect(result).to.equal(3); expect(cls.invoked).to.equal(false, 'Must not be invoked'); - await sleep(100); + await sleep(110); cls.invoked = false; result = await cls.doSomething(uri, 1, 2); From a715423195f93bb3faf20eeb67d1e4bff6603596 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Jan 2019 23:24:59 -0800 Subject: [PATCH 27/49] Always pass value into get configuration --- src/client/common/utils/cacheUtils.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index 5aa1bb055f2b..4953803b45dd 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -27,9 +27,13 @@ const resourceSpecificCacheStores = new Map>(); * @returns */ function getCacheKey(resource: Resource, vscode: VSCodeType = require('vscode')) { - const globalPythonPath = vscode.workspace.getConfiguration('python', null as any).get('pythonPath') || 'python'; + const section = vscode.workspace.getConfiguration('python', vscode.Uri.file(__filename)); + if (!section) { + return 'python'; + } + const globalPythonPath = section.inspect('pythonPath')!.globalValue || 'python'; // Get the workspace related to this resource. - if (!Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { + if (!resource || !Array.isArray(vscode.workspace.workspaceFolders) || vscode.workspace.workspaceFolders.length === 0) { return globalPythonPath; } const folder = resource ? vscode.workspace.getWorkspaceFolder(resource) : vscode.workspace.workspaceFolders[0]; From 6947f18e8b612fa00872110a22f87f0eeb55a6a1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 02:30:27 -0800 Subject: [PATCH 28/49] Fix tests --- src/test/common/utils/cacheUtils.unit.test.ts | 6 +++++- src/test/common/utils/decorators.unit.test.ts | 6 +++++- src/test/format/extension.sort.test.ts | 2 +- src/test/interpreters/activation/service.unit.test.ts | 8 +++++--- src/test/refactor/rename.test.ts | 3 +++ src/test/serviceRegistry.ts | 4 ++++ 6 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/test/common/utils/cacheUtils.unit.test.ts b/src/test/common/utils/cacheUtils.unit.test.ts index 8d2116abf261..e2facb211bf7 100644 --- a/src/test/common/utils/cacheUtils.unit.test.ts +++ b/src/test/common/utils/cacheUtils.unit.test.ts @@ -20,13 +20,17 @@ suite('Common Utils - CacheUtils', () => { return { get: () => { return pythonPath; + }, + inspect: () => { + return { globalValue: pythonPath }; } }; }, getWorkspaceFolder: () => { return; } - } + }, + Uri: Uri } as any; } ['hello', undefined, { date: new Date(), hello: 1234 }].forEach(dataToStore => { diff --git a/src/test/common/utils/decorators.unit.test.ts b/src/test/common/utils/decorators.unit.test.ts index a90cc9f210b2..52249099994a 100644 --- a/src/test/common/utils/decorators.unit.test.ts +++ b/src/test/common/utils/decorators.unit.test.ts @@ -22,13 +22,17 @@ suite('Common Utils - Decorators', () => { return { get: () => { return pythonPath; + }, + inspect: () => { + return { globalValue: pythonPath }; } }; }, getWorkspaceFolder: () => { return; } - } + }, + Uri: Uri } as any; } test('Result must be cached when using cache decorator', async () => { diff --git a/src/test/format/extension.sort.test.ts b/src/test/format/extension.sort.test.ts index d96edbccc3e2..c83c27b00458 100644 --- a/src/test/format/extension.sort.test.ts +++ b/src/test/format/extension.sort.test.ts @@ -20,7 +20,7 @@ const fileToFormatWithConfig1 = path.join(sortingPath, 'withconfig', 'before.1.p const originalFileToFormatWithConfig1 = path.join(sortingPath, 'withconfig', 'original.1.py'); // tslint:disable-next-line:max-func-body-length -suite('Sortingx', () => { +suite('Sorting', () => { let ioc: UnitTestIocContainer; let sorter: ISortImportsEditingProvider; const configTarget = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace; diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts index 9dbdf317270a..1c197e62335e 100644 --- a/src/test/interpreters/activation/service.unit.test.ts +++ b/src/test/interpreters/activation/service.unit.test.ts @@ -8,7 +8,7 @@ import { EOL } from 'os'; import * as path from 'path'; import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; -import { Uri, workspace as workspaceType } from 'vscode'; +import { Uri, workspace as workspaceType, WorkspaceConfiguration } from 'vscode'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IPlatformService } from '../../../client/common/platform/types'; import { CurrentProcess } from '../../../client/common/process/currentProcess'; @@ -61,10 +61,12 @@ suite('Interprters Activation - Python Environment Variables', () => { instance(envVarsService) ); - const cfg = { get: () => 'xyz' }; + const cfg = typemoq.Mock.ofType(); workspace.setup(w => w.getConfiguration(typemoq.It.isValue('python'), typemoq.It.isAny())) - .returns(() => cfg as any); + .returns(() => cfg.object); workspace.setup(w => w.workspaceFolders).returns(() => []); + cfg.setup(c => c.inspect(typemoq.It.isValue('pythonPath'))) + .returns(() => { return { globalValue: 'GlobalValuepython' } as any; }); clearCache(); verify(envVarsService.onDidEnvironmentVariablesChange).once(); diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index 6774da6066c1..47ff7ee9a5a5 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -15,6 +15,7 @@ import { ProcessService } from '../../client/common/process/proc'; import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; import { IConfigurationService, IPythonSettings } from '../../client/common/types'; +import { IEnvironmentActivationService } from '../../client/interpreter/activation/types'; import { IServiceContainer } from '../../client/ioc/types'; import { RefactorProxy } from '../../client/refactor/proxy'; import { PYTHON_PATH } from '../common'; @@ -42,6 +43,8 @@ suite('Refactor Rename', () => { serviceContainer = typeMoq.Mock.ofType(); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IConfigurationService), typeMoq.It.isAny())).returns(() => configService.object); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); + serviceContainer.setup(s => s.get(typeMoq.It.isValue(IEnvironmentActivationService), typeMoq.It.isAny())) + .returns(() => typeMoq.Mock.ofType().object); serviceContainer .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any, diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 8233f09f8b01..1ce925256136 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -22,6 +22,8 @@ import { registerTypes as commonRegisterTypes } from '../client/common/serviceRe import { GLOBAL_MEMENTO, ICurrentProcess, IDisposableRegistry, ILogger, IMemento, IOutputChannel, IPathUtils, IsWindows, WORKSPACE_MEMENTO } from '../client/common/types'; import { registerTypes as variableRegisterTypes } from '../client/common/variables/serviceRegistry'; import { registerTypes as formattersRegisterTypes } from '../client/formatters/serviceRegistry'; +import { EnvironmentActivationService } from '../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../client/interpreter/activation/types'; import { IInterpreterAutoSelectionService, IInterpreterAutoSeletionProxyService } from '../client/interpreter/autoSelection/types'; import { registerTypes as interpretersRegisterTypes } from '../client/interpreter/serviceRegistry'; import { ServiceContainer } from '../client/ioc/container'; @@ -87,6 +89,7 @@ export class IocContainer { } public registerProcessTypes() { processRegisterTypes(this.serviceManager); + this.serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); } public registerVariableTypes() { variableRegisterTypes(this.serviceManager); @@ -115,6 +118,7 @@ export class IocContainer { this.serviceManager.addSingletonInstance(IProcessServiceFactory, processServiceFactory.object); this.serviceManager.addSingleton(IPythonExecutionFactory, PythonExecutionFactory); this.serviceManager.addSingleton(IPythonToolExecutionService, PythonToolExecutionService); + this.serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); } public registerMockProcess() { From b909447c0b498dac963a9598e1a51f5ee55dd9e4 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 02:44:37 -0800 Subject: [PATCH 29/49] Fix tests --- src/test/unittests/debugger.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/unittests/debugger.test.ts b/src/test/unittests/debugger.test.ts index c55792c519ab..0fb4fbbeb8a8 100644 --- a/src/test/unittests/debugger.test.ts +++ b/src/test/unittests/debugger.test.ts @@ -3,6 +3,8 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as path from 'path'; import { ConfigurationTarget } from 'vscode'; import { createDeferred } from '../../client/common/utils/async'; +import { ICondaService } from '../../client/interpreter/contracts'; +import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { TestManagerRunner as NoseTestManagerRunner } from '../../client/unittests//nosetest/runner'; import { TestManagerRunner as PytestManagerRunner } from '../../client/unittests//pytest/runner'; import { TestManagerRunner as UnitTestTestManagerRunner } from '../../client/unittests//unittest/runner'; @@ -83,6 +85,7 @@ suite('Unit Tests - debugging', () => { ioc.serviceManager.add(ITestManagerRunner, UnitTestTestManagerRunner, UNITTEST_PROVIDER); ioc.serviceManager.addSingleton(ITestDebugLauncher, MockDebugLauncher); ioc.serviceManager.addSingleton(ITestMessageService, TestMessageService, PYTEST_PROVIDER); + ioc.serviceManager.addSingleton(ICondaService, CondaService); } async function testStartingDebugger(testProvider: TestProvider) { From ac4414d1093545ce7c85b48b7beb22c9f3e6921f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 03:02:55 -0800 Subject: [PATCH 30/49] cleaner --- pythonFiles/printEnvVariables.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pythonFiles/printEnvVariables.py b/pythonFiles/printEnvVariables.py index fd9af2c5dced..3a5a1e47d7ae 100644 --- a/pythonFiles/printEnvVariables.py +++ b/pythonFiles/printEnvVariables.py @@ -4,8 +4,5 @@ import os import json -data = {} -for item, value in os.environ.items(): - data[item] = value - +data = {item:value for item, value in os.environ.items()} print(json.dumps(data)) From a1faa8bf2c5bae830af1b2fcd10d5adcec59778b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 08:53:35 -0800 Subject: [PATCH 31/49] Register services --- src/test/debugger/portAndHost.test.ts | 2 +- src/test/unittests/debugger.test.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/debugger/portAndHost.test.ts b/src/test/debugger/portAndHost.test.ts index abb30c277f82..77d3b9f19611 100644 --- a/src/test/debugger/portAndHost.test.ts +++ b/src/test/debugger/portAndHost.test.ts @@ -50,7 +50,7 @@ suite(`Standard Debugging of ports and hosts: ${debuggerType}`, () => { cwd: debugFilesPath, stopOnEntry, showReturnValue, - logToFile: true, + logToFile: false, debugOptions: [DebugOptions.RedirectOutput], pythonPath: PYTHON_PATH, args: [], diff --git a/src/test/unittests/debugger.test.ts b/src/test/unittests/debugger.test.ts index 0fb4fbbeb8a8..cd230f05b55e 100644 --- a/src/test/unittests/debugger.test.ts +++ b/src/test/unittests/debugger.test.ts @@ -1,9 +1,11 @@ import { assert, expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { createDeferred } from '../../client/common/utils/async'; -import { ICondaService } from '../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; +import { InterpreterService } from '../../client/interpreter/interpreterService'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { TestManagerRunner as NoseTestManagerRunner } from '../../client/unittests//nosetest/runner'; import { TestManagerRunner as PytestManagerRunner } from '../../client/unittests//pytest/runner'; @@ -85,7 +87,8 @@ suite('Unit Tests - debugging', () => { ioc.serviceManager.add(ITestManagerRunner, UnitTestTestManagerRunner, UNITTEST_PROVIDER); ioc.serviceManager.addSingleton(ITestDebugLauncher, MockDebugLauncher); ioc.serviceManager.addSingleton(ITestMessageService, TestMessageService, PYTEST_PROVIDER); - ioc.serviceManager.addSingleton(ICondaService, CondaService); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function testStartingDebugger(testProvider: TestProvider) { From 46d0bb3291ecdec4b9eece36e7dbd9a95c2f1e6a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 08:58:19 -0800 Subject: [PATCH 32/49] Fix svc registrations --- src/test/unittests/nosetest/nosetest.test.ts | 6 ++++++ src/test/unittests/pytest/pytest.test.ts | 6 ++++++ src/test/unittests/unittest/unittest.test.ts | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/src/test/unittests/nosetest/nosetest.test.ts b/src/test/unittests/nosetest/nosetest.test.ts index 4429186f92c9..9e92dac676aa 100644 --- a/src/test/unittests/nosetest/nosetest.test.ts +++ b/src/test/unittests/nosetest/nosetest.test.ts @@ -1,8 +1,12 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -54,6 +58,8 @@ suite('Unit Tests - nose - discovery against actual python process', () => { ioc.registerProcessTypes(); ioc.registerUnitTestTypes(); ioc.registerVariableTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } test('Discover Tests (single test file)', async () => { diff --git a/src/test/unittests/pytest/pytest.test.ts b/src/test/unittests/pytest/pytest.test.ts index 5ea6e1a36627..2ab281d1d41c 100644 --- a/src/test/unittests/pytest/pytest.test.ts +++ b/src/test/unittests/pytest/pytest.test.ts @@ -1,7 +1,11 @@ import * as assert from 'assert'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -33,6 +37,8 @@ suite('Unit Tests - pytest - discovery against actual python process', () => { ioc.registerProcessTypes(); ioc.registerUnitTestTypes(); ioc.registerVariableTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } test('Discover Tests (single test file)', async () => { diff --git a/src/test/unittests/unittest/unittest.test.ts b/src/test/unittests/unittest/unittest.test.ts index d4bf60eaac43..e4dec2244552 100644 --- a/src/test/unittests/unittest/unittest.test.ts +++ b/src/test/unittests/unittest/unittest.test.ts @@ -3,8 +3,12 @@ import * as assert from 'assert'; import * as fs from 'fs-extra'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory, TestFile, @@ -62,6 +66,8 @@ suite('Unit Tests - unittest - discovery against actual python process', () => { ioc.registerVariableTypes(); ioc.registerUnitTestTypes(); ioc.registerProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } test('Discover Tests (single test file)', async () => { From d1ea8dc957e7642d0c5412bf8b02feb2753d0543 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 09:03:06 -0800 Subject: [PATCH 33/49] Address code review comments --- src/client/common/utils/cacheUtils.ts | 1 - src/client/common/utils/decorators.ts | 2 +- src/client/common/variables/environmentVariablesProvider.ts | 4 ++-- src/client/interpreter/activation/service.ts | 4 ++-- src/test/common/utils/decorators.unit.test.ts | 6 +++--- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/client/common/utils/cacheUtils.ts b/src/client/common/utils/cacheUtils.ts index 4953803b45dd..77df6a971dae 100644 --- a/src/client/common/utils/cacheUtils.ts +++ b/src/client/common/utils/cacheUtils.ts @@ -14,7 +14,6 @@ type CacheData = { value: unknown; expiry: number; }; -// const simpleCache = new Map(); const resourceSpecificCacheStores = new Map>(); /** diff --git a/src/client/common/utils/decorators.ts b/src/client/common/utils/decorators.ts index f53819e72267..e1d62cdf193a 100644 --- a/src/client/common/utils/decorators.ts +++ b/src/client/common/utils/decorators.ts @@ -34,7 +34,7 @@ export function clearCachedResourceSpecificIngterpreterData(key: string, resourc const cache = new InMemoryInterpreterSpecificCache(key, 0, [resource], vscode); cache.clear(); } -export function cacheResourceSpecificIngterpreterData(key: string, expiryDurationMs: number, vscode: VSCodeType = require('vscode')) { +export function cacheResourceSpecificInterpreterData(key: string, expiryDurationMs: number, vscode: VSCodeType = require('vscode')) { return function (_target: Object, _propertyName: string, descriptor: TypedPropertyDescriptor) { const originalMethod = descriptor.value!; descriptor.value = async function (...args: [Uri | undefined, ...any[]]) { diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index e5a3fb1c2ad3..4587e1d3ae60 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode'; import { IPlatformService } from '../platform/types'; import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types'; -import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData } from '../utils/decorators'; +import { cacheResourceSpecificInterpreterData, clearCachedResourceSpecificIngterpreterData } from '../utils/decorators'; import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; const cacheDuration = 60 * 60 * 1000; @@ -33,7 +33,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid watcher.dispose(); }); } - @cacheResourceSpecificIngterpreterData('getEnvironmentVariables', cacheDuration) + @cacheResourceSpecificInterpreterData('getEnvironmentVariables', cacheDuration) public async getEnvironmentVariables(resource?: Uri): Promise { const settings = this.configurationService.getSettings(resource); const workspaceFolderUri = this.getWorkspaceFolderUri(resource); diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index be89a5091e00..2659d157358e 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -11,7 +11,7 @@ import { IPlatformService } from '../../common/platform/types'; import { IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper } from '../../common/terminal/types'; import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; -import { cacheResourceSpecificIngterpreterData, clearCachedResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; +import { cacheResourceSpecificInterpreterData, clearCachedResourceSpecificIngterpreterData, swallowExceptions } from '../../common/utils/decorators'; import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { EXTENSION_ROOT_DIR } from '../../constants'; @@ -45,7 +45,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments) @swallowExceptions('getActivatedEnvironmentVariables') - @cacheResourceSpecificIngterpreterData('ActivatedEnvironmentVariables', cacheDuration) + @cacheResourceSpecificInterpreterData('ActivatedEnvironmentVariables', cacheDuration) public async getActivatedEnvironmentVariables(resource: Resource): Promise { const shell = defaultShells[this.platform.osType]; if (!shell) { diff --git a/src/test/common/utils/decorators.unit.test.ts b/src/test/common/utils/decorators.unit.test.ts index 52249099994a..c85dc1622301 100644 --- a/src/test/common/utils/decorators.unit.test.ts +++ b/src/test/common/utils/decorators.unit.test.ts @@ -7,7 +7,7 @@ import { expect } from 'chai'; import { Uri } from 'vscode'; import { Resource } from '../../../client/common/types'; import { clearCache } from '../../../client/common/utils/cacheUtils'; -import { cacheResourceSpecificIngterpreterData } from '../../../client/common/utils/decorators'; +import { cacheResourceSpecificInterpreterData } from '../../../client/common/utils/decorators'; import { sleep } from '../../core'; // tslint:disable:no-any max-func-body-length no-unnecessary-class @@ -39,7 +39,7 @@ suite('Common Utils - Decorators', () => { const vsc = createMockVSC(''); class TestClass { public invoked = false; - @cacheResourceSpecificIngterpreterData('Something', 100000, vsc) + @cacheResourceSpecificInterpreterData('Something', 100000, vsc) public async doSomething(_resource: Resource, a: number, b: number): Promise { this.invoked = true; return a + b; @@ -70,7 +70,7 @@ suite('Common Utils - Decorators', () => { const vsc = createMockVSC(''); class TestClass { public invoked = false; - @cacheResourceSpecificIngterpreterData('Something', 100, vsc) + @cacheResourceSpecificInterpreterData('Something', 100, vsc) public async doSomething(_resource: Resource, a: number, b: number): Promise { this.invoked = true; return a + b; From 5cd8061069b71a0e92e28961d184ef9ca894eca9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 09:55:16 -0800 Subject: [PATCH 34/49] Add telemetry --- .../common/process/pythonExecutionFactory.ts | 6 +++- src/client/common/terminal/helper.ts | 31 +++++++++++++++++-- src/client/interpreter/activation/service.ts | 3 ++ src/client/telemetry/constants.ts | 3 ++ src/client/telemetry/index.ts | 5 +-- src/client/telemetry/types.ts | 18 +++++++++-- .../terminals/activation.conda.unit.test.ts | 5 ++- src/test/common/terminals/helper.unit.test.ts | 5 ++- 8 files changed, 66 insertions(+), 10 deletions(-) diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index e2972bb649e8..b0b16181a581 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -4,6 +4,8 @@ import { inject, injectable } from 'inversify'; import { IEnvironmentActivationService } from '../../interpreter/activation/types'; import { IServiceContainer } from '../../ioc/types'; +import { sendTelemetryEvent } from '../../telemetry'; +import { PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES } from '../../telemetry/constants'; import { IConfigurationService, Resource } from '../types'; import { ProcessService } from './proc'; import { PythonExecutionService } from './pythonProcess'; @@ -24,7 +26,9 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { } public async createActivatedEnvironment(resource: Resource): Promise { const envVars = await this.activationHelper.getActivatedEnvironmentVariables(resource); - if (!envVars || Object.keys(envVars).length === 0) { + const hasEnvVars = envVars && Object.keys(envVars).length > 0; + sendTelemetryEvent(PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, undefined, { hasEnvVars }); + if (!hasEnvVars) { return this.create({ resource }); } const pythonPath = this.configService.getSettings(resource).pythonPath; diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index 6d73ee813295..4827998f2ac7 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -3,9 +3,12 @@ import { inject, injectable, named } from 'inversify'; import { Terminal, Uri } from 'vscode'; -import { ICondaService } from '../../interpreter/contracts'; +import { ICondaService, IInterpreterService, InterpreterType } from '../../interpreter/contracts'; +import { sendTelemetryEvent } from '../../telemetry'; +import { PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE, PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL } from '../../telemetry/constants'; import { ITerminalManager, IWorkspaceService } from '../application/types'; import '../extensions'; +import { traceError } from '../logger'; import { IPlatformService } from '../platform/types'; import { IConfigurationService, Resource } from '../types'; import { OSType } from '../utils/platform'; @@ -39,6 +42,7 @@ export class TerminalHelper implements ITerminalHelper { @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, @inject(ICondaService) private readonly condaService: ICondaService, + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.conda) private readonly conda: ITerminalActivationCommandProvider, @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.bashCShellFish) private readonly bashCShellFish: ITerminalActivationCommandProvider, @@ -101,7 +105,9 @@ export class TerminalHelper implements ITerminalHelper { } public async getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise { const providers = [this.bashCShellFish, this.commandPromptAndPowerShell, this.pyenv, this.pipenv]; - return this.getActivationCommands(resource || undefined, terminalShellType, providers); + const promise = this.getActivationCommands(resource || undefined, terminalShellType, providers); + this.sendTelemetry(resource, terminalShellType, PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL, promise).ignoreErrors(); + return promise; } public async getEnvironmentActivationShellCommands(resource: Resource): Promise { const shell = defaultOSShells[this.platform.osType]; @@ -109,7 +115,26 @@ export class TerminalHelper implements ITerminalHelper { return; } const providers = [this.bashCShellFish, this.commandPromptAndPowerShell]; - return this.getActivationCommands(resource, shell, providers); + const promise = this.getActivationCommands(resource, shell, providers); + this.sendTelemetry(resource, shell, PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE, promise).ignoreErrors(); + return promise; + } + protected async sendTelemetry(resource: Resource, terminalShellType: TerminalShellType, eventName: string, promise: Promise): Promise { + let hasCommands = false; + const interpreter = await this.interpreterService.getActiveInterpreter(resource); + let failed = false; + try { + const cmds = await promise; + hasCommands = Array.isArray(cmds) && cmds.length > 0; + } catch (ex) { + failed = true; + traceError('Failed to get activation commands', ex); + } + + const pythonVersion = (interpreter && interpreter.version) ? interpreter.version.raw : undefined; + const interpreterType = interpreter ? interpreter.type : InterpreterType.Unknown; + const data = { failed, hasCommands, interpreterType, terminal: terminalShellType, pythonVersion }; + sendTelemetryEvent(eventName, undefined, data); } protected async getActivationCommands(resource: Resource, terminalShellType: TerminalShellType, providers: ITerminalActivationCommandProvider[]): Promise { const settings = this.configurationService.getSettings(resource); diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 2659d157358e..ffa51fdc08f7 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -15,6 +15,8 @@ import { cacheResourceSpecificInterpreterData, clearCachedResourceSpecificIngter import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { EXTENSION_ROOT_DIR } from '../../constants'; +import { captureTelemetry } from '../../telemetry'; +import { PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES } from '../../telemetry/constants'; import { IEnvironmentActivationService } from './types'; const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6'; @@ -45,6 +47,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments) @swallowExceptions('getActivatedEnvironmentVariables') + @captureTelemetry(PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, { failed: false }, true) @cacheResourceSpecificInterpreterData('ActivatedEnvironmentVariables', cacheDuration) public async getActivatedEnvironmentVariables(resource: Resource): Promise { const shell = defaultShells[this.platform.osType]; diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 04652b62f053..d05e97805348 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -24,6 +24,9 @@ export const REPL = 'REPL'; export const PYTHON_INTERPRETER = 'PYTHON_INTERPRETER'; export const PYTHON_INTERPRETER_DISCOVERY = 'PYTHON_INTERPRETER_DISCOVERY'; export const PYTHON_INTERPRETER_AUTO_SELECTION = 'PYTHON_INTERPRETER_AUTO_SELECTION'; +export const PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES = 'PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES'; +export const PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE = 'PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE'; +export const PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL = 'PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL'; export const WORKSPACE_SYMBOLS_BUILD = 'WORKSPACE_SYMBOLS.BUILD'; export const WORKSPACE_SYMBOLS_GO_TO = 'WORKSPACE_SYMBOLS.GO_TO'; export const EXECUTION_CODE = 'EXECUTION_CODE'; diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 396d15471c4b..58ea3e6a212f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1,9 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// tslint:disable-next-line:no-reference +// tslint:disable:no-reference no-any import-name /// -// tslint:disable-next-line:import-name import TelemetryReporter from 'vscode-extension-telemetry'; import { isTestExecution, PVSC_EXTENSION_ID } from '../common/constants'; import { StopWatch } from '../common/utils/stopWatch'; @@ -103,6 +102,8 @@ export function captureTelemetry( // tslint:disable-next-line:promise-function-async .catch(ex => { // tslint:disable-next-line:no-any + properties = properties || {}; + (properties as any).failed = true; sendTelemetryEvent(failureEventName ? failureEventName : eventName, stopWatch.elapsedTime, properties); }); } else { diff --git a/src/client/telemetry/types.ts b/src/client/telemetry/types.ts index 3242aad30776..9759928ce685 100644 --- a/src/client/telemetry/types.ts +++ b/src/client/telemetry/types.ts @@ -46,7 +46,7 @@ export type LintingTelemetry = { export type LinterInstallPromptTelemetry = { tool?: LinterId; - action: 'select'|'disablePrompt'|'install'; + action: 'select' | 'disablePrompt' | 'install'; }; export type LinterSelectionTelemetry = { @@ -166,6 +166,18 @@ export type InterpreterDiscovery = { locator: string; }; +export type InterpreterActivationEnvironmentVariables = { + hasEnvVars?: boolean; + failed?: boolean; +}; + +export type InterpreterActivation = { + hasCommands?: boolean; + failed?: boolean; + terminal: TerminalShellType; + pythonVersion?: string; +}; + export type TelemetryProperties = FormatTelemetry | LanguageServerVersionTelemetry | LanguageServerErrorTelemetry @@ -188,4 +200,6 @@ export type TelemetryProperties = FormatTelemetry | LanguageServePlatformSupported | DebuggerConfigurationPromtpsTelemetry | InterpreterAutoSelection - | InterpreterDiscovery; + | InterpreterDiscovery + | InterpreterActivationEnvironmentVariables + | InterpreterActivation; diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts index b44ac223986a..563589709870 100644 --- a/src/test/common/terminals/activation.conda.unit.test.ts +++ b/src/test/common/terminals/activation.conda.unit.test.ts @@ -35,6 +35,7 @@ import { } from '../../../client/common/types'; import { getNamesAndValues } from '../../../client/common/utils/enum'; import { ICondaService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { IServiceContainer } from '../../../client/ioc/types'; suite('Terminal Environment Activation conda', () => { @@ -83,7 +84,9 @@ suite('Terminal Environment Activation conda', () => { terminalHelper = new TerminalHelper(platformService.object, instance(mock(TerminalManager)), instance(mock(WorkspaceService)), - condaService.object, configService.object, + condaService.object, + instance(mock(InterpreterService)), + configService.object, new CondaActivationCommandProvider(serviceContainer.object), instance(bash), mock(CommandPromptAndPowerShell), diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts index f8fb85ea070c..164f54e67307 100644 --- a/src/test/common/terminals/helper.unit.test.ts +++ b/src/test/common/terminals/helper.unit.test.ts @@ -23,6 +23,7 @@ import { IConfigurationService } from '../../../client/common/types'; import { getNamesAndValues } from '../../../client/common/utils/enum'; import { OSType } from '../../../client/common/utils/platform'; import { ICondaService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; // tslint:disable:max-func-body-length no-any @@ -56,7 +57,9 @@ suite('Terminal Service helpers', () => { helper = new TerminalHelper(instance(platformService), instance(terminalManager), instance(workspaceService), - instance(condaService), instance(configurationService), + instance(condaService), + instance(mock(InterpreterService)), + instance(configurationService), instance(condaActivationProvider), instance(bashActivationProvider), instance(cmdActivationProvider), From a496df3505464259125c9cf880480137c48a94a2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 09:57:23 -0800 Subject: [PATCH 35/49] Log errors --- src/client/common/terminal/helper.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index 4827998f2ac7..155befc565a2 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -8,7 +8,7 @@ import { sendTelemetryEvent } from '../../telemetry'; import { PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE, PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL } from '../../telemetry/constants'; import { ITerminalManager, IWorkspaceService } from '../application/types'; import '../extensions'; -import { traceError } from '../logger'; +import { traceDecorators, traceError } from '../logger'; import { IPlatformService } from '../platform/types'; import { IConfigurationService, Resource } from '../types'; import { OSType } from '../utils/platform'; @@ -119,6 +119,7 @@ export class TerminalHelper implements ITerminalHelper { this.sendTelemetry(resource, shell, PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE, promise).ignoreErrors(); return promise; } + @traceDecorators.error('Failed to capture telemetry') protected async sendTelemetry(resource: Resource, terminalShellType: TerminalShellType, eventName: string, promise: Promise): Promise { let hasCommands = false; const interpreter = await this.interpreterService.getActiveInterpreter(resource); From cdd5d498b93c3c68bb47799343abf32fc0fd3272 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 10:49:44 -0800 Subject: [PATCH 36/49] Fix tests --- src/test/unittests/nosetest/nosetest.disovery.test.ts | 4 ++++ src/test/unittests/nosetest/nosetest.run.test.ts | 4 ++++ src/test/unittests/pytest/pytest.discovery.test.ts | 4 ++++ src/test/unittests/pytest/pytest.run.test.ts | 4 ++++ src/test/unittests/pytest/pytest.testMessageService.test.ts | 4 ++++ src/test/unittests/rediscover.test.ts | 4 ++++ src/test/unittests/unittest/unittest.discovery.test.ts | 4 ++++ src/test/unittests/unittest/unittest.run.test.ts | 4 ++++ 8 files changed, 32 insertions(+) diff --git a/src/test/unittests/nosetest/nosetest.disovery.test.ts b/src/test/unittests/nosetest/nosetest.disovery.test.ts index fbb5120e1868..1027400d2264 100644 --- a/src/test/unittests/nosetest/nosetest.disovery.test.ts +++ b/src/test/unittests/nosetest/nosetest.disovery.test.ts @@ -4,9 +4,12 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -61,6 +64,7 @@ suite('Unit Tests - nose - discovery with mocked process output', () => { ioc.registerVariableTypes(); ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/nosetest/nosetest.run.test.ts b/src/test/unittests/nosetest/nosetest.run.test.ts index 58d1464ef3c1..835a583a5ff3 100644 --- a/src/test/unittests/nosetest/nosetest.run.test.ts +++ b/src/test/unittests/nosetest/nosetest.run.test.ts @@ -4,9 +4,12 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory, TestsToRun } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -59,6 +62,7 @@ suite('Unit Tests - nose - run against actual python process', () => { ioc.registerVariableTypes(); ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/pytest/pytest.discovery.test.ts b/src/test/unittests/pytest/pytest.discovery.test.ts index 11ecbcbfa571..811fd0ec1615 100644 --- a/src/test/unittests/pytest/pytest.discovery.test.ts +++ b/src/test/unittests/pytest/pytest.discovery.test.ts @@ -3,9 +3,12 @@ import * as assert from 'assert'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -43,6 +46,7 @@ suite('Unit Tests - pytest - discovery with mocked process output', () => { // Mocks. ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectTestDiscoveryOutput(output: string) { diff --git a/src/test/unittests/pytest/pytest.run.test.ts b/src/test/unittests/pytest/pytest.run.test.ts index e15a0f98a438..868e6b9f19a5 100644 --- a/src/test/unittests/pytest/pytest.run.test.ts +++ b/src/test/unittests/pytest/pytest.run.test.ts @@ -4,10 +4,13 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IFileSystem } from '../../../client/common/platform/types'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { UnitTestDiagnosticService } from '../../../client/unittests/common/services/unitTestDiagnosticService'; import { FlattenedTestFunction, ITestManager, ITestManagerFactory, Tests, TestStatus, TestsToRun } from '../../../client/unittests/common/types'; @@ -308,6 +311,7 @@ suite('Unit Tests - pytest - run with mocked process output', () => { ioc.registerVariableTypes(); // Mocks. ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/pytest/pytest.testMessageService.test.ts b/src/test/unittests/pytest/pytest.testMessageService.test.ts index d869fba7076d..b32b828b8974 100644 --- a/src/test/unittests/pytest/pytest.testMessageService.test.ts +++ b/src/test/unittests/pytest/pytest.testMessageService.test.ts @@ -6,11 +6,14 @@ import { assert } from 'chai'; import * as fs from 'fs'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as typeMoq from 'typemoq'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { ProductNames } from '../../../client/common/installer/productNames'; import { Product } from '../../../client/common/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { TestResultsService } from '../../../client/unittests/common/services/testResultsService'; import { TestsHelper } from '../../../client/unittests/common/testUtils'; import { TestFlatteningVisitor } from '../../../client/unittests/common/testVisitors/flatteningVisitor'; @@ -107,6 +110,7 @@ suite('Unit Tests - PyTest - TestMessageService', () => { ioc.registerVariableTypes(); // Mocks. ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } // Build tests for the test data that is relevant for this platform. filterdTestScenarios.forEach((scenario) => { diff --git a/src/test/unittests/rediscover.test.ts b/src/test/unittests/rediscover.test.ts index 5e054111eb92..2e17ff2b1635 100644 --- a/src/test/unittests/rediscover.test.ts +++ b/src/test/unittests/rediscover.test.ts @@ -1,7 +1,10 @@ import { assert } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; +import { ICondaService } from '../../client/interpreter/contracts'; +import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../client/unittests/common/constants'; import { ITestManagerFactory, TestProvider } from '../../client/unittests/common/types'; import { deleteDirectory, deleteFile, rootWorkspaceUri, updateSetting } from '../common'; @@ -53,6 +56,7 @@ suite('Unit Tests re-discovery', () => { ioc.registerProcessTypes(); ioc.registerVariableTypes(); ioc.registerUnitTestTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function discoverUnitTests(testProvider: TestProvider) { diff --git a/src/test/unittests/unittest/unittest.discovery.test.ts b/src/test/unittests/unittest/unittest.discovery.test.ts index ceb8c4f9fa03..9ab9e5868532 100644 --- a/src/test/unittests/unittest/unittest.discovery.test.ts +++ b/src/test/unittests/unittest/unittest.discovery.test.ts @@ -5,9 +5,12 @@ import * as assert from 'assert'; import * as fs from 'fs-extra'; import { EOL } from 'os'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; import { rootWorkspaceUri, updateSetting } from '../../common'; @@ -58,6 +61,7 @@ suite('Unit Tests - unittest - discovery with mocked process output', () => { // Mocks. ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectTestDiscoveryOutput(output: string) { diff --git a/src/test/unittests/unittest/unittest.run.test.ts b/src/test/unittests/unittest/unittest.run.test.ts index 4acd798d3985..53193946da92 100644 --- a/src/test/unittests/unittest/unittest.run.test.ts +++ b/src/test/unittests/unittest/unittest.run.test.ts @@ -5,9 +5,12 @@ import * as assert from 'assert'; import * as fs from 'fs-extra'; import { EOL } from 'os'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; +import { ICondaService } from '../../../client/interpreter/contracts'; +import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { ArgumentsHelper } from '../../../client/unittests/common/argumentsHelper'; import { CommandSource, UNITTEST_PROVIDER } from '../../../client/unittests/common/constants'; import { TestRunner } from '../../../client/unittests/common/runner'; @@ -81,6 +84,7 @@ suite('Unit Tests - unittest - run with mocked process output', () => { ioc.serviceManager.add(ITestManagerRunner, TestManagerRunner, UNITTEST_PROVIDER); ioc.serviceManager.add(ITestRunner, TestRunner); ioc.serviceManager.add(IUnitTestHelper, UnitTestHelper); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function ignoreTestLauncher() { From 5e530bf59ce29cc19f77250bf16ed9f44b8857b6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 10:59:25 -0800 Subject: [PATCH 37/49] Add news entry --- news/1 Enhancements/2855.md | 1 + news/2 Fixes/3330.md | 1 + news/2 Fixes/3953.md | 1 + news/3 Code Health/3746.md | 1 + 4 files changed, 4 insertions(+) create mode 100644 news/1 Enhancements/2855.md create mode 100644 news/2 Fixes/3330.md create mode 100644 news/2 Fixes/3953.md create mode 100644 news/3 Code Health/3746.md diff --git a/news/1 Enhancements/2855.md b/news/1 Enhancements/2855.md new file mode 100644 index 000000000000..acc1b147dfeb --- /dev/null +++ b/news/1 Enhancements/2855.md @@ -0,0 +1 @@ +Activate `pipenv` environments in the shell using the command `pipenv shell`. diff --git a/news/2 Fixes/3330.md b/news/2 Fixes/3330.md new file mode 100644 index 000000000000..aa1077cc6a21 --- /dev/null +++ b/news/2 Fixes/3330.md @@ -0,0 +1 @@ +Activate `Python Environment` when running unit tests. diff --git a/news/2 Fixes/3953.md b/news/2 Fixes/3953.md new file mode 100644 index 000000000000..827db121f3fe --- /dev/null +++ b/news/2 Fixes/3953.md @@ -0,0 +1 @@ +Remove duplicates from interpreters listed in the interpreter selection list. diff --git a/news/3 Code Health/3746.md b/news/3 Code Health/3746.md new file mode 100644 index 000000000000..077d28075ad2 --- /dev/null +++ b/news/3 Code Health/3746.md @@ -0,0 +1 @@ +Add support to detect usage of `xonsh` shells. From a25695a9bfe9f15d40fb553544c2b50ada175c46 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 11:03:00 -0800 Subject: [PATCH 38/49] Fix references --- src/test/unittests/nosetest/nosetest.disovery.test.ts | 4 +++- src/test/unittests/nosetest/nosetest.run.test.ts | 4 +++- src/test/unittests/pytest/pytest.discovery.test.ts | 4 +++- src/test/unittests/pytest/pytest.run.test.ts | 4 +++- src/test/unittests/pytest/pytest.testMessageService.test.ts | 4 +++- src/test/unittests/rediscover.test.ts | 4 +++- src/test/unittests/unittest/unittest.discovery.test.ts | 4 +++- src/test/unittests/unittest/unittest.run.test.ts | 4 +++- 8 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/test/unittests/nosetest/nosetest.disovery.test.ts b/src/test/unittests/nosetest/nosetest.disovery.test.ts index 1027400d2264..8099e8ab2d50 100644 --- a/src/test/unittests/nosetest/nosetest.disovery.test.ts +++ b/src/test/unittests/nosetest/nosetest.disovery.test.ts @@ -8,7 +8,8 @@ import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; @@ -65,6 +66,7 @@ suite('Unit Tests - nose - discovery with mocked process output', () => { ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/nosetest/nosetest.run.test.ts b/src/test/unittests/nosetest/nosetest.run.test.ts index 835a583a5ff3..11531162504e 100644 --- a/src/test/unittests/nosetest/nosetest.run.test.ts +++ b/src/test/unittests/nosetest/nosetest.run.test.ts @@ -8,7 +8,8 @@ import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory, TestsToRun } from '../../../client/unittests/common/types'; @@ -63,6 +64,7 @@ suite('Unit Tests - nose - run against actual python process', () => { ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/pytest/pytest.discovery.test.ts b/src/test/unittests/pytest/pytest.discovery.test.ts index 811fd0ec1615..4135c0b0573c 100644 --- a/src/test/unittests/pytest/pytest.discovery.test.ts +++ b/src/test/unittests/pytest/pytest.discovery.test.ts @@ -7,7 +7,8 @@ import { instance, mock } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; @@ -47,6 +48,7 @@ suite('Unit Tests - pytest - discovery with mocked process output', () => { // Mocks. ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function injectTestDiscoveryOutput(output: string) { diff --git a/src/test/unittests/pytest/pytest.run.test.ts b/src/test/unittests/pytest/pytest.run.test.ts index 868e6b9f19a5..be18f0aa3787 100644 --- a/src/test/unittests/pytest/pytest.run.test.ts +++ b/src/test/unittests/pytest/pytest.run.test.ts @@ -9,7 +9,8 @@ import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IFileSystem } from '../../../client/common/platform/types'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { UnitTestDiagnosticService } from '../../../client/unittests/common/services/unitTestDiagnosticService'; @@ -312,6 +313,7 @@ suite('Unit Tests - pytest - run with mocked process output', () => { // Mocks. ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/pytest/pytest.testMessageService.test.ts b/src/test/unittests/pytest/pytest.testMessageService.test.ts index b32b828b8974..da9d8e322a2d 100644 --- a/src/test/unittests/pytest/pytest.testMessageService.test.ts +++ b/src/test/unittests/pytest/pytest.testMessageService.test.ts @@ -12,7 +12,8 @@ import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { ProductNames } from '../../../client/common/installer/productNames'; import { Product } from '../../../client/common/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { TestResultsService } from '../../../client/unittests/common/services/testResultsService'; import { TestsHelper } from '../../../client/unittests/common/testUtils'; @@ -111,6 +112,7 @@ suite('Unit Tests - PyTest - TestMessageService', () => { // Mocks. ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } // Build tests for the test data that is relevant for this platform. filterdTestScenarios.forEach((scenario) => { diff --git a/src/test/unittests/rediscover.test.ts b/src/test/unittests/rediscover.test.ts index 2e17ff2b1635..483c833f6ad7 100644 --- a/src/test/unittests/rediscover.test.ts +++ b/src/test/unittests/rediscover.test.ts @@ -3,7 +3,8 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; -import { ICondaService } from '../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; +import { InterpreterService } from '../../client/interpreter/interpreterService'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../client/unittests/common/constants'; import { ITestManagerFactory, TestProvider } from '../../client/unittests/common/types'; @@ -57,6 +58,7 @@ suite('Unit Tests re-discovery', () => { ioc.registerVariableTypes(); ioc.registerUnitTestTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function discoverUnitTests(testProvider: TestProvider) { diff --git a/src/test/unittests/unittest/unittest.discovery.test.ts b/src/test/unittests/unittest/unittest.discovery.test.ts index 9ab9e5868532..d66479894b4b 100644 --- a/src/test/unittests/unittest/unittest.discovery.test.ts +++ b/src/test/unittests/unittest/unittest.discovery.test.ts @@ -9,7 +9,8 @@ import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { CommandSource } from '../../../client/unittests/common/constants'; import { ITestManagerFactory } from '../../../client/unittests/common/types'; @@ -62,6 +63,7 @@ suite('Unit Tests - unittest - discovery with mocked process output', () => { // Mocks. ioc.registerMockProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function injectTestDiscoveryOutput(output: string) { diff --git a/src/test/unittests/unittest/unittest.run.test.ts b/src/test/unittests/unittest/unittest.run.test.ts index 53193946da92..1c1947427c59 100644 --- a/src/test/unittests/unittest/unittest.run.test.ts +++ b/src/test/unittests/unittest/unittest.run.test.ts @@ -9,7 +9,8 @@ import { instance, mock } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IProcessServiceFactory } from '../../../client/common/process/types'; -import { ICondaService } from '../../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; +import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; import { ArgumentsHelper } from '../../../client/unittests/common/argumentsHelper'; import { CommandSource, UNITTEST_PROVIDER } from '../../../client/unittests/common/constants'; @@ -85,6 +86,7 @@ suite('Unit Tests - unittest - run with mocked process output', () => { ioc.serviceManager.add(ITestRunner, TestRunner); ioc.serviceManager.add(IUnitTestHelper, UnitTestHelper); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function ignoreTestLauncher() { From 34a67799c6f979cfc6997a72767f96e2172de9ce Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 11:10:50 -0800 Subject: [PATCH 39/49] Fixed tests --- src/test/common/installer.test.ts | 5 +++++ src/test/common/moduleInstaller.test.ts | 6 +++++- src/test/format/extension.format.test.ts | 7 +++++++ src/test/format/extension.sort.test.ts | 4 ++++ .../refactor/extension.refactor.extract.method.test.ts | 4 ++++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 34bac00b6ca4..92052bf784c0 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -1,4 +1,5 @@ import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Uri } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; @@ -14,6 +15,8 @@ import { PersistentStateFactory } from '../../client/common/persistentState'; import { PathUtils } from '../../client/common/platform/pathUtils'; import { CurrentProcess } from '../../client/common/process/currentProcess'; import { IProcessServiceFactory } from '../../client/common/process/types'; +import { TerminalHelper } from '../../client/common/terminal/helper'; +import { ITerminalHelper } from '../../client/common/terminal/types'; import { IConfigurationService, ICurrentProcess, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, IsWindows, ModuleNamePurpose, Product, ProductType } from '../../client/common/types'; import { createDeferred } from '../../client/common/utils/async'; import { getNamesAndValues } from '../../client/common/utils/enum'; @@ -94,6 +97,7 @@ suite('Installer', () => { test(`Ensure isInstalled for Product: '${prod.name}' executes the right command`, async () => { ioc.serviceManager.addSingletonInstance(IModuleInstaller, new MockModuleInstaller('one', false)); ioc.serviceManager.addSingletonInstance(IModuleInstaller, new MockModuleInstaller('two', true)); + ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); if (prod.value === Product.ctags || prod.value === Product.unittest || prod.value === Product.isort) { return; } @@ -120,6 +124,7 @@ suite('Installer', () => { test(`Ensure install for Product: '${prod.name}' executes the right command in IModuleInstaller`, async () => { ioc.serviceManager.addSingletonInstance(IModuleInstaller, new MockModuleInstaller('one', false)); ioc.serviceManager.addSingletonInstance(IModuleInstaller, new MockModuleInstaller('two', true)); + ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); if (prod.value === Product.unittest || prod.value === Product.ctags || prod.value === Product.isort) { return; } diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 1aee5db08f7c..d745b327d3b7 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import * as path from 'path'; import { SemVer } from 'semver'; +import { instance, mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; @@ -20,7 +21,8 @@ import { PlatformService } from '../../client/common/platform/platformService'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { CurrentProcess } from '../../client/common/process/currentProcess'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; -import { ITerminalService, ITerminalServiceFactory } from '../../client/common/terminal/types'; +import { TerminalHelper } from '../../client/common/terminal/helper'; +import { ITerminalHelper, ITerminalService, ITerminalServiceFactory } from '../../client/common/terminal/types'; import { IConfigurationService, ICurrentProcess, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, IPythonSettings, IsWindows } from '../../client/common/types'; import { Architecture } from '../../client/common/utils/platform'; import { ICondaService, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType, PIPENV_SERVICE, PythonInterpreter } from '../../client/interpreter/contracts'; @@ -132,6 +134,7 @@ suite('Module Installer', () => { mockInterpreterLocator.setup(p => p.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator.object, INTERPRETER_LOCATOR_SERVICE); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, TypeMoq.Mock.ofType().object, PIPENV_SERVICE); + ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); const processService = await ioc.serviceContainer.get(IProcessServiceFactory).create() as MockProcessService; processService.onExec((file, args, options, callback) => { @@ -167,6 +170,7 @@ suite('Module Installer', () => { ])); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator.object, INTERPRETER_LOCATOR_SERVICE); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, TypeMoq.Mock.ofType().object, PIPENV_SERVICE); + ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); const processService = await ioc.serviceContainer.get(IProcessServiceFactory).create() as MockProcessService; processService.onExec((file, args, options, callback) => { diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index addb7176f1b6..bb73de2ecce1 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -4,15 +4,20 @@ import * as fs from 'fs-extra'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { CancellationTokenSource, Position, Uri, window, workspace } from 'vscode'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; +import { TerminalHelper } from '../../client/common/terminal/helper'; +import { ITerminalHelper } from '../../client/common/terminal/types'; import { AutoPep8Formatter } from '../../client/formatters/autoPep8Formatter'; import { BlackFormatter } from '../../client/formatters/blackFormatter'; import { YapfFormatter } from '../../client/formatters/yapfFormatter'; +import { ICondaService } from '../../client/interpreter/contracts'; +import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { isPythonVersionInProcess } from '../common'; import { closeActiveWindows, initialize, initializeTest } from '../initialize'; import { MockProcessService } from '../mocks/proc'; @@ -97,6 +102,8 @@ suite('Formatting - General', () => { // Mocks. ioc.registerMockProcessTypes(); + ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function injectFormatOutput(outputFileName: string) { diff --git a/src/test/format/extension.sort.test.ts b/src/test/format/extension.sort.test.ts index c83c27b00458..16dc8980ea2a 100644 --- a/src/test/format/extension.sort.test.ts +++ b/src/test/format/extension.sort.test.ts @@ -3,8 +3,11 @@ import { expect } from 'chai'; import * as fs from 'fs'; import { EOL } from 'os'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { commands, ConfigurationTarget, Position, Range, Uri, window, workspace } from 'vscode'; import { Commands } from '../../client/common/constants'; +import { ICondaService } from '../../client/interpreter/contracts'; +import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { SortImportsEditingProvider } from '../../client/providers/importSortProvider'; import { ISortImportsEditingProvider } from '../../client/providers/types'; import { updateSetting } from '../common'; @@ -51,6 +54,7 @@ suite('Sorting', () => { ioc.registerCommonTypes(); ioc.registerVariableTypes(); ioc.registerProcessTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } test('Without Config', async () => { const textDocument = await workspace.openTextDocument(fileToFormatWithoutConfig); diff --git a/src/test/refactor/extension.refactor.extract.method.test.ts b/src/test/refactor/extension.refactor.extract.method.test.ts index d475f41fa0c5..50db1fbb97fb 100644 --- a/src/test/refactor/extension.refactor.extract.method.test.ts +++ b/src/test/refactor/extension.refactor.extract.method.test.ts @@ -3,8 +3,11 @@ import * as assert from 'assert'; import * as fs from 'fs-extra'; import * as path from 'path'; +import { instance, mock } from 'ts-mockito'; import { commands, Position, Range, Selection, TextEditorCursorStyle, TextEditorLineNumbersStyle, TextEditorOptions, Uri, window, workspace } from 'vscode'; import { getTextEditsFromPatch } from '../../client/common/editor'; +import { ICondaService } from '../../client/interpreter/contracts'; +import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { extractMethod } from '../../client/providers/simpleRefactorProvider'; import { RefactorProxy } from '../../client/refactor/proxy'; import { getExtensionSettings } from '../common'; @@ -50,6 +53,7 @@ suite('Method Extraction', () => { ioc.registerCommonTypes(); ioc.registerProcessTypes(); ioc.registerVariableTypes(); + ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); } async function testingMethodExtraction(shouldError: boolean, startPos: Position, endPos: Position): Promise { From 77adb2b8fc9953e02575ea6d4dd9e120b4edaaea Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 11:36:11 -0800 Subject: [PATCH 40/49] Address review comments --- pythonFiles/printEnvVariables.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pythonFiles/printEnvVariables.py b/pythonFiles/printEnvVariables.py index 3a5a1e47d7ae..353149f237de 100644 --- a/pythonFiles/printEnvVariables.py +++ b/pythonFiles/printEnvVariables.py @@ -4,5 +4,4 @@ import os import json -data = {item:value for item, value in os.environ.items()} -print(json.dumps(data)) +print(json.dumps(dict(os.environ))) From 3f5268fb7822e0e163fd0bf8c262bac6deff7324 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Thu, 10 Jan 2019 11:36:32 -0800 Subject: [PATCH 41/49] Update news/3 Code Health/3746.md Co-Authored-By: DonJayamanne --- news/3 Code Health/3746.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/3 Code Health/3746.md b/news/3 Code Health/3746.md index 077d28075ad2..2c95d4fc1318 100644 --- a/news/3 Code Health/3746.md +++ b/news/3 Code Health/3746.md @@ -1 +1 @@ -Add support to detect usage of `xonsh` shells. +Detect usage of `xonsh` shells (this does **not** add support for `xonsh` itself) From eeac0268c959da73250eb4210f88babbe407bd6d Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Thu, 10 Jan 2019 11:36:46 -0800 Subject: [PATCH 42/49] Update news/2 Fixes/3330.md Co-Authored-By: DonJayamanne --- news/2 Fixes/3330.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/2 Fixes/3330.md b/news/2 Fixes/3330.md index aa1077cc6a21..6bb877d4f62c 100644 --- a/news/2 Fixes/3330.md +++ b/news/2 Fixes/3330.md @@ -1 +1 @@ -Activate `Python Environment` when running unit tests. +Activate any selected Python Environment when running unit tests. From ac0a360b7ce36178b90832ffc9257c54556a4867 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 11:42:23 -0800 Subject: [PATCH 43/49] Fix tests --- src/test/format/extension.format.test.ts | 5 +- .../format/extension.formatOnSave.test.ts | 102 ------------------ src/test/format/extension.sort.test.ts | 4 +- .../extension.refactor.extract.method.test.ts | 4 +- src/test/serviceRegistry.ts | 8 +- .../unittests/nosetest/nosetest.run.test.ts | 4 +- src/test/unittests/nosetest/nosetest.test.ts | 4 +- src/test/unittests/pytest/pytest.run.test.ts | 4 +- src/test/unittests/pytest/pytest.test.ts | 4 +- src/test/unittests/unittest/unittest.test.ts | 7 +- 10 files changed, 29 insertions(+), 117 deletions(-) delete mode 100644 src/test/format/extension.formatOnSave.test.ts diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index bb73de2ecce1..7c17f97c709f 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -11,8 +11,6 @@ import { import { IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; -import { TerminalHelper } from '../../client/common/terminal/helper'; -import { ITerminalHelper } from '../../client/common/terminal/types'; import { AutoPep8Formatter } from '../../client/formatters/autoPep8Formatter'; import { BlackFormatter } from '../../client/formatters/blackFormatter'; import { YapfFormatter } from '../../client/formatters/yapfFormatter'; @@ -102,8 +100,7 @@ suite('Formatting - General', () => { // Mocks. ioc.registerMockProcessTypes(); - ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); - ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingleton(ICondaService, CondaService); } async function injectFormatOutput(outputFileName: string) { diff --git a/src/test/format/extension.formatOnSave.test.ts b/src/test/format/extension.formatOnSave.test.ts deleted file mode 100644 index 0508a1a8474e..000000000000 --- a/src/test/format/extension.formatOnSave.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { expect } from 'chai'; -import * as fs from 'fs-extra'; -import * as path from 'path'; -import * as TypeMoq from 'typemoq'; -import * as vscode from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; -import { IConfigurationService } from '../../client/common/types'; -import { PythonFormattingEditProvider } from '../../client/providers/formatProvider'; -import { getExtensionSettings } from '../common'; -import { closeActiveWindows } from '../initialize'; -import { UnitTestIocContainer } from '../unittests/serviceRegistry'; - -const formatFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting'); -const unformattedFile = path.join(formatFilesPath, 'fileToFormat.py'); - -suite('Formating On Save', () => { - let ioc: UnitTestIocContainer; - let config: TypeMoq.IMock; - let editorConfig: TypeMoq.IMock; - let workspace: TypeMoq.IMock; - let documentManager: TypeMoq.IMock; - let commands: TypeMoq.IMock; - let options: TypeMoq.IMock; - let listener: (d: vscode.TextDocument) => Promise; - - setup(initializeDI); - suiteTeardown(async () => { - await closeActiveWindows(); - }); - teardown(async () => { - await ioc.dispose(); - await closeActiveWindows(); - }); - - function initializeDI() { - ioc = new UnitTestIocContainer(); - ioc.registerFormatterTypes(); - ioc.registerFileSystemTypes(); - ioc.registerProcessTypes(); - ioc.registerVariableTypes(); - ioc.registerMockProcess(); - - config = TypeMoq.Mock.ofType(); - config.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => getExtensionSettings(undefined)); - - editorConfig = TypeMoq.Mock.ofType(); - - workspace = TypeMoq.Mock.ofType(); - workspace.setup(x => x.getConfiguration('editor', TypeMoq.It.isAny())).returns(() => editorConfig.object); - - const event = TypeMoq.Mock.ofType>(); - event.setup(x => x(TypeMoq.It.isAny())).callback((s) => { - listener = s as ((d: vscode.TextDocument) => Promise); - // tslint:disable-next-line:no-empty - }).returns(() => new vscode.Disposable(() => { })); - - documentManager = TypeMoq.Mock.ofType(); - documentManager.setup(x => x.onDidSaveTextDocument).returns(() => event.object); - - options = TypeMoq.Mock.ofType(); - options.setup(x => x.insertSpaces).returns(() => true); - options.setup(x => x.tabSize).returns(() => 4); - - commands = TypeMoq.Mock.ofType(); - commands.setup(x => x.executeCommand('editor.action.formatDocument')).returns(() => - new Promise((resolve, reject) => resolve()) - ); - ioc.serviceManager.addSingletonInstance(IConfigurationService, config.object); - ioc.serviceManager.addSingletonInstance(ICommandManager, commands.object); - ioc.serviceManager.addSingletonInstance(IWorkspaceService, workspace.object); - ioc.serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); - } - - test('Workaround VS Code 41194', async () => { - editorConfig.setup(x => x.get('formatOnSave')).returns(() => true); - - const content = await fs.readFile(unformattedFile, 'utf8'); - let version = 1; - - const document = TypeMoq.Mock.ofType(); - document.setup(x => x.getText()).returns(() => content); - document.setup(x => x.uri).returns(() => vscode.Uri.file(unformattedFile)); - document.setup(x => x.isDirty).returns(() => false); - document.setup(x => x.fileName).returns(() => unformattedFile); - document.setup(x => x.save()).callback(() => version += 1); - document.setup(x => x.version).returns(() => version); - - const context = TypeMoq.Mock.ofType(); - const provider = new PythonFormattingEditProvider(context.object, ioc.serviceContainer); - const edits = await provider.provideDocumentFormattingEdits(document.object, options.object, new vscode.CancellationTokenSource().token); - expect(edits.length).be.greaterThan(0, 'Formatter produced no edits'); - - await listener(document.object); - await new Promise((resolve, reject) => setTimeout(resolve, 500)); - - commands.verify(x => x.executeCommand('editor.action.formatDocument'), TypeMoq.Times.once()); - document.verify(x => x.save(), TypeMoq.Times.once()); - }); -}); diff --git a/src/test/format/extension.sort.test.ts b/src/test/format/extension.sort.test.ts index 16dc8980ea2a..6744ab6de1ec 100644 --- a/src/test/format/extension.sort.test.ts +++ b/src/test/format/extension.sort.test.ts @@ -6,7 +6,8 @@ import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import { commands, ConfigurationTarget, Position, Range, Uri, window, workspace } from 'vscode'; import { Commands } from '../../client/common/constants'; -import { ICondaService } from '../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; +import { InterpreterService } from '../../client/interpreter/interpreterService'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { SortImportsEditingProvider } from '../../client/providers/importSortProvider'; import { ISortImportsEditingProvider } from '../../client/providers/types'; @@ -55,6 +56,7 @@ suite('Sorting', () => { ioc.registerVariableTypes(); ioc.registerProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } test('Without Config', async () => { const textDocument = await workspace.openTextDocument(fileToFormatWithoutConfig); diff --git a/src/test/refactor/extension.refactor.extract.method.test.ts b/src/test/refactor/extension.refactor.extract.method.test.ts index 50db1fbb97fb..0ba255b3bc01 100644 --- a/src/test/refactor/extension.refactor.extract.method.test.ts +++ b/src/test/refactor/extension.refactor.extract.method.test.ts @@ -6,7 +6,8 @@ import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import { commands, Position, Range, Selection, TextEditorCursorStyle, TextEditorLineNumbersStyle, TextEditorOptions, Uri, window, workspace } from 'vscode'; import { getTextEditsFromPatch } from '../../client/common/editor'; -import { ICondaService } from '../../client/interpreter/contracts'; +import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts'; +import { InterpreterService } from '../../client/interpreter/interpreterService'; import { CondaService } from '../../client/interpreter/locators/services/condaService'; import { extractMethod } from '../../client/providers/simpleRefactorProvider'; import { RefactorProxy } from '../../client/refactor/proxy'; @@ -54,6 +55,7 @@ suite('Method Extraction', () => { ioc.registerProcessTypes(); ioc.registerVariableTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); + ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); } async function testingMethodExtraction(shouldError: boolean, startPos: Position, endPos: Position): Promise { diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 1ce925256136..61fe09344b1f 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { Container } from 'inversify'; +import { anything, instance, mock, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, Memento, OutputChannel } from 'vscode'; import { STANDARD_OUTPUT_CHANNEL } from '../client/common/constants'; @@ -89,7 +90,9 @@ export class IocContainer { } public registerProcessTypes() { processRegisterTypes(this.serviceManager); - this.serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); + const mockEnvironmentActivationService = mock(EnvironmentActivationService); + when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); + this.serviceManager.addSingletonInstance(IEnvironmentActivationService, instance(mockEnvironmentActivationService)); } public registerVariableTypes() { variableRegisterTypes(this.serviceManager); @@ -119,6 +122,9 @@ export class IocContainer { this.serviceManager.addSingleton(IPythonExecutionFactory, PythonExecutionFactory); this.serviceManager.addSingleton(IPythonToolExecutionService, PythonToolExecutionService); this.serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); + const mockEnvironmentActivationService = mock(EnvironmentActivationService); + when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); + this.serviceManager.rebindInstance(IEnvironmentActivationService, instance(mockEnvironmentActivationService)); } public registerMockProcess() { diff --git a/src/test/unittests/nosetest/nosetest.run.test.ts b/src/test/unittests/nosetest/nosetest.run.test.ts index 11531162504e..e3b15ed6ec19 100644 --- a/src/test/unittests/nosetest/nosetest.run.test.ts +++ b/src/test/unittests/nosetest/nosetest.run.test.ts @@ -63,8 +63,8 @@ suite('Unit Tests - nose - run against actual python process', () => { ioc.registerVariableTypes(); ioc.registerMockProcessTypes(); - ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); - ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); + ioc.serviceManager.addSingleton(ICondaService, CondaService); + ioc.serviceManager.addSingleton(IInterpreterService, InterpreterService); } async function injectTestDiscoveryOutput(outputFileName: string) { diff --git a/src/test/unittests/nosetest/nosetest.test.ts b/src/test/unittests/nosetest/nosetest.test.ts index 9e92dac676aa..28898f0843a6 100644 --- a/src/test/unittests/nosetest/nosetest.test.ts +++ b/src/test/unittests/nosetest/nosetest.test.ts @@ -58,8 +58,8 @@ suite('Unit Tests - nose - discovery against actual python process', () => { ioc.registerProcessTypes(); ioc.registerUnitTestTypes(); ioc.registerVariableTypes(); - ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); - ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); + ioc.serviceManager.addSingleton(ICondaService, CondaService); + ioc.serviceManager.addSingleton(IInterpreterService, InterpreterService); } test('Discover Tests (single test file)', async () => { diff --git a/src/test/unittests/pytest/pytest.run.test.ts b/src/test/unittests/pytest/pytest.run.test.ts index be18f0aa3787..1b149451b22f 100644 --- a/src/test/unittests/pytest/pytest.run.test.ts +++ b/src/test/unittests/pytest/pytest.run.test.ts @@ -4,7 +4,7 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; -import { instance, mock } from 'ts-mockito'; +import { instance, mock, anything } from 'ts-mockito'; import * as vscode from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IFileSystem } from '../../../client/common/platform/types'; @@ -20,6 +20,8 @@ import { MockProcessService } from '../../mocks/proc'; import { UnitTestIocContainer } from '../serviceRegistry'; import { initialize, initializeTest, IS_MULTI_ROOT_TEST } from './../../initialize'; import { ITestDetails, ITestScenarioDetails, testScenarios } from './pytest_run_tests_data'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; const UNITTEST_TEST_FILES_PATH = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'pythonFiles', 'testFiles', 'standard'); const PYTEST_RESULTS_PATH = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'pythonFiles', 'testFiles', 'pytestFiles', 'results'); diff --git a/src/test/unittests/pytest/pytest.test.ts b/src/test/unittests/pytest/pytest.test.ts index 2ab281d1d41c..5841dacc4def 100644 --- a/src/test/unittests/pytest/pytest.test.ts +++ b/src/test/unittests/pytest/pytest.test.ts @@ -37,8 +37,8 @@ suite('Unit Tests - pytest - discovery against actual python process', () => { ioc.registerProcessTypes(); ioc.registerUnitTestTypes(); ioc.registerVariableTypes(); - ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); - ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); + ioc.serviceManager.addSingleton(ICondaService, CondaService); + ioc.serviceManager.addSingleton(IInterpreterService, InterpreterService); } test('Discover Tests (single test file)', async () => { diff --git a/src/test/unittests/unittest/unittest.test.ts b/src/test/unittests/unittest/unittest.test.ts index e4dec2244552..9a01669e7e39 100644 --- a/src/test/unittests/unittest/unittest.test.ts +++ b/src/test/unittests/unittest/unittest.test.ts @@ -3,9 +3,11 @@ import * as assert from 'assert'; import * as fs from 'fs-extra'; import * as path from 'path'; -import { instance, mock } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { CondaService } from '../../../client/interpreter/locators/services/condaService'; @@ -68,6 +70,9 @@ suite('Unit Tests - unittest - discovery against actual python process', () => { ioc.registerProcessTypes(); ioc.serviceManager.addSingletonInstance(ICondaService, instance(mock(CondaService))); ioc.serviceManager.addSingletonInstance(IInterpreterService, instance(mock(InterpreterService))); + const mockEnvironmentActivationService = mock(EnvironmentActivationService); + when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); + ioc.serviceManager.rebindInstance(IEnvironmentActivationService, instance(mockEnvironmentActivationService)); } test('Discover Tests (single test file)', async () => { From 1d22e7b2ca7e81fd4a2d6dbc916829a957959c86 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 11:44:58 -0800 Subject: [PATCH 44/49] More fixes --- src/test/refactor/rename.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index 47ff7ee9a5a5..eabb57f0f8eb 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -39,12 +39,13 @@ suite('Refactor Rename', () => { configService.setup(c => c.getSettings(typeMoq.It.isAny())).returns(() => pythonSettings.object); const processServiceFactory = typeMoq.Mock.ofType(); processServiceFactory.setup(p => p.create(typeMoq.It.isAny())).returns(() => Promise.resolve(new ProcessService(new BufferDecoder()))); - + const envActivationService = typeMoq.Mock.ofType(); + envActivationService.setup(e => e.getActivatedEnvironmentVariables(typeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); serviceContainer = typeMoq.Mock.ofType(); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IConfigurationService), typeMoq.It.isAny())).returns(() => configService.object); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); serviceContainer.setup(s => s.get(typeMoq.It.isValue(IEnvironmentActivationService), typeMoq.It.isAny())) - .returns(() => typeMoq.Mock.ofType().object); + .returns(() => envActivationService.object); serviceContainer .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any, From 2fcd998cd7bef81b1c00c79f6936ec7b62d8cbbf Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 13:02:20 -0800 Subject: [PATCH 45/49] Fixes --- src/test/common/process/execFactory.test.ts | 6 ++++++ .../common/process/pythonProc.simple.multiroot.test.ts | 7 +++++++ src/test/refactor/rename.test.ts | 5 +++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/test/common/process/execFactory.test.ts b/src/test/common/process/execFactory.test.ts index 21eea8523d86..f7c1c525a161 100644 --- a/src/test/common/process/execFactory.test.ts +++ b/src/test/common/process/execFactory.test.ts @@ -10,6 +10,7 @@ import { IFileSystem } from '../../../client/common/platform/types'; import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types'; import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { InterpreterVersionService } from '../../../client/interpreter/interpreterVersion'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -34,6 +35,11 @@ suite('PythonExecutableService', () => { procServiceFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(procService.object)); envVarsProvider.setup(v => v.getEnvironmentVariables(TypeMoq.It.isAny())).returns(() => Promise.resolve({})); + const envActivationService = TypeMoq.Mock.ofType(); + envActivationService.setup(e => e.getActivatedEnvironmentVariables(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(undefined)); + serviceContainer.setup(s => s.get(TypeMoq.It.isValue(IEnvironmentActivationService), TypeMoq.It.isAny())) + .returns(() => envActivationService.object); }); test('Ensure resource is used when getting configuration service settings (undefined resource)', async () => { const pythonPath = `Python_Path_${new Date().toString()}`; diff --git a/src/test/common/process/pythonProc.simple.multiroot.test.ts b/src/test/common/process/pythonProc.simple.multiroot.test.ts index 0ae91f6b7632..d2503712eeb9 100644 --- a/src/test/common/process/pythonProc.simple.multiroot.test.ts +++ b/src/test/common/process/pythonProc.simple.multiroot.test.ts @@ -10,6 +10,7 @@ import * as fs from 'fs-extra'; import { Container } from 'inversify'; import { EOL } from 'os'; import * as path from 'path'; +import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget, Disposable, Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; @@ -30,6 +31,8 @@ import { OSType } from '../../../client/common/utils/platform'; import { registerTypes as variablesRegisterTypes } from '../../../client/common/variables/serviceRegistry'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { IInterpreterAutoSelectionService, IInterpreterAutoSeletionProxyService } from '../../../client/interpreter/autoSelection/types'; import { ServiceContainer } from '../../../client/ioc/container'; import { ServiceManager } from '../../../client/ioc/serviceManager'; @@ -85,6 +88,10 @@ suite('PythonExecutableService', () => { processRegisterTypes(serviceManager); variablesRegisterTypes(serviceManager); + const mockEnvironmentActivationService = mock(EnvironmentActivationService); + when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); + serviceManager.addSingletonInstance(IEnvironmentActivationService, instance(mockEnvironmentActivationService)); + configService = serviceManager.get(IConfigurationService); pythonExecFactory = serviceContainer.get(IPythonExecutionFactory); diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index eabb57f0f8eb..f08788148f22 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -48,8 +48,9 @@ suite('Refactor Rename', () => { .returns(() => envActivationService.object); serviceContainer .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) - .returns(() => new PythonExecutionFactory(serviceContainer.object, undefined as any, undefined as any, - undefined as any, undefined as any)); + .returns(() => new PythonExecutionFactory(serviceContainer.object, + undefined as any, undefined as any, + configService.object, undefined as any)); await initializeTest(); }); teardown(closeActiveWindows); From 66bd123536b33657e9d96c77e775c9e14ae50f58 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 13:07:10 -0800 Subject: [PATCH 46/49] Fixed tests --- .../common/variables/envVarsProvider.multiroot.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 7f0cb344d925..8973db98157e 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -6,15 +6,19 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as fs from 'fs-extra'; import { EOL } from 'os'; import * as path from 'path'; +import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { IS_WINDOWS, NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../../../client/common/platform/constants'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IDisposableRegistry, IPathUtils } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; +import { clearCache } from '../../../client/common/utils/cacheUtils'; import { EnvironmentVariablesService } from '../../../client/common/variables/environment'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; import { EnvironmentVariables } from '../../../client/common/variables/types'; +import { EnvironmentActivationService } from '../../../client/interpreter/activation/service'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { IInterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection/types'; import { clearPythonPathInWorkspaceFolder, updateSetting } from '../../common'; import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST } from '../../initialize'; @@ -46,6 +50,10 @@ suite('Multiroot Environment Variables Provider', () => { ioc.registerCommonTypes(); ioc.registerVariableTypes(); ioc.registerProcessTypes(); + const mockEnvironmentActivationService = mock(EnvironmentActivationService); + when(mockEnvironmentActivationService.getActivatedEnvironmentVariables(anything())).thenResolve(); + ioc.serviceManager.rebindInstance(IEnvironmentActivationService, instance(mockEnvironmentActivationService)); + clearCache(); return initializeTest(); }); suiteTeardown(closeActiveWindows); @@ -55,6 +63,7 @@ suite('Multiroot Environment Variables Provider', () => { await clearPythonPathInWorkspaceFolder(workspace4Path); await updateSetting('envFile', undefined, workspace4PyFile, ConfigurationTarget.WorkspaceFolder); await initializeTest(); + clearCache(); }); function getVariablesProvider(mockVariables: EnvironmentVariables = { ...process.env }) { From c8de453c706af1b216e7dcbe954d99e883b2cf3c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 13:25:59 -0800 Subject: [PATCH 47/49] Fixes --- src/test/refactor/rename.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts index f08788148f22..6459701abc8e 100644 --- a/src/test/refactor/rename.test.ts +++ b/src/test/refactor/rename.test.ts @@ -49,7 +49,7 @@ suite('Refactor Rename', () => { serviceContainer .setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())) .returns(() => new PythonExecutionFactory(serviceContainer.object, - undefined as any, undefined as any, + undefined as any, processServiceFactory.object, configService.object, undefined as any)); await initializeTest(); }); From 58ab538a255c43264a0119f736c5ae6e9e975aed Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 14:35:33 -0800 Subject: [PATCH 48/49] Fix tests --- .../variables/environmentVariablesProvider.ts | 26 ++++++++++++++----- .../envVarsProvider.multiroot.test.ts | 5 +++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index 4587e1d3ae60..7c9fe9b7880f 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -2,9 +2,10 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode'; +import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace, ConfigurationChangeEvent } from 'vscode'; +import { IWorkspaceService } from '../application/types'; import { IPlatformService } from '../platform/types'; -import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types'; +import { IConfigurationService, ICurrentProcess, IDisposableRegistry, Resource } from '../types'; import { cacheResourceSpecificInterpreterData, clearCachedResourceSpecificIngterpreterData } from '../utils/decorators'; import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; @@ -14,13 +15,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid private fileWatchers = new Map(); private disposables: Disposable[] = []; private changeEventEmitter: EventEmitter; + private trackedWorkspaceFolders = new Set(); constructor(@inject(IEnvironmentVariablesService) private envVarsService: IEnvironmentVariablesService, @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IPlatformService) private platformService: IPlatformService, + @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ICurrentProcess) private process: ICurrentProcess) { disposableRegistry.push(this); this.changeEventEmitter = new EventEmitter(); + const disposable = this.workspaceService.onDidChangeConfiguration(this.configurationChanged, this); + this.disposables.push(disposable); } public get onDidEnvironmentVariablesChange(): Event { @@ -37,6 +42,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid public async getEnvironmentVariables(resource?: Uri): Promise { const settings = this.configurationService.getSettings(resource); const workspaceFolderUri = this.getWorkspaceFolderUri(resource); + this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : ''); this.createFileWatcher(settings.envFile, workspaceFolderUri); let mergedVars = await this.envVarsService.parseFile(settings.envFile); if (!mergedVars) { @@ -53,6 +59,14 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid } return mergedVars; } + protected configurationChanged(e: ConfigurationChangeEvent) { + this.trackedWorkspaceFolders.forEach(item => { + const uri = item && item.length > 0 ? Uri.file(item) : undefined; + if (e.affectsConfiguration('python.envFile', uri)) { + this.onEnvironmentFileChanged(uri); + } + }); + } private getWorkspaceFolderUri(resource?: Uri): Uri | undefined { if (!resource) { return; @@ -67,12 +81,12 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid const envFileWatcher = workspace.createFileSystemWatcher(envFile); this.fileWatchers.set(envFile, envFileWatcher); if (envFileWatcher) { - this.disposables.push(envFileWatcher.onDidChange(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri))); - this.disposables.push(envFileWatcher.onDidCreate(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri))); - this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri))); + this.disposables.push(envFileWatcher.onDidChange(() => this.onEnvironmentFileChanged(workspaceFolderUri))); + this.disposables.push(envFileWatcher.onDidCreate(() => this.onEnvironmentFileChanged(workspaceFolderUri))); + this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(workspaceFolderUri))); } } - private onEnvironmentFileChanged(envFile, workspaceFolderUri?: Uri) { + private onEnvironmentFileChanged(workspaceFolderUri?: Uri) { clearCachedResourceSpecificIngterpreterData('getEnvironmentVariables', workspaceFolderUri); clearCachedResourceSpecificIngterpreterData('CustomEnvironmentVariables', workspaceFolderUri); this.changeEventEmitter.fire(workspaceFolderUri); diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 8973db98157e..7b7c61b44f3e 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -8,6 +8,7 @@ import { EOL } from 'os'; import * as path from 'path'; import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode'; +import { WorkspaceService } from '../../../client/common/application/workspace'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { IS_WINDOWS, NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../../../client/common/platform/constants'; import { PlatformService } from '../../../client/common/platform/platformService'; @@ -73,7 +74,9 @@ suite('Multiroot Environment Variables Provider', () => { const disposables = ioc.serviceContainer.get(IDisposableRegistry); ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService()); const cfgService = new ConfigurationService(ioc.serviceContainer); - return new EnvironmentVariablesProvider(variablesService, disposables, new PlatformService(), cfgService, mockProcess); + const workspaceService = new WorkspaceService(); + return new EnvironmentVariablesProvider(variablesService, disposables, + new PlatformService(), workspaceService, cfgService, mockProcess); } test('Custom variables should not be undefined without an env file', async () => { From 1b2975616ffd7370901553f4a71aaf148dc744fc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 10 Jan 2019 14:36:36 -0800 Subject: [PATCH 49/49] Fix tests --- src/test/common/process/pythonProc.simple.multiroot.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/common/process/pythonProc.simple.multiroot.test.ts b/src/test/common/process/pythonProc.simple.multiroot.test.ts index d2503712eeb9..4b7a93fbc79b 100644 --- a/src/test/common/process/pythonProc.simple.multiroot.test.ts +++ b/src/test/common/process/pythonProc.simple.multiroot.test.ts @@ -27,6 +27,7 @@ import { IConfigurationService, ICurrentProcess, IDisposableRegistry, IPathUtils, IsWindows } from '../../../client/common/types'; +import { clearCache } from '../../../client/common/utils/cacheUtils'; import { OSType } from '../../../client/common/utils/platform'; import { registerTypes as variablesRegisterTypes @@ -96,6 +97,7 @@ suite('PythonExecutableService', () => { pythonExecFactory = serviceContainer.get(IPythonExecutionFactory); await configService.updateSetting('envFile', undefined, workspace4PyFile, ConfigurationTarget.WorkspaceFolder); + clearCache(); return initializeTest(); }); suiteTeardown(closeActiveWindows); @@ -106,6 +108,7 @@ suite('PythonExecutableService', () => { await clearPythonPathInWorkspaceFolder(workspace4Path); await configService.updateSetting('envFile', undefined, workspace4PyFile, ConfigurationTarget.WorkspaceFolder); await initializeTest(); + clearCache(); }); test('Importing without a valid PYTHONPATH should fail', async () => {