Skip to content

Commit d7ae4d3

Browse files
authored
fix: remove intrusive error notification for unresolvable Python paths (Fixes #1283) (#1287)
## Summary Removes the intrusive `showErrorMessage` notification that appeared when `handlePythonPath` couldn't resolve a path (e.g., non-Python executables like `node` or `ruby`). This was confusing users who weren't actively selecting a Python interpreter. ## Changes - **`src/common/utils/pythonPath.ts`**: Replaced `showErrorMessage` + `traceError` with `traceWarn` — the unresolvable path is logged but no longer triggers a modal notification - **`src/features/envCommands.ts`**: Added user-facing error message when the interactive "Browse..." flow selects a path that can't be resolved - **`src/common/pickers/environments.ts`**: Added user-facing error message when a browsed path can't be resolved from the environment picker - **`src/test/common/pythonPath.unit.test.ts`**: Added 12 unit tests covering `handlePythonPath` — resolution, fallback, deduplication, cancellation, progress reporting, priority sorting Fixes #1283
1 parent f983b64 commit d7ae4d3

File tree

4 files changed

+201
-8
lines changed

4 files changed

+201
-8
lines changed

src/common/pickers/environments.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ProgressLocation, QuickInputButtons, QuickPickItem, QuickPickItemKind, ThemeIcon, Uri } from 'vscode';
1+
import { ProgressLocation, QuickInputButtons, QuickPickItem, QuickPickItemKind, ThemeIcon, Uri, l10n } from 'vscode';
22
import { CreateEnvironmentOptions, IconPath, PythonEnvironment, PythonProject } from '../../api';
33
import { InternalEnvironmentManager } from '../../internal.api';
44
import { Common, Interpreter, Pickers } from '../localize';
@@ -7,7 +7,13 @@ import { EventNames } from '../telemetry/constants';
77
import { sendTelemetryEvent } from '../telemetry/sender';
88
import { isWindows } from '../utils/platformUtils';
99
import { handlePythonPath } from '../utils/pythonPath';
10-
import { showOpenDialog, showQuickPick, showQuickPickWithButtons, withProgress } from '../window.apis';
10+
import {
11+
showErrorMessage,
12+
showOpenDialog,
13+
showQuickPick,
14+
showQuickPickWithButtons,
15+
withProgress,
16+
} from '../window.apis';
1117
import { pickEnvironmentManager } from './managers';
1218

1319
type QuickPickIcon =
@@ -66,6 +72,11 @@ async function browseForPython(
6672
return env;
6773
},
6874
);
75+
76+
if (!environment) {
77+
showErrorMessage(l10n.t('Selected file is not a valid Python interpreter: {0}', uri.fsPath));
78+
}
79+
6980
return environment;
7081
}
7182

src/common/utils/pythonPath.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { Uri, Progress, CancellationToken } from 'vscode';
1+
import { CancellationToken, Progress, Uri } from 'vscode';
22
import { PythonEnvironment } from '../../api';
33
import { InternalEnvironmentManager } from '../../internal.api';
4-
import { traceVerbose, traceError } from '../logging';
54
import { PYTHON_EXTENSION_ID } from '../constants';
6-
import { showErrorMessage } from '../window.apis';
5+
import { traceVerbose, traceWarn } from '../logging';
76

87
const priorityOrder = [
98
`${PYTHON_EXTENSION_ID}:pyenv`,
@@ -74,7 +73,6 @@ export async function handlePythonPath(
7473
}
7574
}
7675

77-
traceError(`Unable to handle ${interpreterUri.fsPath}`);
78-
showErrorMessage(`Unable to handle ${interpreterUri.fsPath}`);
76+
traceWarn(`Unable to handle ${interpreterUri.fsPath}`);
7977
return undefined;
8078
}

src/features/envCommands.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import * as fs from 'fs-extra';
22
import * as path from 'path';
3-
import { ProgressLocation, QuickInputButtons, TaskExecution, TaskRevealKind, Terminal, Uri, workspace } from 'vscode';
3+
import {
4+
ProgressLocation,
5+
QuickInputButtons,
6+
TaskExecution,
7+
TaskRevealKind,
8+
Terminal,
9+
Uri,
10+
l10n,
11+
workspace,
12+
} from 'vscode';
413
import {
514
CreateEnvironmentOptions,
615
PythonEnvironment,
@@ -93,6 +102,10 @@ async function browseAndResolveInterpreter(
93102
},
94103
);
95104

105+
if (!environment) {
106+
showErrorMessage(l10n.t('Selected file is not a valid Python interpreter: {0}', interpreterUri.fsPath));
107+
}
108+
96109
return environment;
97110
}
98111

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import assert from 'node:assert';
5+
import * as sinon from 'sinon';
6+
import { CancellationTokenSource, Uri } from 'vscode';
7+
import { PythonEnvironment } from '../../api';
8+
import { handlePythonPath } from '../../common/utils/pythonPath';
9+
import { InternalEnvironmentManager } from '../../internal.api';
10+
11+
function createMockManager(
12+
id: string,
13+
displayName: string,
14+
resolveResult: PythonEnvironment | undefined = undefined,
15+
): sinon.SinonStubbedInstance<InternalEnvironmentManager> {
16+
return {
17+
id,
18+
displayName,
19+
resolve: sinon.stub().resolves(resolveResult),
20+
} as unknown as sinon.SinonStubbedInstance<InternalEnvironmentManager>;
21+
}
22+
23+
function createMockEnv(managerId: string): PythonEnvironment {
24+
return {
25+
envId: { id: `env-${managerId}`, managerId },
26+
name: `env-${managerId}`,
27+
displayName: `Env ${managerId}`,
28+
version: '3.11.0',
29+
displayPath: '/usr/bin/python3',
30+
environmentPath: Uri.file('/usr/bin/python3'),
31+
sysPrefix: '/usr',
32+
execInfo: { run: { executable: '/usr/bin/python3' } },
33+
} as PythonEnvironment;
34+
}
35+
36+
suite('handlePythonPath', () => {
37+
const testUri = Uri.file('/test/python3');
38+
39+
teardown(() => {
40+
sinon.restore();
41+
});
42+
43+
test('returns undefined when no managers can resolve the path', async () => {
44+
const manager1 = createMockManager('ms-python.python:venv', 'Venv');
45+
const manager2 = createMockManager('ms-python.python:conda', 'Conda');
46+
47+
const result = await handlePythonPath(testUri, [manager1, manager2], []);
48+
49+
assert.strictEqual(result, undefined);
50+
});
51+
52+
test('returns environment from project manager that resolves first', async () => {
53+
const mockEnv = createMockEnv('ms-python.python:venv');
54+
const projectManager = createMockManager('ms-python.python:venv', 'Venv', mockEnv);
55+
const globalManager = createMockManager('ms-python.python:conda', 'Conda');
56+
57+
const result = await handlePythonPath(testUri, [globalManager], [projectManager]);
58+
59+
assert.strictEqual(result, mockEnv);
60+
// Global manager should NOT have been called since project manager resolved
61+
assert.strictEqual((globalManager.resolve as sinon.SinonStub).called, false);
62+
});
63+
64+
test('falls back to global managers when project managers cannot resolve', async () => {
65+
const mockEnv = createMockEnv('ms-python.python:conda');
66+
const projectManager = createMockManager('ms-python.python:venv', 'Venv');
67+
const globalManager = createMockManager('ms-python.python:conda', 'Conda', mockEnv);
68+
69+
const result = await handlePythonPath(testUri, [globalManager], [projectManager]);
70+
71+
assert.strictEqual(result, mockEnv);
72+
});
73+
74+
test('does not re-check managers already checked as project managers', async () => {
75+
const projectManager = createMockManager('ms-python.python:venv', 'Venv');
76+
const globalManager = createMockManager('ms-python.python:venv', 'Venv');
77+
78+
const result = await handlePythonPath(testUri, [globalManager], [projectManager]);
79+
80+
assert.strictEqual(result, undefined);
81+
// Project manager checked, but global manager with same id should be skipped
82+
assert.strictEqual((projectManager.resolve as sinon.SinonStub).callCount, 1);
83+
assert.strictEqual((globalManager.resolve as sinon.SinonStub).callCount, 0);
84+
});
85+
86+
test('returns undefined and does not throw for unresolvable paths', async () => {
87+
const manager = createMockManager('ms-python.python:system', 'System');
88+
89+
const result = await handlePythonPath(Uri.file('/usr/bin/node'), [manager], []);
90+
91+
assert.strictEqual(result, undefined);
92+
});
93+
94+
test('respects cancellation token', async () => {
95+
const cts = new CancellationTokenSource();
96+
cts.cancel();
97+
98+
const manager = createMockManager('ms-python.python:venv', 'Venv');
99+
100+
const result = await handlePythonPath(testUri, [], [manager], undefined, cts.token);
101+
102+
assert.strictEqual(result, undefined);
103+
assert.strictEqual((manager.resolve as sinon.SinonStub).called, false);
104+
});
105+
106+
test('respects cancellation token for global managers', async () => {
107+
const cts = new CancellationTokenSource();
108+
cts.cancel();
109+
110+
const manager = createMockManager('ms-python.python:venv', 'Venv');
111+
112+
const result = await handlePythonPath(testUri, [manager], [], undefined, cts.token);
113+
114+
assert.strictEqual(result, undefined);
115+
assert.strictEqual((manager.resolve as sinon.SinonStub).called, false);
116+
});
117+
118+
test('reports progress for project managers', async () => {
119+
const reporter = { report: sinon.stub() };
120+
const projectManager = createMockManager('ms-python.python:venv', 'Venv');
121+
122+
await handlePythonPath(testUri, [], [projectManager], reporter);
123+
124+
assert.strictEqual(reporter.report.callCount, 1);
125+
assert.deepStrictEqual(reporter.report.firstCall.args[0], { message: 'Checking Venv' });
126+
});
127+
128+
test('reports progress for global managers', async () => {
129+
const reporter = { report: sinon.stub() };
130+
const manager1 = createMockManager('ms-python.python:venv', 'Venv');
131+
const manager2 = createMockManager('ms-python.python:conda', 'Conda');
132+
133+
await handlePythonPath(testUri, [manager1, manager2], [], reporter);
134+
135+
assert.strictEqual(reporter.report.callCount, 2);
136+
// Conda has higher priority, so it's checked first
137+
assert.deepStrictEqual(reporter.report.firstCall.args[0], { message: 'Checking Conda' });
138+
assert.deepStrictEqual(reporter.report.secondCall.args[0], { message: 'Checking Venv' });
139+
});
140+
141+
test('sorts managers by priority order', async () => {
142+
// Neither resolves, so both get called — lets us verify call order
143+
const systemManager = createMockManager('ms-python.python:system', 'System');
144+
const condaManager = createMockManager('ms-python.python:conda', 'Conda');
145+
146+
// Pass system first in array, but conda should be tried first (higher priority)
147+
await handlePythonPath(testUri, [systemManager, condaManager], []);
148+
149+
assert.ok((condaManager.resolve as sinon.SinonStub).calledBefore(systemManager.resolve as sinon.SinonStub));
150+
});
151+
152+
test('returns first resolving manager and stops checking', async () => {
153+
const venvEnv = createMockEnv('ms-python.python:venv');
154+
const condaEnv = createMockEnv('ms-python.python:conda');
155+
const venvManager = createMockManager('ms-python.python:venv', 'Venv', venvEnv);
156+
const condaManager = createMockManager('ms-python.python:conda', 'Conda', condaEnv);
157+
158+
// Conda is higher priority, so it resolves first
159+
const result = await handlePythonPath(testUri, [venvManager, condaManager], []);
160+
161+
assert.strictEqual(result, condaEnv);
162+
// Venv should NOT have been called since conda resolved first
163+
assert.strictEqual((venvManager.resolve as sinon.SinonStub).called, false);
164+
});
165+
166+
test('returns undefined when both arrays are empty', async () => {
167+
const result = await handlePythonPath(testUri, [], []);
168+
169+
assert.strictEqual(result, undefined);
170+
});
171+
});

0 commit comments

Comments
 (0)