From d2a59dc348909b0f7039918803fb45bd996e16b2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 18 Sep 2020 13:57:39 -0700 Subject: [PATCH 1/6] Modify environment info worker to support new environment type --- src/client/pythonEnvironments/base/info/interpreter.ts | 1 - .../pythonEnvironments/info/environmentInfoService.ts | 8 ++++---- .../info/environmentInfoService.functional.test.ts | 10 ++++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/interpreter.ts b/src/client/pythonEnvironments/base/info/interpreter.ts index 16d82d3280b5..26db361c8e65 100644 --- a/src/client/pythonEnvironments/base/info/interpreter.ts +++ b/src/client/pythonEnvironments/base/info/interpreter.ts @@ -46,7 +46,6 @@ type ShellExecFunc = (command: string, timeout: number) => Promise; } @@ -49,7 +49,7 @@ export class EnvironmentInfoService implements IEnvironmentInfoService { } public async getEnvironmentInfo( - interpreterPath: string, + environment: PythonEnvInfo, priority?: EnvironmentInfoServiceQueuePriority, ): Promise { const result = this.cache.get(interpreterPath); @@ -60,8 +60,8 @@ export class EnvironmentInfoService implements IEnvironmentInfoService { const deferred = createDeferred(); this.cache.set(interpreterPath, deferred); return (priority === EnvironmentInfoServiceQueuePriority.High - ? this.workerPool.addToQueue(interpreterPath, QueuePosition.Front) - : this.workerPool.addToQueue(interpreterPath, QueuePosition.Back) + ? this.workerPool.addToQueue(environment, QueuePosition.Front) + : this.workerPool.addToQueue(environment, QueuePosition.Back) ).then((r) => { deferred.resolve(r); if (r === undefined) { diff --git a/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts b/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts index e1d64c353305..68af36805063 100644 --- a/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts +++ b/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts @@ -54,9 +54,11 @@ suite('Environment Info Service', () => { for (let i = 0; i < 10; i = i + 1) { const path = `any-path${i}`; if (i < 5) { - promises.push(envService.getEnvironmentInfo(path)); + promises.push(envService.getEnvironmentInfo(createEnvInfo(path))); } else { - promises.push(envService.getEnvironmentInfo(path, EnvironmentInfoServiceQueuePriority.High)); + promises.push( + envService.getEnvironmentInfo(createEnvInfo(path), EnvironmentInfoServiceQueuePriority.High), + ); } expected.push(createExpectedEnvInfo(path)); } @@ -79,10 +81,10 @@ suite('Environment Info Service', () => { // Clear call counts stubShellExec.resetHistory(); // Evaluate once so the result is cached. - await envService.getEnvironmentInfo(path); + await envService.getEnvironmentInfo(createEnvInfo(path)); for (let i = 0; i < 10; i = i + 1) { - promises.push(envService.getEnvironmentInfo(path)); + promises.push(envService.getEnvironmentInfo(createEnvInfo(path))); expected.push(createExpectedEnvInfo(path)); } From 65094df0f907d805539bc715fe5b8309194fa27f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 21 Sep 2020 07:03:47 -0700 Subject: [PATCH 2/6] Change worker to return interpreter information instead --- .../pythonEnvironments/info/environmentInfoService.ts | 8 ++++---- .../info/environmentInfoService.functional.test.ts | 10 ++++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/client/pythonEnvironments/info/environmentInfoService.ts b/src/client/pythonEnvironments/info/environmentInfoService.ts index eec6e5aacb98..6ab761c86278 100644 --- a/src/client/pythonEnvironments/info/environmentInfoService.ts +++ b/src/client/pythonEnvironments/info/environmentInfoService.ts @@ -16,7 +16,7 @@ export enum EnvironmentInfoServiceQueuePriority { export const IEnvironmentInfoService = Symbol('IEnvironmentInfoService'); export interface IEnvironmentInfoService { getEnvironmentInfo( - environment: PythonEnvInfo, + interpreterPath: string, priority?: EnvironmentInfoServiceQueuePriority ): Promise; } @@ -49,7 +49,7 @@ export class EnvironmentInfoService implements IEnvironmentInfoService { } public async getEnvironmentInfo( - environment: PythonEnvInfo, + interpreterPath: string, priority?: EnvironmentInfoServiceQueuePriority, ): Promise { const result = this.cache.get(interpreterPath); @@ -60,8 +60,8 @@ export class EnvironmentInfoService implements IEnvironmentInfoService { const deferred = createDeferred(); this.cache.set(interpreterPath, deferred); return (priority === EnvironmentInfoServiceQueuePriority.High - ? this.workerPool.addToQueue(environment, QueuePosition.Front) - : this.workerPool.addToQueue(environment, QueuePosition.Back) + ? this.workerPool.addToQueue(interpreterPath, QueuePosition.Front) + : this.workerPool.addToQueue(interpreterPath, QueuePosition.Back) ).then((r) => { deferred.resolve(r); if (r === undefined) { diff --git a/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts b/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts index 68af36805063..e1d64c353305 100644 --- a/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts +++ b/src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts @@ -54,11 +54,9 @@ suite('Environment Info Service', () => { for (let i = 0; i < 10; i = i + 1) { const path = `any-path${i}`; if (i < 5) { - promises.push(envService.getEnvironmentInfo(createEnvInfo(path))); + promises.push(envService.getEnvironmentInfo(path)); } else { - promises.push( - envService.getEnvironmentInfo(createEnvInfo(path), EnvironmentInfoServiceQueuePriority.High), - ); + promises.push(envService.getEnvironmentInfo(path, EnvironmentInfoServiceQueuePriority.High)); } expected.push(createExpectedEnvInfo(path)); } @@ -81,10 +79,10 @@ suite('Environment Info Service', () => { // Clear call counts stubShellExec.resetHistory(); // Evaluate once so the result is cached. - await envService.getEnvironmentInfo(createEnvInfo(path)); + await envService.getEnvironmentInfo(path); for (let i = 0; i < 10; i = i + 1) { - promises.push(envService.getEnvironmentInfo(createEnvInfo(path))); + promises.push(envService.getEnvironmentInfo(path)); expected.push(createExpectedEnvInfo(path)); } From dab9ee554b641aebc5814d4147c20f49075464c8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 23 Sep 2020 05:23:37 -0700 Subject: [PATCH 3/6] Modify resolveEnv() --- .../collection/environmentsResolver.ts | 133 ++++++++++ src/test/pythonEnvironments/base/common.ts | 4 +- .../environmentsResolver.unit.test.ts | 244 ++++++++++++++++++ 3 files changed, 378 insertions(+), 3 deletions(-) create mode 100644 src/client/pythonEnvironments/collection/environmentsResolver.ts create mode 100644 src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts diff --git a/src/client/pythonEnvironments/collection/environmentsResolver.ts b/src/client/pythonEnvironments/collection/environmentsResolver.ts new file mode 100644 index 000000000000..f35baf20bc2b --- /dev/null +++ b/src/client/pythonEnvironments/collection/environmentsResolver.ts @@ -0,0 +1,133 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { cloneDeep } from 'lodash'; +import { Event, EventEmitter } from 'vscode'; +import { traceVerbose } from '../../common/logger'; +import { areSameEnvironment, PythonEnvInfo } from '../base/info'; +import { InterpreterInformation } from '../base/info/interpreter'; +import { + ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, +} from '../base/locator'; +import { PythonEnvsChangedEvent } from '../base/watcher'; +import { IEnvironmentInfoService } from '../info/environmentInfoService'; + +export class PythonEnvsResolver implements ILocator { + public get onChanged(): Event { + return this.pythonEnvsReducer.onChanged; + } + + constructor( + private readonly pythonEnvsReducer: ILocator, + private readonly environmentInfoService: IEnvironmentInfoService, + ) {} + + public async resolveEnv(env: string | PythonEnvInfo): Promise { + const environment = await this.pythonEnvsReducer.resolveEnv(env); + if (!environment) { + return undefined; + } + const interpreterInfo = await this.environmentInfoService.getEnvironmentInfo(environment.executable.filename); + if (!interpreterInfo) { + return undefined; + } + return getResolvedEnv(interpreterInfo, environment); + } + + public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { + const didUpdate = new EventEmitter(); + const incomingIterator = this.pythonEnvsReducer.iterEnvs(query); + const iterator: IPythonEnvsIterator = this.iterEnvsIterator(incomingIterator, didUpdate); + iterator.onUpdated = didUpdate.event; + return iterator; + } + + private async* iterEnvsIterator( + iterator: IPythonEnvsIterator, + didUpdate: EventEmitter, + ): AsyncIterator { + const state = { + done: false, + pending: 0, + }; + const seen: PythonEnvInfo[] = []; + + if (iterator.onUpdated !== undefined) { + iterator.onUpdated((event) => { + if (event === null) { + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); + } else { + const oldIndex = seen.findIndex((s) => areSameEnvironment(s, event.old)); + if (oldIndex !== -1) { + seen[oldIndex] = event.new; + state.pending += 1; + this.resolveInBackground(oldIndex, state, didUpdate, seen).ignoreErrors(); + } else { + // This implies a problem in a downstream locator + traceVerbose(`Expected already iterated env in resolver, got ${event.old}`); + } + } + }); + } + + let result = await iterator.next(); + while (!result.done) { + const currEnv = result.value; + // Resolver only expects unique environments, so store & yield as-is. + seen.push(currEnv); + yield currEnv; + state.pending += 1; + this.resolveInBackground(seen.indexOf(currEnv), state, didUpdate, seen).ignoreErrors(); + // eslint-disable-next-line no-await-in-loop + result = await iterator.next(); + } + if (iterator.onUpdated === undefined) { + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); + } + } + + private async resolveInBackground( + envIndex: number, + state: { done: boolean; pending: number }, + didUpdate: EventEmitter, + seen: PythonEnvInfo[], + ) { + const interpreterInfo = await this.environmentInfoService.getEnvironmentInfo( + seen[envIndex].executable.filename, + ); + if (interpreterInfo) { + const resolvedEnv = getResolvedEnv(interpreterInfo, seen[envIndex]); + didUpdate.fire({ old: seen[envIndex], new: resolvedEnv }); + seen[envIndex] = resolvedEnv; + } + state.pending -= 1; + checkIfFinishedAndNotify(state, didUpdate); + } +} + +/** + * When all info from incoming iterator has been received and all background calls finishes, notify that we're done + * @param state Carries the current state of progress + * @param didUpdate Used to notify when finished + */ +function checkIfFinishedAndNotify( + state: { done: boolean; pending: number }, + didUpdate: EventEmitter, +) { + if (state.done && state.pending === 0) { + didUpdate.fire(null); + didUpdate.dispose(); + } +} + +function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: PythonEnvInfo) { + // Deep copy into a new object + const resolvedEnv = cloneDeep(environment); + resolvedEnv.version = interpreterInfo.version; + resolvedEnv.executable.filename = interpreterInfo.executable.filename; + resolvedEnv.executable.sysPrefix = interpreterInfo.executable.sysPrefix; + resolvedEnv.arch = interpreterInfo.arch; + return resolvedEnv; +} diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts index d1d3b21cc7cc..7b0516d92a65 100644 --- a/src/test/pythonEnvironments/base/common.ts +++ b/src/test/pythonEnvironments/base/common.ts @@ -11,9 +11,7 @@ import { PythonEnvKind, } from '../../../client/pythonEnvironments/base/info'; import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion'; -import { - IPythonEnvsIterator, Locator, PythonEnvUpdatedEvent, PythonLocatorQuery, -} from '../../../client/pythonEnvironments/base/locator'; +import { IPythonEnvsIterator, Locator, PythonEnvUpdatedEvent, PythonLocatorQuery } from '../../../client/pythonEnvironments/base/locator'; import { PythonEnvsChangedEvent } from '../../../client/pythonEnvironments/base/watcher'; export function createEnv( diff --git a/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts new file mode 100644 index 000000000000..4d6c740422c0 --- /dev/null +++ b/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { assert, expect } from 'chai'; +import { cloneDeep } from 'lodash'; +import * as path from 'path'; +import { ImportMock } from 'ts-mock-imports'; +import { EventEmitter } from 'vscode'; +import { ExecutionResult } from '../../../client/common/process/types'; +import { Architecture } from '../../../client/common/utils/platform'; +import { PythonEnvInfo, PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; +import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion'; +import { PythonEnvUpdatedEvent } from '../../../client/pythonEnvironments/base/locator'; +import { PythonEnvsChangedEvent } from '../../../client/pythonEnvironments/base/watcher'; +import { PythonEnvsResolver } from '../../../client/pythonEnvironments/collection/environmentsResolver'; +import * as ExternalDep from '../../../client/pythonEnvironments/common/externalDependencies'; +import { EnvironmentInfoService } from '../../../client/pythonEnvironments/info/environmentInfoService'; +import { sleep } from '../../core'; +import { createEnv, getEnvs, SimpleLocator } from '../base/common'; + +suite('Environments Resolver', () => { + /** + * Returns the expected environment to be returned by Environment info service + */ + function createExpectedEnvInfo(env: PythonEnvInfo): PythonEnvInfo { + const updatedEnv = cloneDeep(env); + updatedEnv.version = { + ...parseVersion('3.8.3-final'), + sysVersion: '3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]', + }; + updatedEnv.executable.filename = env.executable.filename; + updatedEnv.executable.sysPrefix = 'path'; + updatedEnv.arch = Architecture.x64; + return updatedEnv; + } + suite('iterEnvs()', () => { + let stubShellExec: sinon.SinonStub; + setup(() => { + stubShellExec = ImportMock.mockFunction( + ExternalDep, + 'shellExecute', + new Promise>((resolve) => { + resolve({ + stdout: + '{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}', + }); + }), + ); + }); + + teardown(() => { + stubShellExec.restore(); + }); + + test('Iterator yields environments as-is', async () => { + const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); + const env3 = createEnv('env3', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); + const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); + const environmentsToBeIterated = [env1, env2, env3, env4]; + const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated); + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const iterator = reducer.iterEnvs(); + const envs = await getEnvs(iterator); + + assert.deepEqual(envs, environmentsToBeIterated); + }); + + test('Updates for environments are sent correctly followed by the null event', async () => { + // Arrange + const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec1')); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); + const environmentsToBeIterated = [env1, env2]; + const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated); + const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const iterator = reducer.iterEnvs(); // Act + + // Assert + let { onUpdated } = iterator; + expect(onUpdated).to.not.equal(undefined, ''); + + // Arrange + onUpdated = onUpdated!; + onUpdated((e) => { + onUpdatedEvents.push(e); + }); + + // Act + await getEnvs(iterator); + await sleep(1); // Resolve pending calls in the background + + // Assert + const expectedUpdates = [ + { old: env1, new: createExpectedEnvInfo(env1) }, + { old: env2, new: createExpectedEnvInfo(env2) }, + null, + ]; + assert.deepEqual(expectedUpdates, onUpdatedEvents); + }); + + test('Updates to environments from the incoming iterator are sent correctly followed by the null event', async () => { + // Arrange + const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const updatedEnv = createEnv('env1', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); + const environmentsToBeIterated = [env]; + const didUpdate = new EventEmitter(); + const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); + const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const iterator = reducer.iterEnvs(); // Act + + // Assert + let { onUpdated } = iterator; + expect(onUpdated).to.not.equal(undefined, ''); + + // Arrange + onUpdated = onUpdated!; + onUpdated((e) => { + onUpdatedEvents.push(e); + }); + + // Act + await getEnvs(iterator); + await sleep(1); + didUpdate.fire({ old: env, new: updatedEnv }); + didUpdate.fire(null); // It is essential for the incoming iterator to fire "null" event signifying it's done + await sleep(1); + + // Assert + // The updates can be anything, even the number of updates, but they should lead to the same final state + const { length } = onUpdatedEvents; + assert.deepEqual( + onUpdatedEvents[length - 2]?.new, + createExpectedEnvInfo(updatedEnv), + 'The final update to environment is incorrect', + ); + assert.equal(onUpdatedEvents[length - 1], null, 'Last update should be null'); + didUpdate.dispose(); + }); + }); + + test('onChanged fires iff onChanged from reducer fires', () => { + const pythonEnvReducer = new SimpleLocator([]); + const event1: PythonEnvsChangedEvent = {}; + const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; + const expected = [event1, event2]; + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const events: PythonEnvsChangedEvent[] = []; + reducer.onChanged((e) => events.push(e)); + + pythonEnvReducer.fire(event1); + pythonEnvReducer.fire(event2); + + assert.deepEqual(events, expected); + }); + + suite('resolveEnv()', () => { + let stubShellExec: sinon.SinonStub; + setup(() => { + stubShellExec = ImportMock.mockFunction( + ExternalDep, + 'shellExecute', + new Promise>((resolve) => { + resolve({ + stdout: + '{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}', + }); + }), + ); + }); + + teardown(() => { + stubShellExec.restore(); + }); + + test('Calls into reducer to get resolved environment, then calls environnment service to resolve environment further and return it', async () => { + const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const resolvedEnvReturnedByReducer = createEnv( + 'env1', + '3.8.1', + PythonEnvKind.Conda, + 'resolved/path/to/exec', + ); + const pythonEnvReducer = new SimpleLocator([], { + resolve: async (e: PythonEnvInfo) => { + if (e === env) { + return resolvedEnvReturnedByReducer; + } + throw new Error('Incorrect environment sent to the reducer'); + }, + }); + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const expected = await reducer.resolveEnv(env); + + assert.deepEqual(expected, createExpectedEnvInfo(resolvedEnvReturnedByReducer)); + }); + + test('If the reducer resolves environment, but fetching interpreter info returns undefined, return undefined', async () => { + stubShellExec.returns( + new Promise>((_resolve, reject) => { + reject(); + }), + ); + const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const resolvedEnvReturnedByReducer = createEnv( + 'env1', + '3.8.1', + PythonEnvKind.Conda, + 'resolved/path/to/exec', + ); + const pythonEnvReducer = new SimpleLocator([], { + resolve: async (e: PythonEnvInfo) => { + if (e === env) { + return resolvedEnvReturnedByReducer; + } + throw new Error('Incorrect environment sent to the reducer'); + }, + }); + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const expected = await reducer.resolveEnv(env); + + assert.deepEqual(expected, undefined); + }); + + test("If the reducer isn't able to resolve environment, return undefined", async () => { + const env = createEnv('env', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const pythonEnvReducer = new SimpleLocator([], { + resolve: async () => undefined, + }); + const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + + const expected = await reducer.resolveEnv(env); + + assert.deepEqual(expected, undefined); + }); + }); +}); From 81dcf27a06ce1fa9ea1268538ffd1052d569a163 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 23 Sep 2020 13:23:56 -0700 Subject: [PATCH 4/6] Code reviews --- .../base/info/interpreter.ts | 1 + src/client/pythonEnvironments/base/locator.ts | 2 +- .../collection/environmentsReducer.ts | 4 +- .../collection/environmentsResolver.ts | 12 ++-- src/test/pythonEnvironments/base/common.ts | 4 +- .../environmentsResolver.unit.test.ts | 58 +++++++++---------- 6 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/interpreter.ts b/src/client/pythonEnvironments/base/info/interpreter.ts index 26db361c8e65..16d82d3280b5 100644 --- a/src/client/pythonEnvironments/base/info/interpreter.ts +++ b/src/client/pythonEnvironments/base/info/interpreter.ts @@ -46,6 +46,7 @@ type ShellExecFunc = (command: string, timeout: number) => Promise = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery; +type QueryForEvent = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery; /** * A single Python environment locator. diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index b4c80254a3f9..b4f74ea416e7 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -7,7 +7,7 @@ import { traceVerbose } from '../../common/logger'; import { createDeferred } from '../../common/utils/async'; import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info'; import { - ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, + ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery, } from '../base/locator'; import { PythonEnvsChangedEvent } from '../base/watcher'; @@ -47,7 +47,7 @@ export class PythonEnvsReducer implements ILocator { return this.parentLocator.resolveEnv(environment); } - public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { + public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { const didUpdate = new EventEmitter(); const incomingIterator = this.parentLocator.iterEnvs(query); const iterator: IPythonEnvsIterator = iterEnvsIterator(incomingIterator, didUpdate); diff --git a/src/client/pythonEnvironments/collection/environmentsResolver.ts b/src/client/pythonEnvironments/collection/environmentsResolver.ts index f35baf20bc2b..286e0c33dbc8 100644 --- a/src/client/pythonEnvironments/collection/environmentsResolver.ts +++ b/src/client/pythonEnvironments/collection/environmentsResolver.ts @@ -7,23 +7,23 @@ import { traceVerbose } from '../../common/logger'; import { areSameEnvironment, PythonEnvInfo } from '../base/info'; import { InterpreterInformation } from '../base/info/interpreter'; import { - ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, + ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery, } from '../base/locator'; import { PythonEnvsChangedEvent } from '../base/watcher'; import { IEnvironmentInfoService } from '../info/environmentInfoService'; export class PythonEnvsResolver implements ILocator { public get onChanged(): Event { - return this.pythonEnvsReducer.onChanged; + return this.parentLocator.onChanged; } constructor( - private readonly pythonEnvsReducer: ILocator, + private readonly parentLocator: ILocator, private readonly environmentInfoService: IEnvironmentInfoService, ) {} public async resolveEnv(env: string | PythonEnvInfo): Promise { - const environment = await this.pythonEnvsReducer.resolveEnv(env); + const environment = await this.parentLocator.resolveEnv(env); if (!environment) { return undefined; } @@ -34,9 +34,9 @@ export class PythonEnvsResolver implements ILocator { return getResolvedEnv(interpreterInfo, environment); } - public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { + public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { const didUpdate = new EventEmitter(); - const incomingIterator = this.pythonEnvsReducer.iterEnvs(query); + const incomingIterator = this.parentLocator.iterEnvs(query); const iterator: IPythonEnvsIterator = this.iterEnvsIterator(incomingIterator, didUpdate); iterator.onUpdated = didUpdate.event; return iterator; diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts index 7b0516d92a65..d1d3b21cc7cc 100644 --- a/src/test/pythonEnvironments/base/common.ts +++ b/src/test/pythonEnvironments/base/common.ts @@ -11,7 +11,9 @@ import { PythonEnvKind, } from '../../../client/pythonEnvironments/base/info'; import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion'; -import { IPythonEnvsIterator, Locator, PythonEnvUpdatedEvent, PythonLocatorQuery } from '../../../client/pythonEnvironments/base/locator'; +import { + IPythonEnvsIterator, Locator, PythonEnvUpdatedEvent, PythonLocatorQuery, +} from '../../../client/pythonEnvironments/base/locator'; import { PythonEnvsChangedEvent } from '../../../client/pythonEnvironments/base/watcher'; export function createEnv( diff --git a/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts index 4d6c740422c0..87d1483621ae 100644 --- a/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts @@ -58,10 +58,10 @@ suite('Environments Resolver', () => { const env3 = createEnv('env3', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); const environmentsToBeIterated = [env1, env2, env3, env4]; - const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated); - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const parentLocator = new SimpleLocator(environmentsToBeIterated); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const iterator = reducer.iterEnvs(); + const iterator = resolver.iterEnvs(); const envs = await getEnvs(iterator); assert.deepEqual(envs, environmentsToBeIterated); @@ -72,11 +72,11 @@ suite('Environments Resolver', () => { const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec1')); const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); const environmentsToBeIterated = [env1, env2]; - const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated); + const parentLocator = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const iterator = reducer.iterEnvs(); // Act + const iterator = resolver.iterEnvs(); // Act // Assert let { onUpdated } = iterator; @@ -107,11 +107,11 @@ suite('Environments Resolver', () => { const updatedEnv = createEnv('env1', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env]; const didUpdate = new EventEmitter(); - const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); + const parentLocator = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const iterator = reducer.iterEnvs(); // Act + const iterator = resolver.iterEnvs(); // Act // Assert let { onUpdated } = iterator; @@ -143,18 +143,18 @@ suite('Environments Resolver', () => { }); }); - test('onChanged fires iff onChanged from reducer fires', () => { - const pythonEnvReducer = new SimpleLocator([]); + test('onChanged fires iff onChanged from resolver fires', () => { + const parentLocator = new SimpleLocator([]); const event1: PythonEnvsChangedEvent = {}; const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; const expected = [event1, event2]; - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); const events: PythonEnvsChangedEvent[] = []; - reducer.onChanged((e) => events.push(e)); + resolver.onChanged((e) => events.push(e)); - pythonEnvReducer.fire(event1); - pythonEnvReducer.fire(event2); + parentLocator.fire(event1); + parentLocator.fire(event2); assert.deepEqual(events, expected); }); @@ -178,7 +178,7 @@ suite('Environments Resolver', () => { stubShellExec.restore(); }); - test('Calls into reducer to get resolved environment, then calls environnment service to resolve environment further and return it', async () => { + test('Calls into parent locator to get resolved environment, then calls environnment service to resolve environment further and return it', async () => { const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); const resolvedEnvReturnedByReducer = createEnv( 'env1', @@ -186,22 +186,22 @@ suite('Environments Resolver', () => { PythonEnvKind.Conda, 'resolved/path/to/exec', ); - const pythonEnvReducer = new SimpleLocator([], { + const parentLocator = new SimpleLocator([], { resolve: async (e: PythonEnvInfo) => { if (e === env) { return resolvedEnvReturnedByReducer; } - throw new Error('Incorrect environment sent to the reducer'); + throw new Error('Incorrect environment sent to the resolver'); }, }); - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const expected = await reducer.resolveEnv(env); + const expected = await resolver.resolveEnv(env); assert.deepEqual(expected, createExpectedEnvInfo(resolvedEnvReturnedByReducer)); }); - test('If the reducer resolves environment, but fetching interpreter info returns undefined, return undefined', async () => { + test('If the parent locator resolves environment, but fetching interpreter info returns undefined, return undefined', async () => { stubShellExec.returns( new Promise>((_resolve, reject) => { reject(); @@ -214,29 +214,29 @@ suite('Environments Resolver', () => { PythonEnvKind.Conda, 'resolved/path/to/exec', ); - const pythonEnvReducer = new SimpleLocator([], { + const parentLocator = new SimpleLocator([], { resolve: async (e: PythonEnvInfo) => { if (e === env) { return resolvedEnvReturnedByReducer; } - throw new Error('Incorrect environment sent to the reducer'); + throw new Error('Incorrect environment sent to the resolver'); }, }); - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const expected = await reducer.resolveEnv(env); + const expected = await resolver.resolveEnv(env); assert.deepEqual(expected, undefined); }); - test("If the reducer isn't able to resolve environment, return undefined", async () => { + test("If the parent locator isn't able to resolve environment, return undefined", async () => { const env = createEnv('env', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); - const pythonEnvReducer = new SimpleLocator([], { + const parentLocator = new SimpleLocator([], { resolve: async () => undefined, }); - const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService()); + const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService()); - const expected = await reducer.resolveEnv(env); + const expected = await resolver.resolveEnv(env); assert.deepEqual(expected, undefined); }); From aeab454526aa4dfc43f3f0d459e73e9d6f4afcdf Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 24 Sep 2020 12:36:45 -0700 Subject: [PATCH 5/6] Code reviews --- .../pythonEnvironments/collection/environmentsResolver.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/client/pythonEnvironments/collection/environmentsResolver.ts b/src/client/pythonEnvironments/collection/environmentsResolver.ts index 286e0c33dbc8..e3467dfbb71c 100644 --- a/src/client/pythonEnvironments/collection/environmentsResolver.ts +++ b/src/client/pythonEnvironments/collection/environmentsResolver.ts @@ -12,6 +12,10 @@ import { import { PythonEnvsChangedEvent } from '../base/watcher'; import { IEnvironmentInfoService } from '../info/environmentInfoService'; +/** + * Calls environment info service which runs `interpreterInfo.py` script on environments received + * from the parent locator. Uses information received to populate environments further and pass it on. + */ export class PythonEnvsResolver implements ILocator { public get onChanged(): Event { return this.parentLocator.onChanged; @@ -74,7 +78,6 @@ export class PythonEnvsResolver implements ILocator { let result = await iterator.next(); while (!result.done) { const currEnv = result.value; - // Resolver only expects unique environments, so store & yield as-is. seen.push(currEnv); yield currEnv; state.pending += 1; From 6e3be1148db14648c0bbb35d2228ccb77db7ff80 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 24 Sep 2020 12:39:40 -0700 Subject: [PATCH 6/6] Move stuff --- .../composite}/environmentsReducer.ts | 10 ++++----- .../composite}/environmentsResolver.ts | 12 +++++----- .../environmentsReducer.unit.test.ts | 12 +++++----- .../environmentsResolver.unit.test.ts | 22 +++++++++---------- 4 files changed, 28 insertions(+), 28 deletions(-) rename src/client/pythonEnvironments/{collection => base/locators/composite}/environmentsReducer.ts (93%) rename src/client/pythonEnvironments/{collection => base/locators/composite}/environmentsResolver.ts (91%) rename src/test/pythonEnvironments/{collection => base/locators/composite}/environmentsReducer.unit.test.ts (93%) rename src/test/pythonEnvironments/{collection => base/locators/composite}/environmentsResolver.unit.test.ts (89%) diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts similarity index 93% rename from src/client/pythonEnvironments/collection/environmentsReducer.ts rename to src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts index b4f74ea416e7..ba00eac2a546 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts @@ -3,13 +3,13 @@ import { cloneDeep, isEqual } from 'lodash'; import { Event, EventEmitter } from 'vscode'; -import { traceVerbose } from '../../common/logger'; -import { createDeferred } from '../../common/utils/async'; -import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info'; +import { traceVerbose } from '../../../../common/logger'; +import { createDeferred } from '../../../../common/utils/async'; +import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../../info'; import { ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery, -} from '../base/locator'; -import { PythonEnvsChangedEvent } from '../base/watcher'; +} from '../../locator'; +import { PythonEnvsChangedEvent } from '../../watcher'; /** * Combines duplicate environments received from the incoming locator into one and passes on unique environments diff --git a/src/client/pythonEnvironments/collection/environmentsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts similarity index 91% rename from src/client/pythonEnvironments/collection/environmentsResolver.ts rename to src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts index e3467dfbb71c..347785f73660 100644 --- a/src/client/pythonEnvironments/collection/environmentsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts @@ -3,14 +3,14 @@ import { cloneDeep } from 'lodash'; import { Event, EventEmitter } from 'vscode'; -import { traceVerbose } from '../../common/logger'; -import { areSameEnvironment, PythonEnvInfo } from '../base/info'; -import { InterpreterInformation } from '../base/info/interpreter'; +import { traceVerbose } from '../../../../common/logger'; +import { IEnvironmentInfoService } from '../../../info/environmentInfoService'; +import { areSameEnvironment, PythonEnvInfo } from '../../info'; +import { InterpreterInformation } from '../../info/interpreter'; import { ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery, -} from '../base/locator'; -import { PythonEnvsChangedEvent } from '../base/watcher'; -import { IEnvironmentInfoService } from '../info/environmentInfoService'; +} from '../../locator'; +import { PythonEnvsChangedEvent } from '../../watcher'; /** * Calls environment info service which runs `interpreterInfo.py` script on environments received diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts similarity index 93% rename from src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts rename to src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts index a05ebc772063..9bb67f672768 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts @@ -5,15 +5,15 @@ import { assert, expect } from 'chai'; import { isEqual } from 'lodash'; import * as path from 'path'; import { EventEmitter } from 'vscode'; -import { PythonEnvInfo, PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; -import { PythonEnvUpdatedEvent } from '../../../client/pythonEnvironments/base/locator'; -import { PythonEnvsChangedEvent } from '../../../client/pythonEnvironments/base/watcher'; +import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info'; +import { PythonEnvUpdatedEvent } from '../../../../../client/pythonEnvironments/base/locator'; import { mergeEnvironments, PythonEnvsReducer, -} from '../../../client/pythonEnvironments/collection/environmentsReducer'; -import { sleep } from '../../core'; -import { createEnv, getEnvs, SimpleLocator } from '../base/common'; +} from '../../../../../client/pythonEnvironments/base/locators/composite/environmentsReducer'; +import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher'; +import { sleep } from '../../../../core'; +import { createEnv, getEnvs, SimpleLocator } from '../../common'; suite('Environments Reducer', () => { suite('iterEnvs()', () => { diff --git a/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts similarity index 89% rename from src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts rename to src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts index 87d1483621ae..b0a7d5bbf17b 100644 --- a/src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts @@ -6,17 +6,17 @@ import { cloneDeep } from 'lodash'; import * as path from 'path'; import { ImportMock } from 'ts-mock-imports'; import { EventEmitter } from 'vscode'; -import { ExecutionResult } from '../../../client/common/process/types'; -import { Architecture } from '../../../client/common/utils/platform'; -import { PythonEnvInfo, PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; -import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion'; -import { PythonEnvUpdatedEvent } from '../../../client/pythonEnvironments/base/locator'; -import { PythonEnvsChangedEvent } from '../../../client/pythonEnvironments/base/watcher'; -import { PythonEnvsResolver } from '../../../client/pythonEnvironments/collection/environmentsResolver'; -import * as ExternalDep from '../../../client/pythonEnvironments/common/externalDependencies'; -import { EnvironmentInfoService } from '../../../client/pythonEnvironments/info/environmentInfoService'; -import { sleep } from '../../core'; -import { createEnv, getEnvs, SimpleLocator } from '../base/common'; +import { ExecutionResult } from '../../../../../client/common/process/types'; +import { Architecture } from '../../../../../client/common/utils/platform'; +import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info'; +import { parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion'; +import { PythonEnvUpdatedEvent } from '../../../../../client/pythonEnvironments/base/locator'; +import { PythonEnvsResolver } from '../../../../../client/pythonEnvironments/base/locators/composite/environmentsResolver'; +import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher'; +import * as ExternalDep from '../../../../../client/pythonEnvironments/common/externalDependencies'; +import { EnvironmentInfoService } from '../../../../../client/pythonEnvironments/info/environmentInfoService'; +import { sleep } from '../../../../core'; +import { createEnv, getEnvs, SimpleLocator } from '../../common'; suite('Environments Resolver', () => { /**