Skip to content

Commit 822e884

Browse files
Add unit tests for venv/conda create & remove orchestration (#1638)
Closes #1624 ## Summary The `create`/`remove` orchestration in `VenvManager` (`src/managers/builtin/venvManager.ts`) and `CondaEnvManager` (`src/managers/conda/condaEnvManager.ts`) previously had **no unit-level coverage** of the create/remove lifecycle. While helpers such as `venvUtils.removeVenv` and several `condaEnvManager` sub-behaviors (`setEvents`, `setGlobal`, `findEnvironmentByPath`) were unit-tested, the manager methods that tie discovery, creation, caching, and change-event firing together were only exercised indirectly via integration tests. This PR adds focused unit tests for that lifecycle, plus the minimal supporting changes needed to make those paths testable. ## What changed ### New tests - **`src/test/managers/builtin/venvManager.createRemove.unit.test.ts`** — covers `VenvManager.create`/`remove`: - non-quick create delegates to the create helper, caches the environment, writes `.gitignore`, reveals the folder, and fires an add event - quick create uses the selected global Python and forwards `additionalPackages` - creation errors are reported without adding an environment - the `skipWatcherRefresh` guard is restored even when creation/removal throws - successful removal updates the collection and fires a remove event; a `false` result from the removal helper mutates nothing - **`src/test/managers/conda/condaEnvManager.createRemove.unit.test.ts`** — covers `CondaEnvManager.create`/`remove`: - non-quick create delegates, caches the env, and fires an add event - global quick create resolves a prefix and forwards additional packages; project quick create uses the project root and writes `.gitignore` - no state mutation when creation returns no environment or throws - successful removal updates caches and fires collection + per-project events - a rejected deletion is logged without firing a success event ### Supporting production/test-helper changes - **`src/managers/builtin/venvManager.ts`** (4 lines): route the post-create `revealInExplorer` call through the existing `executeCommand` wrapper in `src/common/command.api.ts` instead of importing `commands` from `vscode` directly. This is a small testability refactor (the wrapper can be stubbed in unit tests) with no behavior change. - **`src/test/mocks/pythonEnvironment.ts`**: add an optional `sysPrefix` override to the shared mock helper (defaults to `envPath`, so existing callers are unaffected). ## Testing - `npm run compile-tests` — clean. - `npm run unittest` (venv/conda manager suites) — all new and existing manager tests pass. The one remaining failure in the suite (`venvManager.loadEnvMap.unit.test.ts` — "projectB should have its env mapped despite projectA failing") pre-exists on `main` and is unrelated to this change. ## Notes - This change is intentionally scoped to test coverage plus the minimal refactor required to enable it; no environment-manager behavior is changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 15d206e commit 822e884

4 files changed

Lines changed: 502 additions & 4 deletions

File tree

src/managers/builtin/venvManager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as fs from 'fs/promises';
22
import * as path from 'path';
33
import {
4-
commands,
54
EventEmitter,
65
l10n,
76
LogOutputChannel,
@@ -28,6 +27,7 @@ import {
2827
ResolveEnvironmentContext,
2928
SetEnvironmentScope,
3029
} from '../../api';
30+
import { executeCommand } from '../../common/command.api';
3131
import { PYTHON_EXTENSION_ID } from '../../common/constants';
3232
import { VenvManagerStrings } from '../../common/localize';
3333
import { traceError, traceWarn } from '../../common/logging';
@@ -238,7 +238,7 @@ export class VenvManager implements EnvironmentManager {
238238
// Open the parent folder of the venv in the current window immediately after creation
239239
const envParent = environment.sysPrefix;
240240
try {
241-
await commands.executeCommand('revealInExplorer', Uri.file(envParent));
241+
await executeCommand('revealInExplorer', Uri.file(envParent));
242242
} catch (error) {
243243
showErrorMessage(
244244
l10n.t(
Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
/* eslint-disable @typescript-eslint/no-explicit-any */
2+
import assert from 'assert';
3+
import * as fse from 'fs-extra';
4+
import * as os from 'os';
5+
import * as path from 'path';
6+
import * as sinon from 'sinon';
7+
import { Uri } from 'vscode';
8+
import {
9+
DidChangeEnvironmentEventArgs,
10+
DidChangeEnvironmentsEventArgs,
11+
EnvironmentChangeKind,
12+
EnvironmentManager,
13+
PythonEnvironment,
14+
PythonEnvironmentApi,
15+
} from '../../../api';
16+
import * as commandApis from '../../../common/command.api';
17+
import { VENV_MANAGER_ID } from '../../../common/constants';
18+
import { normalizePath } from '../../../common/utils/pathUtils';
19+
import * as windowApis from '../../../common/window.apis';
20+
import * as envCommands from '../../../features/envCommands';
21+
import { VenvManager } from '../../../managers/builtin/venvManager';
22+
import * as venvUtils from '../../../managers/builtin/venvUtils';
23+
import { NativePythonFinder } from '../../../managers/common/nativePythonFinder';
24+
import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment';
25+
26+
const TEST_ROOT = Uri.file(path.join(os.tmpdir(), 'vscode-python-envs-tests', 'venv-manager')).fsPath;
27+
28+
function testPath(...segments: string[]): string {
29+
return path.join(TEST_ROOT, ...segments);
30+
}
31+
32+
function venvPythonPath(venvRoot: string): string {
33+
return path.join(venvRoot, process.platform === 'win32' ? 'Scripts' : 'bin', 'python');
34+
}
35+
36+
function createManager(
37+
apiOverrides?: Partial<PythonEnvironmentApi>,
38+
baseEnvironments: PythonEnvironment[] = [],
39+
): VenvManager {
40+
const api = {
41+
getEnvironments: sinon.stub().resolves([]),
42+
getPythonProject: sinon.stub().returns(undefined),
43+
getPythonProjects: sinon.stub().returns([]),
44+
refreshEnvironments: sinon.stub().resolves(undefined),
45+
...apiOverrides,
46+
} as any as PythonEnvironmentApi;
47+
const baseManager = {
48+
getEnvironments: sinon.stub().resolves(baseEnvironments),
49+
} as any as EnvironmentManager;
50+
const manager = new VenvManager(
51+
{} as NativePythonFinder,
52+
api,
53+
baseManager,
54+
{ info: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } as any,
55+
);
56+
(manager as any)._initialized = { completed: true, promise: Promise.resolve() };
57+
(manager as any).collection = [];
58+
return manager;
59+
}
60+
61+
suite('VenvManager.create - orchestration', () => {
62+
let createPythonVenvStub: sinon.SinonStub;
63+
let executeCommandStub: sinon.SinonStub;
64+
let quickCreateVenvStub: sinon.SinonStub;
65+
let showErrorStub: sinon.SinonStub;
66+
let envDir: string;
67+
let pythonPath: string;
68+
let tmpRoot: string;
69+
70+
setup(async () => {
71+
createPythonVenvStub = sinon.stub(venvUtils, 'createPythonVenv');
72+
quickCreateVenvStub = sinon.stub(venvUtils, 'quickCreateVenv');
73+
sinon.stub(envCommands, 'findParentIfFile').callsFake(async (value: string) => value);
74+
showErrorStub = sinon.stub(windowApis, 'showErrorMessage');
75+
executeCommandStub = sinon.stub(commandApis, 'executeCommand').resolves();
76+
77+
tmpRoot = await fse.mkdtemp(path.join(os.tmpdir(), 'venvmgr-'));
78+
envDir = Uri.file(path.join(tmpRoot, 'project', '.venv')).fsPath;
79+
pythonPath = venvPythonPath(envDir);
80+
await fse.outputFile(pythonPath, '');
81+
});
82+
83+
teardown(async () => {
84+
sinon.restore();
85+
if (tmpRoot) {
86+
await fse.remove(tmpRoot);
87+
}
88+
});
89+
90+
function createdEnvironment(): PythonEnvironment {
91+
return createMockPythonEnvironment({
92+
name: '.venv',
93+
envPath: pythonPath,
94+
sysPrefix: envDir,
95+
version: '3.12.0',
96+
managerId: VENV_MANAGER_ID,
97+
});
98+
}
99+
100+
test('non-quick create delegates, caches the environment, and performs side effects', async () => {
101+
const globalEnv = createMockPythonEnvironment({
102+
name: 'global',
103+
envPath: testPath('global', 'python3'),
104+
version: '3.12.0',
105+
});
106+
const created = createdEnvironment();
107+
const manager = createManager({ getEnvironments: sinon.stub().resolves([globalEnv]) });
108+
createPythonVenvStub.resolves({ environment: created });
109+
const events: DidChangeEnvironmentsEventArgs[] = [];
110+
manager.onDidChangeEnvironments((event) => events.push(event));
111+
const scope = Uri.file(path.join(tmpRoot, 'project'));
112+
113+
const result = await manager.create(scope, undefined);
114+
115+
assert.strictEqual(result, created);
116+
assert.deepStrictEqual(createPythonVenvStub.firstCall.args[4], [globalEnv]);
117+
assert.strictEqual(createPythonVenvStub.firstCall.args[5].fsPath, scope.fsPath);
118+
assert.deepStrictEqual(createPythonVenvStub.firstCall.args[6], { showQuickAndCustomOptions: true });
119+
assert.deepStrictEqual((manager as any).collection, [created]);
120+
assert.strictEqual(events.length, 1);
121+
assert.strictEqual(events[0][0].kind, EnvironmentChangeKind.add);
122+
assert.strictEqual(await fse.readFile(path.join(envDir, '.gitignore'), 'utf8'), '*\n');
123+
assert.ok(executeCommandStub.calledOnceWithExactly('revealInExplorer', Uri.file(envDir)));
124+
});
125+
126+
test('quick create uses the selected global Python and forwards additional packages', async () => {
127+
const globalEnv = createMockPythonEnvironment({
128+
name: 'global',
129+
envPath: testPath('global', 'python3'),
130+
version: '3.12.0',
131+
});
132+
const created = createdEnvironment();
133+
const manager = createManager({ getEnvironments: sinon.stub().resolves([globalEnv]) });
134+
(manager as any).globalEnv = globalEnv;
135+
quickCreateVenvStub.resolves({ environment: created });
136+
const scope = Uri.file(path.join(tmpRoot, 'project'));
137+
138+
const result = await manager.create(scope, { quickCreate: true, additionalPackages: ['pytest'] });
139+
140+
assert.strictEqual(result, created);
141+
assert.ok(createPythonVenvStub.notCalled);
142+
assert.strictEqual(quickCreateVenvStub.firstCall.args[4], globalEnv);
143+
assert.strictEqual(quickCreateVenvStub.firstCall.args[5].fsPath, scope.fsPath);
144+
assert.deepStrictEqual(quickCreateVenvStub.firstCall.args[6], ['pytest']);
145+
});
146+
147+
test('reports creation errors without adding an environment', async () => {
148+
const manager = createManager({
149+
getEnvironments: sinon.stub().resolves([
150+
createMockPythonEnvironment({
151+
name: 'global',
152+
envPath: testPath('global', 'python3'),
153+
version: '3.12.0',
154+
}),
155+
]),
156+
});
157+
createPythonVenvStub.resolves({ envCreationErr: 'creation failed' });
158+
159+
const result = await manager.create(Uri.file(path.join(tmpRoot, 'project')), undefined);
160+
161+
assert.strictEqual(result, undefined);
162+
assert.deepStrictEqual((manager as any).collection, []);
163+
assert.ok(showErrorStub.calledOnce);
164+
});
165+
166+
test('restores the watcher guard when creation throws', async () => {
167+
const manager = createManager({
168+
getEnvironments: sinon.stub().resolves([
169+
createMockPythonEnvironment({
170+
name: 'global',
171+
envPath: testPath('global', 'python3'),
172+
version: '3.12.0',
173+
}),
174+
]),
175+
});
176+
createPythonVenvStub.rejects(new Error('creation failed'));
177+
178+
await assert.rejects(manager.create(Uri.file(path.join(tmpRoot, 'project')), undefined), /creation failed/);
179+
180+
assert.strictEqual((manager as any).skipWatcherRefresh, false);
181+
});
182+
});
183+
184+
suite('VenvManager.remove - orchestration', () => {
185+
let removeVenvStub: sinon.SinonStub;
186+
187+
setup(() => {
188+
removeVenvStub = sinon.stub(venvUtils, 'removeVenv');
189+
sinon.stub(venvUtils, 'setVenvForGlobal').resolves();
190+
sinon.stub(venvUtils, 'getVenvForGlobal').resolves(undefined);
191+
});
192+
193+
teardown(() => {
194+
sinon.restore();
195+
});
196+
197+
function environment(): PythonEnvironment {
198+
const root = testPath('workspace', 'project', '.venv');
199+
return createMockPythonEnvironment({
200+
name: '.venv',
201+
envPath: venvPythonPath(root),
202+
sysPrefix: root,
203+
version: '3.12.0',
204+
managerId: VENV_MANAGER_ID,
205+
});
206+
}
207+
208+
test('successful removal updates the collection and fires a remove event', async () => {
209+
const manager = createManager();
210+
const env = environment();
211+
(manager as any).collection = [env];
212+
removeVenvStub.resolves(true);
213+
const events: DidChangeEnvironmentsEventArgs[] = [];
214+
manager.onDidChangeEnvironments((event) => events.push(event));
215+
216+
await manager.remove(env);
217+
218+
assert.deepStrictEqual((manager as any).collection, []);
219+
assert.strictEqual(events.length, 1);
220+
assert.strictEqual(events[0][0].kind, EnvironmentChangeKind.remove);
221+
assert.strictEqual(events[0][0].environment, env);
222+
});
223+
224+
test('does not mutate state when the removal helper returns false', async () => {
225+
const manager = createManager();
226+
const env = environment();
227+
(manager as any).collection = [env];
228+
removeVenvStub.resolves(false);
229+
const events: DidChangeEnvironmentsEventArgs[] = [];
230+
manager.onDidChangeEnvironments((event) => events.push(event));
231+
232+
await manager.remove(env);
233+
234+
assert.deepStrictEqual((manager as any).collection, [env]);
235+
assert.strictEqual(events.length, 0);
236+
});
237+
238+
test('clears mapped project state and reports the effective fallback', async () => {
239+
const projectUri = Uri.file(testPath('workspace', 'project'));
240+
const project = { name: 'project', uri: projectUri };
241+
const fallback = createMockPythonEnvironment({
242+
name: 'global',
243+
envPath: testPath('global', 'python3'),
244+
version: '3.13.0',
245+
});
246+
const manager = createManager({ getPythonProject: sinon.stub().returns(project) }, [fallback]);
247+
const env = environment();
248+
(manager as any).collection = [env];
249+
(manager as any).globalEnv = fallback;
250+
(manager as any).fsPathToEnv = new Map([[normalizePath(projectUri.fsPath), env]]);
251+
removeVenvStub.resolves(true);
252+
const events: DidChangeEnvironmentEventArgs[] = [];
253+
manager.onDidChangeEnvironment((event) => events.push(event));
254+
255+
await manager.remove(env);
256+
257+
assert.strictEqual((manager as any).fsPathToEnv.size, 0);
258+
assert.strictEqual(events.length, 1);
259+
assert.strictEqual(normalizePath(events[0].uri!.fsPath), normalizePath(projectUri.fsPath));
260+
assert.strictEqual(events[0].old, env);
261+
assert.strictEqual(events[0].new, fallback);
262+
});
263+
264+
test('clears the current global environment', async () => {
265+
const manager = createManager();
266+
const env = environment();
267+
(manager as any).collection = [env];
268+
(manager as any).globalEnv = env;
269+
removeVenvStub.resolves(true);
270+
const events: DidChangeEnvironmentEventArgs[] = [];
271+
manager.onDidChangeEnvironment((event) => events.push(event));
272+
273+
await manager.remove(env);
274+
275+
assert.strictEqual((manager as any).globalEnv, undefined);
276+
assert.strictEqual(events.length, 1);
277+
assert.deepStrictEqual(events[0], { uri: undefined, old: env, new: undefined });
278+
});
279+
280+
test('restores the watcher guard when removal throws', async () => {
281+
const manager = createManager();
282+
removeVenvStub.rejects(new Error('removal failed'));
283+
284+
await assert.rejects(manager.remove(environment()), /removal failed/);
285+
286+
assert.strictEqual((manager as any).skipWatcherRefresh, false);
287+
});
288+
});

0 commit comments

Comments
 (0)