From 4396943e2dbb035d8e3a2e2914f47204465bbd55 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 26 Sep 2022 20:38:32 -0700 Subject: [PATCH 1/2] Rename `EnvironmentId` to `EnvironmentPath` --- src/client/deprecatedProposedApi.ts | 8 ------ src/client/deprecatedProposedApiTypes.ts | 6 +---- src/client/proposedApi.ts | 28 ++++++++++++------- src/client/proposedApiTypes.ts | 18 +++++++------ src/test/proposedApi.unit.test.ts | 34 ++++++++++++------------ 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/client/deprecatedProposedApi.ts b/src/client/deprecatedProposedApi.ts index 84340772901a..ef4d8a1d5eaa 100644 --- a/src/client/deprecatedProposedApi.ts +++ b/src/client/deprecatedProposedApi.ts @@ -85,14 +85,6 @@ export function buildDeprecatedProposedApi( const env = await interpreterService.getActiveInterpreter(resource); return env ? { execCommand: [env.path] } : { execCommand: undefined }; }, - async getActiveEnvironmentPath(resource?: Resource) { - sendApiTelemetry('getActiveEnvironmentPath'); - const env = await interpreterService.getActiveInterpreter(resource); - if (!env) { - return undefined; - } - return getEnvPath(env.path, env.envPath); - }, async getEnvironmentDetails( path: string, options?: EnvironmentDetailsOptions, diff --git a/src/client/deprecatedProposedApiTypes.ts b/src/client/deprecatedProposedApiTypes.ts index cf6c01f21219..4193c8b18212 100644 --- a/src/client/deprecatedProposedApiTypes.ts +++ b/src/client/deprecatedProposedApiTypes.ts @@ -74,10 +74,6 @@ export interface DeprecatedProposedAPI { */ execCommand: string[] | undefined; }>; - /** - * @deprecated Use {@link getActiveEnvironmentId} instead. This will soon be removed. - */ - getActiveEnvironmentPath(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 @@ -135,7 +131,7 @@ export interface DeprecatedProposedAPI { */ onDidEnvironmentsChanged: Event; /** - * @deprecated Use {@link ProposedExtensionAPI.environment} `onDidChangeActiveEnvironmentId` instead. This will soon be removed. + * @deprecated Use {@link ProposedExtensionAPI.environment} `onDidChangeActiveEnvironmentPath` instead. This will soon be removed. */ onDidActiveEnvironmentChanged: Event; }; diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index e85c4009d2c9..22c38f1dfa0d 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -17,7 +17,7 @@ import { Resource, EnvironmentType, EnvironmentTools, - EnvironmentId, + EnvironmentPath, } from './proposedApiTypes'; import { PythonEnvInfo, PythonEnvKind, PythonEnvType } from './pythonEnvironments/base/info'; import { getEnvPath } from './pythonEnvironments/base/info/env'; @@ -164,24 +164,34 @@ export function buildProposedApi( const proposed: ProposedExtensionAPI = { environment: { - getActiveEnvironmentId(resource?: Resource) { - sendApiTelemetry('getActiveEnvironmentId'); + getActiveEnvironmentPath(resource?: Resource) { + sendApiTelemetry('getActiveEnvironmentPath'); resource = resource && 'uri' in resource ? resource.uri : resource; const path = configService.getSettings(resource).pythonPath; const id = path === 'python' ? 'DEFAULT_PYTHON' : getEnvID(path); - return { id, path }; + return { + id, + path, + /** + * @deprecated Only provided for backwards compatibility and will soon be removed. + */ + pathType: 'interpreterPath', + }; }, - updateActiveEnvironmentId(env: Environment | EnvironmentId | string, resource?: Resource): Promise { - sendApiTelemetry('updateActiveEnvironmentId'); + updateActiveEnvironmentPath( + env: Environment | EnvironmentPath | string, + resource?: Resource, + ): Promise { + sendApiTelemetry('updateActiveEnvironmentPath'); const path = typeof env !== 'string' ? env.path : env; resource = resource && 'uri' in resource ? resource.uri : resource; return interpreterPathService.update(resource, ConfigurationTarget.WorkspaceFolder, path); }, - get onDidChangeActiveEnvironmentId() { - sendApiTelemetry('onDidChangeActiveEnvironmentId'); + get onDidChangeActiveEnvironmentPath() { + sendApiTelemetry('onDidChangeActiveEnvironmentPath'); return onDidActiveInterpreterChangedEvent.event; }, - resolveEnvironment: async (env: Environment | EnvironmentId | string) => { + resolveEnvironment: async (env: Environment | EnvironmentPath | string) => { let path = typeof env !== 'string' ? env.path : env; if (pathUtils.basename(path) === path) { // Value can be `python`, `python3`, `python3.9` etc. diff --git a/src/client/proposedApiTypes.ts b/src/client/proposedApiTypes.ts index 89f9f1e4e5f4..b67632e14b86 100644 --- a/src/client/proposedApiTypes.ts +++ b/src/client/proposedApiTypes.ts @@ -12,7 +12,7 @@ export interface ProposedExtensionAPI { * @param resource : Uri of a file or workspace folder. This is used to determine the env in a multi-root * scenario. If `undefined`, then the API returns what ever is set for the workspace. */ - getActiveEnvironmentId(resource?: Resource): EnvironmentId; + getActiveEnvironmentPath(resource?: Resource): EnvironmentPath; /** * Sets the active environment path for the python extension for the resource. Configuration target will always * be the workspace folder. @@ -20,14 +20,14 @@ export interface ProposedExtensionAPI { * the environment itself. * @param resource : [optional] File or workspace to scope to a particular workspace folder. */ - updateActiveEnvironmentId( - environment: Environment | EnvironmentId | string, + updateActiveEnvironmentPath( + environment: string | EnvironmentPath | Environment, resource?: Resource, ): Promise; /** * This event is triggered when the active environment setting changes. */ - readonly onDidChangeActiveEnvironmentId: Event; + readonly onDidChangeActiveEnvironmentPath: Event; /** * Carries environments found by the extension at the time of fetching the property. Note this may not * contain all environments in the system as a refresh might be going on. @@ -55,7 +55,9 @@ export interface ProposedExtensionAPI { * @param environment : Full path to environment folder or python executable for the environment. Can also pass * the environment id or the environment itself. */ - resolveEnvironment(environment: Environment | EnvironmentId | string): Promise; + resolveEnvironment( + environment: Environment | EnvironmentPath | string, + ): Promise; }; } @@ -70,7 +72,7 @@ export type RefreshOptions = { /** * Details about the environment. Note the environment folder, type and name never changes over time. */ -export type Environment = EnvironmentId & { +export type Environment = EnvironmentPath & { /** * Carries details about python executable. */ @@ -175,7 +177,7 @@ export type EnvironmentsChangeEvent = { readonly type: 'add' | 'remove' | 'update'; }; -export type ActiveEnvironmentIdChangeEvent = EnvironmentId & { +export type ActiveEnvironmentIdChangeEvent = EnvironmentPath & { /** * Workspace folder the environment changed for. */ @@ -187,7 +189,7 @@ export type ActiveEnvironmentIdChangeEvent = EnvironmentId & { */ export type Resource = Uri | WorkspaceFolder; -export type EnvironmentId = { +export type EnvironmentPath = { /** * The ID of the environment. */ diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index 816bf1051d25..b72e20c6d768 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -26,12 +26,12 @@ import { sleep } from './core'; import { PythonEnvKind, PythonEnvSource } from '../client/pythonEnvironments/base/info'; import { Architecture } from '../client/common/utils/platform'; import { PythonEnvCollectionChangedEvent } from '../client/pythonEnvironments/base/watcher'; +import { normCasePath } from '../client/common/platform/fs-paths'; import { - ProposedExtensionAPI, ActiveEnvironmentIdChangeEvent, EnvironmentsChangeEvent, + ProposedExtensionAPI, } from '../client/proposedApiTypes'; -import { normCasePath } from '../client/common/platform/fs-paths'; suite('Proposed Extension API', () => { let serviceContainer: typemoq.IMock; @@ -75,7 +75,7 @@ suite('Proposed Extension API', () => { test('Provide an event to track when active environment details change', async () => { const events: ActiveEnvironmentIdChangeEvent[] = []; - proposed.environment.onDidChangeActiveEnvironmentId((e) => { + proposed.environment.onDidChangeActiveEnvironmentPath((e) => { events.push(e); }); reportActiveInterpreterChanged({ path: 'path/to/environment', resource: undefined }); @@ -85,31 +85,31 @@ suite('Proposed Extension API', () => { ]); }); - test('getActiveEnvironmentId: No resource', () => { + test('getActiveEnvironmentPath: No resource', () => { const pythonPath = 'this/is/a/test/path'; configService .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); - const actual = proposed.environment.getActiveEnvironmentId(); + const actual = proposed.environment.getActiveEnvironmentPath(); assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath }); }); - test('getActiveEnvironmentId: default python', () => { + test('getActiveEnvironmentPath: default python', () => { const pythonPath = 'python'; configService .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); - const actual = proposed.environment.getActiveEnvironmentId(); + const actual = proposed.environment.getActiveEnvironmentPath(); assert.deepEqual(actual, { id: 'DEFAULT_PYTHON', path: pythonPath }); }); - test('getActiveEnvironmentId: With resource', () => { + test('getActiveEnvironmentPath: With resource', () => { const pythonPath = 'this/is/a/test/path'; const resource = Uri.file(__filename); configService .setup((c) => c.getSettings(resource)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); - const actual = proposed.environment.getActiveEnvironmentId(resource); + const actual = proposed.environment.getActiveEnvironmentPath(resource); assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath }); }); @@ -317,24 +317,24 @@ suite('Proposed Extension API', () => { assert.deepEqual(eventValues, expectedEvents); }); - test('updateActiveEnvironmentId: no resource', async () => { + test('updateActiveEnvironmentPath: no resource', async () => { interpreterPathService .setup((i) => i.update(undefined, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path')) .returns(() => Promise.resolve()) .verifiable(typemoq.Times.once()); - await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path'); + await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path'); interpreterPathService.verifyAll(); }); - test('updateActiveEnvironmentId: passed as Environment', async () => { + test('updateActiveEnvironmentPath: passed as Environment', async () => { interpreterPathService .setup((i) => i.update(undefined, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path')) .returns(() => Promise.resolve()) .verifiable(typemoq.Times.once()); - await proposed.environment.updateActiveEnvironmentId({ + await proposed.environment.updateActiveEnvironmentPath({ id: normCasePath('this/is/a/test/python/path'), path: 'this/is/a/test/python/path', }); @@ -342,19 +342,19 @@ suite('Proposed Extension API', () => { interpreterPathService.verifyAll(); }); - test('updateActiveEnvironmentId: with uri', async () => { + test('updateActiveEnvironmentPath: with uri', async () => { const uri = Uri.parse('a'); interpreterPathService .setup((i) => i.update(uri, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path')) .returns(() => Promise.resolve()) .verifiable(typemoq.Times.once()); - await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path', uri); + await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', uri); interpreterPathService.verifyAll(); }); - test('updateActiveEnvironmentId: with workspace folder', async () => { + test('updateActiveEnvironmentPath: with workspace folder', async () => { const uri = Uri.parse('a'); interpreterPathService .setup((i) => i.update(uri, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path')) @@ -366,7 +366,7 @@ suite('Proposed Extension API', () => { index: 0, }; - await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path', workspace); + await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', workspace); interpreterPathService.verifyAll(); }); From bd7c5bd68b05de4598b1f89c0490ccabe6b3c62d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 26 Sep 2022 20:52:13 -0700 Subject: [PATCH 2/2] Fix tests --- src/client/proposedApi.ts | 4 ++-- src/client/proposedApiTypes.ts | 4 ++-- src/test/proposedApi.unit.test.ts | 23 ++++++++++++++++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 22c38f1dfa0d..1147e839d0a5 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -8,7 +8,7 @@ import { IConfigurationService, IDisposableRegistry, IExtensions, IInterpreterPa import { Architecture } from './common/utils/platform'; import { IServiceContainer } from './ioc/types'; import { - ActiveEnvironmentIdChangeEvent, + ActiveEnvironmentPathChangeEvent, Environment, EnvironmentsChangeEvent, ProposedExtensionAPI, @@ -38,7 +38,7 @@ type ActiveEnvironmentChangeEvent = { path: string; }; -const onDidActiveInterpreterChangedEvent = new EventEmitter(); +const onDidActiveInterpreterChangedEvent = new EventEmitter(); export function reportActiveInterpreterChanged(e: ActiveEnvironmentChangeEvent): void { onDidActiveInterpreterChangedEvent.fire({ id: getEnvID(e.path), path: e.path, resource: e.resource }); reportActiveInterpreterChangedDeprecated({ path: e.path, resource: e.resource?.uri }); diff --git a/src/client/proposedApiTypes.ts b/src/client/proposedApiTypes.ts index b67632e14b86..ee6e8e3384d7 100644 --- a/src/client/proposedApiTypes.ts +++ b/src/client/proposedApiTypes.ts @@ -27,7 +27,7 @@ export interface ProposedExtensionAPI { /** * This event is triggered when the active environment setting changes. */ - readonly onDidChangeActiveEnvironmentPath: Event; + readonly onDidChangeActiveEnvironmentPath: Event; /** * Carries environments found by the extension at the time of fetching the property. Note this may not * contain all environments in the system as a refresh might be going on. @@ -177,7 +177,7 @@ export type EnvironmentsChangeEvent = { readonly type: 'add' | 'remove' | 'update'; }; -export type ActiveEnvironmentIdChangeEvent = EnvironmentPath & { +export type ActiveEnvironmentPathChangeEvent = EnvironmentPath & { /** * Workspace folder the environment changed for. */ diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index b72e20c6d768..2bee65e70f65 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -28,7 +28,8 @@ import { Architecture } from '../client/common/utils/platform'; import { PythonEnvCollectionChangedEvent } from '../client/pythonEnvironments/base/watcher'; import { normCasePath } from '../client/common/platform/fs-paths'; import { - ActiveEnvironmentIdChangeEvent, + ActiveEnvironmentPathChangeEvent, + EnvironmentPath, EnvironmentsChangeEvent, ProposedExtensionAPI, } from '../client/proposedApiTypes'; @@ -74,7 +75,7 @@ suite('Proposed Extension API', () => { }); test('Provide an event to track when active environment details change', async () => { - const events: ActiveEnvironmentIdChangeEvent[] = []; + const events: ActiveEnvironmentPathChangeEvent[] = []; proposed.environment.onDidChangeActiveEnvironmentPath((e) => { events.push(e); }); @@ -91,7 +92,11 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environment.getActiveEnvironmentPath(); - assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath }); + assert.deepEqual(actual, ({ + id: normCasePath(pythonPath), + path: pythonPath, + pathType: 'interpreterPath', + } as unknown) as EnvironmentPath); }); test('getActiveEnvironmentPath: default python', () => { @@ -100,7 +105,11 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environment.getActiveEnvironmentPath(); - assert.deepEqual(actual, { id: 'DEFAULT_PYTHON', path: pythonPath }); + assert.deepEqual(actual, ({ + id: 'DEFAULT_PYTHON', + path: pythonPath, + pathType: 'interpreterPath', + } as unknown) as EnvironmentPath); }); test('getActiveEnvironmentPath: With resource', () => { @@ -110,7 +119,11 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(resource)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environment.getActiveEnvironmentPath(resource); - assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath }); + assert.deepEqual(actual, ({ + id: normCasePath(pythonPath), + path: pythonPath, + pathType: 'interpreterPath', + } as unknown) as EnvironmentPath); }); test('resolveEnvironment: invalid environment (when passed as string)', async () => {