From 4cf7c168268308c08657bcc78b7d2067012b27df Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 17 Sep 2020 13:17:23 -0700 Subject: [PATCH 1/7] Add environments reducer --- .../pythonEnvironments/base/info/index.ts | 20 +++ src/client/pythonEnvironments/base/locator.ts | 2 +- .../collection/environmentsReducer.ts | 127 ++++++++++++++++++ .../environmentsReducer.unit.test.ts | 29 ++++ 4 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 src/client/pythonEnvironments/collection/environmentsReducer.ts create mode 100644 src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts diff --git a/src/client/pythonEnvironments/base/info/index.ts b/src/client/pythonEnvironments/base/info/index.ts index 11398217a512..4ea48e1fa26a 100644 --- a/src/client/pythonEnvironments/base/info/index.ts +++ b/src/client/pythonEnvironments/base/info/index.ts @@ -4,6 +4,7 @@ import { Uri } from 'vscode'; import { Architecture } from '../../../common/utils/platform'; import { BasicVersionInfo, VersionInfo } from '../../../common/utils/version'; +import { arePathsSame } from '../../common/externalDependencies'; /** * IDs for the various supported Python environments. @@ -143,3 +144,22 @@ export type PythonEnvInfo = _PythonEnvInfo & { defaultDisplayName?: string; searchLocation?: Uri; }; + +/** + * Determine if the given infos correspond to the same env. + * + * @param environment1 - one of the two envs to compare + * @param environment2 - one of the two envs to compare + */ +export function areSameEnvironment( + environment1: PythonEnvInfo, + environment2: PythonEnvInfo, +): boolean { + if (!environment1 || !environment2) { + return false; + } + if (arePathsSame(environment1.executable.filename, environment2.executable.filename)) { + return true; + } + return false; +} diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index 03eb206445bd..c6a77c473d05 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -92,7 +92,7 @@ export type PythonLocatorQuery = BasicPythonLocatorQuery & { searchLocations?: Uri[]; }; -type QueryForEvent = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery; +export 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 new file mode 100644 index 000000000000..0d51041c9212 --- /dev/null +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Event, EventEmitter } from 'vscode'; +import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info'; +import { + ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, +} from '../base/locator'; +import { PythonEnvsChangedEvent } from '../base/watcher'; + +export class PythonEnvsReducer implements ILocator { + public get onChanged(): Event { + return this.pythonEnvsManager.onChanged; + } + + constructor(private readonly pythonEnvsManager: ILocator) {} + + public resolveEnv(env: string | PythonEnvInfo): Promise { + return this.pythonEnvsManager.resolveEnv(env); + } + + public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { + const didUpdate = new EventEmitter(); + const iterator: IPythonEnvsIterator = this.iterEnvsIterator(didUpdate, query); + iterator.onUpdated = didUpdate.event; + return iterator; + } + + private async* iterEnvsIterator( + didUpdate: EventEmitter, + query?: QueryForEvent, + ): AsyncIterator { + const state = { + done: false, + pending: 0, + }; + const seen: PythonEnvInfo[] = []; + const iterator = this.pythonEnvsManager.iterEnvs(query); + + if (iterator.onUpdated !== undefined) { + iterator.onUpdated((event) => { + if (event === null) { + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); + } else { + const old = seen.find((s) => areSameEnvironment(s, event.old)); + if (old !== undefined) { + state.pending += 1; + resolveDifferencesInBackground(old, event.new, { seen, ...state }, didUpdate).ignoreErrors(); + } + } + }); + } + + let result = await iterator.next(); + while (!result.done) { + const currEnv = result.value; + const old = seen.find((s) => areSameEnvironment(s, currEnv)); + if (old !== undefined) { + state.pending += 1; + resolveDifferencesInBackground(old, currEnv, { seen, ...state }, didUpdate).ignoreErrors(); + } else { + yield currEnv; + seen.push(currEnv); + } + // eslint-disable-next-line no-await-in-loop + result = await iterator.next(); + } + if (iterator.onUpdated === undefined) { + state.done = true; + } + } +} + +async function resolveDifferencesInBackground( + oldEnv: PythonEnvInfo, + newEnv: PythonEnvInfo, + state: { seen: PythonEnvInfo[]; done: boolean; pending: number }, + didUpdate: EventEmitter, +) { + const merged = mergeEnvironments(oldEnv, newEnv); + didUpdate.fire({ old: oldEnv, new: merged }); + state.pending -= 1; + state.seen[state.seen.indexOf(oldEnv)] = merged; + 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(); + } +} + +export function mergeEnvironments(environment: PythonEnvInfo, other: PythonEnvInfo): PythonEnvInfo { + // Preserve type information. + // Possible we identified environment as unknown, but a later provider has identified env type. + if (environment.kind === PythonEnvKind.Unknown && other.kind && other.kind !== PythonEnvKind.Unknown) { + environment.kind = other.kind; + } + const props: (keyof PythonEnvInfo)[] = [ + 'version', + 'kind', + 'executable', + 'name', + 'arch', + 'distro', + 'defaultDisplayName', + 'searchLocation', + ]; + props.forEach((prop) => { + if (!environment[prop] && other[prop]) { + // tslint:disable: no-any + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (environment as any)[prop] = other[prop]; + } + }); + return environment; +} diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts new file mode 100644 index 000000000000..93224862d16c --- /dev/null +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -0,0 +1,29 @@ +import { assert } from 'chai'; +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; +import { PythonEnvsReducer } from '../../../client/pythonEnvironments/collection/environmentsReducer'; +import { + createLocatedEnv, getEnvs, SimpleLocator, +} from '../base/common'; + +suite('Environments Reducer', () => { + test('Duplicated incoming environments from locator manager are removed', async () => { + const env1 = createLocatedEnv('path/to/env1', '3.5.12b1', PythonEnvKind.Venv); + const env2 = createLocatedEnv('path/to/env2', '3.8.1', PythonEnvKind.Conda); + const env3 = createLocatedEnv('path/to/env3', '2.7', PythonEnvKind.System); + const env4 = createLocatedEnv('path/to/env2', '3.9.0rc2', PythonEnvKind.Pyenv); + const env5 = createLocatedEnv('path/to/env1', '3.8', PythonEnvKind.System); + const environments = [env1, env2, env3, env4, env5]; + const pythonEnvManager = new SimpleLocator(environments); + + const reducer = new PythonEnvsReducer(pythonEnvManager); + + const iterator = reducer.iterEnvs(); + const envs = await getEnvs(iterator); + + const expected = [env1, env2, env3]; + assert.deepEqual(envs.sort(), expected.sort()); + }); +}); From 6f9c4d98563d6155d1562c5af136fbfa5b11e67b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 18 Sep 2020 06:02:51 -0700 Subject: [PATCH 2/7] Added tests --- .../collection/environmentsReducer.ts | 10 +- src/test/pythonEnvironments/base/common.ts | 17 +- .../environmentsReducer.unit.test.ts | 181 ++++++++++++++++-- 3 files changed, 182 insertions(+), 26 deletions(-) diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index 0d51041c9212..bf0e5b26f8ce 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -46,7 +46,7 @@ export class PythonEnvsReducer implements ILocator { const old = seen.find((s) => areSameEnvironment(s, event.old)); if (old !== undefined) { state.pending += 1; - resolveDifferencesInBackground(old, event.new, { seen, ...state }, didUpdate).ignoreErrors(); + resolveDifferencesInBackground(old, event.new, state, didUpdate, seen).ignoreErrors(); } } }); @@ -58,7 +58,7 @@ export class PythonEnvsReducer implements ILocator { const old = seen.find((s) => areSameEnvironment(s, currEnv)); if (old !== undefined) { state.pending += 1; - resolveDifferencesInBackground(old, currEnv, { seen, ...state }, didUpdate).ignoreErrors(); + resolveDifferencesInBackground(old, currEnv, state, didUpdate, seen).ignoreErrors(); } else { yield currEnv; seen.push(currEnv); @@ -68,6 +68,7 @@ export class PythonEnvsReducer implements ILocator { } if (iterator.onUpdated === undefined) { state.done = true; + checkIfFinishedAndNotify(state, didUpdate); } } } @@ -75,13 +76,14 @@ export class PythonEnvsReducer implements ILocator { async function resolveDifferencesInBackground( oldEnv: PythonEnvInfo, newEnv: PythonEnvInfo, - state: { seen: PythonEnvInfo[]; done: boolean; pending: number }, + state: { done: boolean; pending: number }, didUpdate: EventEmitter, + seen: PythonEnvInfo[], ) { const merged = mergeEnvironments(oldEnv, newEnv); didUpdate.fire({ old: oldEnv, new: merged }); + seen[seen.indexOf(oldEnv)] = merged; state.pending -= 1; - state.seen[state.seen.indexOf(oldEnv)] = merged; checkIfFinishedAndNotify(state, didUpdate); } diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts index 6f56011f1f03..d1d3b21cc7cc 100644 --- a/src/test/pythonEnvironments/base/common.ts +++ b/src/test/pythonEnvironments/base/common.ts @@ -1,14 +1,19 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { createDeferred, flattenIterator, iterable, mapToIterator } from '../../../client/common/utils/async'; +import { Event } from 'vscode'; +import { + createDeferred, flattenIterator, iterable, mapToIterator, +} from '../../../client/common/utils/async'; import { Architecture } from '../../../client/common/utils/platform'; import { PythonEnvInfo, PythonEnvKind, } from '../../../client/pythonEnvironments/base/info'; import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion'; -import { IPythonEnvsIterator, Locator, 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( @@ -66,6 +71,7 @@ export class SimpleLocator extends Locator { resolve?: null | ((env: PythonEnvInfo) => Promise); before?: Promise; after?: Promise; + onUpdated?: Event; beforeEach?(e: PythonEnvInfo): Promise; afterEach?(e: PythonEnvInfo): Promise; onQuery?(query: PythonLocatorQuery | undefined, envs: PythonEnvInfo[]): Promise; @@ -83,7 +89,7 @@ export class SimpleLocator extends Locator { const deferred = this.deferred; const callbacks = this.callbacks; let envs = this.envs; - async function* iterator() { + const iterator: IPythonEnvsIterator = async function*() { if (callbacks?.onQuery !== undefined) { envs = await callbacks.onQuery(query, envs); } @@ -114,8 +120,9 @@ export class SimpleLocator extends Locator { await callbacks.after; } deferred.resolve(); - } - return iterator(); + }(); + iterator.onUpdated = this.callbacks?.onUpdated; + return iterator; } public async resolveEnv(env: string | PythonEnvInfo): Promise { const envInfo: PythonEnvInfo = typeof env === 'string' ? createEnv('', '', undefined, env) : env; diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts index 93224862d16c..2bf3e2574bd4 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -1,29 +1,176 @@ -import { assert } from 'chai'; // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; -import { PythonEnvsReducer } from '../../../client/pythonEnvironments/collection/environmentsReducer'; +import { assert, expect } from 'chai'; +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 { - createLocatedEnv, getEnvs, SimpleLocator, -} from '../base/common'; + mergeEnvironments, + PythonEnvsReducer, +} from '../../../client/pythonEnvironments/collection/environmentsReducer'; +import { sleep } from '../../core'; +import { createEnv, getEnvs, SimpleLocator } from '../base/common'; suite('Environments Reducer', () => { - test('Duplicated incoming environments from locator manager are removed', async () => { - const env1 = createLocatedEnv('path/to/env1', '3.5.12b1', PythonEnvKind.Venv); - const env2 = createLocatedEnv('path/to/env2', '3.8.1', PythonEnvKind.Conda); - const env3 = createLocatedEnv('path/to/env3', '2.7', PythonEnvKind.System); - const env4 = createLocatedEnv('path/to/env2', '3.9.0rc2', PythonEnvKind.Pyenv); - const env5 = createLocatedEnv('path/to/env1', '3.8', PythonEnvKind.System); - const environments = [env1, env2, env3, env4, env5]; - const pythonEnvManager = new SimpleLocator(environments); + suite('iterEnvs()', () => { + test('Iterator only yields unique environments', async () => { + const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Venv, 'path/to/exec1'); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Conda, 'path/to/exec2'); + const env3 = createEnv('env3', '2.7', PythonEnvKind.System, 'path/to/exec3'); + const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, 'path/to/exec2'); // Same as env2 + const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, 'path/to/exec1'); // Same as env1 + const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); + const reducer = new PythonEnvsReducer(pythonEnvManager); + const iterator = reducer.iterEnvs(); + const envs = await getEnvs(iterator); + + const expected = [env1, env2, env3]; + assert.deepEqual(envs, expected); + }); + + test('Single updates for multiple environments are sent correctly followed by the null event', async () => { + // Arrange + const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, 'path/to/exec1'); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Unknown, 'path/to/exec2'); + const env3 = createEnv('env3', '2.7', PythonEnvKind.System, 'path/to/exec3'); + const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Conda, 'path/to/exec2'); // Same as env2 + const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, 'path/to/exec1'); // Same as env1 + const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); + const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; + const reducer = new PythonEnvsReducer(pythonEnvManager); + + 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: env2, new: mergeEnvironments(env2, env4) }, + { old: env1, new: mergeEnvironments(env1, env5) }, + null, + ]; + assert.deepEqual(expectedUpdates, onUpdatedEvents); + }); + + test('Multiple updates for the same environment are sent correctly followed by the null event', async () => { + // Arrange + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, 'path/to/exec'); + const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, 'path/to/exec'); + const environmentsToBeIterated = [env1, env2, env3]; // All refer to the same environment + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); + const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; + const reducer = new PythonEnvsReducer(pythonEnvManager); + + 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 env12 = mergeEnvironments(env1, env2); + const expectedUpdates = [ + { old: env1, new: env12 }, + { old: env12, new: mergeEnvironments(env12, env3) }, + null, + ]; + assert.deepEqual(expectedUpdates, onUpdatedEvents); + }); + + test('Updates to environments from the incoming iterator are passed on correctly followed by the null event', async () => { + // Arrange + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, 'path/to/exec'); + const environmentsToBeIterated = [env1]; + const didUpdate = new EventEmitter(); + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); + const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; + const reducer = new PythonEnvsReducer(pythonEnvManager); + + 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); + didUpdate.fire({ old: env1, new: env2 }); + didUpdate.fire(null); // It is essential for the incoming iterator to fire "null" event signifying it's done + await sleep(1); + + // Assert + const expectedUpdates = [{ old: env1, new: mergeEnvironments(env1, env2) }, null]; + assert.deepEqual(expectedUpdates, onUpdatedEvents); + didUpdate.dispose(); + }); + }); + + test('onChanged fires iff onChanged from locator manager fires', () => { + const pythonEnvManager = new SimpleLocator([]); + const event1: PythonEnvsChangedEvent = {}; + const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; + const expected = [event1, event2]; + const reducer = new PythonEnvsReducer(pythonEnvManager); + + const events: PythonEnvsChangedEvent[] = []; + reducer.onChanged((e) => events.push(e)); + + pythonEnvManager.fire(event1); + pythonEnvManager.fire(event2); + + assert.deepEqual(events, expected); + }); + + test('Calls locator manager to resolves environments', async () => { + const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); + const resolvedEnv = createEnv('env1', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); + const pythonEnvManager = new SimpleLocator([], { + resolve: async (e: PythonEnvInfo) => { + if (e === env) { + return resolvedEnv; + } + return undefined; + }, + }); const reducer = new PythonEnvsReducer(pythonEnvManager); - const iterator = reducer.iterEnvs(); - const envs = await getEnvs(iterator); + const expected = await reducer.resolveEnv(env); - const expected = [env1, env2, env3]; - assert.deepEqual(envs.sort(), expected.sort()); + assert.deepEqual(expected, resolvedEnv); }); }); From e6a9260d0f2bdc60381ae14e5562e95700b71dc8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 18 Sep 2020 06:14:56 -0700 Subject: [PATCH 3/7] Use path.join to construct paths --- .../environmentsReducer.unit.test.ts | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts index 2bf3e2574bd4..51dd4a0c2569 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { assert, expect } from 'chai'; +import * as path from 'path'; import { EventEmitter } from 'vscode'; import { PythonEnvInfo, PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; import { PythonEnvUpdatedEvent } from '../../../client/pythonEnvironments/base/locator'; @@ -16,11 +17,11 @@ import { createEnv, getEnvs, SimpleLocator } from '../base/common'; suite('Environments Reducer', () => { suite('iterEnvs()', () => { test('Iterator only yields unique environments', async () => { - const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Venv, 'path/to/exec1'); - const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Conda, 'path/to/exec2'); - const env3 = createEnv('env3', '2.7', PythonEnvKind.System, 'path/to/exec3'); - const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, 'path/to/exec2'); // Same as env2 - const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, 'path/to/exec1'); // Same as env1 + 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')); // Same as env2 + const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1 const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); const reducer = new PythonEnvsReducer(pythonEnvManager); @@ -34,11 +35,11 @@ suite('Environments Reducer', () => { test('Single updates for multiple environments are sent correctly followed by the null event', async () => { // Arrange - const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, 'path/to/exec1'); - const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Unknown, 'path/to/exec2'); - const env3 = createEnv('env3', '2.7', PythonEnvKind.System, 'path/to/exec3'); - const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Conda, 'path/to/exec2'); // Same as env2 - const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, 'path/to/exec1'); // Same as env1 + 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 env3 = createEnv('env3', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); + const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); // Same as env2; + const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1; const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; @@ -71,9 +72,9 @@ suite('Environments Reducer', () => { test('Multiple updates for the same environment are sent correctly followed by the null event', async () => { // Arrange - const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); - const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, 'path/to/exec'); - const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, 'path/to/exec'); + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); + const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env1, env2, env3]; // All refer to the same environment const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; @@ -107,8 +108,8 @@ suite('Environments Reducer', () => { test('Updates to environments from the incoming iterator are passed on correctly followed by the null event', async () => { // Arrange - const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); - const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, 'path/to/exec'); + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env1]; const didUpdate = new EventEmitter(); const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); @@ -157,7 +158,7 @@ suite('Environments Reducer', () => { }); test('Calls locator manager to resolves environments', async () => { - const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, 'path/to/exec'); + const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); const resolvedEnv = createEnv('env1', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); const pythonEnvManager = new SimpleLocator([], { resolve: async (e: PythonEnvInfo) => { From f2b5f80f92dc7d9b41683e98a69778c4d95efa04 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 21 Sep 2020 13:41:54 -0700 Subject: [PATCH 4/7] Code reviews --- .../collection/environmentsReducer.ts | 98 +++++++++++-------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index bf0e5b26f8ce..1e6559542e73 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -1,13 +1,18 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { isEqual } from 'lodash'; import { Event, EventEmitter } from 'vscode'; +import { traceVerbose } from '../../common/logger'; import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info'; import { ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, } from '../base/locator'; import { PythonEnvsChangedEvent } from '../base/watcher'; +/** + * Combines duplicate environments received from the incoming locator into one and passes on unique environments + */ export class PythonEnvsReducer implements ILocator { public get onChanged(): Event { return this.pythonEnvsManager.onChanged; @@ -21,68 +26,75 @@ export class PythonEnvsReducer implements ILocator { public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { const didUpdate = new EventEmitter(); - const iterator: IPythonEnvsIterator = this.iterEnvsIterator(didUpdate, query); + const incomingIterator = this.pythonEnvsManager.iterEnvs(query); + const iterator: IPythonEnvsIterator = iterEnvsIterator(incomingIterator, didUpdate); iterator.onUpdated = didUpdate.event; return iterator; } +} - private async* iterEnvsIterator( - didUpdate: EventEmitter, - query?: QueryForEvent, - ): AsyncIterator { - const state = { - done: false, - pending: 0, - }; - const seen: PythonEnvInfo[] = []; - const iterator = this.pythonEnvsManager.iterEnvs(query); +async function* 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); + 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) { + state.pending += 1; + resolveDifferencesInBackground(oldIndex, event.new, state, didUpdate, seen).ignoreErrors(); } else { - const old = seen.find((s) => areSameEnvironment(s, event.old)); - if (old !== undefined) { - state.pending += 1; - resolveDifferencesInBackground(old, event.new, state, didUpdate, seen).ignoreErrors(); - } + // This implies a problem in a downstream locator + traceVerbose(`Expected already iterated env, got ${event.old}`); } - }); - } - - let result = await iterator.next(); - while (!result.done) { - const currEnv = result.value; - const old = seen.find((s) => areSameEnvironment(s, currEnv)); - if (old !== undefined) { - state.pending += 1; - resolveDifferencesInBackground(old, currEnv, state, didUpdate, seen).ignoreErrors(); - } else { - yield currEnv; - seen.push(currEnv); } - // eslint-disable-next-line no-await-in-loop - result = await iterator.next(); - } - if (iterator.onUpdated === undefined) { - state.done = true; - checkIfFinishedAndNotify(state, didUpdate); + }); + } + + let result = await iterator.next(); + while (!result.done) { + const currEnv = result.value; + const oldIndex = seen.findIndex((s) => areSameEnvironment(s, currEnv)); + if (oldIndex !== -1) { + state.pending += 1; + resolveDifferencesInBackground(oldIndex, currEnv, state, didUpdate, seen).ignoreErrors(); + } else { + // We haven't yielded a matching env so yield this one as-is. + yield currEnv; + seen.push(currEnv); } + // eslint-disable-next-line no-await-in-loop + result = await iterator.next(); + } + if (iterator.onUpdated === undefined) { + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); } } async function resolveDifferencesInBackground( - oldEnv: PythonEnvInfo, + oldIndex: number, newEnv: PythonEnvInfo, state: { done: boolean; pending: number }, didUpdate: EventEmitter, seen: PythonEnvInfo[], ) { + const oldEnv = seen[oldIndex]; const merged = mergeEnvironments(oldEnv, newEnv); - didUpdate.fire({ old: oldEnv, new: merged }); - seen[seen.indexOf(oldEnv)] = merged; + if (!isEqual(oldEnv, merged)) { + didUpdate.fire({ old: oldEnv, new: merged }); + seen[oldIndex] = merged; + } state.pending -= 1; checkIfFinishedAndNotify(state, didUpdate); } From 1f00f38040c7ade633c44c0d8aabd065addaaa48 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 22 Sep 2020 07:36:13 -0700 Subject: [PATCH 5/7] Correct dummy implementations and adjust tests --- .../collection/environmentsReducer.ts | 11 ++++++----- .../collection/environmentsReducer.unit.test.ts | 15 +++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index 1e6559542e73..cf3c10a5e6cb 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { isEqual } from 'lodash'; +import { cloneDeep, isEqual } from 'lodash'; import { Event, EventEmitter } from 'vscode'; import { traceVerbose } from '../../common/logger'; import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info'; @@ -115,10 +115,11 @@ function checkIfFinishedAndNotify( } export function mergeEnvironments(environment: PythonEnvInfo, other: PythonEnvInfo): PythonEnvInfo { + const result = cloneDeep(environment); // Preserve type information. // Possible we identified environment as unknown, but a later provider has identified env type. if (environment.kind === PythonEnvKind.Unknown && other.kind && other.kind !== PythonEnvKind.Unknown) { - environment.kind = other.kind; + result.kind = other.kind; } const props: (keyof PythonEnvInfo)[] = [ 'version', @@ -131,11 +132,11 @@ export function mergeEnvironments(environment: PythonEnvInfo, other: PythonEnvIn 'searchLocation', ]; props.forEach((prop) => { - if (!environment[prop] && other[prop]) { + if (!result[prop] && other[prop]) { // tslint:disable: no-any // eslint-disable-next-line @typescript-eslint/no-explicit-any - (environment as any)[prop] = other[prop]; + (result as any)[prop] = other[prop]; } }); - return environment; + return result; } diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts index 51dd4a0c2569..2876cddb7e22 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. 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'; @@ -98,12 +99,14 @@ suite('Environments Reducer', () => { // Assert const env12 = mergeEnvironments(env1, env2); - const expectedUpdates = [ - { old: env1, new: env12 }, - { old: env12, new: mergeEnvironments(env12, env3) }, - null, - ]; - assert.deepEqual(expectedUpdates, onUpdatedEvents); + const env123 = mergeEnvironments(env12, env3); + const expectedUpdates: (PythonEnvUpdatedEvent | null)[] = []; + if (isEqual(env12, env123)) { + expectedUpdates.push({ old: env1, new: env12 }, null); + } else { + expectedUpdates.push({ old: env1, new: env12 }, { old: env12, new: env123 }, null); + } + assert.deepEqual(onUpdatedEvents, expectedUpdates); }); test('Updates to environments from the incoming iterator are passed on correctly followed by the null event', async () => { From 3b24031d8c043225f2e569ff811321b7db556ad7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 22 Sep 2020 14:31:51 -0700 Subject: [PATCH 6/7] Modify resolveEnv() --- .../pythonEnvironments/base/info/index.ts | 19 +++-- .../collection/environmentsReducer.ts | 27 ++++++- .../environmentsReducer.unit.test.ts | 80 ++++++++++++++----- 3 files changed, 98 insertions(+), 28 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/index.ts b/src/client/pythonEnvironments/base/info/index.ts index 4ea48e1fa26a..c24cf8dc2aa3 100644 --- a/src/client/pythonEnvironments/base/info/index.ts +++ b/src/client/pythonEnvironments/base/info/index.ts @@ -152,13 +152,22 @@ export type PythonEnvInfo = _PythonEnvInfo & { * @param environment2 - one of the two envs to compare */ export function areSameEnvironment( - environment1: PythonEnvInfo, - environment2: PythonEnvInfo, + environment1: PythonEnvInfo | string, + environment2: PythonEnvInfo | string, ): boolean { - if (!environment1 || !environment2) { - return false; + let path1: string; + let path2: string; + if (typeof environment1 === 'string') { + path1 = environment1; + } else { + path1 = environment1.executable.filename; } - if (arePathsSame(environment1.executable.filename, environment2.executable.filename)) { + if (typeof environment2 === 'string') { + path2 = environment2; + } else { + path2 = environment2.executable.filename; + } + if (arePathsSame(path1, path2)) { return true; } return false; diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index cf3c10a5e6cb..42143762f3c6 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -4,6 +4,7 @@ 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 { ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent, @@ -20,8 +21,30 @@ export class PythonEnvsReducer implements ILocator { constructor(private readonly pythonEnvsManager: ILocator) {} - public resolveEnv(env: string | PythonEnvInfo): Promise { - return this.pythonEnvsManager.resolveEnv(env); + public async resolveEnv(env: string | PythonEnvInfo): Promise { + let environment: PythonEnvInfo | undefined; + const waitForUpdatesDeferred = createDeferred(); + const iterator = this.iterEnvs(); + iterator.onUpdated!((event) => { + if (event === null) { + waitForUpdatesDeferred.resolve(); + } else if (environment && areSameEnvironment(environment, event.new)) { + environment = event.new; + } + }); + let result = await iterator.next(); + while (!result.done) { + if (areSameEnvironment(result.value, env)) { + environment = result.value; + } + // eslint-disable-next-line no-await-in-loop + result = await iterator.next(); + } + if (!environment) { + return undefined; + } + await waitForUpdatesDeferred.promise; + return this.pythonEnvsManager.resolveEnv(environment); } public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts index 2876cddb7e22..82f6f5874e05 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -18,11 +18,11 @@ import { createEnv, getEnvs, SimpleLocator } from '../base/common'; suite('Environments Reducer', () => { suite('iterEnvs()', () => { test('Iterator only yields unique environments', 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 env1 = createEnv('env1', '3.5', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); + const env2 = createEnv('env2', '3.8', 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')); // Same as env2 - const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1 + const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); // Same as env2 + const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1 const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); const reducer = new PythonEnvsReducer(pythonEnvManager); @@ -36,11 +36,11 @@ suite('Environments Reducer', () => { test('Single updates for multiple 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 env1 = createEnv('env1', '3.5', PythonEnvKind.Unknown, path.join('path', 'to', 'exec1')); + const env2 = createEnv('env2', '3.8', PythonEnvKind.Unknown, 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.Conda, path.join('path', 'to', 'exec2')); // Same as env2; - const env5 = createEnv('env5', '3.8', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1; + const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); // Same as env2; + const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1; const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; @@ -160,21 +160,59 @@ suite('Environments Reducer', () => { assert.deepEqual(events, expected); }); - test('Calls locator manager to resolves environments', async () => { - const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); - const resolvedEnv = createEnv('env1', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); - const pythonEnvManager = new SimpleLocator([], { - resolve: async (e: PythonEnvInfo) => { - if (e === env) { - return resolvedEnv; - } - return undefined; - }, + suite('resolveEnv()', () => { + test('Iterates environments from the reducer to get resolved environment, then calls into locator manager to resolve environment further and return it', async () => { + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const env2 = createEnv('env2', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); + const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); + const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); + const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); + const env6 = createEnv('env6', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); + const environmentsToBeIterated = [env1, env2, env3, env4, env5, env6]; // env1 env3 env6 are same + + const env13 = mergeEnvironments(env1, env3); + const env136 = mergeEnvironments(env13, env6); + const expectedResolvedEnv = createEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { + resolve: async (e: PythonEnvInfo) => { + if (isEqual(e, env136)) { + return expectedResolvedEnv; + } + return undefined; + }, + }); + const reducer = new PythonEnvsReducer(pythonEnvManager); + + // Trying to resolve the environment corresponding to env1 env3 env6 + const expected = await reducer.resolveEnv(path.join('path', 'to', 'exec')); + + assert.deepEqual(expected, expectedResolvedEnv); }); - const reducer = new PythonEnvsReducer(pythonEnvManager); - const expected = await reducer.resolveEnv(env); + test("If the reducer isn't able to resolve environment, return undefined", async () => { + const env1 = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); + const env2 = createEnv('env2', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); + const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); + const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); + const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); + const env6 = createEnv('env6', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); + const environmentsToBeIterated = [env1, env2, env3, env4, env5, env6]; // env1 env3 env6 are same + + const env13 = mergeEnvironments(env1, env3); + const env136 = mergeEnvironments(env13, env6); + const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { + resolve: async (e: PythonEnvInfo) => { + if (isEqual(e, env136)) { + return createEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); + } + return undefined; + }, + }); + const reducer = new PythonEnvsReducer(pythonEnvManager); + + const expected = await reducer.resolveEnv(path.join('path', 'to', 'execNeverSeenBefore')); - assert.deepEqual(expected, resolvedEnv); + assert.deepEqual(expected, undefined); + }); }); }); From 5ec529a3e9aa88f2c184871be979c26f726c897f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 23 Sep 2020 11:36:34 -0700 Subject: [PATCH 7/7] Rename to a general parentLocator --- .../collection/environmentsReducer.ts | 8 ++--- .../environmentsReducer.unit.test.ts | 32 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/client/pythonEnvironments/collection/environmentsReducer.ts b/src/client/pythonEnvironments/collection/environmentsReducer.ts index 42143762f3c6..b4c80254a3f9 100644 --- a/src/client/pythonEnvironments/collection/environmentsReducer.ts +++ b/src/client/pythonEnvironments/collection/environmentsReducer.ts @@ -16,10 +16,10 @@ import { PythonEnvsChangedEvent } from '../base/watcher'; */ export class PythonEnvsReducer implements ILocator { public get onChanged(): Event { - return this.pythonEnvsManager.onChanged; + return this.parentLocator.onChanged; } - constructor(private readonly pythonEnvsManager: ILocator) {} + constructor(private readonly parentLocator: ILocator) {} public async resolveEnv(env: string | PythonEnvInfo): Promise { let environment: PythonEnvInfo | undefined; @@ -44,12 +44,12 @@ export class PythonEnvsReducer implements ILocator { return undefined; } await waitForUpdatesDeferred.promise; - return this.pythonEnvsManager.resolveEnv(environment); + return this.parentLocator.resolveEnv(environment); } public iterEnvs(query?: QueryForEvent): IPythonEnvsIterator { const didUpdate = new EventEmitter(); - const incomingIterator = this.pythonEnvsManager.iterEnvs(query); + const incomingIterator = this.parentLocator.iterEnvs(query); const iterator: IPythonEnvsIterator = iterEnvsIterator(incomingIterator, didUpdate); iterator.onUpdated = didUpdate.event; return iterator; diff --git a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts index 82f6f5874e05..a05ebc772063 100644 --- a/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/collection/environmentsReducer.unit.test.ts @@ -24,8 +24,8 @@ suite('Environments Reducer', () => { const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); // Same as env2 const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1 const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); - const reducer = new PythonEnvsReducer(pythonEnvManager); + const parentLocator = new SimpleLocator(environmentsToBeIterated); + const reducer = new PythonEnvsReducer(parentLocator); const iterator = reducer.iterEnvs(); const envs = await getEnvs(iterator); @@ -42,9 +42,9 @@ suite('Environments Reducer', () => { const env4 = createEnv('env4', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec2')); // Same as env2; const env5 = createEnv('env5', '3.5.12b1', PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); // Same as env1; const environmentsToBeIterated = [env1, env2, env3, env4, env5]; // Contains 3 unique environments - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); + const parentLocator = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); const iterator = reducer.iterEnvs(); // Act @@ -77,9 +77,9 @@ suite('Environments Reducer', () => { const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); const env3 = createEnv('env3', '3.8.1', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env1, env2, env3]; // All refer to the same environment - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated); + const parentLocator = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); const iterator = reducer.iterEnvs(); // Act @@ -115,9 +115,9 @@ suite('Environments Reducer', () => { const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env1]; const didUpdate = new EventEmitter(); - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); + const parentLocator = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); const iterator = reducer.iterEnvs(); // Act @@ -145,17 +145,17 @@ suite('Environments Reducer', () => { }); test('onChanged fires iff onChanged from locator manager fires', () => { - const pythonEnvManager = new SimpleLocator([]); + const parentLocator = new SimpleLocator([]); const event1: PythonEnvsChangedEvent = {}; const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; const expected = [event1, event2]; - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); const events: PythonEnvsChangedEvent[] = []; reducer.onChanged((e) => events.push(e)); - pythonEnvManager.fire(event1); - pythonEnvManager.fire(event2); + parentLocator.fire(event1); + parentLocator.fire(event2); assert.deepEqual(events, expected); }); @@ -173,7 +173,7 @@ suite('Environments Reducer', () => { const env13 = mergeEnvironments(env1, env3); const env136 = mergeEnvironments(env13, env6); const expectedResolvedEnv = createEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { + const parentLocator = new SimpleLocator(environmentsToBeIterated, { resolve: async (e: PythonEnvInfo) => { if (isEqual(e, env136)) { return expectedResolvedEnv; @@ -181,7 +181,7 @@ suite('Environments Reducer', () => { return undefined; }, }); - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); // Trying to resolve the environment corresponding to env1 env3 env6 const expected = await reducer.resolveEnv(path.join('path', 'to', 'exec')); @@ -200,7 +200,7 @@ suite('Environments Reducer', () => { const env13 = mergeEnvironments(env1, env3); const env136 = mergeEnvironments(env13, env6); - const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { + const parentLocator = new SimpleLocator(environmentsToBeIterated, { resolve: async (e: PythonEnvInfo) => { if (isEqual(e, env136)) { return createEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); @@ -208,7 +208,7 @@ suite('Environments Reducer', () => { return undefined; }, }); - const reducer = new PythonEnvsReducer(pythonEnvManager); + const reducer = new PythonEnvsReducer(parentLocator); const expected = await reducer.resolveEnv(path.join('path', 'to', 'execNeverSeenBefore'));