From 217c0324caaffb0274f4c71ee105062bfe427a0e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 4 Jun 2019 12:14:47 -0700 Subject: [PATCH] Changes to identification of shell --- .vscode/launch.json | 2 +- CHANGELOG.md | 10 +- package-lock.json | 2 +- package.json | 2 +- src/client/common/terminal/activator/base.ts | 3 +- .../activator/powershellFailedHandler.ts | 4 +- src/client/common/terminal/helper.ts | 93 +++++++++++++------ src/client/common/terminal/service.ts | 3 +- src/client/common/terminal/types.ts | 3 +- src/client/extension.ts | 2 +- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 7 ++ .../terminals/activation.conda.unit.test.ts | 7 +- .../terminals/activator/base.unit.test.ts | 3 - .../powerShellFailedHandler.unit.test.ts | 6 -- src/test/common/terminals/helper.unit.test.ts | 54 +++-------- .../common/terminals/service.unit.test.ts | 12 --- 17 files changed, 108 insertions(+), 106 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 7609fc076e7f..dcbc11b4c024 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -210,4 +210,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/CHANGELOG.md b/CHANGELOG.md index f0f21fb9f7c0..c8f67ad6a79a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ # Changelog -## 2019.5.1 (30 May 2019) +## 2019.5.2 (4 June 2019) + +### Fixes + +1. Changes to identificaction of `shell` for the activation of environments in the terminal. + ([#5743](https://github.com/microsoft/vscode-python/issues/5743)) + + +## 2019.5.17517 (30 May 2019) ### Fixes diff --git a/package-lock.json b/package-lock.json index 415cc79803eb..9784376d2605 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2019.5.1", + "version": "2019.5.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 25dad5ec6c7f..31261bcea00e 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, code formatting, refactoring, unit tests, snippets, and more.", - "version": "2019.5.1", + "version": "2019.5.2", "languageServerVersion": "0.2.82", "publisher": "ms-python", "author": { diff --git a/src/client/common/terminal/activator/base.ts b/src/client/common/terminal/activator/base.ts index 0c3f23f812af..cae3f4108132 100644 --- a/src/client/common/terminal/activator/base.ts +++ b/src/client/common/terminal/activator/base.ts @@ -16,8 +16,7 @@ export class BaseTerminalActivator implements ITerminalActivator { } const deferred = createDeferred(); this.activatedTerminals.set(terminal, deferred.promise); - const shellPath = this.helper.getTerminalShellPath(); - const terminalShellType = !shellPath || shellPath.length === 0 ? TerminalShellType.other : this.helper.identifyTerminalShell(shellPath); + const terminalShellType = this.helper.identifyTerminalShell(terminal); const activationCommamnds = await this.helper.getEnvironmentActivationCommands(terminalShellType, resource); let activated = false; diff --git a/src/client/common/terminal/activator/powershellFailedHandler.ts b/src/client/common/terminal/activator/powershellFailedHandler.ts index 3196555ca9be..574b0532ae01 100644 --- a/src/client/common/terminal/activator/powershellFailedHandler.ts +++ b/src/client/common/terminal/activator/powershellFailedHandler.ts @@ -17,11 +17,11 @@ export class PowershellTerminalActivationFailedHandler implements ITerminalActiv @inject(IPlatformService) private readonly platformService: IPlatformService, @inject(IDiagnosticsService) @named(PowerShellActivationHackDiagnosticsServiceId) private readonly diagnosticService: IDiagnosticsService) { } - public async handleActivation(_terminal: Terminal, resource: Resource, _preserveFocus: boolean, activated: boolean) { + public async handleActivation(terminal: Terminal, resource: Resource, _preserveFocus: boolean, activated: boolean) { if (activated || !this.platformService.isWindows) { return; } - const shell = this.helper.identifyTerminalShell(this.helper.getTerminalShellPath()); + const shell = this.helper.identifyTerminalShell(terminal); if (shell !== TerminalShellType.powershell && shell !== TerminalShellType.powershellCore) { return; } diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index ec9369501c96..ff67db9a74d6 100644 --- a/src/client/common/terminal/helper.ts +++ b/src/client/common/terminal/helper.ts @@ -2,15 +2,16 @@ // Licensed under the MIT License. import { inject, injectable, named } from 'inversify'; +import * as os from 'os'; import { Terminal, Uri } from 'vscode'; import { ICondaService, IInterpreterService, InterpreterType, PythonInterpreter } from '../../interpreter/contracts'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { ITerminalManager, IWorkspaceService } from '../application/types'; +import { ITerminalManager } from '../application/types'; import '../extensions'; import { traceDecorators, traceError } from '../logger'; import { IPlatformService } from '../platform/types'; -import { IConfigurationService, Resource } from '../types'; +import { IConfigurationService, ICurrentProcess, Resource } from '../types'; import { OSType } from '../utils/platform'; import { ITerminalActivationCommandProvider, ITerminalHelper, TerminalActivationProviders, TerminalShellType } from './types'; @@ -21,7 +22,7 @@ const IS_BASH = /(bash.exe$|bash$)/i; const IS_WSL = /(wsl.exe$)/i; const IS_ZSH = /(zsh$)/i; const IS_KSH = /(ksh$)/i; -const IS_COMMAND = /cmd.exe$/i; +const IS_COMMAND = /(cmd.exe$|cmd$)/i; const IS_POWERSHELL = /(powershell.exe$|powershell$)/i; const IS_POWERSHELL_CORE = /(pwsh.exe$|pwsh$)/i; const IS_FISH = /(fish$)/i; @@ -41,7 +42,6 @@ export class TerminalHelper implements ITerminalHelper { private readonly detectableShells: Map; constructor(@inject(IPlatformService) private readonly platform: IPlatformService, @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, @@ -49,7 +49,8 @@ export class TerminalHelper implements ITerminalHelper { @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 + @inject(ITerminalActivationCommandProvider) @named(TerminalActivationProviders.pipenv) private readonly pipenv: ITerminalActivationCommandProvider, + @inject(IConfigurationService) private readonly currentProcess: ICurrentProcess ) { this.detectableShells = new Map(); this.detectableShells.set(TerminalShellType.powershell, IS_POWERSHELL); @@ -68,37 +69,44 @@ export class TerminalHelper implements ITerminalHelper { public createTerminal(title?: string): Terminal { return this.terminalManager.createTerminal({ name: title }); } - public identifyTerminalShell(shellPath: string): TerminalShellType { + public identifyTerminalShell(terminal?: Terminal): TerminalShellType { + let shell = TerminalShellType.other; + let usingDefaultShell = false; + const terminalProvided = !!terminal; + // Determine shell based on the name of the terminal. + // See solution here https://github.com/microsoft/vscode/issues/74233#issuecomment-497527337 + if (terminal) { + shell = this.identifyTerminalShellByName(terminal.name); + } + + // If still unable to identify, then use fall back to determine path to the default shell. + if (shell === TerminalShellType.other) { + const shellPath = getDefaultShell(this.platform.osType, this.currentProcess); + shell = Array.from(this.detectableShells.keys()) + .reduce((matchedShell, shellToDetect) => { + if (matchedShell === TerminalShellType.other && this.detectableShells.get(shellToDetect)!.test(shellPath)) { + return shellToDetect; + } + return matchedShell; + }, TerminalShellType.other); + + // We have restored to using the default shell. + usingDefaultShell = shell !== TerminalShellType.other; + } + const properties = { failed: shell === TerminalShellType.other, usingDefaultShell, terminalProvided }; + sendTelemetryEvent(EventName.TERMINAL_SHELL_IDENTIFICATION, undefined, properties); + return shell; + } + public identifyTerminalShellByName(name: string): TerminalShellType { return Array.from(this.detectableShells.keys()) .reduce((matchedShell, shellToDetect) => { - if (matchedShell === TerminalShellType.other && this.detectableShells.get(shellToDetect)!.test(shellPath)) { + if (matchedShell === TerminalShellType.other && this.detectableShells.get(shellToDetect)!.test(name)) { return shellToDetect; } return matchedShell; }, TerminalShellType.other); } - public getTerminalShellPath(): string { - const shellConfig = this.workspace.getConfiguration('terminal.integrated.shell'); - let osSection = ''; - switch (this.platform.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)!; - } + public buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]) { const isPowershell = terminalShellType === TerminalShellType.powershell || terminalShellType === TerminalShellType.powershellCore; const commandPrefix = isPowershell ? '& ' : ''; @@ -173,3 +181,30 @@ export class TerminalHelper implements ITerminalHelper { } } } + +/* + The following code is based on VS Code from https://github.com/microsoft/vscode/blob/5c65d9bfa4c56538150d7f3066318e0db2c6151f/src/vs/workbench/contrib/terminal/node/terminal.ts#L12-L55 + This is only a fall back to identify the default shell used by VSC. + On Windows, determine the default shell. + On others, default to bash. +*/ +function getDefaultShell(osType: OSType, currentProcess: ICurrentProcess): string { + if (osType === OSType.Windows) { + return getTerminalDefaultShellWindows(osType, currentProcess); + } + return '/bin/bash'; +} +let _TERMINAL_DEFAULT_SHELL_WINDOWS: string | null = null; +function getTerminalDefaultShellWindows(osType: OSType, currentProcess: ICurrentProcess): string { + if (!_TERMINAL_DEFAULT_SHELL_WINDOWS) { + const isAtLeastWindows10 = osType === OSType.Windows && parseFloat(os.release()) >= 10; + const is32ProcessOn64Windows = process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'); + const powerShellPath = `${process.env.windir}\\${is32ProcessOn64Windows ? 'Sysnative' : 'System32'}\\WindowsPowerShell\\v1.0\\powershell.exe`; + _TERMINAL_DEFAULT_SHELL_WINDOWS = isAtLeastWindows10 ? powerShellPath : getWindowsShell(currentProcess); + } + return _TERMINAL_DEFAULT_SHELL_WINDOWS; +} + +function getWindowsShell(currentProcess: ICurrentProcess): string { + return currentProcess.env.comspec || 'cmd.exe'; +} diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 2db3b9f5c3c3..ec3710aefa8c 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -58,8 +58,7 @@ export class TerminalService implements ITerminalService, Disposable { if (this.terminal) { return; } - const shellPath = this.terminalHelper.getTerminalShellPath(); - this.terminalShellType = !shellPath || shellPath.length === 0 ? TerminalShellType.other : this.terminalHelper.identifyTerminalShell(shellPath); + this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal); this.terminal = this.terminalManager.createTerminal({ name: this.title }); // Sometimes the terminal takes some time to start up before it can start accepting input. diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index 193e2e1efbb3..ec92bdc1e7f8 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -54,8 +54,7 @@ export const ITerminalHelper = Symbol('ITerminalHelper'); export interface ITerminalHelper { createTerminal(title?: string): Terminal; - identifyTerminalShell(shellPath: string): TerminalShellType; - getTerminalShellPath(): string; + identifyTerminalShell(terminal?: Terminal): TerminalShellType; buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]): string; getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise; getEnvironmentActivationShellCommands(resource: Resource, interpreter?: PythonInterpreter): Promise; diff --git a/src/client/extension.ts b/src/client/extension.ts index 3507f53b9ae9..eaccea7e8f52 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -354,7 +354,7 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer): // be able to partially populate as much as possible instead // (through granular try-catch statements). const terminalHelper = serviceContainer.get(ITerminalHelper); - const terminalShellType = terminalHelper.identifyTerminalShell(terminalHelper.getTerminalShellPath()); + const terminalShellType = terminalHelper.identifyTerminalShell(); const condaLocator = serviceContainer.get(ICondaService); const interpreterService = serviceContainer.get(IInterpreterService); const workspaceService = serviceContainer.get(IWorkspaceService); diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index ce7aa952d146..cc612b596513 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -29,6 +29,7 @@ export enum EventName { PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES = 'PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES', PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE = 'PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE', PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL = 'PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL', + TERMINAL_SHELL_IDENTIFICATION = 'TERMINAL_SHELL_IDENTIFICATION', PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT = 'PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT', ENVFILE_VARIABLE_SUBSTITUTION = 'ENVFILE_VARIABLE_SUBSTITUTION', WORKSPACE_SYMBOLS_BUILD = 'WORKSPACE_SYMBOLS.BUILD', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 325c039d26b1..dca4b863c90a 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -368,4 +368,11 @@ export interface IEventNamePropertyMapping { [EventName.UNITTEST_NAVIGATE_TEST_FUNCTION]: { focus_code: boolean }; [EventName.UNITTEST_NAVIGATE_TEST_SUITE]: { focus_code: boolean }; [EventName.UNITTEST_EXPLORER_WORK_SPACE_COUNT]: { count: number }; + /* + Telemetry event sent to provide information on whether we have successfully identify the type of shell used. + failed - If true, indicates we have failed to identify the shell. Note this impacts impacts ability to activate environments in the terminal & code. + usingDefaultShell - If true, this indicates we failed to identify the shell and we have reverted to using the default shell on user machine. + terminalProvided - If true, we used the terminal provided to detec the shell. If not provided, we use the default shell on user machine. + */ + [EventName.TERMINAL_SHELL_IDENTIFICATION]: { failed: boolean; usingDefaultShell: boolean; terminalProvided: boolean }; } diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts index 7afbb07574a4..974f1997bf59 100644 --- a/src/test/common/terminals/activation.conda.unit.test.ts +++ b/src/test/common/terminals/activation.conda.unit.test.ts @@ -10,11 +10,11 @@ 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 } from '../../../client/common/platform/types'; +import { CurrentProcess } from '../../../client/common/process/currentProcess'; import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types'; @@ -84,7 +84,7 @@ suite('Terminal Environment Activation conda', () => { pythonSettings.setup(s => s.terminal).returns(() => terminalSettings.object); terminalHelper = new TerminalHelper(platformService.object, - instance(mock(TerminalManager)), instance(mock(WorkspaceService)), + instance(mock(TerminalManager)), condaService.object, instance(mock(InterpreterService)), configService.object, @@ -92,7 +92,8 @@ suite('Terminal Environment Activation conda', () => { instance(bash), mock(CommandPromptAndPowerShell), mock(PyEnvActivationCommandProvider), - mock(PipEnvActivationCommandProvider)); + mock(PipEnvActivationCommandProvider), + instance(mock(CurrentProcess))); }); teardown(() => { diff --git a/src/test/common/terminals/activator/base.unit.test.ts b/src/test/common/terminals/activator/base.unit.test.ts index 80584ebbe3ca..54d68bfc6cf8 100644 --- a/src/test/common/terminals/activator/base.unit.test.ts +++ b/src/test/common/terminals/activator/base.unit.test.ts @@ -30,7 +30,6 @@ suite('Terminal Base Activator', () => { const titleSuffix = `(${item.commandCount} activation command, and preserve focus in terminal is ${item.preserveFocus})`; const activationCommands = item.commandCount === 1 ? ['CMD1'] : ['CMD1', 'CMD2']; test(`Terminal is activated ${titleSuffix}`, async () => { - helper.setup(h => h.getTerminalShellPath()).returns(() => ''); helper.setup(h => h.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(activationCommands)); const terminal = TypeMoq.Mock.ofType(); @@ -50,7 +49,6 @@ suite('Terminal Base Activator', () => { terminal.verifyAll(); }); test(`Terminal is activated only once ${titleSuffix}`, async () => { - helper.setup(h => h.getTerminalShellPath()).returns(() => ''); helper.setup(h => h.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(activationCommands)); const terminal = TypeMoq.Mock.ofType(); @@ -72,7 +70,6 @@ suite('Terminal Base Activator', () => { terminal.verifyAll(); }); test(`Terminal is activated only once ${titleSuffix} (even when not waiting)`, async () => { - helper.setup(h => h.getTerminalShellPath()).returns(() => ''); helper.setup(h => h.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(activationCommands)); const terminal = TypeMoq.Mock.ofType(); diff --git a/src/test/common/terminals/activator/powerShellFailedHandler.unit.test.ts b/src/test/common/terminals/activator/powerShellFailedHandler.unit.test.ts index 1711af2988f4..eb2384483090 100644 --- a/src/test/common/terminals/activator/powerShellFailedHandler.unit.test.ts +++ b/src/test/common/terminals/activator/powerShellFailedHandler.unit.test.ts @@ -20,18 +20,12 @@ suite('Terminal Activation Powershell Failed Handler', () => { async function testDiagnostics(mustHandleDiagnostics: boolean, isWindows: boolean, activatedSuccessfully: boolean, shellType: TerminalShellType, cmdPromptHasActivationCommands: boolean) { platform.setup(p => p.isWindows).returns(() => isWindows); - helper - .setup(p => p.getTerminalShellPath()) - .returns(() => ''); - // .verifiable(TypeMoq.Times.atMostOnce()); helper .setup(p => p.identifyTerminalShell(TypeMoq.It.isAny())) .returns(() => shellType); - // .verifiable(TypeMoq.Times.atMostOnce());c const cmdPromptCommands = cmdPromptHasActivationCommands ? ['a'] : []; helper.setup(h => h.getEnvironmentActivationCommands(TypeMoq.It.isValue(TerminalShellType.commandPrompt), TypeMoq.It.isAny())) .returns(() => Promise.resolve(cmdPromptCommands)); - // .verifiable(TypeMoq.Times.atMostOnce()); diagnosticService .setup(d => d.handle(TypeMoq.It.isAny())) diff --git a/src/test/common/terminals/helper.unit.test.ts b/src/test/common/terminals/helper.unit.test.ts index ec592448892f..28f255278d8a 100644 --- a/src/test/common/terminals/helper.unit.test.ts +++ b/src/test/common/terminals/helper.unit.test.ts @@ -3,16 +3,14 @@ import { expect } from 'chai'; import { SemVer } from 'semver'; import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; -import * as TypeMoq from 'typemoq'; -import { Uri, WorkspaceConfiguration } from 'vscode'; - +import { Uri } 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 { ITerminalManager } from '../../../client/common/application/types'; 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 { CurrentProcess } from '../../../client/common/process/currentProcess'; import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash'; import { CommandPromptAndPowerShell } from '../../../client/common/terminal/environmentActivationProviders/commandPrompt'; import { @@ -27,7 +25,6 @@ import { import { TerminalHelper } from '../../../client/common/terminal/helper'; import { ITerminalActivationCommandProvider, - ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; import { IConfigurationService } from '../../../client/common/types'; @@ -40,10 +37,9 @@ import { CondaService } from '../../../client/interpreter/locators/services/cond // tslint:disable:max-func-body-length no-any suite('Terminal Service helpers', () => { - let helper: ITerminalHelper; + let helper: TerminalHelper; let terminalManager: ITerminalManager; let platformService: IPlatformService; - let workspaceService: IWorkspaceService; let condaService: ICondaService; let configurationService: IConfigurationService; let condaActivationProvider: ITerminalActivationCommandProvider; @@ -65,7 +61,6 @@ suite('Terminal Service helpers', () => { function doSetup() { terminalManager = mock(TerminalManager); platformService = mock(PlatformService); - workspaceService = mock(WorkspaceService); condaService = mock(CondaService); configurationService = mock(ConfigurationService); condaActivationProvider = mock(CondaActivationCommandProvider); @@ -76,7 +71,6 @@ suite('Terminal Service helpers', () => { pythonSettings = mock(PythonSettings); helper = new TerminalHelper(instance(platformService), instance(terminalManager), - instance(workspaceService), instance(condaService), instance(mock(InterpreterService)), instance(configurationService), @@ -84,7 +78,8 @@ suite('Terminal Service helpers', () => { instance(bashActivationProvider), instance(cmdActivationProvider), instance(pyenvActivationProvider), - instance(pipenvActivationProvider)); + instance(pipenvActivationProvider), + instance(mock(CurrentProcess))); } suite('Misc', () => { setup(doSetup); @@ -112,6 +107,13 @@ suite('Terminal Service helpers', () => { expect(term).to.be.deep.equal(terminal); expect(args.name).to.be.deep.equal(theTitle); }); + test('Revert to default shell', async () => { + when(platformService.osType).thenReturn(OSType.OSX); + + const shellType = helper.identifyTerminalShell(); + + expect(shellType).to.be.equal(TerminalShellType.bash); + }); test('Test identification of Terminal Shells', async () => { const shellPathsAndIdentification = new Map(); shellPathsAndIdentification.set('c:\\windows\\system32\\cmd.exe', TerminalShellType.commandPrompt); @@ -140,37 +142,9 @@ suite('Terminal Service helpers', () => { shellPathsAndIdentification.set('/usr/bin/xonshx', TerminalShellType.other); shellPathsAndIdentification.forEach((shellType, shellPath) => { - expect(helper.identifyTerminalShell(shellPath)).to.equal(shellType, `Incorrect Shell Type for path '${shellPath}'`); + expect(helper.identifyTerminalShellByName(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); - - const shellPath = helper.getTerminalShellPath(); - - 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'; diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 24814c12decb..9afc97d5eb53 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -79,7 +79,6 @@ suite('Terminal Service', () => { const args = ['1', '2']; const commandToExpect = [commandToSend].concat(args).join(' '); terminalHelper.setup(h => h.buildCommandForTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => commandToExpect); - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); @@ -93,7 +92,6 @@ suite('Terminal Service', () => { terminalHelper.setup(helper => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); service = new TerminalService(mockServiceContainer.object); const textToSend = 'Some Text'; - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); @@ -106,7 +104,6 @@ suite('Terminal Service', () => { test('Ensure terminal shown', async () => { terminalHelper.setup(helper => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); service = new TerminalService(mockServiceContainer.object); - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); @@ -118,7 +115,6 @@ suite('Terminal Service', () => { test('Ensure terminal shown and focus is set to the Terminal', async () => { terminalHelper.setup(helper => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); service = new TerminalService(mockServiceContainer.object); - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); @@ -129,9 +125,6 @@ suite('Terminal Service', () => { test('Ensure terminal is activated once after creation', async () => { service = new TerminalService(mockServiceContainer.object); - terminalHelper - .setup(h => h.getTerminalShellPath()).returns(() => '') - .verifiable(TypeMoq.Times.once()); terminalActivator .setup(h => h.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(true)) @@ -153,9 +146,6 @@ suite('Terminal Service', () => { test('Ensure terminal is activated once before sending text', async () => { service = new TerminalService(mockServiceContainer.object); const textToSend = 'Some Text'; - terminalHelper - .setup(h => h.getTerminalShellPath()).returns(() => '') - .verifiable(TypeMoq.Times.once()); terminalActivator .setup(h => h.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(true)) @@ -185,7 +175,6 @@ suite('Terminal Service', () => { }); service = new TerminalService(mockServiceContainer.object); service.onDidCloseTerminal(() => eventFired = true, service); - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); @@ -209,7 +198,6 @@ suite('Terminal Service', () => { service = new TerminalService(mockServiceContainer.object); service.onDidCloseTerminal(() => eventFired = true); - terminalHelper.setup(h => h.getTerminalShellPath()).returns(() => ''); terminalHelper.setup(h => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); terminalManager.setup(t => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object);