diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index fe02e022..a9f2f8c5 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -514,21 +514,17 @@ export async function createTerminalCommand( api: PythonEnvironmentApi, tm: TerminalManager, ): Promise { - 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) { diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index 311e67d3..07df27b2 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/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; @@ -37,8 +39,7 @@ suite('Create Any Environment Command Tests', () => { env = typeMoq.Mock.ofType(); 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(); 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; + let tm: typeMoq.IMock; + let env: typeMoq.IMock; + let terminal: typeMoq.IMock; + 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(); + env.setup((e) => e.envId).returns(() => ({ id: 'env1', managerId: 'test' })); + setupNonThenable(env); + + terminal = typeMoq.Mock.ofType(); + setupNonThenable(terminal); + + api = typeMoq.Mock.ofType(); + tm = typeMoq.Mock.ofType(); + + 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'); + }); +}); diff --git a/src/test/mocks/helper.ts b/src/test/mocks/helper.ts index b612aaab..d63be170 100644 --- a/src/test/mocks/helper.ts +++ b/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(); + * setupNonThenable(mock); + */ +export function setupNonThenable(mock: TypeMoq.IMock): void { + mock.setup((x) => (x as unknown as Thenable).then).returns(() => undefined); +} + export function createTypeMoq( targetCtor?: common.CtorWithArgs, behavior?: TypeMoq.MockBehavior, @@ -22,6 +67,6 @@ export function createTypeMoq( // 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(targetCtor, behavior, shouldOverrideTarget, ...targetCtorArgs); - result.setup((x: any) => x.then).returns(() => undefined); + setupNonThenable(result); return result; }