Added tests

Author: Kartik Raj

src/client/pythonEnvironments/collection/environmentsResolver.ts

@@ -50,12 +50,11 @@ export class PythonEnvsResolver implements ILocator {
                     state.done = true;
                     checkIfFinishedAndNotify(state, didUpdate);
                 } else {
-                    const old = seen.find((s) => areSameEnvironment(s, event.old));
-                    if (old !== undefined) {
-                        didUpdate.fire({ old, new: event.new });
-                        seen[seen.indexOf(old)] = event.new;
+                    const oldIndex = seen.findIndex((s) => areSameEnvironment(s, event.old));
+                    if (oldIndex !== -1) {
+                        seen[oldIndex] = event.new;
                         state.pending += 1;
-                        this.resolveInBackground(event.new, state, didUpdate, seen).ignoreErrors();
+                        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}`);
@@ -71,7 +70,7 @@ export class PythonEnvsResolver implements ILocator {
             seen.push(currEnv);
             yield currEnv;
             state.pending += 1;
-            this.resolveInBackground(currEnv, state, didUpdate, seen).ignoreErrors();
+            this.resolveInBackground(seen.indexOf(currEnv), state, didUpdate, seen).ignoreErrors();
             // eslint-disable-next-line no-await-in-loop
             result = await iterator.next();
         }
@@ -82,18 +81,18 @@ export class PythonEnvsResolver implements ILocator {
     }
 
     private async resolveInBackground(
-        env: PythonEnvInfo,
+        envIndex: number,
         state: { done: boolean; pending: number },
         didUpdate: EventEmitter<PythonEnvUpdatedEvent | null>,
         seen: PythonEnvInfo[],
     ) {
-        const interpreterInfo = await this.environmentInfoService.getEnvironmentInfo(env.executable.filename);
-        // Old environment to be fired might have changed until now, search for it again
-        const seenEnv = seen.find((s) => areSameEnvironment(s, env));
-        if (interpreterInfo && seenEnv) {
-            const resolvedEnv = getResolvedEnv(interpreterInfo, seenEnv);
-            didUpdate.fire({ old: seenEnv, new: resolvedEnv });
-            seen[seen.indexOf(seenEnv)] = resolvedEnv;
+        const interpreterInfo = await this.environmentInfoService.getEnvironmentInfo(
+            seen[envIndex].executable.filename,
+        );
+        if (interpreterInfo && seen[envIndex]) {
+            const resolvedEnv = getResolvedEnv(interpreterInfo, seen[envIndex]);
+            didUpdate.fire({ old: seen[envIndex], new: resolvedEnv });
+            seen[envIndex] = resolvedEnv;
         }
         state.pending -= 1;
         checkIfFinishedAndNotify(state, didUpdate);

src/test/pythonEnvironments/base/common.ts

@@ -1,14 +1,17 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { Event } from 'vscode';
 import { createDeferred, flattenIterator, iterable, mapToIterator } from '../../../client/common/utils/async';
 import { Architecture } from '../../../client/common/utils/platform';
+import { EMPTY_VERSION, parseBasicVersionInfo } from '../../../client/common/utils/version';
 import {
     PythonEnvInfo,
     PythonEnvKind,
+    PythonReleaseLevel,
+    PythonVersion
 } 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(
@@ -43,6 +46,36 @@ export function createEnv(
     };
 }
 
+function parseVersion(versionStr: string): PythonVersion {
+    const parsed = parseBasicVersionInfo<PythonVersion>(versionStr);
+    if (!parsed) {
+        if (versionStr === '') {
+            return EMPTY_VERSION as PythonVersion;
+        }
+        throw Error(`invalid version ${versionStr}`);
+    }
+    const { version, after } = parsed;
+    const match = after.match(/^(a|b|rc)(\d+)$/);
+    if (match) {
+        const [, levelStr, serialStr ] = match;
+        let level: PythonReleaseLevel;
+        if (levelStr === 'a') {
+            level = PythonReleaseLevel.Alpha;
+        } else if (levelStr === 'b') {
+            level = PythonReleaseLevel.Beta;
+        } else if (levelStr === 'rc') {
+            level = PythonReleaseLevel.Candidate;
+        } else {
+            throw Error('unreachable!');
+        }
+        version.release = {
+            level,
+            serial: parseInt(serialStr, 10)
+        };
+    }
+    return version;
+}
+
 export function createLocatedEnv(
     location: string,
     versionStr: string,
@@ -66,6 +99,7 @@ export class SimpleLocator extends Locator {
             resolve?: null | ((env: PythonEnvInfo) => Promise<PythonEnvInfo | undefined>);
             before?: Promise<void>;
             after?: Promise<void>;
+            onUpdated?: Event<PythonEnvUpdatedEvent | null>;
             beforeEach?(e: PythonEnvInfo): Promise<void>;
             afterEach?(e: PythonEnvInfo): Promise<void>;
             onQuery?(query: PythonLocatorQuery | undefined, envs: PythonEnvInfo[]): Promise<PythonEnvInfo[]>;
@@ -83,7 +117,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 +148,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<PythonEnvInfo | undefined> {
         const envInfo: PythonEnvInfo = typeof env === 'string' ? createEnv('', '', undefined, env) : env;

src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts

@@ -2,20 +2,26 @@
 // 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, IEnvironmentInfoService } from '../../../client/pythonEnvironments/info/environmentInfoService';
+import {
+    EnvironmentInfoService,
+    IEnvironmentInfoService,
+} from '../../../client/pythonEnvironments/info/environmentInfoService';
 import { sleep } from '../../core';
 import { createEnv, getEnvs, SimpleLocator } from '../base/common';
 
-suite('Environments Reducer', () => {
+suite('Environments Resolver', () => {
     suite('iterEnvs()', () => {
         let stubShellExec: sinon.SinonStub;
         let envService: IEnvironmentInfoService;
@@ -28,40 +34,45 @@ suite('Environments Reducer', () => {
                 new Promise<ExecutionResult<string>>((resolve) => {
                     resolve({
                         stdout:
-                            '{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "version": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
+                            '{"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 only yields unique environments', async () => {
+        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;
+        }
+
+        test('Iterator only yields 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')); // 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 env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2'));
+            const environmentsToBeIterated = [env1, env2, env3, env4];
             const pythonEnvManager = new SimpleLocator(environmentsToBeIterated);
             const reducer = new PythonEnvsResolver(pythonEnvManager, envService);
 
             const iterator = reducer.iterEnvs();
             const envs = await getEnvs(iterator);
 
-            const expected = [env1, env2, env3];
-            assert.deepEqual(envs, expected);
+            assert.deepEqual(envs, environmentsToBeIterated);
         });
 
-        test('Single updates for multiple environments are sent correctly followed by the null event', async () => {
+        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 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 environmentsToBeIterated = [env1, env2];
             const pythonEnvManager = new SimpleLocator(environmentsToBeIterated);
             const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = [];
             const reducer = new PythonEnvsResolver(pythonEnvManager, envService);
@@ -84,54 +95,18 @@ suite('Environments Reducer', () => {
 
             // Assert
             const expectedUpdates = [
-                { old: env2, new: mergeEnvironments(env2, env4) },
-                { old: env1, new: mergeEnvironments(env1, env5) },
+                { old: env1, new: createExpectedEnvInfo(env1) },
+                { old: env2, new: createExpectedEnvInfo(env2) },
                 null,
             ];
             assert.deepEqual(expectedUpdates, onUpdatedEvents);
         });
 
-        test('Multiple updates for the same environment are sent correctly followed by the null event', async () => {
+        test('Updates to environments from the incoming iterator are sent correctly followed by the null event', async () => {
             // Arrange
-            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)[] = [];
-            const reducer = new PythonEnvsResolver(pythonEnvManager, envService);
-
-            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.join('path', 'to', 'exec'));
-            const env2 = createEnv('env2', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec'));
-            const environmentsToBeIterated = [env1];
+            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<PythonEnvUpdatedEvent | null>();
             const pythonEnvManager = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event });
             const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = [];
@@ -151,13 +126,16 @@ suite('Environments Reducer', () => {
 
             // Act
             await getEnvs(iterator);
-            didUpdate.fire({ old: env1, new: env2 });
+            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
-            const expectedUpdates = [{ old: env1, new: mergeEnvironments(env1, env2) }, null];
-            assert.deepEqual(expectedUpdates, onUpdatedEvents);
+            // 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();
         });
     });