Skip to content

Commit b3a23df

Browse files
authored
fix: deduplicate workspaceSearchPaths error and skip PET resolve for unresolved variables (Fixes #1289) (#1290)
Fix noisy startup errors reported in #1289. - Add module-level `workspaceSearchPathsGlobalWarningShown` flag so the `workspaceSearchPaths` global-level error is logged at most once per session - Check for unresolved variables (`${...}`) in `defaultInterpreterPath` after `resolveVariables()` and skip PET resolve call, falling through to auto-discovery with a clear warning - Localize the `SettingResolutionError.reason` for unresolved variables since it's surfaced to users via `notifyUserOfSettingErrors()` - Export `resetWorkspaceSearchPathsGlobalWarningFlag()` for test isolation - Add test: "should skip native resolution when defaultInterpreterPath has unresolved variables" - Add test: "Global workspace setting warning is only logged once across multiple calls" Fixes #1289
1 parent d7ae4d3 commit b3a23df

File tree

4 files changed

+97
-15
lines changed

4 files changed

+97
-15
lines changed

src/features/interpreterSelection.ts

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,32 @@ async function resolvePriorityChainCore(
107107
const userInterpreterPath = getUserConfiguredSetting<string>('python', 'defaultInterpreterPath', scope);
108108
if (userInterpreterPath) {
109109
const expandedInterpreterPath = resolveVariables(userInterpreterPath, scope);
110-
const resolved = await tryResolveInterpreterPath(nativeFinder, api, expandedInterpreterPath, envManagers);
111-
if (resolved) {
112-
traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`);
113-
return { result: resolved, errors };
110+
if (expandedInterpreterPath.includes('${')) {
111+
traceWarn(
112+
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' contains unresolved variables, falling back to auto-discovery`,
113+
);
114+
const error: SettingResolutionError = {
115+
setting: 'defaultInterpreterPath',
116+
configuredValue: userInterpreterPath,
117+
reason: l10n.t('Path contains unresolved variables'),
118+
};
119+
errors.push(error);
120+
} else {
121+
const resolved = await tryResolveInterpreterPath(nativeFinder, api, expandedInterpreterPath, envManagers);
122+
if (resolved) {
123+
traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`);
124+
return { result: resolved, errors };
125+
}
126+
const error: SettingResolutionError = {
127+
setting: 'defaultInterpreterPath',
128+
configuredValue: userInterpreterPath,
129+
reason: `Could not resolve interpreter path '${userInterpreterPath}'`,
130+
};
131+
errors.push(error);
132+
traceWarn(
133+
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' unresolvable, falling back to auto-discovery`,
134+
);
114135
}
115-
const error: SettingResolutionError = {
116-
setting: 'defaultInterpreterPath',
117-
configuredValue: userInterpreterPath,
118-
reason: `Could not resolve interpreter path '${userInterpreterPath}'`,
119-
};
120-
errors.push(error);
121-
traceWarn(
122-
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' unresolvable, falling back to auto-discovery`,
123-
);
124136
}
125137

126138
// PRIORITY 4: Auto-discovery (no user-configured settings matched)

src/managers/common/nativePythonFinder.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,15 @@ function getGlobalSearchPaths(): string[] {
842842
}
843843
}
844844

845+
let workspaceSearchPathsGlobalWarningShown = false;
846+
847+
/**
848+
* @internal Test-only helper to reset the workspaceSearchPaths global-level warning flag.
849+
*/
850+
export function resetWorkspaceSearchPathsGlobalWarningFlag(): void {
851+
workspaceSearchPathsGlobalWarningShown = false;
852+
}
853+
845854
/**
846855
* Gets the most specific workspace-level setting available for workspaceSearchPaths.
847856
* Supports glob patterns which are expanded by PET.
@@ -851,7 +860,8 @@ function getWorkspaceSearchPaths(): string[] {
851860
const envConfig = getConfiguration('python-envs');
852861
const inspection = envConfig.inspect<string[]>('workspaceSearchPaths');
853862

854-
if (inspection?.globalValue) {
863+
if (inspection?.globalValue && !workspaceSearchPathsGlobalWarningShown) {
864+
workspaceSearchPathsGlobalWarningShown = true;
855865
traceError(
856866
'python-envs.workspaceSearchPaths is set at the user/global level, but this setting can only be set at the workspace or workspace folder level.',
857867
);

src/test/features/interpreterSelection.unit.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,36 @@ suite('Interpreter Selection - Priority Chain', () => {
264264
assert.ok(mockNativeFinder.resolve.calledOnceWithExactly(expandedInterpreterPath));
265265
});
266266

267+
test('should skip native resolution when defaultInterpreterPath has unresolved variables', async () => {
268+
// When resolveVariables can't resolve ${workspaceFolder} (e.g., global scope with no workspace),
269+
// the path still contains '${' and should be skipped without calling nativeFinder.resolve
270+
sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration);
271+
sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]);
272+
sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(undefined);
273+
sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => {
274+
if (section === 'python' && key === 'defaultInterpreterPath') {
275+
return '${workspaceFolder}/.venv/bin/python3';
276+
}
277+
return undefined;
278+
});
279+
mockVenvManager.get.resolves(mockVenvEnv);
280+
281+
const result = await resolveEnvironmentByPriority(
282+
testUri,
283+
mockEnvManagers as unknown as EnvironmentManagers,
284+
mockProjectManager as unknown as PythonProjectManager,
285+
mockNativeFinder as unknown as NativePythonFinder,
286+
mockApi as unknown as PythonEnvironmentApi,
287+
);
288+
289+
// Should fall through to auto-discovery without calling nativeFinder.resolve
290+
assert.strictEqual(result.source, 'autoDiscovery');
291+
assert.ok(
292+
mockNativeFinder.resolve.notCalled,
293+
'nativeFinder.resolve should not be called with unresolved variables',
294+
);
295+
});
296+
267297
test('should fall through to Priority 4 when defaultInterpreterPath cannot be resolved', async () => {
268298
sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration);
269299
sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]);

src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as pathUtils from '../../../common/utils/pathUtils';
66
import * as workspaceApis from '../../../common/workspace.apis';
77

88
// Import the function under test
9-
import { getAllExtraSearchPaths } from '../../../managers/common/nativePythonFinder';
9+
import { getAllExtraSearchPaths, resetWorkspaceSearchPathsGlobalWarningFlag } from '../../../managers/common/nativePythonFinder';
1010

1111
interface MockWorkspaceConfig {
1212
get: sinon.SinonStub;
@@ -26,6 +26,8 @@ suite('getAllExtraSearchPaths Integration Tests', () => {
2626
let envConfig: MockWorkspaceConfig;
2727

2828
setup(() => {
29+
resetWorkspaceSearchPathsGlobalWarningFlag();
30+
2931
// Mock VS Code workspace APIs
3032
mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration');
3133
mockGetWorkspaceFolders = sinon.stub(workspaceApis, 'getWorkspaceFolders');
@@ -87,6 +89,7 @@ suite('getAllExtraSearchPaths Integration Tests', () => {
8789

8890
teardown(() => {
8991
sinon.restore();
92+
resetWorkspaceSearchPathsGlobalWarningFlag();
9093
});
9194

9295
suite('Legacy Path Consolidation Tests', () => {
@@ -332,6 +335,33 @@ suite('getAllExtraSearchPaths Integration Tests', () => {
332335
);
333336
});
334337

338+
test('Global workspace setting warning is only logged once across multiple calls', async () => {
339+
// Mock → Workspace setting incorrectly set at global level
340+
pythonConfig.get.withArgs('venvPath').returns(undefined);
341+
pythonConfig.get.withArgs('venvFolders').returns(undefined);
342+
envConfig.inspect.withArgs('globalSearchPaths').returns({ globalValue: [] });
343+
envConfig.inspect.withArgs('workspaceSearchPaths').returns({
344+
globalValue: ['should-be-ignored'],
345+
});
346+
347+
// Run multiple times
348+
await getAllExtraSearchPaths();
349+
await getAllExtraSearchPaths();
350+
await getAllExtraSearchPaths();
351+
352+
// Assert - error should only be logged once, not three times
353+
const matchingCalls = mockTraceError
354+
.getCalls()
355+
.filter((call: sinon.SinonSpyCall) =>
356+
/workspaceSearchPaths.*global.*level/i.test(String(call.args[0])),
357+
);
358+
assert.strictEqual(
359+
matchingCalls.length,
360+
1,
361+
`Expected exactly 1 error about workspaceSearchPaths global level, got ${matchingCalls.length}`,
362+
);
363+
});
364+
335365
test('Configuration read errors return empty arrays', async () => {
336366
// Mock → Configuration throws errors
337367
pythonConfig.get.withArgs('venvPath').returns(undefined);

0 commit comments

Comments
 (0)