Skip to content

Commit 4d58d74

Browse files
committed
Ensure to show terminal on run Python file with command activated terminals
1 parent d0edb90 commit 4d58d74

2 files changed

Lines changed: 184 additions & 0 deletions

File tree

src/features/terminal/terminalManager.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,11 @@ export class TerminalManagerImpl implements TerminalManager {
306306
// We add it to skip activation on open to prevent double activation.
307307
// We can activate it ourselves since we are creating it.
308308
this.skipActivationOnOpen.add(newTerminal);
309+
310+
// Show terminal before activation so users can see the activation happening, requested script running.
311+
// Necessary for scenarios such as when terminal is awaiting user input, etc.
312+
newTerminal.show();
313+
309314
await this.autoActivateOnTerminalOpen(newTerminal, environment);
310315
}
311316

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'assert';
5+
import * as sinon from 'sinon';
6+
import { EventEmitter, Progress, Terminal, TerminalOptions } from 'vscode';
7+
import { PythonEnvironment } from '../../../api';
8+
import * as windowApis from '../../../common/window.apis';
9+
import * as workspaceApis from '../../../common/workspace.apis';
10+
import { TerminalActivationInternal } from '../../../features/terminal/terminalActivationState';
11+
import { TerminalManagerImpl } from '../../../features/terminal/terminalManager';
12+
import {
13+
ShellEnvsProvider,
14+
ShellStartupScriptProvider,
15+
} from '../../../features/terminal/shells/startupProvider';
16+
import * as terminalUtils from '../../../features/terminal/utils';
17+
import * as activationUtils from '../../../features/common/activation';
18+
19+
suite('TerminalManager - create()', () => {
20+
let terminalActivation: TerminalActivationInternal;
21+
let mockGetAutoActivationType: sinon.SinonStub;
22+
let terminalManager: TerminalManagerImpl;
23+
24+
// Tracking variables for show() and activate() call order
25+
let callOrder: string[];
26+
let mockTerminal: Partial<Terminal> & { show: sinon.SinonStub };
27+
28+
const createMockEnvironment = (): PythonEnvironment =>
29+
({
30+
envId: { id: 'test-env-id', managerId: 'test-manager' },
31+
environmentPath: { fsPath: '/path/to/python' },
32+
displayName: 'Test Environment',
33+
execInfo: {
34+
activation: { executable: 'source', args: ['/path/to/activate'] },
35+
},
36+
} as unknown as PythonEnvironment);
37+
38+
setup(() => {
39+
callOrder = [];
40+
41+
// Create mock terminal with tracking
42+
mockTerminal = {
43+
name: 'Test Terminal',
44+
creationOptions: {} as TerminalOptions,
45+
shellIntegration: undefined,
46+
show: sinon.stub().callsFake(() => {
47+
callOrder.push('show');
48+
}),
49+
sendText: sinon.stub(),
50+
};
51+
52+
// Mock terminal activation using unknown cast for simpler typing
53+
const onDidChangeEmitter = new EventEmitter<unknown>();
54+
terminalActivation = {
55+
isActivated: sinon.stub().returns(false),
56+
activate: sinon.stub().callsFake(async () => {
57+
callOrder.push('activate');
58+
}),
59+
deactivate: sinon.stub().resolves(),
60+
getEnvironment: sinon.stub().returns(undefined),
61+
updateActivationState: sinon.stub(),
62+
onDidChangeTerminalActivationState: onDidChangeEmitter.event,
63+
dispose: sinon.stub(),
64+
} as unknown as TerminalActivationInternal;
65+
66+
// Stub terminalUtils
67+
mockGetAutoActivationType = sinon.stub(terminalUtils, 'getAutoActivationType');
68+
sinon.stub(terminalUtils, 'waitForShellIntegration').resolves(false);
69+
70+
// Stub isActivatableEnvironment to return true
71+
sinon.stub(activationUtils, 'isActivatableEnvironment').returns(true);
72+
73+
// Stub window APIs
74+
sinon.stub(windowApis, 'createTerminal').returns(mockTerminal as unknown as Terminal);
75+
sinon.stub(windowApis, 'onDidOpenTerminal').returns({ dispose: sinon.stub() });
76+
sinon.stub(windowApis, 'onDidCloseTerminal').returns({ dispose: sinon.stub() });
77+
sinon.stub(windowApis, 'onDidChangeWindowState').returns({ dispose: sinon.stub() });
78+
sinon.stub(windowApis, 'terminals').returns([]);
79+
80+
// Stub withProgress to execute the callback directly
81+
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
82+
const mockProgress: Progress<{ message?: string; increment?: number }> = { report: () => {} };
83+
const mockCancellationToken = {
84+
isCancellationRequested: false,
85+
onCancellationRequested: () => ({ dispose: () => {} }),
86+
};
87+
return task(mockProgress, mockCancellationToken as never);
88+
});
89+
90+
// Stub workspace APIs
91+
sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns({ dispose: sinon.stub() });
92+
});
93+
94+
teardown(() => {
95+
sinon.restore();
96+
});
97+
98+
function createTerminalManager(): TerminalManagerImpl {
99+
const emptyEnvProviders: ShellEnvsProvider[] = [];
100+
const emptyScriptProviders: ShellStartupScriptProvider[] = [];
101+
return new TerminalManagerImpl(
102+
terminalActivation as TerminalActivationInternal,
103+
emptyEnvProviders,
104+
emptyScriptProviders,
105+
);
106+
}
107+
108+
// Regression test for https://github.com/microsoft/vscode-python-environments/issues/640
109+
// With ACT_TYPE_COMMAND, create() awaits activation which blocks returning the terminal.
110+
// Without showing the terminal early, users wouldn't see it until activation completes (2-5 seconds).
111+
test('ACT_TYPE_COMMAND: shows terminal before awaiting activation to prevent hidden terminal during activation', async () => {
112+
// Mock
113+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
114+
terminalManager = createTerminalManager();
115+
116+
// Run
117+
const env = createMockEnvironment();
118+
await terminalManager.create(env, { cwd: '/workspace' });
119+
120+
// Assert - show() must be called before activate() so terminal is visible during activation
121+
assert.ok(callOrder.includes('show'), 'Terminal show() should be called');
122+
assert.ok(callOrder.includes('activate'), 'Terminal activate() should be called');
123+
const showIndex = callOrder.indexOf('show');
124+
const activateIndex = callOrder.indexOf('activate');
125+
assert.ok(
126+
showIndex < activateIndex,
127+
`show() at index ${showIndex} must precede activate() at index ${activateIndex}`,
128+
);
129+
});
130+
131+
// With ACT_TYPE_SHELL/OFF, create() returns immediately without blocking.
132+
// The caller (runInTerminal) handles showing the terminal, so create() shouldn't call show().
133+
test('ACT_TYPE_SHELL: does not call show() since create() returns immediately and caller handles visibility', async () => {
134+
// Mock
135+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_SHELL);
136+
terminalManager = createTerminalManager();
137+
138+
// Run
139+
const env = createMockEnvironment();
140+
await terminalManager.create(env, { cwd: '/workspace' });
141+
142+
// Assert - no blocking activation means caller (runInTerminal) will show terminal
143+
assert.strictEqual(callOrder.includes('show'), false, 'show() deferred to caller');
144+
assert.strictEqual(callOrder.includes('activate'), false, 'No command activation for shell startup mode');
145+
});
146+
147+
test('ACT_TYPE_OFF: does not call show() since create() returns immediately and caller handles visibility', async () => {
148+
// Mock
149+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_OFF);
150+
terminalManager = createTerminalManager();
151+
152+
// Run
153+
const env = createMockEnvironment();
154+
await terminalManager.create(env, { cwd: '/workspace' });
155+
156+
// Assert - no activation means caller (runInTerminal) will show terminal
157+
assert.strictEqual(callOrder.includes('show'), false, 'show() deferred to caller');
158+
assert.strictEqual(callOrder.includes('activate'), false, 'Activation disabled');
159+
});
160+
161+
test('disableActivation option: skips both show() and activation, returns terminal immediately', async () => {
162+
// Mock
163+
mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
164+
terminalManager = createTerminalManager();
165+
166+
// Run
167+
const env = createMockEnvironment();
168+
const terminal = await terminalManager.create(env, { cwd: '/workspace', disableActivation: true });
169+
170+
// Assert - terminal returned without any activation logic
171+
assert.ok(terminal, 'Terminal should be returned');
172+
assert.strictEqual(callOrder.includes('show'), false, 'No show() when activation skipped');
173+
assert.strictEqual(
174+
(terminalActivation.activate as sinon.SinonStub).called,
175+
false,
176+
'No activate() when disableActivation is true',
177+
);
178+
});
179+
});

0 commit comments

Comments
 (0)