Skip to content

Commit 30810b2

Browse files
Preserve other projects' selections when a venv cannot resolve (#1854)
Isolate a saved virtual-environment resolution failure to its project instead of abandoning selection restoration for the entire workspace. Fixes #1853. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 52f6ace commit 30810b2

2 files changed

Lines changed: 130 additions & 1 deletion

File tree

‎src/managers/builtin/venvManager.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ export class VenvManager implements EnvironmentManager {
574574

575575
/**
576576
* Loads and maps Python environments to their corresponding project paths in the workspace. about O(p × e) where p = projects.len and e = environments.len
577+
* Skips unresolvable selections without preventing other projects from restoring their environments.
577578
*/
578579
private async loadEnvMap() {
579580
const globals = await this.baseManager.getEnvironments('global');
@@ -608,7 +609,7 @@ export class VenvManager implements EnvironmentManager {
608609
foundEnv = resolved;
609610
} else {
610611
this.log.error(`Failed to resolve python environment: ${env}`);
611-
return;
612+
continue;
612613
}
613614
}
614615
// Given found env, add it to the map and fire the event if needed.
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'assert';
5+
import * as path from 'path';
6+
import * as sinon from 'sinon';
7+
import * as typemoq from 'typemoq';
8+
import { CancellationTokenSource, LogOutputChannel, Uri } from 'vscode';
9+
import {
10+
DidChangeEnvironmentEventArgs,
11+
EnvironmentManager,
12+
PythonEnvironmentApi,
13+
PythonProject,
14+
} from '../../../api';
15+
import { VENV_MANAGER_ID } from '../../../common/constants';
16+
import * as windowApis from '../../../common/window.apis';
17+
import { VenvManager } from '../../../managers/builtin/venvManager';
18+
import * as venvUtils from '../../../managers/builtin/venvUtils';
19+
import { NativePythonFinder } from '../../../managers/common/nativePythonFinder';
20+
import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment';
21+
22+
suite('VenvManager - project selection recovery', () => {
23+
let sandbox: sinon.SinonSandbox;
24+
let cancellation: CancellationTokenSource;
25+
26+
setup(() => {
27+
sandbox = sinon.createSandbox();
28+
cancellation = new CancellationTokenSource();
29+
sandbox.stub(windowApis, 'withProgress').callsFake(async (_options, task) =>
30+
task({ report: () => {} }, cancellation.token),
31+
);
32+
sandbox.stub(venvUtils, 'getVenvForGlobal').resolves(undefined);
33+
});
34+
35+
teardown(() => {
36+
sandbox.restore();
37+
cancellation.dispose();
38+
});
39+
40+
for (const operation of ['initialize', 'refresh'] as const) {
41+
for (const brokenIndex of [0, 1]) {
42+
test(`${operation} restores other projects when saved selection ${brokenIndex + 1} cannot resolve`, async () => {
43+
const projects: PythonProject[] = ['a', 'b', 'c'].map((name) => ({
44+
name,
45+
uri: Uri.file(path.resolve('selection-recovery', `project-${name}`)),
46+
}));
47+
const environments = projects.map((project) =>
48+
createMockPythonEnvironment({
49+
name: project.name,
50+
envPath: path.resolve(
51+
'selection-recovery',
52+
`external-${project.name}`,
53+
process.platform === 'win32' ? 'Scripts' : 'bin',
54+
'python',
55+
),
56+
managerId: VENV_MANAGER_ID,
57+
}),
58+
);
59+
const fallback = createMockPythonEnvironment({
60+
name: 'global',
61+
envPath: path.resolve('selection-recovery', 'global', 'python'),
62+
});
63+
const api = typemoq.Mock.ofType<PythonEnvironmentApi>();
64+
api.setup((a) => a.getPythonProjects()).returns(() => projects);
65+
api.setup((a) => a.getPythonProject(typemoq.It.isAny())).returns((uri: Uri) =>
66+
projects.find((project) => project.uri.fsPath === uri.fsPath),
67+
);
68+
const baseManager = typemoq.Mock.ofType<EnvironmentManager>();
69+
baseManager.setup((m) => m.getEnvironments('global')).returns(async () => [fallback]);
70+
const log = typemoq.Mock.ofType<LogOutputChannel>();
71+
const manager = new VenvManager(
72+
typemoq.Mock.ofType<NativePythonFinder>().object,
73+
api.object,
74+
baseManager.object,
75+
log.object,
76+
);
77+
sandbox.stub(venvUtils, 'findVirtualEnvironments').callsFake(async () => []);
78+
sandbox.stub(venvUtils, 'getVenvForWorkspace').callsFake(async (fsPath) => {
79+
const index = projects.findIndex((project) => project.uri.fsPath === fsPath);
80+
return environments[index]?.environmentPath.fsPath;
81+
});
82+
const resolve = sandbox
83+
.stub(venvUtils, 'resolveVenvPythonEnvironmentPath')
84+
.callsFake(async (fsPath) => environments.find((env) => env.environmentPath.fsPath === fsPath));
85+
const events: DidChangeEnvironmentEventArgs[] = [];
86+
const subscription = manager.onDidChangeEnvironment((event) => events.push(event));
87+
try {
88+
if (operation === 'refresh') {
89+
await manager.initialize();
90+
for (const [index, project] of projects.entries()) {
91+
assert.strictEqual(await manager.get(project.uri), environments[index]);
92+
}
93+
events.length = 0;
94+
}
95+
96+
const brokenPath = environments[brokenIndex].environmentPath.fsPath;
97+
resolve.withArgs(brokenPath).resolves(undefined);
98+
if (operation === 'initialize') {
99+
await manager.initialize();
100+
} else {
101+
await manager.refresh(undefined);
102+
}
103+
104+
for (const [index, project] of projects.entries()) {
105+
if (index !== brokenIndex) {
106+
assert.strictEqual(
107+
(await manager.get(project.uri))?.environmentPath.fsPath,
108+
environments[index].environmentPath.fsPath,
109+
`Project ${project.name} must retain its selected interpreter`,
110+
);
111+
}
112+
}
113+
assert.deepStrictEqual(
114+
events.map((event) => event.uri?.fsPath),
115+
projects.filter((_, index) => index !== brokenIndex).map((project) => project.uri.fsPath),
116+
'Healthy projects must still publish their restored selections',
117+
);
118+
log.verify(
119+
(l) => l.error(`Failed to resolve python environment: ${brokenPath}`),
120+
typemoq.Times.once(),
121+
);
122+
} finally {
123+
subscription.dispose();
124+
}
125+
});
126+
}
127+
}
128+
});

0 commit comments

Comments
 (0)