Fix createTerminal command to prompt for project selection in multiroot workspaces

src/features/envCommands.ts

@@ -514,21 +514,17 @@ export async function createTerminalCommand(
     api: PythonEnvironmentApi,
     tm: TerminalManager,
 ): Promise<Terminal | undefined> {
-    if (context === undefined) {
-        const pw = await pickProject(api.getPythonProjects());
-        if (pw) {
-            const env = await api.getEnvironment(pw.uri);
-            const cwd = await findParentIfFile(pw.uri.fsPath);
-            if (env) {
-                return await tm.create(env, { cwd });
-            }
+    if (context === undefined || context instanceof Uri) {
+        // For undefined context or Uri context, check for multiroot and prompt if needed
+        const projects = api.getPythonProjects();
+        const pw = await pickProject(projects);
+        if (!pw) {
+            // User cancelled project selection
+            return undefined;
         }
-    } else if (context instanceof Uri) {
-        const uri = context as Uri;
-        const env = await api.getEnvironment(uri);
-        const pw = api.getPythonProject(uri);
-        if (env && pw) {
-            const cwd = await findParentIfFile(pw.uri.fsPath);
+        const env = await api.getEnvironment(pw.uri);
+        const cwd = await findParentIfFile(pw.uri.fsPath);
+        if (env) {
             return await tm.create(env, { cwd });
         }
     } else if (context instanceof ProjectItem) {

src/test/features/envCommands.unit.test.ts

@@ -1,12 +1,14 @@
 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 typeMoq from 'typemoq';
+import { Terminal, Uri } from 'vscode';
+import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api';
 import * as managerApi from '../../common/pickers/managers';
-import { PythonEnvironment, PythonProject } from '../../api';
-import { createAnyEnvironmentCommand } from '../../features/envCommands';
-import { Uri } from 'vscode';
+import * as projectApi from '../../common/pickers/projects';
+import { createAnyEnvironmentCommand, createTerminalCommand } from '../../features/envCommands';
+import { TerminalManager } from '../../features/terminal/terminalManager';
+import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api';
+import { setupNonThenable } from '../mocks/helper';
 
 suite('Create Any Environment Command Tests', () => {
     let em: typeMoq.IMock<EnvironmentManagers>;
@@ -37,8 +39,7 @@ suite('Create Any Environment Command Tests', () => {
 
         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);
+        setupNonThenable(env);
 
         em = typeMoq.Mock.ofType<EnvironmentManagers>();
         em.setup((e) => e.managers).returns(() => [manager.object]);
@@ -175,3 +176,122 @@ suite('Create Any Environment Command Tests', () => {
         em.verifyAll();
     });
 });
+
+suite('Create Terminal Command Tests', () => {
+    let api: typeMoq.IMock<PythonEnvironmentApi>;
+    let tm: typeMoq.IMock<TerminalManager>;
+    let env: typeMoq.IMock<PythonEnvironment>;
+    let terminal: typeMoq.IMock<Terminal>;
+    let pickProjectStub: sinon.SinonStub;
+    let project1: PythonProject = {
+        uri: Uri.file('/tmp'),
+        name: 'folder1',
+    };
+    let project2: PythonProject = {
+        uri: Uri.file('/home'),
+        name: 'folder2',
+    };
+
+    setup(() => {
+        env = typeMoq.Mock.ofType<PythonEnvironment>();
+        env.setup((e) => e.envId).returns(() => ({ id: 'env1', managerId: 'test' }));
+        setupNonThenable(env);
+
+        terminal = typeMoq.Mock.ofType<Terminal>();
+        setupNonThenable(terminal);
+
+        api = typeMoq.Mock.ofType<PythonEnvironmentApi>();
+        tm = typeMoq.Mock.ofType<TerminalManager>();
+
+        pickProjectStub = sinon.stub(projectApi, 'pickProject');
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('Single project: should create terminal without prompting', async () => {
+        // Setup: single project
+        api.setup((a) => a.getPythonProjects()).returns(() => [project1]);
+        api.setup((a) => a.getEnvironment(project1.uri)).returns(() => Promise.resolve(env.object));
+        tm.setup((t) => t.create(env.object, typeMoq.It.isAny())).returns(() => Promise.resolve(terminal.object));
+
+        // pickProject should return the single project without prompting
+        pickProjectStub.resolves(project1);
+
+        const result = await createTerminalCommand(undefined, api.object, tm.object);
+
+        assert.strictEqual(result, terminal.object, 'Expected terminal to be created');
+        assert.strictEqual(pickProjectStub.callCount, 1, 'pickProject should be called once');
+    });
+
+    test('Multiple projects: should prompt user to select project', async () => {
+        // Setup: multiple projects
+        api.setup((a) => a.getPythonProjects()).returns(() => [project1, project2]);
+        api.setup((a) => a.getEnvironment(project2.uri)).returns(() => Promise.resolve(env.object));
+        tm.setup((t) => t.create(env.object, typeMoq.It.isAny())).returns(() => Promise.resolve(terminal.object));
+
+        // User selects project2
+        pickProjectStub.resolves(project2);
+
+        const result = await createTerminalCommand(undefined, api.object, tm.object);
+
+        assert.strictEqual(result, terminal.object, 'Expected terminal to be created');
+        assert.strictEqual(pickProjectStub.callCount, 1, 'pickProject should be called once');
+        // Verify pickProject was called with both projects
+        assert.deepStrictEqual(
+            pickProjectStub.firstCall.args[0],
+            [project1, project2],
+            'pickProject should be called with all projects',
+        );
+    });
+
+    test('Uri context with single project: should create terminal without prompting', async () => {
+        // Setup: single project
+        api.setup((a) => a.getPythonProjects()).returns(() => [project1]);
+        api.setup((a) => a.getEnvironment(project1.uri)).returns(() => Promise.resolve(env.object));
+        tm.setup((t) => t.create(env.object, typeMoq.It.isAny())).returns(() => Promise.resolve(terminal.object));
+
+        // pickProject should return the single project without prompting
+        pickProjectStub.resolves(project1);
+
+        const result = await createTerminalCommand(project1.uri, api.object, tm.object);
+
+        assert.strictEqual(result, terminal.object, 'Expected terminal to be created');
+        assert.strictEqual(pickProjectStub.callCount, 1, 'pickProject should be called once');
+    });
+
+    test('Uri context with multiple projects: should prompt user to select project', async () => {
+        // Setup: multiple projects, context is project1.uri but user should still be prompted
+        api.setup((a) => a.getPythonProjects()).returns(() => [project1, project2]);
+        api.setup((a) => a.getEnvironment(project2.uri)).returns(() => Promise.resolve(env.object));
+        tm.setup((t) => t.create(env.object, typeMoq.It.isAny())).returns(() => Promise.resolve(terminal.object));
+
+        // User selects project2 (different from context)
+        pickProjectStub.resolves(project2);
+
+        const result = await createTerminalCommand(project1.uri, api.object, tm.object);
+
+        assert.strictEqual(result, terminal.object, 'Expected terminal to be created');
+        assert.strictEqual(pickProjectStub.callCount, 1, 'pickProject should be called once');
+        // Verify pickProject was called with all projects, not just the context
+        assert.deepStrictEqual(
+            pickProjectStub.firstCall.args[0],
+            [project1, project2],
+            'pickProject should be called with all projects',
+        );
+    });
+
+    test('User cancels project selection: should not create terminal', async () => {
+        // Setup: multiple projects
+        api.setup((a) => a.getPythonProjects()).returns(() => [project1, project2]);
+
+        // User cancels selection
+        pickProjectStub.resolves(undefined);
+
+        const result = await createTerminalCommand(undefined, api.object, tm.object);
+
+        assert.strictEqual(result, undefined, 'Expected no terminal to be created when user cancels');
+        assert.strictEqual(pickProjectStub.callCount, 1, 'pickProject should be called once');
+    });
+});

src/test/mocks/helper.ts

@@ -1,10 +1,11 @@
-
 /* eslint-disable @typescript-eslint/no-explicit-any */
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
-import * as TypeMoq from 'typemoq';
+import * as sinon from 'sinon';
 import { Readable } from 'stream';
+import * as TypeMoq from 'typemoq';
 import * as common from 'typemoq/Common/_all';
+import { LogOutputChannel } from 'vscode';
 
 export class FakeReadableStream extends Readable {
     _read(_size: unknown): void | null {
@@ -13,6 +14,50 @@ export class FakeReadableStream extends Readable {
     }
 }
 
+/**
+ * Creates a mock LogOutputChannel for testing.
+ * @returns A mock LogOutputChannel with stubbed methods
+ */
+export function createMockLogOutputChannel(): LogOutputChannel {
+    return {
+        info: sinon.stub(),
+        error: sinon.stub(),
+        warn: sinon.stub(),
+        append: sinon.stub(),
+        debug: sinon.stub(),
+        trace: sinon.stub(),
+        show: sinon.stub(),
+        hide: sinon.stub(),
+        dispose: sinon.stub(),
+        clear: sinon.stub(),
+        replace: sinon.stub(),
+        appendLine: sinon.stub(),
+        name: 'test-log',
+        logLevel: 1,
+        onDidChangeLogLevel: sinon.stub() as LogOutputChannel['onDidChangeLogLevel'],
+    } as unknown as LogOutputChannel;
+}
+
+/**
+ * Type helper for accessing the `.then` property on mocks.
+ * Used to prevent TypeMoq from treating mocks as thenables (Promise-like objects).
+ * See: https://github.com/florinn/typemoq/issues/67
+ */
+export type Thenable = { then?: unknown };
+
+/**
+ * Sets up a mock to not be treated as a thenable (Promise-like object).
+ * This is necessary due to a TypeMoq limitation where mocks can be confused with Promises.
+ *
+ * @param mock - The TypeMoq mock to configure
+ * @example
+ * const mock = TypeMoq.Mock.ofType<MyInterface>();
+ * setupNonThenable(mock);
+ */
+export function setupNonThenable<T>(mock: TypeMoq.IMock<T>): void {
+    mock.setup((x) => (x as unknown as Thenable).then).returns(() => undefined);
+}
+
 export function createTypeMoq<T>(
     targetCtor?: common.CtorWithArgs<T>,
     behavior?: TypeMoq.MockBehavior,
@@ -22,6 +67,6 @@ export function createTypeMoq<T>(
     // Use typemoqs for those things that are resolved as promises. mockito doesn't allow nesting of mocks. ES6 Proxy class
     // is the problem. We still need to make it thenable though. See this issue: https://github.com/florinn/typemoq/issues/67
     const result = TypeMoq.Mock.ofType<T>(targetCtor, behavior, shouldOverrideTarget, ...targetCtorArgs);
-    result.setup((x: any) => x.then).returns(() => undefined);
+    setupNonThenable(result);
     return result;
 }