Handle venv creation when running in no-workspace case (#145)

karthiknadig · web-flow · commit 3bb989374f25 · 2025-01-29T10:27:24.000-08:00
Fixes #140
diff --git a/src/common/pickers/projects.ts b/src/common/pickers/projects.ts
@@ -49,6 +49,8 @@ export async function pickProjectMany(
         }
     } else if (projects.length === 1) {
         return [...projects];
+    } else if (projects.length === 0) {
+        return [];
     }
     return undefined;
 }
diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts
@@ -111,44 +111,47 @@ export async function createAnyEnvironmentCommand(
     options?: any,
 ): Promise<PythonEnvironment | undefined> {
     const select = options?.selectEnvironment;
-    const projects = await pickProjectMany(pm.getProjects());
-    if (projects && projects.length > 0) {
-        const defaultManagers: InternalEnvironmentManager[] = [];
-
-        projects.forEach((p) => {
-            const manager = em.getEnvironmentManager(p.uri);
-            if (manager && manager.supportsCreate && !defaultManagers.includes(manager)) {
-                defaultManagers.push(manager);
-            }
-        });
-
-        const managerId = await pickEnvironmentManager(
-            em.managers.filter((m) => m.supportsCreate),
-            defaultManagers,
-        );
-
-        const manager = em.managers.find((m) => m.id === managerId);
-        if (manager) {
-            const env = await manager.create(projects.map((p) => p.uri));
-            if (select) {
-                await em.setEnvironments(
-                    projects.map((p) => p.uri),
-                    env,
-                );
-            }
-            return env;
-        }
-    } else if (projects && projects.length === 0) {
+    const projects = pm.getProjects();
+    if (projects.length === 0) {
         const managerId = await pickEnvironmentManager(em.managers.filter((m) => m.supportsCreate));
-
         const manager = em.managers.find((m) => m.id === managerId);
         if (manager) {
             const env = await manager.create('global');
-            if (select) {
+            if (select && env) {
                 await manager.set(undefined, env);
             }
             return env;
         }
+    } else if (projects.length > 0) {
+        const selected = await pickProjectMany(projects);
+
+        if (selected && selected.length > 0) {
+            const defaultManagers: InternalEnvironmentManager[] = [];
+
+            selected.forEach((p) => {
+                const manager = em.getEnvironmentManager(p.uri);
+                if (manager && manager.supportsCreate && !defaultManagers.includes(manager)) {
+                    defaultManagers.push(manager);
+                }
+            });
+
+            const managerId = await pickEnvironmentManager(
+                em.managers.filter((m) => m.supportsCreate),
+                defaultManagers,
+            );
+
+            const manager = em.managers.find((m) => m.id === managerId);
+            if (manager) {
+                const env = await manager.create(selected.map((p) => p.uri));
+                if (select && env) {
+                    await em.setEnvironments(
+                        selected.map((p) => p.uri),
+                        env,
+                    );
+                }
+                return env;
+            }
+        }
     }
 }
 
diff --git a/src/test/copilotTools.unit.test.ts b/src/test/copilotTools.unit.test.ts
@@ -18,6 +18,9 @@ suite('GetPackagesTool Tests', () => {
         mockApi = typeMoq.Mock.ofType<PythonProjectEnvironmentApi & PythonPackageGetterApi>();
         mockEnvironment = typeMoq.Mock.ofType<PythonEnvironment>();
 
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
+
         // refresh will always return a resolved promise
         mockApi.setup((x) => x.refreshPackages(typeMoq.It.isAny())).returns(() => Promise.resolve());
 
@@ -30,9 +33,6 @@ suite('GetPackagesTool Tests', () => {
     });
 
     test('should throw error if filePath is undefined', async () => {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         const testFile: IGetActiveFile = {
             filePath: '',
         };
@@ -61,9 +61,6 @@ suite('GetPackagesTool Tests', () => {
     });
 
     test('should throw error for notebook cells', async () => {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         const testFile: IGetActiveFile = {
             filePath: 'test.ipynb#123',
         };
@@ -84,9 +81,6 @@ suite('GetPackagesTool Tests', () => {
             filePath: 'test.py',
         };
 
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         mockApi
             .setup((x) => x.getEnvironment(typeMoq.It.isAny()))
             .returns(() => {
@@ -109,9 +103,6 @@ suite('GetPackagesTool Tests', () => {
             filePath: 'test.py',
         };
 
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         mockApi
             .setup((x) => x.getEnvironment(typeMoq.It.isAny()))
             .returns(() => {
@@ -152,9 +143,6 @@ suite('GetPackagesTool Tests', () => {
             filePath: 'test.py',
         };
 
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         mockApi
             .setup((x) => x.getEnvironment(typeMoq.It.isAny()))
             .returns(() => {
@@ -197,9 +185,6 @@ suite('GetPackagesTool Tests', () => {
             filePath: 'test.py',
         };
 
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        mockEnvironment.setup((x: any) => x.then).returns(() => undefined);
-
         mockApi
             .setup((x) => x.getEnvironment(typeMoq.It.isAny()))
             .returns(async () => {
diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts
@@ -0,0 +1,177 @@
+import * as assert from 'assert';
+import * as typeMoq from 'typemoq';
+import * as sinon from 'sinon';
+import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api';
+import * as projectApi from '../../common/pickers/projects';
+import * as managerApi from '../../common/pickers/managers';
+import { PythonEnvironment, PythonProject } from '../../api';
+import { createAnyEnvironmentCommand } from '../../features/envCommands';
+import { Uri } from 'vscode';
+
+suite('Create Any Environment Command Tests', () => {
+    let em: typeMoq.IMock<EnvironmentManagers>;
+    let pm: typeMoq.IMock<PythonProjectManager>;
+    let manager: typeMoq.IMock<InternalEnvironmentManager>;
+    let env: typeMoq.IMock<PythonEnvironment>;
+    let pickProjectManyStub: sinon.SinonStub;
+    let pickEnvironmentManagerStub: sinon.SinonStub;
+    let project: PythonProject = {
+        uri: Uri.file('/some/test/workspace/folder'),
+        name: 'test-folder',
+    };
+    let project2: PythonProject = {
+        uri: Uri.file('/some/test/workspace/folder2'),
+        name: 'test-folder2',
+    };
+    let project3: PythonProject = {
+        uri: Uri.file('/some/test/workspace/folder3'),
+        name: 'test-folder3',
+    };
+
+    setup(() => {
+        manager = typeMoq.Mock.ofType<InternalEnvironmentManager>();
+        manager.setup((m) => m.id).returns(() => 'test');
+        manager.setup((m) => m.displayName).returns(() => 'Test Manager');
+        manager.setup((m) => m.description).returns(() => 'Test Manager Description');
+        manager.setup((m) => m.supportsCreate).returns(() => true);
+
+        env = typeMoq.Mock.ofType<PythonEnvironment>();
+        env.setup((e) => e.envId).returns(() => ({ id: 'env1', managerId: 'test' }));
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        env.setup((e: any) => e.then).returns(() => undefined);
+
+        em = typeMoq.Mock.ofType<EnvironmentManagers>();
+        em.setup((e) => e.managers).returns(() => [manager.object]);
+        em.setup((e) => e.getEnvironmentManager(typeMoq.It.isAnyString())).returns(() => manager.object);
+
+        pm = typeMoq.Mock.ofType<PythonProjectManager>();
+
+        pickEnvironmentManagerStub = sinon.stub(managerApi, 'pickEnvironmentManager');
+        pickProjectManyStub = sinon.stub(projectApi, 'pickProjectMany');
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('Create global venv (no-workspace): no-select', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => []);
+        manager
+            .setup((m) => m.create('global'))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: false });
+        // Add assertions to verify the result
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+    });
+
+    test('Create global venv (no-workspace): select', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => []);
+        manager
+            .setup((m) => m.create('global'))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        manager.setup((m) => m.set(undefined, env.object)).verifiable(typeMoq.Times.once());
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });
+        // Add assertions to verify the result
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+    });
+
+    test('Create workspace venv: no-select', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => [project]);
+        manager
+            .setup((m) => m.create([project.uri]))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+        em.setup((e) => e.setEnvironments(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([project]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: false });
+
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+        em.verifyAll();
+    });
+
+    test('Create workspace venv: select', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => [project]);
+        manager
+            .setup((m) => m.create([project.uri]))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        // This is a case where env managers handler does this in batch to avoid writing to files for each case
+        manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+        em.setup((e) => e.setEnvironments([project.uri], env.object)).verifiable(typeMoq.Times.once());
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([project]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });
+
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+        em.verifyAll();
+    });
+
+    test('Create multi-workspace venv: select all', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => [project, project2, project3]);
+        manager
+            .setup((m) => m.create([project.uri, project2.uri, project3.uri]))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        // This is a case where env managers handler does this in batch to avoid writing to files for each case
+        manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+        em.setup((e) => e.setEnvironments([project.uri, project2.uri, project3.uri], env.object)).verifiable(
+            typeMoq.Times.once(),
+        );
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([project, project2, project3]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });
+
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+        em.verifyAll();
+    });
+
+    test('Create multi-workspace venv: select some', async () => {
+        pm.setup((p) => p.getProjects()).returns(() => [project, project2, project3]);
+        manager
+            .setup((m) => m.create([project.uri, project3.uri]))
+            .returns(() => Promise.resolve(env.object))
+            .verifiable(typeMoq.Times.once());
+
+        // This is a case where env managers handler does this in batch to avoid writing to files for each case
+        manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
+        em.setup((e) => e.setEnvironments([project.uri, project3.uri], env.object)).verifiable(typeMoq.Times.once());
+
+        pickEnvironmentManagerStub.resolves(manager.object.id);
+        pickProjectManyStub.resolves([project, project3]);
+
+        const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });
+
+        assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
+        manager.verifyAll();
+        em.verifyAll();
+    });
+});

Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,8 @@ export async function pickProjectMany(`
`49`	`49`	`}`
`50`	`50`	`} else if (projects.length === 1) {`
`51`	`51`	`return [...projects];`
	`52`	`+ } else if (projects.length === 0) {`
	`53`	`+ return [];`
`52`	`54`	`}`
`53`	`55`	`return undefined;`
`54`	`56`	`}`