From e65ec36c185a8b43ad076f755aa4842d50315274 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 20 Apr 2020 12:24:23 -0700 Subject: [PATCH] Added tests --- .../configuration/interpreterSelector.ts | 12 +- src/client/interpreter/configuration/types.ts | 2 + .../interpreterSelector.unit.test.ts | 388 ++++++++++++++++-- 3 files changed, 360 insertions(+), 42 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector.ts index 23f3502ff48a..40f48836067e 100644 --- a/src/client/interpreter/configuration/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector.ts @@ -24,10 +24,10 @@ import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector, + InterpreterStateArgs, IPythonPathUpdaterServiceManager } from './types'; -type InterpreterStateArgs = { path?: string; workspace: Resource }; @injectable() export class InterpreterSelector implements IInterpreterSelector { private disposables: Disposable[] = []; @@ -105,10 +105,10 @@ export class InterpreterSelector implements IInterpreterSelector { if (selection === undefined) { return; - } else if (selection !== enterInterpreterPathSuggestion) { - state.path = (selection as IInterpreterQuickPickItem).path; + } else if (selection.label === enterInterpreterPathSuggestion.label) { + return this._enterOrBrowseInterpreterPath(input, state); } else { - return this._enterOrBrowseInterpreterPath.bind(this); + state.path = (selection as IInterpreterQuickPickItem).path; } } @@ -129,7 +129,9 @@ export class InterpreterSelector implements IInterpreterSelector { acceptFilterBoxTextAsSelection: true }); - if (typeof selection === 'string') { + if (selection === undefined) { + return; + } else if (typeof selection === 'string') { // User entered text in the filter box to enter path to python, store it state.path = selection; } else if (selection.label === 'Browse...') { diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 4fcfecb4ff0a..5e5fa68fecb3 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -44,3 +44,5 @@ export const IInterpreterComparer = Symbol('IInterpreterComparer'); export interface IInterpreterComparer { compare(a: PythonInterpreter, b: PythonInterpreter): number; } + +export type InterpreterStateArgs = { path?: string; workspace: Resource }; diff --git a/src/test/configuration/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector.unit.test.ts index 3f0ca171ac84..515f5cf5b2e8 100644 --- a/src/test/configuration/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector.unit.test.ts @@ -2,10 +2,12 @@ // Licensed under the MIT License. import * as assert from 'assert'; +import { expect } from 'chai'; import * as path from 'path'; import { SemVer } from 'semver'; +import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; -import { ConfigurationTarget, Uri } from 'vscode'; +import { ConfigurationTarget, QuickPickItem, Uri } from 'vscode'; import { IApplicationShell, ICommandManager, @@ -14,15 +16,17 @@ import { } from '../../client/common/application/types'; import { DeprecatePythonPath } from '../../client/common/experimentGroups'; import { PathUtils } from '../../client/common/platform/pathUtils'; -import { IFileSystem } from '../../client/common/platform/types'; +import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IConfigurationService, IExperimentsManager, IPythonSettings } from '../../client/common/types'; -import { Interpreters } from '../../client/common/utils/localize'; +import { InterpreterQuickPickList, Interpreters } from '../../client/common/utils/localize'; +import { IMultiStepInput, IMultiStepInputFactory } from '../../client/common/utils/multiStepInput'; import { Architecture } from '../../client/common/utils/platform'; import { IInterpreterSecurityService } from '../../client/interpreter/autoSelection/types'; import { InterpreterSelector } from '../../client/interpreter/configuration/interpreterSelector'; import { IInterpreterComparer, IInterpreterQuickPickItem, + InterpreterStateArgs, IPythonPathUpdaterServiceManager } from '../../client/interpreter/configuration/types'; import { @@ -72,6 +76,8 @@ suite('Interpreters - selector', () => { let interpreterSecurityService: TypeMoq.IMock; let configurationService: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; + let platformService: TypeMoq.IMock; + let multiStepInputFactory: TypeMoq.IMock; const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; @@ -100,6 +106,8 @@ suite('Interpreters - selector', () => { let selector: TestInterpreterSelector; setup(() => { + multiStepInputFactory = TypeMoq.Mock.ofType(); + platformService = TypeMoq.Mock.ofType(); experimentsManager = TypeMoq.Mock.ofType(); experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); experimentsManager @@ -136,10 +144,16 @@ suite('Interpreters - selector', () => { configurationService.object, commandManager.object, experimentsManager.object, - interpreterSecurityService.object + interpreterSecurityService.object, + multiStepInputFactory.object, + platformService.object ); }); + teardown(() => { + sinon.restore(); + }); + [true, false].forEach((isWindows) => { test(`Suggestions (${isWindows ? 'Windows' : 'Non-Windows'})`, async () => { const interpreterSelector = new InterpreterSelector( @@ -154,7 +168,9 @@ suite('Interpreters - selector', () => { configurationService.object, commandManager.object, experimentsManager.object, - interpreterSecurityService.object + interpreterSecurityService.object, + multiStepInputFactory.object, + platformService.object ); const initial: PythonInterpreter[] = [ @@ -219,6 +235,222 @@ suite('Interpreters - selector', () => { assert.deepEqual(suggestion, ['interpreter1', 'interpreter3']); }); + suite('Test method _enterOrBrowseInterpreterPath()', async () => { + // tslint:disable-next-line: no-any + let _enterOrBrowseInterpreterPath: sinon.SinonStub; + // tslint:disable-next-line: no-any + let getSuggestions: sinon.SinonStub; + const item: IInterpreterQuickPickItem = { + description: '', + detail: '', + label: '', + path: 'This is the selected Python path', + // tslint:disable-next-line: no-any + interpreter: {} as any + }; + const expectedEnterInterpreterPathSuggestion = { + label: InterpreterQuickPickList.enterPath.label(), + detail: InterpreterQuickPickList.enterPath.detail(), + alwaysShow: true + }; + const currentPythonPath = 'python'; + setup(() => { + _enterOrBrowseInterpreterPath = sinon.stub(InterpreterSelector.prototype, '_enterOrBrowseInterpreterPath'); + _enterOrBrowseInterpreterPath.resolves(); + getSuggestions = sinon.stub(InterpreterSelector.prototype, 'getSuggestions'); + getSuggestions.resolves([item]); + pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); + selector = new TestInterpreterSelector( + interpreterService.object, + workspace.object, + appShell.object, + documentManager.object, + new PathUtils(false), + comparer.object, + pythonPathUpdater.object, + shebangProvider.object, + configurationService.object, + commandManager.object, + experimentsManager.object, + interpreterSecurityService.object, + multiStepInputFactory.object, + platformService.object + ); + }); + teardown(() => { + sinon.restore(); + }); + + test('Existing state path must be removed before displaying picker', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(undefined as any)); + + await selector._pickInterpreter(multiStepInput.object, state); + + expect(state.path).to.equal(undefined, ''); + }); + + test('Picker should be displayed with expected items', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + const suggestions = [expectedEnterInterpreterPathSuggestion, item]; + const expectedParameters = { + placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), + items: suggestions, + activeItem: item, + matchOnDetail: true, + matchOnDescription: true + }; + multiStepInput + .setup((i) => i.showQuickPick(expectedParameters)) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(undefined as any)) + .verifiable(TypeMoq.Times.once()); + + await selector._pickInterpreter(multiStepInput.object, state); + + multiStepInput.verifyAll(); + }); + + test('If an item is selected, update state and return', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(item as any)); + + await selector._pickInterpreter(multiStepInput.object, state); + + expect(state.path).to.equal(item.path, ''); + }); + + test('If `Enter or browse...` option is selected, call the corresponding method with correct arguments', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(expectedEnterInterpreterPathSuggestion as any)); + + await selector._pickInterpreter(multiStepInput.object, state); + + assert( + _enterOrBrowseInterpreterPath.calledOnceWith(multiStepInput.object, { + path: undefined, + workspace: undefined + }) + ); + }); + }); + + suite('Test method _enterOrBrowseInterpreterPath()', async () => { + const items: QuickPickItem[] = [ + { + label: InterpreterQuickPickList.browsePath.label(), + detail: InterpreterQuickPickList.browsePath.detail() + } + ]; + const expectedParameters = { + placeholder: InterpreterQuickPickList.enterPath.placeholder(), + items, + acceptFilterBoxTextAsSelection: true + }; + + test('Picker should be displayed with expected items', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(expectedParameters)) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(undefined as any)) + .verifiable(TypeMoq.Times.once()); + + await selector._enterOrBrowseInterpreterPath(multiStepInput.object, state); + + multiStepInput.verifyAll(); + }); + + test('If user enters path to interpreter in the filter box, get path and update state', async () => { + const state: InterpreterStateArgs = { path: undefined, workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve('enteredPath' as any)); + + await selector._enterOrBrowseInterpreterPath(multiStepInput.object, state); + + expect(state.path).to.equal('enteredPath', ''); + }); + + test('If `Browse...` is selected, open the file browser to get path and update state', async () => { + const state: InterpreterStateArgs = { path: undefined, workspace: undefined }; + const expectedPathUri = Uri.parse('browsed path'); + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(items[0] as any)); + appShell + .setup((a) => a.showOpenDialog(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([expectedPathUri])); + + await selector._enterOrBrowseInterpreterPath(multiStepInput.object, state); + + expect(state.path).to.equal(expectedPathUri.fsPath, ''); + }); + + test('If `Browse...` option is selected on Windows, file browser is opened using expected parameters', async () => { + const state: InterpreterStateArgs = { path: undefined, workspace: undefined }; + const filtersKey = 'Executables'; + const filtersObject: { [name: string]: string[] } = {}; + filtersObject[filtersKey] = ['exe']; + const expectedParams = { + filters: filtersObject, + openLabel: InterpreterQuickPickList.browsePath.openButtonLabel(), + canSelectMany: false + }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(items[0] as any)); + appShell + // tslint:disable-next-line: no-any + .setup((a) => a.showOpenDialog(expectedParams as any)) + .verifiable(TypeMoq.Times.once()); + platformService.setup((p) => p.isWindows).returns(() => true); + + await selector._enterOrBrowseInterpreterPath(multiStepInput.object, state); + + appShell.verifyAll(); + }); + + test('If `Browse...` option is selected on non-Windows, file browser is opened using expected parameters', async () => { + const state: InterpreterStateArgs = { path: undefined, workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + const expectedParams = { + filters: undefined, + openLabel: InterpreterQuickPickList.browsePath.openButtonLabel(), + canSelectMany: false + }; + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + // tslint:disable-next-line: no-any + .returns(() => Promise.resolve(items[0] as any)); + appShell.setup((a) => a.showOpenDialog(expectedParams)).verifiable(TypeMoq.Times.once()); + platformService.setup((p) => p.isWindows).returns(() => false); + + await selector._enterOrBrowseInterpreterPath(multiStepInput.object, state); + + appShell.verifyAll(); + }); + }); // tslint:disable-next-line: max-func-body-length suite('Test method setInterpreter()', async () => { test('Update Global settings when there are no workspaces', async () => { @@ -235,10 +467,17 @@ suite('Interpreters - selector', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); selector.getSuggestions = () => Promise.resolve([]); - appShell - .setup((s) => s.showQuickPick(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => Promise.resolve(selectedItem)) - .verifiable(TypeMoq.Times.once()); + const multiStepInput = { + // tslint:disable-next-line: no-any + run: (_: any, state: InterpreterStateArgs) => { + state.path = selectedItem.path; + return Promise.resolve(); + } + }; + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any + .returns(() => multiStepInput as any); pythonPathUpdater .setup((p) => p.updatePythonPath( @@ -253,7 +492,6 @@ suite('Interpreters - selector', () => { await selector.setInterpreter(); - appShell.verifyAll(); workspace.verifyAll(); pythonPathUpdater.verifyAll(); }); @@ -273,10 +511,19 @@ suite('Interpreters - selector', () => { workspace.setup((w) => w.workspaceFolders).returns(() => [folder]); selector.getSuggestions = () => Promise.resolve([]); - appShell - .setup((s) => s.showQuickPick(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => Promise.resolve(selectedItem)) - .verifiable(TypeMoq.Times.once()); + + const multiStepInput = { + // tslint:disable-next-line: no-any + run: (_: any, state: InterpreterStateArgs) => { + state.path = selectedItem.path; + return Promise.resolve(); + } + }; + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any + .returns(() => multiStepInput as any); + pythonPathUpdater .setup((p) => p.updatePythonPath( @@ -291,7 +538,6 @@ suite('Interpreters - selector', () => { await selector.setInterpreter(); - appShell.verifyAll(); workspace.verifyAll(); pythonPathUpdater.verifyAll(); }); @@ -324,10 +570,18 @@ suite('Interpreters - selector', () => { } ]; selector.getSuggestions = () => Promise.resolve([]); - appShell - .setup((s) => s.showQuickPick(TypeMoq.It.isValue([]), TypeMoq.It.isAny())) - .returns(() => Promise.resolve(selectedItem)) - .verifiable(TypeMoq.Times.once()); + + const multiStepInput = { + // tslint:disable-next-line: no-any + run: (_: any, state: InterpreterStateArgs) => { + state.path = selectedItem.path; + return Promise.resolve(); + } + }; + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any + .returns(() => multiStepInput as any); appShell .setup((s) => s.showQuickPick(TypeMoq.It.isValue(expectedItems), TypeMoq.It.isAny())) .returns(() => @@ -385,12 +639,17 @@ suite('Interpreters - selector', () => { } ]; selector.getSuggestions = () => Promise.resolve([selectedItem]); - appShell - .setup((s) => - s.showQuickPick(TypeMoq.It.isValue([selectedItem]), TypeMoq.It.isAny()) - ) - .returns(() => Promise.resolve(selectedItem)) - .verifiable(TypeMoq.Times.once()); + const multiStepInput = { + // tslint:disable-next-line: no-any + run: (_: any, state: InterpreterStateArgs) => { + state.path = selectedItem.path; + return Promise.resolve(); + } + }; + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any + .returns(() => multiStepInput as any); appShell .setup((s) => s.showQuickPick(TypeMoq.It.isValue(expectedItems), TypeMoq.It.isAny())) .returns(() => @@ -419,21 +678,12 @@ suite('Interpreters - selector', () => { pythonPathUpdater.verifyAll(); }); test('Do not update anything when user does not select a workspace folder and there is more than one workspace folder', async () => { - const selectedItem: IInterpreterQuickPickItem = { - description: '', - detail: '', - label: '', - path: 'This is the selected Python path', - // tslint:disable-next-line: no-any - interpreter: {} as any - }; - workspace.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); selector.getSuggestions = () => Promise.resolve([]); - appShell - .setup((s) => s.showQuickPick(TypeMoq.It.isValue([]), TypeMoq.It.isAny())) - .returns(() => Promise.resolve(selectedItem)) + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any .verifiable(TypeMoq.Times.never()); const expectedItems = [ @@ -469,6 +719,70 @@ suite('Interpreters - selector', () => { appShell.verifyAll(); workspace.verifyAll(); pythonPathUpdater.verifyAll(); + multiStepInputFactory.verifyAll(); + }); + test('Make sure multiStepInput.run is called with the correct arguments', async () => { + const pickInterpreter = sinon.stub(InterpreterSelector.prototype, '_pickInterpreter'); + selector = new TestInterpreterSelector( + interpreterService.object, + workspace.object, + appShell.object, + documentManager.object, + new PathUtils(false), + comparer.object, + pythonPathUpdater.object, + shebangProvider.object, + configurationService.object, + commandManager.object, + experimentsManager.object, + interpreterSecurityService.object, + multiStepInputFactory.object, + platformService.object + ); + let inputStep!: Function; + pythonSettings.setup((p) => p.pythonPath).returns(() => 'python'); + const selectedItem: IInterpreterQuickPickItem = { + description: '', + detail: '', + label: '', + path: 'This is the selected Python path', + // tslint:disable-next-line: no-any + interpreter: {} as any + }; + + workspace.setup((w) => w.workspaceFolders).returns(() => undefined); + + selector.getSuggestions = () => Promise.resolve([]); + const multiStepInput = { + // tslint:disable-next-line: no-any + run: (inputStepArg: any, state: InterpreterStateArgs) => { + inputStep = inputStepArg; + state.path = selectedItem.path; + return Promise.resolve(); + } + }; + multiStepInputFactory + .setup((f) => f.create()) + // tslint:disable-next-line: no-any + .returns(() => multiStepInput as any); + pythonPathUpdater + .setup((p) => + p.updatePythonPath( + TypeMoq.It.isValue(selectedItem.path), + TypeMoq.It.isValue(ConfigurationTarget.Global), + TypeMoq.It.isValue('ui'), + TypeMoq.It.isValue(undefined) + ) + ) + .returns(() => Promise.resolve()); + + await selector.setInterpreter(); + + expect(inputStep).to.not.equal(undefined, ''); + + assert(pickInterpreter.notCalled); + await inputStep(); + assert(pickInterpreter.calledOnce); }); });