From 6cc121554f375370517bdb4c18dd13315c1e9f9c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 4 Feb 2021 12:05:01 -0800 Subject: [PATCH 1/2] Correct pipenv activation provider when in discovery experiment --- .../pipEnvActivationProvider.ts | 24 +++++++++++++------ src/client/pythonEnvironments/legacyIOC.ts | 8 +------ .../pipEnvActivationProvider.unit.test.ts | 8 ++++++- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts index 1589bd43ce54..39d5bf053be7 100644 --- a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -7,10 +7,12 @@ import { inject, injectable, named } from 'inversify'; import { Uri } from 'vscode'; import '../../../common/extensions'; import { IInterpreterService } from '../../../interpreter/contracts'; +import { isPipenvEnvironmentRelatedToFolder } from '../../../pythonEnvironments/discovery/locators/services/pipEnvHelper'; import { EnvironmentType } from '../../../pythonEnvironments/info'; import { IWorkspaceService } from '../../application/types'; +import { inDiscoveryExperiment } from '../../experiments/helpers'; import { IFileSystem } from '../../platform/types'; -import { IToolExecutionPath, ToolExecutionPath } from '../../types'; +import { IExperimentService, IToolExecutionPath, ToolExecutionPath } from '../../types'; import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; @injectable() @@ -22,6 +24,7 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma private readonly pipEnvExecution: IToolExecutionPath, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IFileSystem) private readonly fs: IFileSystem, + @inject(IExperimentService) private readonly experimentService: IExperimentService, ) {} public isShellSupported(_targetShell: TerminalShellType): boolean { @@ -35,12 +38,19 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma } // Activate using `pipenv shell` only if the current folder relates pipenv environment. const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; - if ( - workspaceFolder && - interpreter.pipEnvWorkspaceFolder && - !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) - ) { - return; + if (workspaceFolder) { + if (await inDiscoveryExperiment(this.experimentService)) { + if (!(await isPipenvEnvironmentRelatedToFolder(interpreter.path, workspaceFolder?.uri.fsPath))) { + return; + } + } else { + if ( + interpreter.pipEnvWorkspaceFolder && + !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) + ) { + return; + } + } } const execName = this.pipEnvExecution.executable; return [`${execName.fileToCommandArgument()} shell`]; diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index bcd5fc5ca73b..5777097f3cfb 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -85,7 +85,7 @@ const convertedKinds = new Map( ); function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment { - const { name, location, executable, arch, kind, searchLocation, version, distro } = info; + const { name, location, executable, arch, kind, version, distro } = info; const { filename, sysPrefix } = executable; const env: PythonEnvironment = { sysPrefix, @@ -102,12 +102,6 @@ function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment { } // Otherwise it stays Unknown. - if (searchLocation !== undefined) { - if (kind === PythonEnvKind.Pipenv) { - env.pipEnvWorkspaceFolder = searchLocation.fsPath; - } - } - if (version !== undefined) { const { release, sysVersion } = version; if (release === undefined) { diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index 1e0e5d506043..e9686dd65303 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -9,11 +9,12 @@ import * as TypeMoq from 'typemoq'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../../../client/common/application/types'; import { WorkspaceService } from '../../../../client/common/application/workspace'; +import { DiscoveryVariants } from '../../../../client/common/experiments/groups'; import { FileSystem } from '../../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../../client/common/platform/types'; import { PipEnvActivationCommandProvider } from '../../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; import { ITerminalActivationCommandProvider, TerminalShellType } from '../../../../client/common/terminal/types'; -import { IToolExecutionPath } from '../../../../client/common/types'; +import { IExperimentService, IToolExecutionPath } from '../../../../client/common/types'; import { getNamesAndValues } from '../../../../client/common/utils/enum'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { InterpreterService } from '../../../../client/interpreter/interpreterService'; @@ -28,17 +29,22 @@ suite('Terminals Activation - Pipenv', () => { let pipEnvExecution: TypeMoq.IMock; let workspaceService: IWorkspaceService; let fs: IFileSystem; + let experimentService: IExperimentService; setup(() => { interpreterService = mock(InterpreterService); fs = mock(FileSystem); workspaceService = mock(WorkspaceService); interpreterService = mock(InterpreterService); + experimentService = mock(); + when(experimentService.inExperiment(DiscoveryVariants.discoverWithFileWatching)).thenResolve(false); + when(experimentService.inExperiment(DiscoveryVariants.discoveryWithoutFileWatching)).thenResolve(false); pipEnvExecution = TypeMoq.Mock.ofType(); activationProvider = new PipEnvActivationCommandProvider( instance(interpreterService), pipEnvExecution.object, instance(workspaceService), instance(fs), + instance(experimentService), ); pipEnvExecution.setup((p) => p.executable).returns(() => pipenvExecFile); From 911c0e0193c6a227ada850fb3a801e4c0133fb43 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 9 Feb 2021 10:17:29 -0800 Subject: [PATCH 2/2] Fix ESLint errors --- .eslintignore | 2 -- .../pipEnvActivationProvider.ts | 32 ++++++++----------- .../pipEnvActivationProvider.unit.test.ts | 3 ++ 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/.eslintignore b/.eslintignore index 5bd7c3feb1c2..916c4bb20277 100644 --- a/.eslintignore +++ b/.eslintignore @@ -188,7 +188,6 @@ src/test/common/terminals/service.unit.test.ts src/test/common/terminals/helper.unit.test.ts src/test/common/terminals/activation.unit.test.ts src/test/common/terminals/shellDetectors/shellDetectors.unit.test.ts -src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts src/test/common/socketStream.test.ts src/test/common/configSettings.test.ts @@ -557,7 +556,6 @@ src/client/common/terminal/shellDetectors/vscEnvironmentShellDetector.ts src/client/common/terminal/shellDetectors/terminalNameShellDetector.ts src/client/common/terminal/shellDetectors/settingsShellDetector.ts src/client/common/terminal/shellDetectors/baseShellDetector.ts -src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts src/client/common/terminal/environmentActivationProviders/baseActivationProvider.ts src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts src/client/common/terminal/environmentActivationProviders/commandPrompt.ts diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts index 39d5bf053be7..49e60e109a03 100644 --- a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -5,7 +5,7 @@ import { inject, injectable, named } from 'inversify'; import { Uri } from 'vscode'; -import '../../../common/extensions'; +import '../../extensions'; import { IInterpreterService } from '../../../interpreter/contracts'; import { isPipenvEnvironmentRelatedToFolder } from '../../../pythonEnvironments/discovery/locators/services/pipEnvHelper'; import { EnvironmentType } from '../../../pythonEnvironments/info'; @@ -13,7 +13,7 @@ import { IWorkspaceService } from '../../application/types'; import { inDiscoveryExperiment } from '../../experiments/helpers'; import { IFileSystem } from '../../platform/types'; import { IExperimentService, IToolExecutionPath, ToolExecutionPath } from '../../types'; -import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; +import { ITerminalActivationCommandProvider } from '../types'; @injectable() export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider { @@ -27,42 +27,38 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma @inject(IExperimentService) private readonly experimentService: IExperimentService, ) {} - public isShellSupported(_targetShell: TerminalShellType): boolean { + // eslint-disable-next-line class-methods-use-this + public isShellSupported(): boolean { return false; } - public async getActivationCommands(resource: Uri | undefined, _: TerminalShellType): Promise { + public async getActivationCommands(resource: Uri | undefined): Promise { const interpreter = await this.interpreterService.getActiveInterpreter(resource); if (!interpreter || interpreter.envType !== EnvironmentType.Pipenv) { - return; + return undefined; } // Activate using `pipenv shell` only if the current folder relates pipenv environment. const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; if (workspaceFolder) { if (await inDiscoveryExperiment(this.experimentService)) { if (!(await isPipenvEnvironmentRelatedToFolder(interpreter.path, workspaceFolder?.uri.fsPath))) { - return; - } - } else { - if ( - interpreter.pipEnvWorkspaceFolder && - !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) - ) { - return; + return undefined; } + } else if ( + interpreter.pipEnvWorkspaceFolder && + !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) + ) { + return undefined; } } const execName = this.pipEnvExecution.executable; return [`${execName.fileToCommandArgument()} shell`]; } - public async getActivationCommandsForInterpreter( - pythonPath: string, - _targetShell: TerminalShellType, - ): Promise { + public async getActivationCommandsForInterpreter(pythonPath: string): Promise { const interpreter = await this.interpreterService.getInterpreterDetails(pythonPath); if (!interpreter || interpreter.envType !== EnvironmentType.Pipenv) { - return; + return undefined; } const execName = this.pipEnvExecution.executable; diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index e9686dd65303..89bc04005eab 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -66,6 +66,7 @@ suite('Terminals Activation - Pipenv', () => { for (const interpreterType of nonPipInterpreterTypes) { when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: interpreterType, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) { @@ -78,6 +79,7 @@ suite('Terminals Activation - Pipenv', () => { test('pipenv shell is returned for pipenv interpeter', async () => { when(interpreterService.getActiveInterpreter(resource)).thenResolve({ envType: EnvironmentType.Pipenv, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) { @@ -90,6 +92,7 @@ suite('Terminals Activation - Pipenv', () => { pipenvExecFile = 'my pipenv'; when(interpreterService.getActiveInterpreter(resource)).thenResolve({ envType: EnvironmentType.Pipenv, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) {