fix bug: getDedicatedTerminal errors on string value for supported terminalKey (#1231)

fixes
#1230

Author: eleanorjboyd

src/features/terminal/terminalManager.ts

@@ -59,12 +59,7 @@ export interface TerminalInit {
 }
 
 export interface TerminalManager
-    extends TerminalEnvironment,
-        TerminalInit,
-        TerminalActivation,
-        TerminalCreation,
-        TerminalGetters,
-        Disposable {}
+    extends TerminalEnvironment, TerminalInit, TerminalActivation, TerminalCreation, TerminalGetters, Disposable {}
 
 export class TerminalManagerImpl implements TerminalManager {
     private disposables: Disposable[] = [];
@@ -321,7 +316,7 @@ export class TerminalManagerImpl implements TerminalManager {
 
     private dedicatedTerminals = new Map<string, Terminal>();
     async getDedicatedTerminal(
-        terminalKey: Uri,
+        terminalKey: Uri | string,
         project: Uri | PythonProject,
         environment: PythonEnvironment,
         createNew: boolean = false,
@@ -336,17 +331,26 @@ export class TerminalManagerImpl implements TerminalManager {
         }
 
         const puri = project instanceof Uri ? project : project.uri;
-        const config = getConfiguration('python', terminalKey);
+        const config = getConfiguration('python', terminalKey instanceof Uri ? terminalKey : puri);
         const projectStat = await fsapi.stat(puri.fsPath);
         const projectDir = projectStat.isDirectory() ? puri.fsPath : path.dirname(puri.fsPath);
 
-        const uriStat = await fsapi.stat(terminalKey.fsPath);
-        const uriDir = uriStat.isDirectory() ? terminalKey.fsPath : path.dirname(terminalKey.fsPath);
-        const cwd = config.get<boolean>('terminal.executeInFileDir', false) ? uriDir : projectDir;
+        let cwd: string;
+        let name: string;
+
+        if (terminalKey instanceof Uri) {
+            const uriStat = await fsapi.stat(terminalKey.fsPath);
+            const uriDir = uriStat.isDirectory() ? terminalKey.fsPath : path.dirname(terminalKey.fsPath);
+            cwd = config.get<boolean>('terminal.executeInFileDir', false) ? uriDir : projectDir;
+            // Follow Python extension's naming: 'Python: {filename}' for dedicated terminals
+            const fileName = path.basename(terminalKey.fsPath).replace('.py', '');
+            name = `Python: ${fileName}`;
+        } else {
+            // When terminalKey is a string, use project directory and the string as name
+            cwd = projectDir;
+            name = `Python: ${terminalKey}`;
+        }
 
-        // Follow Python extension's naming: 'Python: {filename}' for dedicated terminals
-        const fileName = path.basename(terminalKey.fsPath).replace('.py', '');
-        const name = `Python: ${fileName}`;
         const newTerminal = await this.create(environment, { cwd, name });
         this.dedicatedTerminals.set(key, newTerminal);
 

src/test/features/terminal/terminalManager.unit.test.ts

@@ -6,16 +6,22 @@ import * as fsapi from 'fs-extra';
 import * as os from 'os';
 import * as path from 'path';
 import * as sinon from 'sinon';
-import { Disposable, Event, EventEmitter, Progress, Terminal, TerminalOptions, Uri, WorkspaceConfiguration } from 'vscode';
+import {
+    Disposable,
+    Event,
+    EventEmitter,
+    Progress,
+    Terminal,
+    TerminalOptions,
+    Uri,
+    WorkspaceConfiguration,
+} from 'vscode';
 import { PythonEnvironment } from '../../../api';
 import * as windowApis from '../../../common/window.apis';
 import * as workspaceApis from '../../../common/workspace.apis';
 import * as activationUtils from '../../../features/common/activation';
 import * as shellDetector from '../../../features/common/shellDetector';
-import {
-    ShellEnvsProvider,
-    ShellStartupScriptProvider,
-} from '../../../features/terminal/shells/startupProvider';
+import { ShellEnvsProvider, ShellStartupScriptProvider } from '../../../features/terminal/shells/startupProvider';
 import {
     DidChangeTerminalActivationStateEvent,
     TerminalActivationInternal,
@@ -309,13 +315,109 @@ suite('TerminalManager - terminal naming', () => {
         try {
             await terminalManager.getProjectTerminal(projectUri, env);
 
+            assert.strictEqual(optionsList[0]?.name, 'Python', 'Project terminal should use the Python title');
+        } finally {
+            await fsapi.remove(tempRoot);
+        }
+    });
+
+    // Regression test for https://github.com/microsoft/vscode-python-environments/issues/1230
+    test('getDedicatedTerminal with string key uses string as terminal name', async () => {
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_OFF);
+        terminalManager = createTerminalManager();
+        const env = createMockEnvironment();
+
+        const optionsList: TerminalOptions[] = [];
+        createTerminalStub.callsFake((options) => {
+            optionsList.push(options);
+            return mockTerminal as Terminal;
+        });
+
+        const tempRoot = await fsapi.mkdtemp(path.join(os.tmpdir(), 'py-envs-'));
+        const projectPath = path.join(tempRoot, 'project');
+        await fsapi.ensureDir(projectPath);
+        const projectUri = Uri.file(projectPath);
+
+        const config = { get: sinon.stub().returns(false) } as unknown as WorkspaceConfiguration;
+        sinon.stub(workspaceApis, 'getConfiguration').returns(config);
+
+        try {
+            await terminalManager.getDedicatedTerminal('my-terminal-key', projectUri, env);
+
             assert.strictEqual(
                 optionsList[0]?.name,
-                'Python',
-                'Project terminal should use the Python title',
+                'Python: my-terminal-key',
+                'Dedicated terminal with string key should use the string in the title',
+            );
+            assert.strictEqual(
+                Uri.file(optionsList[0]?.cwd as string).fsPath,
+                Uri.file(projectPath).fsPath,
+                'Dedicated terminal with string key should use project directory as cwd',
             );
         } finally {
             await fsapi.remove(tempRoot);
         }
     });
+
+    // Regression test for https://github.com/microsoft/vscode-python-environments/issues/1230
+    test('getDedicatedTerminal with string key reuses terminal for same key', async () => {
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_OFF);
+        terminalManager = createTerminalManager();
+        const env = createMockEnvironment();
+
+        let createCount = 0;
+        createTerminalStub.callsFake((options) => {
+            createCount++;
+            return { ...mockTerminal, name: options.name } as Terminal;
+        });
+
+        const tempRoot = await fsapi.mkdtemp(path.join(os.tmpdir(), 'py-envs-'));
+        const projectPath = path.join(tempRoot, 'project');
+        await fsapi.ensureDir(projectPath);
+        const projectUri = Uri.file(projectPath);
+
+        const config = { get: sinon.stub().returns(false) } as unknown as WorkspaceConfiguration;
+        sinon.stub(workspaceApis, 'getConfiguration').returns(config);
+
+        try {
+            const terminal1 = await terminalManager.getDedicatedTerminal('my-key', projectUri, env);
+            const terminal2 = await terminalManager.getDedicatedTerminal('my-key', projectUri, env);
+
+            assert.strictEqual(terminal1, terminal2, 'Same string key should return the same terminal');
+            assert.strictEqual(createCount, 1, 'Terminal should be created only once');
+        } finally {
+            await fsapi.remove(tempRoot);
+        }
+    });
+
+    // Regression test for https://github.com/microsoft/vscode-python-environments/issues/1230
+    test('getDedicatedTerminal with string key uses different terminals for different keys', async () => {
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_OFF);
+        terminalManager = createTerminalManager();
+        const env = createMockEnvironment();
+
+        let createCount = 0;
+        createTerminalStub.callsFake((options) => {
+            createCount++;
+            return { ...mockTerminal, name: options.name, id: createCount } as unknown as Terminal;
+        });
+
+        const tempRoot = await fsapi.mkdtemp(path.join(os.tmpdir(), 'py-envs-'));
+        const projectPath = path.join(tempRoot, 'project');
+        await fsapi.ensureDir(projectPath);
+        const projectUri = Uri.file(projectPath);
+
+        const config = { get: sinon.stub().returns(false) } as unknown as WorkspaceConfiguration;
+        sinon.stub(workspaceApis, 'getConfiguration').returns(config);
+
+        try {
+            const terminal1 = await terminalManager.getDedicatedTerminal('key-1', projectUri, env);
+            const terminal2 = await terminalManager.getDedicatedTerminal('key-2', projectUri, env);
+
+            assert.notStrictEqual(terminal1, terminal2, 'Different string keys should return different terminals');
+            assert.strictEqual(createCount, 2, 'Two terminals should be created');
+        } finally {
+            await fsapi.remove(tempRoot);
+        }
+    });
 });