From 7b273b3a5c7046c40728bd8a54025bb4bd1d34ad Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 17 Nov 2021 12:39:52 -0800 Subject: [PATCH 01/14] Proposed API initial commit --- src/client/api.ts | 85 +----------- src/client/apiTypes.ts | 125 ++++++++++++++++++ src/client/extension.ts | 9 +- src/client/interpreter/interpreterService.ts | 5 + src/client/proposedApi.ts | 113 ++++++++++++++++ .../locators/composite/envsCollectionCache.ts | 4 + src/test/common.ts | 4 +- src/test/initialize.ts | 2 +- 8 files changed, 257 insertions(+), 90 deletions(-) create mode 100644 src/client/apiTypes.ts create mode 100644 src/client/proposedApi.ts diff --git a/src/client/api.ts b/src/client/api.ts index 16eb647df7df..366515da3e83 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -4,98 +4,15 @@ 'use strict'; import { noop } from 'lodash'; -import { Event, Uri } from 'vscode'; +import { IExtensionApi } from './apiTypes'; import { isTestExecution } from './common/constants'; import { IConfigurationService, Resource } from './common/types'; import { getDebugpyLauncherArgs, getDebugpyPackagePath } from './debugger/extension/adapter/remoteLaunchers'; import { IInterpreterService } from './interpreter/contracts'; import { IServiceContainer, IServiceManager } from './ioc/types'; import { JupyterExtensionIntegration } from './jupyter/jupyterIntegration'; -import { IDataViewerDataProvider, IJupyterUriProvider } from './jupyter/types'; import { traceError } from './logging'; -/* - * Do not introduce any breaking changes to this API. - * This is the public API for other extensions to interact with this extension. - */ - -export interface IExtensionApi { - /** - * Promise indicating whether all parts of the extension have completed loading or not. - * @type {Promise} - * @memberof IExtensionApi - */ - ready: Promise; - jupyter: { - registerHooks(): void; - }; - debug: { - /** - * Generate an array of strings for commands to pass to the Python executable to launch the debugger for remote debugging. - * Users can append another array of strings of what they want to execute along with relevant arguments to Python. - * E.g `['/Users/..../pythonVSCode/pythonFiles/lib/python/debugpy', '--listen', 'localhost:57039', '--wait-for-client']` - * @param {string} host - * @param {number} port - * @param {boolean} [waitUntilDebuggerAttaches=true] - * @returns {Promise} - */ - getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean): Promise; - - /** - * Gets the path to the debugger package used by the extension. - * @returns {Promise} - */ - getDebuggerPackagePath(): Promise; - }; - /** - * Return internal settings within the extension which are stored in VSCode storage - */ - settings: { - /** - * An event that is emitted when execution details (for a resource) change. For instance, when interpreter configuration changes. - */ - readonly onDidChangeExecutionDetails: Event; - /** - * Returns all the details the consumer needs to execute code within the selected environment, - * corresponding to the specified resource taking into account any workspace-specific settings - * for the workspace to which this resource belongs. - * @param {Resource} [resource] A resource for which the setting is asked for. - * * When no resource is provided, the setting scoped to the first workspace folder is returned. - * * If no folder is present, it returns the global setting. - * @returns {({ execCommand: string[] | undefined })} - */ - getExecutionDetails( - resource?: Resource, - ): { - /** - * E.g of execution commands returned could be, - * * `['']` - * * `['']` - * * `['conda', 'run', 'python']` which is used to run from within Conda environments. - * or something similar for some other Python environments. - * - * @type {(string[] | undefined)} When return value is `undefined`, it means no interpreter is set. - * Otherwise, join the items returned using space to construct the full execution command. - */ - execCommand: string[] | undefined; - }; - }; - - datascience: { - /** - * Launches Data Viewer component. - * @param {IDataViewerDataProvider} dataProvider Instance that will be used by the Data Viewer component to fetch data. - * @param {string} title Data Viewer title - */ - showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; - /** - * Registers a remote server provider component that's used to pick remote jupyter server URIs - * @param serverProvider object called back when picking jupyter server URI - */ - registerRemoteServerProvider(serverProvider: IJupyterUriProvider): void; - }; -} - export function buildApi( ready: Promise, serviceManager: IServiceManager, diff --git a/src/client/apiTypes.ts b/src/client/apiTypes.ts new file mode 100644 index 000000000000..3a4a37764344 --- /dev/null +++ b/src/client/apiTypes.ts @@ -0,0 +1,125 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Event, Uri } from 'vscode'; +import { Resource } from './common/types'; +import { IDataViewerDataProvider, IJupyterUriProvider } from './jupyter/types'; + +/* + * Do not introduce any breaking changes to this API. + * This is the public API for other extensions to interact with this extension. + */ + +export interface IExtensionApi { + /** + * Promise indicating whether all parts of the extension have completed loading or not. + * @type {Promise} + * @memberof IExtensionApi + */ + ready: Promise; + jupyter: { + registerHooks(): void; + }; + debug: { + /** + * Generate an array of strings for commands to pass to the Python executable to launch the debugger for remote debugging. + * Users can append another array of strings of what they want to execute along with relevant arguments to Python. + * E.g `['/Users/..../pythonVSCode/pythonFiles/lib/python/debugpy', '--listen', 'localhost:57039', '--wait-for-client']` + * @param {string} host + * @param {number} port + * @param {boolean} [waitUntilDebuggerAttaches=true] + * @returns {Promise} + */ + getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean): Promise; + + /** + * Gets the path to the debugger package used by the extension. + * @returns {Promise} + */ + getDebuggerPackagePath(): Promise; + }; + /** + * Return internal settings within the extension which are stored in VSCode storage + */ + settings: { + /** + * An event that is emitted when execution details (for a resource) change. For instance, when interpreter configuration changes. + */ + readonly onDidChangeExecutionDetails: Event; + /** + * Returns all the details the consumer needs to execute code within the selected environment, + * corresponding to the specified resource taking into account any workspace-specific settings + * for the workspace to which this resource belongs. + * @param {Resource} [resource] A resource for which the setting is asked for. + * * When no resource is provided, the setting scoped to the first workspace folder is returned. + * * If no folder is present, it returns the global setting. + * @returns {({ execCommand: string[] | undefined })} + */ + getExecutionDetails( + resource?: Resource, + ): { + /** + * E.g of execution commands returned could be, + * * `['']` + * * `['']` + * * `['conda', 'run', 'python']` which is used to run from within Conda environments. + * or something similar for some other Python environments. + * + * @type {(string[] | undefined)} When return value is `undefined`, it means no interpreter is set. + * Otherwise, join the items returned using space to construct the full execution command. + */ + execCommand: string[] | undefined; + }; + }; + + datascience: { + /** + * Launches Data Viewer component. + * @param {IDataViewerDataProvider} dataProvider Instance that will be used by the Data Viewer component to fetch data. + * @param {string} title Data Viewer title + */ + showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; + /** + * Registers a remote server provider component that's used to pick remote jupyter server URIs + * @param serverProvider object called back when picking jupyter server URI + */ + registerRemoteServerProvider(serverProvider: IJupyterUriProvider): void; + }; +} + +export interface InterpreterDetailsOptions { + useCache: boolean; +} + +export interface InterpreterDetails { + path: string; + version: string[]; + environmentType: string[]; + metadata: Record; +} + +export interface InterpretersChangedParams { + path?: string; + type: 'add' | 'remove' | 'update' | 'clear-all'; +} + +export interface ActiveInterpreterChangedParams { + interpreterPath?: string; + resource?: Uri; +} + +export interface IProposedExtensionAPI { + environment: { + getActiveInterpreterPath(resource?: Resource): Promise; + getInterpreterDetails( + interpreterPath: string, + options?: InterpreterDetailsOptions, + ): Promise; + getInterpreterPaths(): Promise; + setActiveInterpreter(interpreterPath: string, resource?: Resource): Promise; + refreshInterpreters(): Promise; + getRefreshPromise(): Promise | undefined; + onDidInterpretersChanged: Event; + onDidActiveInterpreterChanged: Event; + }; +} diff --git a/src/client/extension.ts b/src/client/extension.ts index c2c756d678d3..d5bc7374b8a0 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -30,7 +30,7 @@ initializeFileLogging(logDispose); import { ProgressLocation, ProgressOptions, window } from 'vscode'; -import { buildApi, IExtensionApi } from './api'; +import { buildApi } from './api'; import { IApplicationShell, IWorkspaceService } from './common/application/types'; import { IAsyncDisposableRegistry, IDisposableRegistry, IExperimentService, IExtensionContext } from './common/types'; import { createDeferred } from './common/utils/async'; @@ -42,6 +42,8 @@ import { sendErrorTelemetry, sendStartupTelemetry } from './startupTelemetry'; import { IStartupDurations } from './types'; import { runAfterActivation } from './common/utils/runAfterActivation'; import { IInterpreterService } from './interpreter/contracts'; +import { IExtensionApi, IProposedExtensionAPI } from './apiTypes'; +import { buildProposedApi } from './proposedApi'; import { WorkspaceService } from './common/application/workspace'; durations.codeLoadingTime = stopWatch.elapsedTime; @@ -106,7 +108,7 @@ async function activateUnsafe( context: IExtensionContext, startupStopWatch: StopWatch, startupDurations: IStartupDurations, -): Promise<[IExtensionApi, Promise, IServiceContainer]> { +): Promise<[IExtensionApi & IProposedExtensionAPI, Promise, IServiceContainer]> { // Add anything that we got from initializing logs to dispose. context.subscriptions.push(...logDispose); @@ -158,7 +160,8 @@ async function activateUnsafe( }); const api = buildApi(activationPromise, ext.legacyIOC.serviceManager, ext.legacyIOC.serviceContainer); - return [api, activationPromise, ext.legacyIOC.serviceContainer]; + const proposedApi = buildProposedApi(components.pythonEnvs, ext.legacyIOC.serviceContainer); + return [{ ...api, ...proposedApi }, activationPromise, ext.legacyIOC.serviceContainer]; } function displayProgress(promise: Promise) { diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 5b46c303804f..46d542929012 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -23,6 +23,7 @@ import { PythonLocatorQuery } from '../pythonEnvironments/base/locator'; import { traceError } from '../logging'; import { PYTHON_LANGUAGE } from '../common/constants'; import { InterpreterStatusBarPosition } from '../common/experiments/groups'; +import { reportActiveInterpreterChanged } from '../proposedApi'; type StoredPythonEnvironment = PythonEnvironment & { store?: boolean }; @@ -176,6 +177,10 @@ export class InterpreterService implements Disposable, IInterpreterService { if (this._pythonPathSetting === '' || this._pythonPathSetting !== pySettings.pythonPath) { this._pythonPathSetting = pySettings.pythonPath; this.didChangeInterpreterEmitter.fire(); + reportActiveInterpreterChanged({ + interpreterPath: pySettings.pythonPath === '' ? undefined : pySettings.pythonPath, + resource, + }); const interpreterDisplay = this.serviceContainer.get(IInterpreterDisplay); interpreterDisplay.refresh().catch((ex) => traceError('Python Extension: display.refresh', ex)); } diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts new file mode 100644 index 000000000000..91d4b819f57b --- /dev/null +++ b/src/client/proposedApi.ts @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ConfigurationTarget, EventEmitter } from 'vscode'; +import { + ActiveInterpreterChangedParams, + InterpreterDetails, + InterpreterDetailsOptions, + InterpretersChangedParams, + IProposedExtensionAPI, +} from './apiTypes'; +import { IConfigurationService, IInterpreterPathService, Resource } from './common/types'; +import { IComponentAdapter } from './interpreter/contracts'; +import { IServiceContainer } from './ioc/types'; +import { PythonEnvInfo } from './pythonEnvironments/base/info'; +import { IDiscoveryAPI } from './pythonEnvironments/base/locator'; +import { PythonEnvironment } from './pythonEnvironments/info'; + +const onDidInterpretersChangedEvent = new EventEmitter(); +export function reportInterpretersChanged(e: InterpretersChangedParams[]): void { + onDidInterpretersChangedEvent.fire(e); +} + +const onDidActiveInterpreterChangedEvent = new EventEmitter(); +export function reportActiveInterpreterChanged(e: ActiveInterpreterChangedParams): void { + onDidActiveInterpreterChangedEvent.fire(e); +} + +function getVersionString(env: PythonEnvInfo): string[] { + return [ + `${env.version.major}`, + `${env.version.minor}`, + `${env.version.micro}`, + `${env.version.release ?? ''}`, + `${env.version.sysVersion ?? ''}`, + ]; +} + +function getVersionString2(env: PythonEnvironment): string[] { + return [`${env.version?.major ?? ''}`, `${env.version?.minor ?? ''}`, `${env.version?.patch ?? ''}`] + .concat(env.version?.prerelease ?? ['']) + .concat(env.version?.build ?? ['']); +} + +export function buildProposedApi( + pythonEnvironments: IDiscoveryAPI, + serviceContainer: IServiceContainer, +): IProposedExtensionAPI { + const configurationService = serviceContainer.get(IConfigurationService); + const interpreterPathService = serviceContainer.get(IInterpreterPathService); + const pyenvs = serviceContainer.get(IComponentAdapter); + + const proposed: IProposedExtensionAPI = { + environment: { + getActiveInterpreterPath(resource?: Resource): Promise { + const { pythonPath } = configurationService.getSettings(resource); + return Promise.resolve(pythonPath === '' ? undefined : pythonPath); + }, + async getInterpreterDetails( + interpreterPath: string, + options?: InterpreterDetailsOptions, + ): Promise { + if (options?.useCache) { + const interpreter = pythonEnvironments + .getEnvs() + .find((v) => v.executable.filename === interpreterPath); + if (interpreter) { + return { + path: interpreterPath, + version: getVersionString(interpreter), + environmentType: [`${interpreter.kind}`], + metadata: { + sysPrefix: interpreter.executable.sysPrefix, + }, + }; + } + return undefined; + } + + const interpreter = await pyenvs.getInterpreterDetails(interpreterPath); + if (interpreter) { + return { + path: interpreterPath, + version: getVersionString2(interpreter), + environmentType: [`${interpreter.envType}`], + metadata: { + sysPrefix: interpreter.sysPrefix, + }, + }; + } + return undefined; + }, + getInterpreterPaths(): Promise { + const paths = pythonEnvironments.getEnvs().map((e) => e.executable.filename); + return Promise.resolve(paths); + }, + setActiveInterpreter(interpreterPath: string, resource?: Resource): Promise { + return interpreterPathService.update(resource, ConfigurationTarget.Workspace, interpreterPath); + }, + async refreshInterpreters(): Promise { + await pythonEnvironments.triggerRefresh(undefined); + const paths = pythonEnvironments.getEnvs().map((e) => e.executable.filename); + return Promise.resolve(paths); + }, + getRefreshPromise(): Promise | undefined { + return pythonEnvironments.refreshPromise; + }, + onDidInterpretersChanged: onDidInterpretersChangedEvent.event, + onDidActiveInterpreterChanged: onDidActiveInterpreterChangedEvent.event, + }, + }; + return proposed; +} diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts index b9c729888c91..c1d6ef5d7213 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts @@ -3,6 +3,7 @@ import { Event } from 'vscode'; import { traceInfo } from '../../../../logging'; +import { reportInterpretersChanged } from '../../../../proposedApi'; import { pathExists } from '../../../common/externalDependencies'; import { PythonEnvInfo } from '../../info'; import { areSameEnv } from '../../info/env'; @@ -82,6 +83,7 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher { const env = this.envs.splice(index, 1)[0]; this.fire({ old: env, new: undefined }); + reportInterpretersChanged([{ path: env.executable.filename, type: 'remove' }]); }); } @@ -97,6 +99,7 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher(fn: () => Promise, timeoutMs: number = 6 // Capture result, if no exceptions return that. return result; } catch (ex) { - lastEx = ex; + lastEx = ex as Error | undefined; } await sleep(10); } diff --git a/src/test/initialize.ts b/src/test/initialize.ts index 559a033e1a3b..1fce36607df5 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -1,6 +1,6 @@ import * as path from 'path'; import * as vscode from 'vscode'; -import type { IExtensionApi } from '../client/api'; +import type { IExtensionApi } from '../client/apiTypes'; import { clearPythonPathInWorkspaceFolder, IExtensionTestApi, From 43ced1d89cd89e61c5f60ad382afd0655d124700 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 23 Nov 2021 11:30:23 -0800 Subject: [PATCH 02/14] Add API description. --- src/client/apiTypes.ts | 45 +++++++++++++++++++++++++++++++++++++++ src/client/proposedApi.ts | 1 - 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/client/apiTypes.ts b/src/client/apiTypes.ts index 3a4a37764344..92895a4ce355 100644 --- a/src/client/apiTypes.ts +++ b/src/client/apiTypes.ts @@ -110,16 +110,61 @@ export interface ActiveInterpreterChangedParams { export interface IProposedExtensionAPI { environment: { + /** + * Returns the path to the python binary selected by the user or as in the settings. + * This is just the path to the python binary, this does not provide activation or any + * other activation command. The `resource` if provided will be used to determine the + * python binary in a multi-root scenario. If resource is `undefined` then the API + * returns what ever is set for the workspace. + * @param resource : Uri of a file or workspace + */ getActiveInterpreterPath(resource?: Resource): Promise; + /** + * Returns details for the given interpreter. Details such as absolute interpreter path, + * version, type (conda, pyenv, etc). Metadata such as `sysPrefix` can be found under + * metadata field. + * @param interpreterPath : Path of the interpreter whose details you need. + * @param options : [optional] + * * useCache : When true, cache is checked first for any data, returns even if there + * is partial data. + */ getInterpreterDetails( interpreterPath: string, options?: InterpreterDetailsOptions, ): Promise; + /** + * Returns paths to interpreters found by the extension at the time of calling. This API + * will *not* trigger a refresh. If a refresh is going on it will *not* wait for the refresh + * to finish. This will return what is known so far. To get complete list `await` on promise + * returned by `getRefreshPromise()`. + */ getInterpreterPaths(): Promise; + /** + * Sets the active interpreter path for the python extension. Configuration target will + * always be the workspace. + * @param interpreterPath : Interpreter path to set for a given workspace. + * @param resource : [optional] Uri of a file ro workspace to scope to a particular workspace + * folder. + */ setActiveInterpreter(interpreterPath: string, resource?: Resource): Promise; + /** + * This API will re-trigger environment discovery. Extensions can wait on the returned + * promise to get the updated interpreters list. + */ refreshInterpreters(): Promise; + /** + * Returns a promise for the ongoing refresh. Returns `undefined` if there are no active + * refreshes going on. + */ getRefreshPromise(): Promise | undefined; + /** + * This event is triggered when the known interpreters list changes, like when a interpreter + * is found, or existing interpreter is removed. + */ onDidInterpretersChanged: Event; + /** + * This event is triggered when the active interpreter changes. + */ onDidActiveInterpreterChanged: Event; }; } diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 91d4b819f57b..d80e19331e39 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -74,7 +74,6 @@ export function buildProposedApi( }, }; } - return undefined; } const interpreter = await pyenvs.getInterpreterDetails(interpreterPath); From 64cb75dd57fca7f97a288bb532294c372215a843 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Mon, 29 Nov 2021 11:23:36 -0800 Subject: [PATCH 03/14] Adding tests initial --- src/client/proposedApi.ts | 14 +++++------- src/test/proposedApi.functional.tests.ts | 28 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 src/test/proposedApi.functional.tests.ts diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index d80e19331e39..19425d82936d 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -43,7 +43,7 @@ function getVersionString2(env: PythonEnvironment): string[] { } export function buildProposedApi( - pythonEnvironments: IDiscoveryAPI, + discoveryApi: IDiscoveryAPI, serviceContainer: IServiceContainer, ): IProposedExtensionAPI { const configurationService = serviceContainer.get(IConfigurationService); @@ -61,9 +61,7 @@ export function buildProposedApi( options?: InterpreterDetailsOptions, ): Promise { if (options?.useCache) { - const interpreter = pythonEnvironments - .getEnvs() - .find((v) => v.executable.filename === interpreterPath); + const interpreter = discoveryApi.getEnvs().find((v) => v.executable.filename === interpreterPath); if (interpreter) { return { path: interpreterPath, @@ -90,19 +88,19 @@ export function buildProposedApi( return undefined; }, getInterpreterPaths(): Promise { - const paths = pythonEnvironments.getEnvs().map((e) => e.executable.filename); + const paths = discoveryApi.getEnvs().map((e) => e.executable.filename); return Promise.resolve(paths); }, setActiveInterpreter(interpreterPath: string, resource?: Resource): Promise { return interpreterPathService.update(resource, ConfigurationTarget.Workspace, interpreterPath); }, async refreshInterpreters(): Promise { - await pythonEnvironments.triggerRefresh(undefined); - const paths = pythonEnvironments.getEnvs().map((e) => e.executable.filename); + await discoveryApi.triggerRefresh(undefined); + const paths = discoveryApi.getEnvs().map((e) => e.executable.filename); return Promise.resolve(paths); }, getRefreshPromise(): Promise | undefined { - return pythonEnvironments.refreshPromise; + return discoveryApi.refreshPromise; }, onDidInterpretersChanged: onDidInterpretersChangedEvent.event, onDidActiveInterpreterChanged: onDidActiveInterpreterChangedEvent.event, diff --git a/src/test/proposedApi.functional.tests.ts b/src/test/proposedApi.functional.tests.ts new file mode 100644 index 000000000000..5bbf9b014155 --- /dev/null +++ b/src/test/proposedApi.functional.tests.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as typemoq from 'typemoq'; +import { IConfigurationService } from '../client/common/types'; +import { IComponentAdapter, IInterpreterService } from '../client/interpreter/contracts'; +import { IServiceContainer } from '../client/ioc/types'; +import { IDiscoveryAPI } from '../client/pythonEnvironments/base/locator'; + +suite('Proposed Extension API', () => { + let serviceContainer: typemoq.IMock; + let discoverAPI: typemoq.IMock; + let interpreterService: typemoq.IMock; + let configService: typemoq.IMock; + let pyenvs: typemoq.IMock; + + setup(() => { + serviceContainer = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + discoverAPI = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + interpreterService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + configService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + pyenvs = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + + serviceContainer.setup((s) => s.get(IInterpreterService)).returns(() => interpreterService.object); + serviceContainer.setup((s) => s.get(IConfigurationService)).returns(() => configService.object); + serviceContainer.setup((s) => s.get(IComponentAdapter)).returns(() => pyenvs.object); + }); +}); From 662229b7f44a0b77815fabd3eebefc51a8f56867 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 13 Jan 2022 14:20:25 -0800 Subject: [PATCH 04/14] Added some tests --- src/test/proposedApi.functional.tests.ts | 28 --------- src/test/proposedApi.unit.test.ts | 72 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 28 deletions(-) delete mode 100644 src/test/proposedApi.functional.tests.ts create mode 100644 src/test/proposedApi.unit.test.ts diff --git a/src/test/proposedApi.functional.tests.ts b/src/test/proposedApi.functional.tests.ts deleted file mode 100644 index 5bbf9b014155..000000000000 --- a/src/test/proposedApi.functional.tests.ts +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import * as typemoq from 'typemoq'; -import { IConfigurationService } from '../client/common/types'; -import { IComponentAdapter, IInterpreterService } from '../client/interpreter/contracts'; -import { IServiceContainer } from '../client/ioc/types'; -import { IDiscoveryAPI } from '../client/pythonEnvironments/base/locator'; - -suite('Proposed Extension API', () => { - let serviceContainer: typemoq.IMock; - let discoverAPI: typemoq.IMock; - let interpreterService: typemoq.IMock; - let configService: typemoq.IMock; - let pyenvs: typemoq.IMock; - - setup(() => { - serviceContainer = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); - discoverAPI = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); - interpreterService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); - configService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); - pyenvs = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); - - serviceContainer.setup((s) => s.get(IInterpreterService)).returns(() => interpreterService.object); - serviceContainer.setup((s) => s.get(IConfigurationService)).returns(() => configService.object); - serviceContainer.setup((s) => s.get(IComponentAdapter)).returns(() => pyenvs.object); - }); -}); diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts new file mode 100644 index 000000000000..c875c429e940 --- /dev/null +++ b/src/test/proposedApi.unit.test.ts @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as typemoq from 'typemoq'; +import { expect } from 'chai'; +import { Uri } from 'vscode'; +import { IProposedExtensionAPI } from '../client/apiTypes'; +import { IConfigurationService, IInterpreterPathService, IPythonSettings } from '../client/common/types'; +import { IComponentAdapter } from '../client/interpreter/contracts'; +import { IServiceContainer } from '../client/ioc/types'; +import { buildProposedApi } from '../client/proposedApi'; +import { IDiscoveryAPI } from '../client/pythonEnvironments/base/locator'; + +suite('Proposed Extension API', () => { + let serviceContainer: typemoq.IMock; + let discoverAPI: typemoq.IMock; + let interpreterPathService: typemoq.IMock; + let configService: typemoq.IMock; + let pyenvs: typemoq.IMock; + + let proposed: IProposedExtensionAPI; + + setup(() => { + serviceContainer = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + discoverAPI = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + interpreterPathService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + configService = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + pyenvs = typemoq.Mock.ofType(undefined, typemoq.MockBehavior.Strict); + + serviceContainer.setup((s) => s.get(IInterpreterPathService)).returns(() => interpreterPathService.object); + serviceContainer.setup((s) => s.get(IConfigurationService)).returns(() => configService.object); + serviceContainer.setup((s) => s.get(IComponentAdapter)).returns(() => pyenvs.object); + + proposed = buildProposedApi(discoverAPI.object, serviceContainer.object); + }); + + test('getActiveInterpreterPath: No resource', async () => { + const pythonPath = 'this/is/a/test/path'; + configService + .setup((c) => c.getSettings(undefined)) + .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); + const actual = await proposed.environment.getActiveInterpreterPath(); + expect(actual).to.be.equals(pythonPath); + }); + test('getActiveInterpreterPath: With resource', async () => { + const resource = Uri.file(__filename); + const pythonPath = 'this/is/a/test/path'; + configService + .setup((c) => c.getSettings(resource)) + .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); + const actual = await proposed.environment.getActiveInterpreterPath(resource); + expect(actual).to.be.equals(pythonPath); + }); + + test('getInterpreterDetails: no discovered python', async () => { + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); + pyenvs.setup((p) => p.getInterpreterDetails(typemoq.It.isAny())).returns(() => Promise.resolve(undefined)); + + const pythonPath = 'this/is/a/test/path'; + const actual = await proposed.environment.getInterpreterDetails(pythonPath); + expect(actual).to.be.equal(undefined); + }); + + test('getInterpreterDetails: no discovered python (with cache)', async () => { + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); + pyenvs.setup((p) => p.getInterpreterDetails(typemoq.It.isAny())).returns(() => Promise.resolve(undefined)); + + const pythonPath = 'this/is/a/test/path'; + const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: true }); + expect(actual).to.be.equal(undefined); + }); +}); From 6f231182358ce1e0b5845b6e7cfde40d2df8ffa0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 11:32:15 -0800 Subject: [PATCH 05/14] add getInterpreterDetails tests --- src/client/proposedApi.ts | 17 ++-- src/test/proposedApi.unit.test.ts | 143 +++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 9 deletions(-) diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 19425d82936d..6ca3d1262969 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -27,13 +27,14 @@ export function reportActiveInterpreterChanged(e: ActiveInterpreterChangedParams } function getVersionString(env: PythonEnvInfo): string[] { - return [ - `${env.version.major}`, - `${env.version.minor}`, - `${env.version.micro}`, - `${env.version.release ?? ''}`, - `${env.version.sysVersion ?? ''}`, - ]; + const ver = [`${env.version.major}`, `${env.version.minor}`, `${env.version.micro}`]; + if (env.version.release) { + ver.push(`${env.version.release}`); + if (env.version.sysVersion) { + ver.push(`${env.version.release}`); + } + } + return ver; } function getVersionString2(env: PythonEnvironment): string[] { @@ -69,6 +70,7 @@ export function buildProposedApi( environmentType: [`${interpreter.kind}`], metadata: { sysPrefix: interpreter.executable.sysPrefix, + bitness: interpreter.arch, }, }; } @@ -82,6 +84,7 @@ export function buildProposedApi( environmentType: [`${interpreter.envType}`], metadata: { sysPrefix: interpreter.sysPrefix, + bitness: interpreter.architecture, }, }; } diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index c875c429e940..dbb61c383a12 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -4,12 +4,15 @@ import * as typemoq from 'typemoq'; import { expect } from 'chai'; import { Uri } from 'vscode'; -import { IProposedExtensionAPI } from '../client/apiTypes'; +import { InterpreterDetails, IProposedExtensionAPI } from '../client/apiTypes'; import { IConfigurationService, IInterpreterPathService, IPythonSettings } from '../client/common/types'; import { IComponentAdapter } from '../client/interpreter/contracts'; import { IServiceContainer } from '../client/ioc/types'; import { buildProposedApi } from '../client/proposedApi'; import { IDiscoveryAPI } from '../client/pythonEnvironments/base/locator'; +import { EnvironmentType } from '../client/pythonEnvironments/info'; +import { PythonEnvKind, PythonEnvSource } from '../client/pythonEnvironments/base/info'; +import { Architecture } from '../client/common/utils/platform'; suite('Proposed Extension API', () => { let serviceContainer: typemoq.IMock; @@ -56,7 +59,7 @@ suite('Proposed Extension API', () => { discoverAPI.setup((d) => d.getEnvs()).returns(() => []); pyenvs.setup((p) => p.getInterpreterDetails(typemoq.It.isAny())).returns(() => Promise.resolve(undefined)); - const pythonPath = 'this/is/a/test/path'; + const pythonPath = 'this/is/a/test/path (without cache)'; const actual = await proposed.environment.getInterpreterDetails(pythonPath); expect(actual).to.be.equal(undefined); }); @@ -69,4 +72,140 @@ suite('Proposed Extension API', () => { const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: true }); expect(actual).to.be.equal(undefined); }); + + test('getInterpreterDetails: without cache', async () => { + const pythonPath = 'this/is/a/test/path'; + + const expected: InterpreterDetails = { + path: pythonPath, + version: ['3', '9', '0'], + environmentType: [`${EnvironmentType.System}`], + metadata: { + sysPrefix: 'prefix/path', + bitness: Architecture.x64, + }, + }; + + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); + pyenvs + .setup((p) => p.getInterpreterDetails(pythonPath)) + .returns(() => + Promise.resolve({ + path: pythonPath, + version: { + raw: '3.9.0', + major: 3, + minor: 9, + patch: 0, + build: [], + prerelease: [], + }, + envType: EnvironmentType.System, + architecture: Architecture.x64, + sysPrefix: 'prefix/path', + }), + ); + + const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: false }); + expect(actual).to.be.deep.equal(expected); + }); + + test('getInterpreterDetails: from cache', async () => { + const pythonPath = 'this/is/a/test/path'; + + const expected: InterpreterDetails = { + path: pythonPath, + version: ['3', '9', '0'], + environmentType: [`${PythonEnvKind.System}`], + metadata: { + sysPrefix: 'prefix/path', + bitness: Architecture.x64, + }, + }; + + discoverAPI + .setup((d) => d.getEnvs()) + .returns(() => [ + { + executable: { + filename: pythonPath, + ctime: 1, + mtime: 2, + sysPrefix: 'prefix/path', + }, + version: { + major: 3, + minor: 9, + micro: 0, + }, + kind: PythonEnvKind.System, + arch: Architecture.x64, + name: '', + location: '', + source: [PythonEnvSource.PathEnvVar], + distro: { + org: '', + }, + }, + ]); + pyenvs + .setup((p) => p.getInterpreterDetails(pythonPath)) + .returns(() => + Promise.resolve({ + path: pythonPath, + version: { + raw: '3.9.0', + major: 3, + minor: 9, + patch: 0, + build: [], + prerelease: [], + }, + envType: EnvironmentType.System, + architecture: Architecture.x64, + sysPrefix: 'prefix/path', + }), + ); + + const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: true }); + expect(actual).to.be.deep.equal(expected); + }); + + test('getInterpreterDetails: cache miss', async () => { + const pythonPath = 'this/is/a/test/path'; + + const expected: InterpreterDetails = { + path: pythonPath, + version: ['3', '9', '0'], + environmentType: [`${EnvironmentType.System}`], + metadata: { + sysPrefix: 'prefix/path', + bitness: Architecture.x64, + }, + }; + + // Force this API to return empty to cause a cache miss. + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); + pyenvs + .setup((p) => p.getInterpreterDetails(pythonPath)) + .returns(() => + Promise.resolve({ + path: pythonPath, + version: { + raw: '3.9.0', + major: 3, + minor: 9, + patch: 0, + build: [], + prerelease: [], + }, + envType: EnvironmentType.System, + architecture: Architecture.x64, + sysPrefix: 'prefix/path', + }), + ); + + const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: true }); + expect(actual).to.be.deep.equal(expected); + }); }); From 6e7a79d26415165a731600f73d8aec65258ba68d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 11:59:32 -0800 Subject: [PATCH 06/14] Added tests for getInterpreterPaths --- src/test/proposedApi.unit.test.ts | 57 +++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index dbb61c383a12..834257cc52ee 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -208,4 +208,61 @@ suite('Proposed Extension API', () => { const actual = await proposed.environment.getInterpreterDetails(pythonPath, { useCache: true }); expect(actual).to.be.deep.equal(expected); }); + + test('getInterpreterPaths: no pythons found', async () => { + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); + const actual = await proposed.environment.getInterpreterPaths(); + expect(actual).to.be.deep.equal([]); + }); + + test('getInterpreterPaths: python found', async () => { + discoverAPI + .setup((d) => d.getEnvs()) + .returns(() => [ + { + executable: { + filename: 'this/is/a/test/python/path1', + ctime: 1, + mtime: 2, + sysPrefix: 'prefix/path', + }, + version: { + major: 3, + minor: 9, + micro: 0, + }, + kind: PythonEnvKind.System, + arch: Architecture.x64, + name: '', + location: '', + source: [PythonEnvSource.PathEnvVar], + distro: { + org: '', + }, + }, + { + executable: { + filename: 'this/is/a/test/python/path2', + ctime: 1, + mtime: 2, + sysPrefix: 'prefix/path', + }, + version: { + major: 3, + minor: 10, + micro: 0, + }, + kind: PythonEnvKind.Venv, + arch: Architecture.x64, + name: '', + location: '', + source: [PythonEnvSource.PathEnvVar], + distro: { + org: '', + }, + }, + ]); + const actual = await proposed.environment.getInterpreterPaths(); + expect(actual).to.be.deep.equal(['this/is/a/test/python/path1', 'this/is/a/test/python/path2']); + }); }); From 73a70ee500c66fbc3bb29a04bd777dab99966c7b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 12:23:56 -0800 Subject: [PATCH 07/14] added tests for reportActiveInterpreterChanged --- .../activation/activationService.unit.test.ts | 4 +- src/test/common/process/logger.unit.test.ts | 66 ++++++++++++------- .../interpreterService.unit.test.ts | 23 ++++++- 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 867e0b9c4079..7017506ca0f8 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -601,7 +601,7 @@ suite('Language Server Activation - ActivationService', () => { await activationService.get(resource, interpreter); - traceLogStub.calledOnceWithExactly(outputString); + sinon.assert.calledOnceWithExactly(traceLogStub, outputString); activator.verify((a) => a.start(resource, interpreter), TypeMoq.Times.once()); }); }); @@ -624,7 +624,7 @@ suite('Language Server Activation - ActivationService', () => { await activationService.get(resource, interpreter); - traceLogStub.calledOnceWithExactly(LanguageService.startingPylance()); + sinon.assert.calledOnceWithExactly(traceLogStub, LanguageService.startingPylance()); activator.verify((a) => a.start(resource, interpreter), TypeMoq.Times.once()); }); }); diff --git a/src/test/common/process/logger.unit.test.ts b/src/test/common/process/logger.unit.test.ts index 040a3032de52..65e48c741197 100644 --- a/src/test/common/process/logger.unit.test.ts +++ b/src/test/common/process/logger.unit.test.ts @@ -40,72 +40,88 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar'], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger adds quotes around arguments if they contain spaces', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', 'import test'], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar "import test"`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, + `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, + ); }); test('Logger preserves quotes around arguments if they contain spaces', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', '"import test"'], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar "import test"`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, + `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, + ); }); test('Logger converts single quotes around arguments to double quotes if they contain spaces', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', "'import test'"], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar "import test"`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, + `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, + ); }); test('Logger removes single quotes around arguments if they do not contain spaces', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', "'importtest'"], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar importtest`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar importtest`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, + `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, + ); }); test('Logger replaces the path/to/home with ~ in the current working directory', async () => { const options = { cwd: path.join(untildify('~'), 'debug', 'path') }; logger.logProcess('test', ['--foo', '--bar'], options); - traceInfoStub.calledOnceWithExactly(`> test --foo --bar`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${path.join('~', 'debug', 'path')}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, + `${Logging.currentWorkingDirectory()} ${path.join('~', 'debug', 'path')}`, + ); }); test('Logger replaces the path/to/home with ~ in the command path', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - traceInfoStub.calledOnceWithExactly(`> ${path.join('~', 'test')} --foo --bar`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ if shell command is provided', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess(`"${path.join(untildify('~'), 'test')}" "--foo" "--bar"`, undefined, options); - traceInfoStub.calledOnceWithExactly(`> "${path.join('~', 'test')}" "--foo" "--bar"`); - traceInfoStub.calledOnceWithExactly(`${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> "${path.join('~', 'test')}" "--foo" "--bar"`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger replaces the path to workspace with . if exactly one workspace folder is opened', async () => { const options = { cwd: path.join('path', 'to', 'workspace', 'debug', 'path') }; logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - traceInfoStub.calledOnceWithExactly(`> ".${path.sep}test" "--foo" "--bar"`); - traceInfoStub.calledOnceWithExactly( + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); }); @@ -118,8 +134,9 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - traceInfoStub.calledOnceWithExactly(`> ".${path.sep}test" "--foo" "--bar"`); - traceInfoStub.calledOnceWithExactly( + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); traceInfoStub.resetHistory(); @@ -127,8 +144,9 @@ suite('ProcessLogger suite', () => { options = { cwd: path.join('path\\to\\workspace', 'debug', 'path') }; logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - traceInfoStub.calledOnceWithExactly(`> ".${path.sep}test" "--foo" "--bar"`); - traceInfoStub.calledOnceWithExactly( + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledOnceWithExactly( + traceInfoStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); }); @@ -136,13 +154,13 @@ suite('ProcessLogger suite', () => { test("Logger doesn't display the working directory line if there is no options parameter", async () => { logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar']); - traceInfoStub.calledOnceWithExactly(`> ${path.join('~', 'test')} --foo --bar`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar`); }); test("Logger doesn't display the working directory line if there is no cwd key in the options parameter", async () => { const options = {}; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - traceInfoStub.calledOnceWithExactly(`> ${path.join('~', 'test')} --foo --bar\n`); + sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar\n`); }); }); diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index d3119f589815..d20acd6b2991 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -8,6 +8,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import { Container } from 'inversify'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; +import * as sinon from 'sinon'; import { ConfigurationTarget, Disposable, TextDocument, TextEditor, Uri, WorkspaceConfiguration } from 'vscode'; import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; import { InterpreterStatusBarPosition } from '../../client/common/experiments/groups'; @@ -34,6 +35,7 @@ import { ServiceContainer } from '../../client/ioc/container'; import { ServiceManager } from '../../client/ioc/serviceManager'; import { PYTHON_PATH } from '../common'; import { MockAutoSelectionService } from '../mocks/autoSelector'; +import * as proposedApi from '../../client/proposedApi'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -56,8 +58,9 @@ suite('Interpreters service', () => { let interpreterPathService: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; let experiments: TypeMoq.IMock; + let reportActiveInterpreterChangedStub: sinon.SinonStub; - function setupSuite() { + setup(() => { const cont = new Container(); serviceManager = new ServiceManager(cont); serviceContainer = new ServiceContainer(cont); @@ -131,9 +134,14 @@ suite('Interpreters service', () => { MockAutoSelectionService, ); serviceManager.addSingletonInstance(IConfigurationService, configService.object); - } - setup(setupSuite); + reportActiveInterpreterChangedStub = sinon.stub(proposedApi, 'reportActiveInterpreterChanged'); + }); + + teardown(() => { + sinon.restore(); + }); + [undefined, Uri.file('xyz')].forEach((resource) => { const resourceTestSuffix = `(${resource ? 'with' : 'without'} a resource)`; @@ -249,6 +257,10 @@ suite('Interpreters service', () => { .verifiable(TypeMoq.Times.once()); service._onConfigChanged(resource); interpreterDisplay.verifyAll(); + sinon.assert.calledOnceWithExactly(reportActiveInterpreterChangedStub, { + interpreterPath: 'current path', + resource, + }); }); test('If stored setting is not equal to current interpreter path setting, refresh the interpreter display', async () => { @@ -263,6 +275,10 @@ suite('Interpreters service', () => { .verifiable(TypeMoq.Times.once()); service._onConfigChanged(resource); interpreterDisplay.verifyAll(); + sinon.assert.calledOnceWithExactly(reportActiveInterpreterChangedStub, { + interpreterPath: 'current path', + resource, + }); }); test('If stored setting is equal to current interpreter path setting, do not refresh the interpreter display', async () => { @@ -277,5 +293,6 @@ suite('Interpreters service', () => { .verifiable(TypeMoq.Times.never()); service._onConfigChanged(resource); interpreterDisplay.verifyAll(); + expect(reportActiveInterpreterChangedStub.notCalled).to.be.equal(true); }); }); From 2ea3b26afcb97fd9b542f2b1c4dda2be7e5021de Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 15:03:40 -0800 Subject: [PATCH 08/14] Tests for setActiveInterpreter --- src/test/proposedApi.unit.test.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index 834257cc52ee..e1c041f91182 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -3,7 +3,7 @@ import * as typemoq from 'typemoq'; import { expect } from 'chai'; -import { Uri } from 'vscode'; +import { ConfigurationTarget, Uri } from 'vscode'; import { InterpreterDetails, IProposedExtensionAPI } from '../client/apiTypes'; import { IConfigurationService, IInterpreterPathService, IPythonSettings } from '../client/common/types'; import { IComponentAdapter } from '../client/interpreter/contracts'; @@ -265,4 +265,26 @@ suite('Proposed Extension API', () => { const actual = await proposed.environment.getInterpreterPaths(); expect(actual).to.be.deep.equal(['this/is/a/test/python/path1', 'this/is/a/test/python/path2']); }); + + test('setActiveInterpreter: no resource', async () => { + interpreterPathService + .setup((i) => i.update(undefined, ConfigurationTarget.Workspace, 'this/is/a/test/python/path')) + .returns(() => Promise.resolve()) + .verifiable(typemoq.Times.once()); + + await proposed.environment.setActiveInterpreter('this/is/a/test/python/path'); + + interpreterPathService.verifyAll(); + }); + test('setActiveInterpreter: with resource', async () => { + const resource = Uri.parse('a'); + interpreterPathService + .setup((i) => i.update(resource, ConfigurationTarget.Workspace, 'this/is/a/test/python/path')) + .returns(() => Promise.resolve()) + .verifiable(typemoq.Times.once()); + + await proposed.environment.setActiveInterpreter('this/is/a/test/python/path', resource); + + interpreterPathService.verifyAll(); + }); }); From 3583a7a5524e5ccb6ff6854b1dae61653170e7d2 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 18:04:13 -0800 Subject: [PATCH 09/14] Add tests for reportInterpretersChanged --- .../envsCollectionService.unit.test.ts | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) diff --git a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts index 3b50e75d436e..f8903adb6fae 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts @@ -4,9 +4,11 @@ import { assert, expect } from 'chai'; import { cloneDeep } from 'lodash'; import * as path from 'path'; +import * as sinon from 'sinon'; import { EventEmitter, Uri } from 'vscode'; import { FileChangeType } from '../../../../../client/common/platform/fileSystemWatcher'; import { createDeferred, createDeferredFromPromise, sleep } from '../../../../../client/common/utils/async'; +import * as proposedApi from '../../../../../client/proposedApi'; import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info'; import { buildEnvInfo } from '../../../../../client/pythonEnvironments/base/info/env'; import { PythonEnvUpdatedEvent } from '../../../../../client/pythonEnvironments/base/locator'; @@ -24,6 +26,8 @@ import { assertEnvEqual, assertEnvsEqual } from '../envTestUtils'; suite('Python envs locator - Environments Collection', async () => { let collectionService: EnvsCollectionService; let storage: PythonEnvInfo[]; + let reportInterpretersChangedStub: sinon.SinonStub; + const updatedName = 'updatedName'; function applyChangeEventToEnvList(envs: PythonEnvInfo[], event: PythonEnvCollectionChangedEvent) { @@ -107,6 +111,11 @@ suite('Python envs locator - Environments Collection', async () => { }, }); collectionService = new EnvsCollectionService(cache, parentLocator); + reportInterpretersChangedStub = sinon.stub(proposedApi, 'reportInterpretersChanged'); + }); + + teardown(() => { + sinon.restore(); }); test('getEnvs() returns valid envs from cache', () => { @@ -148,6 +157,15 @@ suite('Python envs locator - Environments Collection', async () => { return e; }), ); + envs.forEach((e) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [ + { + path: e.executable.filename, + type: 'add', + }, + ]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, envs.length); }); test('triggerRefresh() refreshes the collection and storage with any new environments', async () => { @@ -180,6 +198,67 @@ suite('Python envs locator - Environments Collection', async () => { const expected = getExpectedEnvs(); assertEnvsEqual(envs, expected); assertEnvsEqual(storage, expected); + + const eventData = [ + { + path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), + type: 'remove', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'add', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'pipenv', 'project1', '.venv', 'Scripts', 'python.exe'), + type: 'update', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'remove', + }, + ]; + eventData.forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, eventData.length); }); test('Ensure correct events are fired when collection changes on refresh', async () => { @@ -221,6 +300,67 @@ suite('Python envs locator - Environments Collection', async () => { }); const expected = getExpectedEnvs(); assertEnvsEqual(envs, expected); + + const eventData = [ + { + path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), + type: 'remove', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'add', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'pipenv', 'project1', '.venv', 'Scripts', 'python.exe'), + type: 'update', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'update', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'remove', + }, + ]; + eventData.forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, eventData.length); }); test('refreshPromise() correctly indicates the status of the refresh', async () => { @@ -256,6 +396,38 @@ suite('Python envs locator - Environments Collection', async () => { true, 'Any previous refresh promises should be resolved when refresh is over', ); + + const eventData = [ + { + path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), + type: 'remove', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'add', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'add', + }, + ]; + eventData.forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, eventData.length); }); test('onRefreshStart() is fired if refresh is triggered', async () => { @@ -266,6 +438,34 @@ suite('Python envs locator - Environments Collection', async () => { collectionService.triggerRefresh().ignoreErrors(); await sleep(1); expect(isFired).to.equal(true); + + const eventData = [ + { + path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), + type: 'add', + }, + { + path: path.join( + TEST_LAYOUT_ROOT, + 'pyenv2', + '.pyenv', + 'pyenv-win', + 'versions', + '3.6.9', + 'bin', + 'python.exe', + ), + type: 'add', + }, + { + path: path.join(TEST_LAYOUT_ROOT, 'virtualhome', '.venvs', 'win1', 'python.exe'), + type: 'add', + }, + ]; + eventData.forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, eventData.length); }); test('resolveEnv() uses cache if complete info is available', async () => { @@ -288,6 +488,7 @@ suite('Python envs locator - Environments Collection', async () => { collectionService = new EnvsCollectionService(cache, parentLocator); const resolved = await collectionService.resolveEnv(env.executable.filename); assertEnvEqual(resolved, env); + sinon.assert.calledOnce(reportInterpretersChangedStub); }); test('resolveEnv() uses underlying locator if cache does not have complete info for env', async () => { @@ -310,6 +511,22 @@ suite('Python envs locator - Environments Collection', async () => { collectionService = new EnvsCollectionService(cache, parentLocator); const resolved = await collectionService.resolveEnv(env.executable.filename); assertEnvEqual(resolved, resolvedViaLocator); + + const eventData = [ + { + path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), + type: 'remove', + }, + + { + path: 'Resolved via locator', + type: 'add', + }, + ]; + eventData.forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.callCount(reportInterpretersChangedStub, eventData.length); }); test('resolveEnv() adds env to cache after resolving using downstream locator', async () => { @@ -333,6 +550,7 @@ suite('Python envs locator - Environments Collection', async () => { const envs = collectionService.getEnvs(); expect(resolved?.hasCompleteInfo).to.equal(true); assertEnvsEqual(envs, [resolved]); + sinon.assert.calledOnce(reportInterpretersChangedStub); }); test('Ensure events from downstream locators do not trigger new refreshes if a refresh is already scheduled', async () => { @@ -385,5 +603,6 @@ suite('Python envs locator - Environments Collection', async () => { events.sort((a, b) => (a.type && b.type ? a.type?.localeCompare(b.type) : 0)), downstreamEvents.sort((a, b) => (a.type && b.type ? a.type?.localeCompare(b.type) : 0)), ); + sinon.assert.notCalled(reportInterpretersChangedStub); }); }); From 3c1892d0f4f9ecd6263b4be79e01c9039e56a6e6 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 18:04:41 -0800 Subject: [PATCH 10/14] Tests for refreshInterpreters and getRefreshPromise --- src/test/proposedApi.unit.test.ts | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index e1c041f91182..a85b04e23b70 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -287,4 +287,72 @@ suite('Proposed Extension API', () => { interpreterPathService.verifyAll(); }); + + test('refreshInterpreters: common scenario', async () => { + discoverAPI + .setup((d) => d.triggerRefresh(undefined)) + .returns(() => Promise.resolve()) + .verifiable(typemoq.Times.once()); + discoverAPI + .setup((d) => d.getEnvs()) + .returns(() => [ + { + executable: { + filename: 'this/is/a/test/python/path1', + ctime: 1, + mtime: 2, + sysPrefix: 'prefix/path', + }, + version: { + major: 3, + minor: 9, + micro: 0, + }, + kind: PythonEnvKind.System, + arch: Architecture.x64, + name: '', + location: '', + source: [PythonEnvSource.PathEnvVar], + distro: { + org: '', + }, + }, + { + executable: { + filename: 'this/is/a/test/python/path2', + ctime: 1, + mtime: 2, + sysPrefix: 'prefix/path', + }, + version: { + major: 3, + minor: 10, + micro: 0, + }, + kind: PythonEnvKind.Venv, + arch: Architecture.x64, + name: '', + location: '', + source: [PythonEnvSource.PathEnvVar], + distro: { + org: '', + }, + }, + ]); + + const actual = await proposed.environment.refreshInterpreters(); + expect(actual).to.be.deep.equal(['this/is/a/test/python/path1', 'this/is/a/test/python/path2']); + discoverAPI.verifyAll(); + }); + + test('getRefreshPromise: common scenario', () => { + const expected = Promise.resolve(); + discoverAPI.setup((d) => d.refreshPromise).returns(() => expected); + const actual = proposed.environment.getRefreshPromise(); + + // We are comparing instances here, they should be the same instance. + // So '==' is ok here. + // eslint-disable-next-line eqeqeq + expect(actual == expected).is.equal(true); + }); }); From 91b604560c0507512a63e356d2085558e6944fab Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 18:19:52 -0800 Subject: [PATCH 11/14] Add news item. --- news/1 Enhancements/17905.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/17905.md diff --git a/news/1 Enhancements/17905.md b/news/1 Enhancements/17905.md new file mode 100644 index 000000000000..7eb29a0a1c0c --- /dev/null +++ b/news/1 Enhancements/17905.md @@ -0,0 +1 @@ +Public API for environments (proposed). From 46daaeaa65973a01859c7c5e315b72cea310b3bc Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 18:37:35 -0800 Subject: [PATCH 12/14] Update documentation and support for 'clear-all' event. --- src/client/apiTypes.ts | 5 +++-- .../composite/envsCollectionService.ts | 8 ++++++++ .../envsCollectionService.unit.test.ts | 19 +++++++++++++++++-- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/client/apiTypes.ts b/src/client/apiTypes.ts index 92895a4ce355..94a33692f491 100644 --- a/src/client/apiTypes.ts +++ b/src/client/apiTypes.ts @@ -149,7 +149,8 @@ export interface IProposedExtensionAPI { setActiveInterpreter(interpreterPath: string, resource?: Resource): Promise; /** * This API will re-trigger environment discovery. Extensions can wait on the returned - * promise to get the updated interpreters list. + * promise to get the updated interpreters list. If there is a refresh already going on + * then it returns the promise for that refresh. */ refreshInterpreters(): Promise; /** @@ -159,7 +160,7 @@ export interface IProposedExtensionAPI { getRefreshPromise(): Promise | undefined; /** * This event is triggered when the known interpreters list changes, like when a interpreter - * is found, or existing interpreter is removed. + * is found, existing interpreter is removed, or some details changed on an interpreter. */ onDidInterpretersChanged: Event; /** diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index c74661db0eec..13218eb63b80 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -6,6 +6,7 @@ import '../../../../common/extensions'; import { createDeferred } from '../../../../common/utils/async'; import { StopWatch } from '../../../../common/utils/stopWatch'; import { traceError } from '../../../../logging'; +import { reportInterpretersChanged } from '../../../../proposedApi'; import { sendTelemetryEvent } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; import { PythonEnvInfo } from '../../info'; @@ -91,6 +92,13 @@ export class EnvsCollectionService extends PythonEnvsWatcher { const stopWatch = new StopWatch(); const deferred = createDeferred(); + + if (!query) { + // `undefined` query means this is full refresh of environments. + // Trigger an event indicating that we need to clear everything and start + // fresh. + reportInterpretersChanged([{ path: undefined, type: 'clear-all' }]); + } // Ensure we set this before we trigger the promise to accurately track when a refresh has started. this.refreshPromises.set(query, deferred.promise); this.refreshStarted.fire(); diff --git a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts index f8903adb6fae..e4956b8ea0ac 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts @@ -157,6 +157,8 @@ suite('Python envs locator - Environments Collection', async () => { return e; }), ); + + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [{ path: undefined, type: 'clear-all' }]); envs.forEach((e) => { sinon.assert.calledWithExactly(reportInterpretersChangedStub, [ { @@ -165,7 +167,9 @@ suite('Python envs locator - Environments Collection', async () => { }, ]); }); - sinon.assert.callCount(reportInterpretersChangedStub, envs.length); + + // One for each environment and one for global 'clear-all' event. + sinon.assert.callCount(reportInterpretersChangedStub, envs.length + 1); }); test('triggerRefresh() refreshes the collection and storage with any new environments', async () => { @@ -200,6 +204,7 @@ suite('Python envs locator - Environments Collection', async () => { assertEnvsEqual(storage, expected); const eventData = [ + { path: undefined, type: 'clear-all' }, { path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), type: 'remove', @@ -302,6 +307,7 @@ suite('Python envs locator - Environments Collection', async () => { assertEnvsEqual(envs, expected); const eventData = [ + { path: undefined, type: 'clear-all' }, { path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), type: 'remove', @@ -398,6 +404,7 @@ suite('Python envs locator - Environments Collection', async () => { ); const eventData = [ + { path: undefined, type: 'clear-all' }, { path: path.join(TEST_LAYOUT_ROOT, 'doesNotExist'), type: 'remove', @@ -440,6 +447,7 @@ suite('Python envs locator - Environments Collection', async () => { expect(isFired).to.equal(true); const eventData = [ + { path: undefined, type: 'clear-all' }, { path: path.join(TEST_LAYOUT_ROOT, 'conda1', 'python.exe'), type: 'add', @@ -603,6 +611,13 @@ suite('Python envs locator - Environments Collection', async () => { events.sort((a, b) => (a.type && b.type ? a.type?.localeCompare(b.type) : 0)), downstreamEvents.sort((a, b) => (a.type && b.type ? a.type?.localeCompare(b.type) : 0)), ); - sinon.assert.notCalled(reportInterpretersChangedStub); + + [ + { path: undefined, type: 'clear-all' }, + { path: undefined, type: 'clear-all' }, + ].forEach((d) => { + sinon.assert.calledWithExactly(reportInterpretersChangedStub, [d]); + }); + sinon.assert.calledTwice(reportInterpretersChangedStub); }); }); From 74836e2f5b421cb144cc4f2870d294175d51959f Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 18:55:12 -0800 Subject: [PATCH 13/14] Fix tests and linting. --- .github/ISSUE_TEMPLATE/config.yml | 22 +++--- src/test/common/process/logger.unit.test.ts | 70 +++++++++---------- .../envsCollectionService.unit.test.ts | 4 +- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 89988bf5c8f4..82af056110ab 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,20 +1,20 @@ blank_issues_enabled: false contact_links: - - name: "Bug 🐜" + - name: 'Bug 🐜' url: https://aka.ms/pvsc-bug - about: "Use the `Python: Report Issue...` command (follow the link for instructions)" - - name: "Pylance" + about: 'Use the `Python: Report Issue...` command (follow the link for instructions)' + - name: 'Pylance' url: https://github.com/microsoft/pylance-release/issues - about: "For issues relating to the Pylance language server extension" - - name: "Jupyter" + about: 'For issues relating to the Pylance language server extension' + - name: 'Jupyter' url: https://github.com/microsoft/vscode-jupyter/issues - about: "For issues relating to the Jupyter extension (including the interactive window)" - - name: "Debugpy" + about: 'For issues relating to the Jupyter extension (including the interactive window)' + - name: 'Debugpy' url: https://github.com/microsoft/debugpy/issues - about: "For issues relating to the debugpy debugger" + about: 'For issues relating to the debugpy debugger' - name: Help/Support url: https://github.com/microsoft/vscode-python/discussions/categories/q-a - about: "Having trouble with the extension? Need help getting something to work?" - - name: "Chat" + about: 'Having trouble with the extension? Need help getting something to work?' + - name: 'Chat' url: https://aka.ms/python-discord - about: "You can ask for help or chat in the `#vscode` channel of our microsoft-python Discord server" + about: 'You can ask for help or chat in the `#vscode` channel of our microsoft-python Discord server' diff --git a/src/test/common/process/logger.unit.test.ts b/src/test/common/process/logger.unit.test.ts index 65e48c741197..786d5784e92e 100644 --- a/src/test/common/process/logger.unit.test.ts +++ b/src/test/common/process/logger.unit.test.ts @@ -18,7 +18,7 @@ import * as logging from '../../../client/logging'; suite('ProcessLogger suite', () => { let workspaceService: TypeMoq.IMock; let logger: ProcessLogger; - let traceInfoStub: sinon.SinonStub; + let traceLogStub: sinon.SinonStub; suiteSetup(() => { workspaceService = TypeMoq.Mock.ofType(); @@ -29,7 +29,7 @@ suite('ProcessLogger suite', () => { }); setup(() => { - traceInfoStub = sinon.stub(logging, 'traceInfo'); + traceLogStub = sinon.stub(logging, 'traceLog'); }); teardown(() => { @@ -40,17 +40,17 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar`); - sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger adds quotes around arguments if they contain spaces', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', 'import test'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, ); }); @@ -59,9 +59,9 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', '"import test"'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, ); }); @@ -70,9 +70,9 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', "'import test'"], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar "import test"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar "import test"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, ); }); @@ -81,9 +81,9 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess('test', ['--foo', '--bar', "'importtest'"], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar importtest`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar importtest`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} ${path.join('debug', 'path')}`, ); }); @@ -92,9 +92,9 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join(untildify('~'), 'debug', 'path') }; logger.logProcess('test', ['--foo', '--bar'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> test --foo --bar`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> test --foo --bar`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} ${path.join('~', 'debug', 'path')}`, ); }); @@ -103,25 +103,25 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar`); - sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger replaces the path/to/home with ~ if shell command is provided', async () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess(`"${path.join(untildify('~'), 'test')}" "--foo" "--bar"`, undefined, options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> "${path.join('~', 'test')}" "--foo" "--bar"`); - sinon.assert.calledOnceWithExactly(traceInfoStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); + sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" "--foo" "--bar"`); + sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); test('Logger replaces the path to workspace with . if exactly one workspace folder is opened', async () => { const options = { cwd: path.join('path', 'to', 'workspace', 'debug', 'path') }; logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); }); @@ -134,19 +134,19 @@ suite('ProcessLogger suite', () => { logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); - traceInfoStub.resetHistory(); + traceLogStub.resetHistory(); options = { cwd: path.join('path\\to\\workspace', 'debug', 'path') }; logger.logProcess(`"${path.join('path', 'to', 'workspace', 'test')}" "--foo" "--bar"`, undefined, options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ".${path.sep}test" "--foo" "--bar"`); - sinon.assert.calledOnceWithExactly( - traceInfoStub, + sinon.assert.calledWithExactly(traceLogStub, `> ".${path.sep}test" "--foo" "--bar"`); + sinon.assert.calledWithExactly( + traceLogStub, `${Logging.currentWorkingDirectory()} .${path.sep + path.join('debug', 'path')}`, ); }); @@ -154,13 +154,13 @@ suite('ProcessLogger suite', () => { test("Logger doesn't display the working directory line if there is no options parameter", async () => { logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar']); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); }); test("Logger doesn't display the working directory line if there is no cwd key in the options parameter", async () => { const options = {}; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - sinon.assert.calledOnceWithExactly(traceInfoStub, `> ${path.join('~', 'test')} --foo --bar\n`); + sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); }); }); diff --git a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts index e4956b8ea0ac..a6fe7f7de23c 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts @@ -558,7 +558,9 @@ suite('Python envs locator - Environments Collection', async () => { const envs = collectionService.getEnvs(); expect(resolved?.hasCompleteInfo).to.equal(true); assertEnvsEqual(envs, [resolved]); - sinon.assert.calledOnce(reportInterpretersChangedStub); + sinon.assert.calledOnceWithExactly(reportInterpretersChangedStub, [ + { path: resolved?.executable.filename, type: 'add' }, + ]); }); test('Ensure events from downstream locators do not trigger new refreshes if a refresh is already scheduled', async () => { From bb38236e41e4a240503f89f35e90835affb3841d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 18 Jan 2022 20:42:00 -0800 Subject: [PATCH 14/14] Fix logger tests. --- src/test/common/process/logger.unit.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/common/process/logger.unit.test.ts b/src/test/common/process/logger.unit.test.ts index 786d5784e92e..48d8bca82220 100644 --- a/src/test/common/process/logger.unit.test.ts +++ b/src/test/common/process/logger.unit.test.ts @@ -103,7 +103,7 @@ suite('ProcessLogger suite', () => { const options = { cwd: path.join('debug', 'path') }; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('~', 'test')} --foo --bar`); sinon.assert.calledWithExactly(traceLogStub, `${Logging.currentWorkingDirectory()} ${options.cwd}`); }); @@ -154,13 +154,13 @@ suite('ProcessLogger suite', () => { test("Logger doesn't display the working directory line if there is no options parameter", async () => { logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar']); - sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('~', 'test')} --foo --bar`); }); test("Logger doesn't display the working directory line if there is no cwd key in the options parameter", async () => { const options = {}; logger.logProcess(path.join(untildify('~'), 'test'), ['--foo', '--bar'], options); - sinon.assert.calledWithExactly(traceLogStub, `> "${path.join('~', 'test')}" --foo --bar`); + sinon.assert.calledWithExactly(traceLogStub, `> ${path.join('~', 'test')} --foo --bar`); }); });