From 4b8c1749697587849103dada4bfa79094bcafcfa Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 20:38:36 -0800 Subject: [PATCH 1/6] overhaul to interpreter selection prioritization code to align with desired outcomes --- .github/instructions/generic.instructions.md | 4 + .../testing-workflow.instructions.md | 164 +- src/common/constants.ts | 2 + src/extension.ts | 18 +- src/features/creators/newPackageProject.ts | 2 +- src/features/envManagers.ts | 81 +- src/features/interpreterSelection.ts | 445 ++++++ src/features/settings/settingHelpers.ts | 80 +- src/helpers.ts | 79 +- src/internal.api.ts | 44 +- .../interpreterSelection.unit.test.ts | 1410 +++++++++++++++++ .../settings/settingHelpers.unit.test.ts | 158 +- 12 files changed, 2116 insertions(+), 371 deletions(-) create mode 100644 src/features/interpreterSelection.ts create mode 100644 src/test/features/interpreterSelection.unit.test.ts diff --git a/.github/instructions/generic.instructions.md b/.github/instructions/generic.instructions.md index 3f43f207..c9e3c87e 100644 --- a/.github/instructions/generic.instructions.md +++ b/.github/instructions/generic.instructions.md @@ -30,3 +30,7 @@ Provide project context and coding guidelines that AI should follow when generat ## Documentation - Add clear docstrings to public functions, describing their purpose, parameters, and behavior. + +## Learnings + +- When using `getConfiguration().inspect()`, always pass a scope/Uri to `getConfiguration(section, scope)` β€” otherwise `workspaceFolderValue` will be `undefined` because VS Code doesn't know which folder to inspect (1) diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index be56e4a8..88a3e656 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -20,18 +20,18 @@ This guide covers the full testing lifecycle: **User Requests Testing:** -- "Write tests for this function" -- "Run the tests" -- "Fix the failing tests" -- "Test this code" -- "Add test coverage" +- "Write tests for this function" +- "Run the tests" +- "Fix the failing tests" +- "Test this code" +- "Add test coverage" **File Context Triggers:** -- Working in `**/test/**` directories -- Files ending in `.test.ts` or `.unit.test.ts` -- Test failures or compilation errors -- Coverage reports or test output analysis +- Working in `**/test/**` directories +- Files ending in `.test.ts` or `.unit.test.ts` +- Test failures or compilation errors +- Coverage reports or test output analysis ## Test Types @@ -39,17 +39,17 @@ When implementing tests as an AI agent, choose between two main types: ### Unit Tests (`*.unit.test.ts`) -- **Fast isolated testing** - Mock all external dependencies -- **Use for**: Pure functions, business logic, data transformations -- **Execute with**: `runTests` tool with specific file patterns -- **Mock everything** - VS Code APIs automatically mocked via `/src/test/unittests.ts` +- **Fast isolated testing** - Mock all external dependencies +- **Use for**: Pure functions, business logic, data transformations +- **Execute with**: `runTests` tool with specific file patterns +- **Mock everything** - VS Code APIs automatically mocked via `/src/test/unittests.ts` ### Extension Tests (`*.test.ts`) -- **Full VS Code integration** - Real environment with actual APIs -- **Use for**: Command registration, UI interactions, extension lifecycle -- **Execute with**: VS Code launch configurations or `runTests` tool -- **Slower but comprehensive** - Tests complete user workflows +- **Full VS Code integration** - Real environment with actual APIs +- **Use for**: Command registration, UI interactions, extension lifecycle +- **Execute with**: VS Code launch configurations or `runTests` tool +- **Slower but comprehensive** - Tests complete user workflows ## πŸ€– Agent Tool Usage for Test Execution @@ -172,17 +172,17 @@ function analyzeFailure(failure: TestFailure): TestFailureAnalysis { **Choose Unit Tests (`*.unit.test.ts`) when analyzing:** -- Functions with clear inputs/outputs and no VS Code API dependencies -- Data transformation, parsing, or utility functions -- Business logic that can be isolated with mocks -- Error handling scenarios with predictable inputs +- Functions with clear inputs/outputs and no VS Code API dependencies +- Data transformation, parsing, or utility functions +- Business logic that can be isolated with mocks +- Error handling scenarios with predictable inputs **Choose Extension Tests (`*.test.ts`) when analyzing:** -- Functions that register VS Code commands or use `vscode.*` APIs -- UI components, tree views, or command palette interactions -- File system operations requiring workspace context -- Extension lifecycle events (activation, deactivation) +- Functions that register VS Code commands or use `vscode.*` APIs +- UI components, tree views, or command palette interactions +- File system operations requiring workspace context +- Extension lifecycle events (activation, deactivation) **Agent Implementation Pattern:** @@ -300,22 +300,22 @@ function generateTestScenarios(analysis: FunctionAnalysis): TestScenario[] { #### Main Flows -- βœ… **Happy path scenarios** - normal expected usage -- βœ… **Alternative paths** - different configuration combinations -- βœ… **Integration scenarios** - multiple features working together +- βœ… **Happy path scenarios** - normal expected usage +- βœ… **Alternative paths** - different configuration combinations +- βœ… **Integration scenarios** - multiple features working together #### Edge Cases -- πŸ”Έ **Boundary conditions** - empty inputs, missing data -- πŸ”Έ **Error scenarios** - network failures, permission errors -- πŸ”Έ **Data validation** - invalid inputs, type mismatches +- πŸ”Έ **Boundary conditions** - empty inputs, missing data +- πŸ”Έ **Error scenarios** - network failures, permission errors +- πŸ”Έ **Data validation** - invalid inputs, type mismatches #### Real-World Scenarios -- βœ… **Fresh install** - clean slate -- βœ… **Existing user** - migration scenarios -- βœ… **Power user** - complex configurations -- πŸ”Έ **Error recovery** - graceful degradation +- βœ… **Fresh install** - clean slate +- βœ… **Existing user** - migration scenarios +- βœ… **Power user** - complex configurations +- πŸ”Έ **Error recovery** - graceful degradation ### Example Test Plan Structure @@ -324,30 +324,30 @@ function generateTestScenarios(analysis: FunctionAnalysis): TestScenario[] { ### 1. Configuration Migration Tests -- No legacy settings exist -- Legacy settings already migrated -- Fresh migration needed -- Partial migration required -- Migration failures +- No legacy settings exist +- Legacy settings already migrated +- Fresh migration needed +- Partial migration required +- Migration failures ### 2. Configuration Source Tests -- Global search paths -- Workspace search paths -- Settings precedence -- Configuration errors +- Global search paths +- Workspace search paths +- Settings precedence +- Configuration errors ### 3. Path Resolution Tests -- Absolute vs relative paths -- Workspace folder resolution -- Path validation and filtering +- Absolute vs relative paths +- Workspace folder resolution +- Path validation and filtering ### 4. Integration Scenarios -- Combined configurations -- Deduplication logic -- Error handling flows +- Combined configurations +- Deduplication logic +- Error handling flows ``` ## πŸ”§ Step 4: Set Up Your Test Infrastructure @@ -514,47 +514,47 @@ envConfig.inspect ### Configuration Tests -- Test different setting combinations -- Test setting precedence (workspace > user > default) -- Test configuration errors and recovery -- Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility +- Test different setting combinations +- Test setting precedence (workspace > user > default) +- Test configuration errors and recovery +- Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility ### Data Flow Tests -- Test how data moves through the system -- Test transformations (path resolution, filtering) -- Test state changes (migrations, updates) +- Test how data moves through the system +- Test transformations (path resolution, filtering) +- Test state changes (migrations, updates) ### Error Handling Tests -- Test graceful degradation -- Test error logging -- Test fallback behaviors +- Test graceful degradation +- Test error logging +- Test fallback behaviors ### Integration Tests -- Test multiple features together -- Test real-world scenarios -- Test edge case combinations +- Test multiple features together +- Test real-world scenarios +- Test edge case combinations ## πŸ“Š Step 8: Review and Refine ### Test Quality Checklist -- [ ] **Clear naming** - test names describe the scenario and expected outcome -- [ ] **Good coverage** - main flows, edge cases, error scenarios -- [ ] **Resilient assertions** - won't break due to minor changes -- [ ] **Readable structure** - follows Mock β†’ Run β†’ Assert pattern -- [ ] **Isolated tests** - each test is independent -- [ ] **Fast execution** - tests run quickly with proper mocking +- [ ] **Clear naming** - test names describe the scenario and expected outcome +- [ ] **Good coverage** - main flows, edge cases, error scenarios +- [ ] **Resilient assertions** - won't break due to minor changes +- [ ] **Readable structure** - follows Mock β†’ Run β†’ Assert pattern +- [ ] **Isolated tests** - each test is independent +- [ ] **Fast execution** - tests run quickly with proper mocking ### Common Anti-Patterns to Avoid -- ❌ Testing implementation details instead of behavior -- ❌ Brittle assertions that break on cosmetic changes -- ❌ Order-dependent tests that fail due to processing changes -- ❌ Tests that don't clean up mocks properly -- ❌ Overly complex test setup that's hard to understand +- ❌ Testing implementation details instead of behavior +- ❌ Brittle assertions that break on cosmetic changes +- ❌ Order-dependent tests that fail due to processing changes +- ❌ Tests that don't clean up mocks properly +- ❌ Overly complex test setup that's hard to understand ## πŸ”„ Reviewing and Improving Existing Tests @@ -567,13 +567,15 @@ envConfig.inspect ### Common Fixes -- Over-complex mocks β†’ Minimal mocks with only needed methods -- Brittle assertions β†’ Behavior-focused with error messages -- Vague test names β†’ Clear scenario descriptions (transform "should return X when Y" into "should [expected behavior] when [scenario context]") -- Missing structure β†’ Mock β†’ Run β†’ Assert pattern -- Untestable Node.js APIs β†’ Create proxy abstraction functions (use function overloads to preserve intelligent typing while making functions mockable) +- Over-complex mocks β†’ Minimal mocks with only needed methods +- Brittle assertions β†’ Behavior-focused with error messages +- Vague test names β†’ Clear scenario descriptions (transform "should return X when Y" into "should [expected behavior] when [scenario context]") +- Missing structure β†’ Mock β†’ Run β†’ Assert pattern +- Untestable Node.js APIs β†’ Create proxy abstraction functions (use function overloads to preserve intelligent typing while making functions mockable) ## 🧠 Agent Learnings -- Avoid testing exact error messages or log output - assert only that errors are thrown or rejection occurs to prevent brittle tests (1) -- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1) +- Avoid testing exact error messages or log output - assert only that errors are thrown or rejection occurs to prevent brittle tests (1) +- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1) +- Always compile tests (`npm run compile-tests`) before running them after adding new test cases - test counts will be wrong if running against stale compiled output (1) +- Never create "documentation tests" that just `assert.ok(true)` β€” if mocking limitations prevent testing, either test a different layer that IS mockable, or skip the test entirely with a clear explanation (1) diff --git a/src/common/constants.ts b/src/common/constants.ts index 08612bf7..a804df6e 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -7,6 +7,8 @@ export const EXTENSION_ROOT_DIR = path.dirname(__dirname); export const DEFAULT_PACKAGE_MANAGER_ID = 'ms-python.python:pip'; export const DEFAULT_ENV_MANAGER_ID = 'ms-python.python:venv'; +export const VENV_MANAGER_ID = 'ms-python.python:venv'; +export const SYSTEM_MANAGER_ID = 'ms-python.python:system'; export const KNOWN_FILES = [ 'requirements.txt', diff --git a/src/extension.ts b/src/extension.ts index 74ee761d..124b3fae 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -45,6 +45,10 @@ import { } from './features/envCommands'; import { PythonEnvironmentManagers } from './features/envManagers'; import { EnvVarManager, PythonEnvVariableManager } from './features/execution/envVariableManager'; +import { + applyInitialEnvironmentSelection, + registerInterpreterSettingsChangeListener, +} from './features/interpreterSelection'; import { PythonProjectManagerImpl } from './features/projectManager'; import { getPythonApi, setPythonApi } from './features/pythonApi'; import { registerCompletionProvider } from './features/settings/settingCompletions'; @@ -67,12 +71,7 @@ import { PythonStatusBarImpl } from './features/views/pythonStatusBar'; import { updateViewsAndStatus } from './features/views/revealHandler'; import { TemporaryStateManager } from './features/views/temporaryStateManager'; import { ProjectItem, PythonEnvTreeItem } from './features/views/treeViewItems'; -import { - collectEnvironmentInfo, - getEnvManagerAndPackageManagerConfigLevels, - resolveDefaultInterpreter, - runPetInTerminalImpl, -} from './helpers'; +import { collectEnvironmentInfo, getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl } from './helpers'; import { EnvironmentManagers, ProjectCreators, PythonProjectManager } from './internal.api'; import { registerSystemPythonFeatures } from './managers/builtin/main'; import { SysPythonManager } from './managers/builtin/sysPythonManager'; @@ -460,7 +459,12 @@ export async function activate(context: ExtensionContext): Promise { + public async setEnvironment( + scope: SetEnvironmentScope, + environment?: PythonEnvironment, + shouldPersistSettings: boolean = true, + ): Promise { if (Array.isArray(scope)) { - return this.setEnvironments(scope, environment); + return this.setEnvironments(scope, environment, shouldPersistSettings); } const customScope = environment ? environment : scope; @@ -284,7 +316,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { await manager.set(scope, environment); const project = scope ? this.pm.get(scope) : undefined; - if (scope) { + // Only persist to settings when explicitly requested + if (shouldPersistSettings && scope) { const packageManager = this.getPackageManager(environment); if (project && packageManager) { await setAllManagerSettings([ @@ -310,8 +343,18 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { /** * Sets the given environment for the specified project URIs or globally. * If a list of URIs is provided, sets the environment for each project; if 'global', sets it as the global environment. + * + * @param scope - Array of URIs or 'global' + * @param environment - The environment to set (optional) + * @param shouldPersistSettings - Whether to persist to settings.json (default: true). + * Pass `false` when setting environments during initial selection/auto-discovery + * to avoid writing to settings.json. */ - public async setEnvironments(scope: Uri[] | string, environment?: PythonEnvironment): Promise { + public async setEnvironments( + scope: Uri[] | string, + environment?: PythonEnvironment, + shouldPersistSettings: boolean = true, + ): Promise { if (environment) { const manager = this.managers.find((m) => m.id === environment.envId.managerId); if (!manager) { @@ -320,7 +363,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { environment.environmentPath ? environment.environmentPath.fsPath : '' }`, ); - traceError(this.managers.map((m) => m.id).join(', ')); + traceError(`Available managers: ${this.managers.map((m) => m.id).join(', ')}`); return; } @@ -331,7 +374,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { promises.push(manager.set(scope, environment)); scope.forEach((uri) => { const m = this.getEnvironmentManager(uri); - if (manager.id !== m?.id) { + // Always add settings when persisting, OR when manager differs + if (shouldPersistSettings || manager.id !== m?.id) { settings.push({ project: this.pm.get(uri), envManager: manager.id, @@ -350,7 +394,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } else if (typeof scope === 'string' && scope === 'global') { const m = this.getEnvironmentManager(undefined); promises.push(manager.set(undefined, environment)); - if (manager.id !== m?.id) { + // Always add settings when persisting, OR when manager differs + if (shouldPersistSettings || manager.id !== m?.id) { settings.push({ project: undefined, envManager: manager.id, @@ -365,7 +410,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } await Promise.all(promises); - await setAllManagerSettings(settings); + // Only persist to settings when explicitly requested + if (shouldPersistSettings) { + await setAllManagerSettings(settings); + } setImmediate(() => events.forEach((e) => this._onDidChangeEnvironmentFiltered.fire(e))); } else { const promises: Promise[] = []; @@ -427,19 +475,19 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (typeof scope === 'string' && scope === 'global') { const current = await this.getEnvironment(undefined); if (!current) { - await this.setEnvironments('global', environment); + await this.setEnvironments('global', environment, true); } } else if (Array.isArray(scope)) { const urisToSet: Uri[] = []; for (const uri of scope) { const current = await this.getEnvironment(uri); - if (!current || current.envId.managerId === 'ms-python.python:system') { + if (!current || current.envId.managerId === SYSTEM_MANAGER_ID) { // If the current environment is not set or is the system environment, set the new environment. urisToSet.push(uri); } } if (urisToSet.length > 0) { - await this.setEnvironments(urisToSet, environment); + await this.setEnvironments(urisToSet, environment, true); } } } @@ -464,6 +512,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { // Always get the new first, then compare with the old. This has minor impact on the ordering of // events. But it ensures that we always get the latest environment at the time of this call. const newEnv = await manager.get(scope); + const key = project ? project.uri.toString() : 'global'; const oldEnv = this._previousEnvironments.get(key); if (oldEnv?.envId.id !== newEnv?.envId.id) { diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts new file mode 100644 index 00000000..b954669b --- /dev/null +++ b/src/features/interpreterSelection.ts @@ -0,0 +1,445 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { commands, ConfigurationChangeEvent, Disposable, l10n, Uri, workspace } from 'vscode'; +import { PythonEnvironment, PythonEnvironmentApi } from '../api'; +import { SYSTEM_MANAGER_ID, VENV_MANAGER_ID } from '../common/constants'; +import { traceInfo, traceVerbose, traceWarn } from '../common/logging'; +import { showWarningMessage } from '../common/window.apis'; +import { getConfiguration, onDidChangeConfiguration } from '../common/workspace.apis'; +import { getUserConfiguredSetting } from '../helpers'; +import { + EnvironmentManagers, + InternalEnvironmentManager, + PythonProjectManager, + PythonProjectSettings, +} from '../internal.api'; +import { NativeEnvInfo, NativePythonFinder } from '../managers/common/nativePythonFinder'; + +/** + * Result from the priority chain resolution. + */ +export interface PriorityChainResult { + /** The environment manager to use */ + manager: InternalEnvironmentManager; + /** Optional specific environment - if undefined, let the manager decide via get() */ + environment?: PythonEnvironment; + /** Which priority level matched */ + source: 'pythonProjects' | 'defaultEnvManager' | 'defaultInterpreterPath' | 'autoDiscovery'; +} + +/** + * Error information when a user-configured setting could not be applied. + */ +export interface SettingResolutionError { + /** The setting that failed */ + setting: 'pythonProjects' | 'defaultEnvManager' | 'defaultInterpreterPath'; + /** The configured value */ + configuredValue: string; + /** Reason for failure */ + reason: string; +} + +/** + * Core priority chain logic shared between workspace folder and global resolution. + * + * @param scope - The workspace folder URI (for workspace scope) or undefined (for global scope) + * @param envManagers - The environment managers registry + * @param projectManager - The project manager for pythonProjects[] lookups (only used for workspace scope) + * @param nativeFinder - Native Python finder for path resolution + * @param api - The Python environment API + * @returns The resolved PriorityChainResult and any errors encountered + */ +async function resolvePriorityChainCore( + scope: Uri | undefined, + envManagers: EnvironmentManagers, + projectManager: PythonProjectManager | undefined, + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, +): Promise<{ result: PriorityChainResult; errors: SettingResolutionError[] }> { + const errors: SettingResolutionError[] = []; + const logPrefix = scope ? `[resolveEnvironmentByPriority] ${scope.fsPath}` : '[resolveGlobalEnvironmentByPriority]'; + + traceVerbose(`${logPrefix} Starting priority chain resolution`); + + // PRIORITY 1: Check pythonProjects[] for this workspace path (workspace scope only) + if (scope && projectManager) { + const projectManagerId = getProjectSpecificEnvManager(projectManager, scope); + if (projectManagerId) { + const manager = envManagers.getEnvironmentManager(projectManagerId); + if (manager) { + traceVerbose(`${logPrefix} Priority 1: Using pythonProjects[] manager: ${projectManagerId}`); + return { result: { manager, source: 'pythonProjects' }, errors }; + } + const error: SettingResolutionError = { + setting: 'pythonProjects', + configuredValue: projectManagerId, + reason: `Environment manager '${projectManagerId}' is not registered`, + }; + errors.push(error); + traceWarn(`${logPrefix} ${error.reason}`); + } + } + + // PRIORITY 2: User-configured defaultEnvManager (skip if only fallback) + const userConfiguredManager = getUserConfiguredSetting('python-envs', 'defaultEnvManager', scope); + if (userConfiguredManager) { + const manager = envManagers.getEnvironmentManager(userConfiguredManager); + if (manager) { + traceVerbose(`${logPrefix} Priority 2: Using user-configured defaultEnvManager: ${userConfiguredManager}`); + return { result: { manager, source: 'defaultEnvManager' }, errors }; + } + const error: SettingResolutionError = { + setting: 'defaultEnvManager', + configuredValue: userConfiguredManager, + reason: `Environment manager '${userConfiguredManager}' is not registered`, + }; + errors.push(error); + traceWarn(`${logPrefix} ${error.reason}`); + } + + // PRIORITY 3: User-configured python.defaultInterpreterPath + const userInterpreterPath = getUserConfiguredSetting('python', 'defaultInterpreterPath', scope); + if (userInterpreterPath) { + const resolved = await tryResolveInterpreterPath(nativeFinder, api, userInterpreterPath, envManagers); + if (resolved) { + traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`); + return { result: resolved, errors }; + } + const error: SettingResolutionError = { + setting: 'defaultInterpreterPath', + configuredValue: userInterpreterPath, + reason: `Could not resolve interpreter path '${userInterpreterPath}'`, + }; + errors.push(error); + traceWarn(`${logPrefix} ${error.reason}, falling through to auto-discovery`); + } + + // PRIORITY 4: Auto-discovery + traceVerbose(`${logPrefix} Priority 4: Falling through to auto-discovery`); + const autoDiscoverResult = await autoDiscoverEnvironment(scope, envManagers); + return { result: autoDiscoverResult, errors }; +} + +/** + * Determine the environment for a workspace folder by walking the priority chain: + * + * PRIORITY 1: pythonProjects[] entry for this path + * PRIORITY 2: User-configured defaultEnvManager (not fallback) + * PRIORITY 3: User-configured python.defaultInterpreterPath + * PRIORITY 4: Auto-discovery (local venv β†’ global Python) + * + * Returns the manager (and optionally specific environment) without persisting to settings. + * + * @param scope - The workspace folder URI to resolve + * @param envManagers - The environment managers registry + * @param projectManager - The project manager for pythonProjects[] lookups + * @param nativeFinder - Native Python finder for path resolution + * @param api - The Python environment API + */ +export async function resolveEnvironmentByPriority( + scope: Uri, + envManagers: EnvironmentManagers, + projectManager: PythonProjectManager, + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, +): Promise { + const { result } = await resolvePriorityChainCore(scope, envManagers, projectManager, nativeFinder, api); + return result; +} + +/** + * Determine the environment for global scope (no workspace folder) by walking the priority chain: + * + * PRIORITY 1: (Skipped - pythonProjects[] doesn't apply to global scope) + * PRIORITY 2: User-configured defaultEnvManager (not fallback) + * PRIORITY 3: User-configured python.defaultInterpreterPath + * PRIORITY 4: Auto-discovery (system Python) + * + * Returns the manager (and optionally specific environment) without persisting to settings. + * + * @param envManagers - The environment managers registry + * @param nativeFinder - Native Python finder for path resolution + * @param api - The Python environment API + */ +export async function resolveGlobalEnvironmentByPriority( + envManagers: EnvironmentManagers, + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, +): Promise { + const { result } = await resolvePriorityChainCore(undefined, envManagers, undefined, nativeFinder, api); + return result; +} + +/** + * Auto-discovery for any scope: try local venv first (if scope provided), then fall back to system manager. + */ +async function autoDiscoverEnvironment( + scope: Uri | undefined, + envManagers: EnvironmentManagers, +): Promise { + // Try venv manager first for local environments (workspace scope only) + if (scope) { + const venvManager = envManagers.getEnvironmentManager(VENV_MANAGER_ID); + if (venvManager) { + try { + const localEnv = await venvManager.get(scope); + if (localEnv) { + traceVerbose(`[autoDiscover] Priority 4: Auto-discovered local venv: ${localEnv.displayName}`); + return { manager: venvManager, environment: localEnv, source: 'autoDiscovery' }; + } + } catch (err) { + traceWarn(`[autoDiscover] Error checking venv manager: ${err}`); + } + } + } + + // Fall back to system manager + const systemManager = envManagers.getEnvironmentManager(SYSTEM_MANAGER_ID); + if (systemManager) { + traceVerbose('[autoDiscover] Priority 4: Using system Python (fallback)'); + return { manager: systemManager, source: 'autoDiscovery' }; + } + + // Last resort: use any available manager + const anyManager = envManagers.managers[0]; + if (anyManager) { + traceVerbose(`[autoDiscover] Priority 4: No venv or system manager, using first available: ${anyManager.id}`); + return { manager: anyManager, source: 'autoDiscovery' }; + } + + // This should never happen if managers are registered properly + throw new Error('No environment managers available'); +} + +/** + * Called once at extension activation. Runs priority chain for all workspace folders + * AND the global scope, and caches results WITHOUT writing to settings.json. + * + * This ensures users see an interpreter immediately while respecting: + * - Existing project-specific settings (pythonProjects[]) + * - User's defaultEnvManager preference + * - Legacy defaultInterpreterPath migration + * - Auto-discovered local environments + * + * If user-configured settings cannot be applied, shows a warning notification + * with an option to open settings. + * + * @param envManagers - The environment managers registry + * @param projectManager - The project manager + * @param nativeFinder - Native Python finder for path resolution + * @param api - The Python environment API + */ +export async function applyInitialEnvironmentSelection( + envManagers: EnvironmentManagers, + projectManager: PythonProjectManager, + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, +): Promise { + const folders = workspace.workspaceFolders ?? []; + traceVerbose(`[applyInitialEnvironmentSelection] Starting - ${folders.length} workspace folder(s)`); + + const allErrors: SettingResolutionError[] = []; + + for (const folder of folders) { + try { + const { result, errors } = await resolvePriorityChainCore( + folder.uri, + envManagers, + projectManager, + nativeFinder, + api, + ); + allErrors.push(...errors); + + // Get the specific environment if not already resolved + const env = result.environment ?? (await result.manager.get(folder.uri)); + + // Cache only β€” NO settings.json write (shouldPersistSettings = false) + await envManagers.setEnvironment(folder.uri, env, false); + + traceVerbose( + `[applyInitialEnvironmentSelection] Set environment for ${folder.uri.fsPath}: ` + + `manager=${result.manager.id}, source=${result.source}, env=${env?.displayName ?? 'none'}`, + ); + } catch (err) { + traceWarn(`[applyInitialEnvironmentSelection] Error setting environment for ${folder.uri.fsPath}: ${err}`); + } + } + + // Also apply initial selection for global scope (no workspace folder) + // This ensures defaultInterpreterPath is respected even without a workspace + try { + const { result, errors } = await resolvePriorityChainCore(undefined, envManagers, undefined, nativeFinder, api); + allErrors.push(...errors); + + // Get the specific environment if not already resolved + const env = result.environment ?? (await result.manager.get(undefined)); + + // Cache only β€” NO settings.json write (shouldPersistSettings = false) + await envManagers.setEnvironments('global', env, false); + + traceVerbose( + `[applyInitialEnvironmentSelection] Set global environment: ` + + `manager=${result.manager.id}, source=${result.source}, env=${env?.displayName ?? 'none'}`, + ); + } catch (err) { + traceWarn(`[applyInitialEnvironmentSelection] Error setting global environment: ${err}`); + } + + // Notify user if any settings could not be applied + if (allErrors.length > 0) { + await notifyUserOfSettingErrors(allErrors); + } +} + +/** + * Notify the user when their configured settings could not be applied. + * Shows a warning message with an option to open settings. + */ +async function notifyUserOfSettingErrors(errors: SettingResolutionError[]): Promise { + // Group errors by setting type to avoid spamming the user + const uniqueSettings = [...new Set(errors.map((e) => e.setting))]; + + for (const setting of uniqueSettings) { + const settingErrors = errors.filter((e) => e.setting === setting); + const firstError = settingErrors[0]; + + let message: string; + let settingKey: string; + + switch (setting) { + case 'pythonProjects': + message = l10n.t( + "Python project setting for environment manager '{0}' could not be applied: {1}", + firstError.configuredValue, + firstError.reason, + ); + settingKey = 'python-envs.pythonProjects'; + break; + case 'defaultEnvManager': + message = l10n.t( + "Default environment manager '{0}' could not be applied: {1}", + firstError.configuredValue, + firstError.reason, + ); + settingKey = 'python-envs.defaultEnvManager'; + break; + case 'defaultInterpreterPath': + message = l10n.t( + "Default interpreter path '{0}' could not be resolved: {1}", + firstError.configuredValue, + firstError.reason, + ); + settingKey = 'python.defaultInterpreterPath'; + break; + default: + continue; + } + + const openSettings = l10n.t('Open Settings'); + const result = await showWarningMessage(message, openSettings); + if (result === openSettings) { + await commands.executeCommand('workbench.action.openSettings', settingKey); + } + } +} + +/** + * Extract the pythonProjects[] setting lookup into a dedicated function. + * Returns the manager ID if found in pythonProjects[] for the given scope, else undefined. + */ +function getProjectSpecificEnvManager(projectManager: PythonProjectManager, scope: Uri): string | undefined { + const config = getConfiguration('python-envs', scope); + const overrides = config.get('pythonProjects', []); + + if (overrides.length > 0) { + const pw = projectManager.get(scope); + const w = workspace.getWorkspaceFolder(scope); + if (pw && w) { + const pwPath = path.normalize(pw.uri.fsPath); + const matching = overrides.find((s) => path.resolve(w.uri.fsPath, s.path) === pwPath); + if (matching && matching.envManager && matching.envManager.length > 0) { + return matching.envManager; + } + } + } + return undefined; +} + +/** + * Try to resolve an interpreter path via nativeFinder and return a PriorityChainResult. + * Returns undefined if resolution fails. + */ +async function tryResolveInterpreterPath( + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, + interpreterPath: string, + envManagers: EnvironmentManagers, +): Promise { + try { + const resolved: NativeEnvInfo = await nativeFinder.resolve(interpreterPath); + + if (resolved && resolved.executable) { + const resolvedEnv = await api.resolveEnvironment(Uri.file(resolved.executable)); + + // Find the appropriate manager - prefer the one from the resolved env, fall back to system + let manager = envManagers.managers.find((m) => m.id === resolvedEnv?.envId.managerId); + if (!manager) { + manager = envManagers.getEnvironmentManager(SYSTEM_MANAGER_ID); + } + + if (manager && resolvedEnv) { + // Create a wrapper environment that uses the user's specified path + const newEnv: PythonEnvironment = { + envId: { + id: `defaultInterpreterPath:${interpreterPath}`, + managerId: manager.id, + }, + name: 'defaultInterpreterPath: ' + (resolved.version ?? ''), + displayName: 'defaultInterpreterPath: ' + (resolved.version ?? ''), + version: resolved.version ?? '', + displayPath: interpreterPath, + environmentPath: Uri.file(interpreterPath), + sysPrefix: resolved.prefix ?? '', + execInfo: { + run: { + executable: interpreterPath, + }, + }, + }; + return { manager, environment: newEnv, source: 'defaultInterpreterPath' }; + } + } + } catch (err) { + traceWarn(`[tryResolveInterpreterPath] Error resolving interpreter path: ${err}`); + } + return undefined; +} + +/** + * Register a configuration change listener for interpreter-related settings. When relevant settings change (defaultInterpreterPath, defaultEnvManager, pythonProjects), + * re-run the priority chain to apply the new settings immediately. + */ +export function registerInterpreterSettingsChangeListener( + envManagers: EnvironmentManagers, + projectManager: PythonProjectManager, + nativeFinder: NativePythonFinder, + api: PythonEnvironmentApi, +): Disposable { + return onDidChangeConfiguration(async (e: ConfigurationChangeEvent) => { + const relevantSettingsChanged = + e.affectsConfiguration('python.defaultInterpreterPath') || + e.affectsConfiguration('python-envs.defaultEnvManager') || + e.affectsConfiguration('python-envs.pythonProjects'); + + if (relevantSettingsChanged) { + traceInfo( + '[interpreterSelection] Interpreter settings changed, re-evaluating priority chain for all scopes', + ); + // Re-run the interpreter selection priority chain to apply new settings immediately + await applyInitialEnvironmentSelection(envManagers, projectManager, nativeFinder, api); + } + }); +} diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 4f571c42..10d484a1 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -147,29 +147,10 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr packageManager: e.packageManager, }); } else { - // Only write settings if: - // 1. There's already an explicit setting (we're updating it), OR - // 2. The new value is not the implicit fallback (system manager is the fallback) - const isSystemManager = e.envManager === 'ms-python.python:system'; - const envManagerInspect = config.inspect('defaultEnvManager'); - const hasExplicitEnvManager = - envManagerInspect?.workspaceFolderValue !== undefined || - envManagerInspect?.workspaceValue !== undefined; - - // Write if changing an existing setting, OR if setting to non-system manager - if ((hasExplicitEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) { + if (config.get('defaultEnvManager') !== e.envManager) { promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace)); } - - const pkgManagerInspect = config.inspect('defaultPackageManager'); - const hasExplicitPkgManager = - pkgManagerInspect?.workspaceFolderValue !== undefined || - pkgManagerInspect?.workspaceValue !== undefined; - // For package manager, write if there's an explicit setting OR if env manager is being written - if ( - (hasExplicitPkgManager || !isSystemManager) && - config.get('defaultPackageManager') !== e.packageManager - ) { + if (config.get('defaultPackageManager') !== e.packageManager) { promises.push( config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace), ); @@ -196,23 +177,10 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr edits .filter((e) => !e.project) .forEach((e) => { - // Only write global settings if: - // 1. There's already an explicit global setting (we're updating it), OR - // 2. The new value is not the implicit fallback (system manager) - const isSystemManager = e.envManager === 'ms-python.python:system'; - const envManagerInspect = config.inspect('defaultEnvManager'); - const hasExplicitGlobalEnvManager = envManagerInspect?.globalValue !== undefined; - - if ((hasExplicitGlobalEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) { + if (config.get('defaultEnvManager') !== e.envManager) { promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); } - - const pkgManagerInspect = config.inspect('defaultPackageManager'); - const hasExplicitGlobalPkgManager = pkgManagerInspect?.globalValue !== undefined; - if ( - (hasExplicitGlobalPkgManager || !isSystemManager) && - config.get('defaultPackageManager') !== e.packageManager - ) { + if (config.get('defaultPackageManager') !== e.packageManager) { promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); } }); @@ -266,14 +234,8 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr if (index >= 0) { overrides[index].envManager = e.envManager; projectsModified = true; - } else { - // Only write settings if updating existing OR setting non-system manager - const isSystemManager = e.envManager === 'ms-python.python:system'; - const envManagerInspect = config.inspect('defaultEnvManager'); - const hasExplicitEnvManager = envManagerInspect?.workspaceValue !== undefined; - if ((hasExplicitEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) { - promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace)); - } + } else if (config.get('defaultEnvManager') !== e.envManager) { + promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace)); } }); @@ -289,11 +251,7 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr edits .filter((e) => !e.project) .forEach((e) => { - // Only write global settings if updating existing OR setting non-system manager - const isSystemManager = e.envManager === 'ms-python.python:system'; - const envManagerInspect = config.inspect('defaultEnvManager'); - const hasExplicitGlobalEnvManager = envManagerInspect?.globalValue !== undefined; - if ((hasExplicitGlobalEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) { + if (config.get('defaultEnvManager') !== e.envManager) { promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); } }); @@ -350,19 +308,8 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr if (index >= 0) { overrides[index].packageManager = e.packageManager; projectsModified = true; - } else { - // Only write settings if updating existing OR setting non-default package manager - const isPipManager = e.packageManager === 'ms-python.python:pip'; - const pkgManagerInspect = config.inspect('defaultPackageManager'); - const hasExplicitPkgManager = pkgManagerInspect?.workspaceValue !== undefined; - if ( - (hasExplicitPkgManager || !isPipManager) && - config.get('defaultPackageManager') !== e.packageManager - ) { - promises.push( - config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace), - ); - } + } else if (config.get('defaultPackageManager') !== e.packageManager) { + promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace)); } }); @@ -378,14 +325,7 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr edits .filter((e) => !e.project) .forEach((e) => { - // Only write global settings if updating existing OR setting non-default package manager - const isPipManager = e.packageManager === 'ms-python.python:pip'; - const pkgManagerInspect = config.inspect('defaultPackageManager'); - const hasExplicitGlobalPkgManager = pkgManagerInspect?.globalValue !== undefined; - if ( - (hasExplicitGlobalPkgManager || !isPipManager) && - config.get('defaultPackageManager') !== e.packageManager - ) { + if (config.get('defaultPackageManager') !== e.packageManager) { promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); } }); diff --git a/src/helpers.ts b/src/helpers.ts index 6ccfb7bd..19655289 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -1,6 +1,6 @@ import { ExtensionContext, extensions, QuickInputButtons, Uri, window, workspace } from 'vscode'; -import { PythonEnvironment, PythonEnvironmentApi } from './api'; -import { traceError, traceInfo, traceWarn } from './common/logging'; +import { PythonEnvironment } from './api'; +import { traceInfo } from './common/logging'; import { isWindows } from './common/utils/platformUtils'; import { createTerminal, showInputBoxWithButtons } from './common/window.apis'; import { getConfiguration } from './common/workspace.apis'; @@ -9,7 +9,7 @@ import { identifyTerminalShell } from './features/common/shellDetector'; import { quoteArgs } from './features/execution/execUtils'; import { getAutoActivationType } from './features/terminal/utils'; import { EnvironmentManagers, PythonProjectManager } from './internal.api'; -import { getNativePythonToolsPath, NativeEnvInfo, NativePythonFinder } from './managers/common/nativePythonFinder'; +import { getNativePythonToolsPath } from './managers/common/nativePythonFinder'; /** * Collects relevant Python environment information for issue reporting @@ -122,9 +122,12 @@ export function getEnvManagerAndPackageManagerConfigLevels() { /** * Returns the user-configured value for a configuration key if set at any level (workspace folder, workspace, or global), * otherwise returns undefined. + * @param section - The configuration section (e.g., 'python', 'python-envs') + * @param key - The configuration key within the section + * @param scope - Optional scope (Uri) to get workspace-folder-specific settings */ -export function getUserConfiguredSetting(section: string, key: string): T | undefined { - const config = getConfiguration(section); +export function getUserConfiguredSetting(section: string, key: string, scope?: Uri): T | undefined { + const config = getConfiguration(section, scope); const inspect = config.inspect(key); if (!inspect) { return undefined; @@ -247,69 +250,3 @@ export async function runPetInTerminalImpl(): Promise { } } } - -/** - * Sets the default Python interpreter for the workspace if the user has not explicitly set 'defaultEnvManager'. - * @param nativeFinder - used to resolve interpreter paths. - * @param envManagers - contains all registered managers. - * @param api - The PythonEnvironmentApi for environment resolution and setting. - */ -export async function resolveDefaultInterpreter( - nativeFinder: NativePythonFinder, - envManagers: EnvironmentManagers, - api: PythonEnvironmentApi, -) { - const userSetdefaultInterpreter = getUserConfiguredSetting('python', 'defaultInterpreterPath'); - const userSetDefaultManager = getUserConfiguredSetting('python-envs', 'defaultEnvManager'); - traceInfo( - `[resolveDefaultInterpreter] User configured defaultInterpreterPath: ${userSetdefaultInterpreter} and defaultEnvManager: ${userSetDefaultManager}`, - ); - - // Only proceed if the user has explicitly set defaultInterpreterPath but nothing is saved for defaultEnvManager - if (userSetdefaultInterpreter && !userSetDefaultManager) { - try { - const resolved: NativeEnvInfo = await nativeFinder.resolve(userSetdefaultInterpreter); - if (resolved && resolved.executable) { - const resolvedEnv = await api.resolveEnvironment(Uri.file(resolved.executable)); - traceInfo(`[resolveDefaultInterpreter] API resolved environment: ${JSON.stringify(resolvedEnv)}`); - - let findEnvManager = envManagers.managers.find((m) => m.id === resolvedEnv?.envId.managerId); - if (!findEnvManager) { - findEnvManager = envManagers.managers.find((m) => m.id === 'ms-python.python:system'); - } - const randomString = Math.random().toString(36).substring(2, 15); - if (resolvedEnv) { - const newEnv: PythonEnvironment = { - envId: { - id: `${userSetdefaultInterpreter}_${randomString}`, - managerId: resolvedEnv?.envId.managerId ?? '', - }, - name: 'defaultInterpreterPath: ' + (resolved.version ?? ''), - displayName: 'defaultInterpreterPath: ' + (resolved.version ?? ''), - version: resolved.version ?? '', - displayPath: userSetdefaultInterpreter ?? '', - environmentPath: userSetdefaultInterpreter ? Uri.file(userSetdefaultInterpreter) : Uri.file(''), - sysPrefix: resolved.prefix ?? '', - execInfo: { - run: { - executable: userSetdefaultInterpreter ?? '', - }, - }, - }; - if (workspace.workspaceFolders?.[0] && findEnvManager) { - traceInfo( - `[resolveDefaultInterpreter] Setting environment for workspace: ${workspace.workspaceFolders[0].uri.fsPath}`, - ); - await api.setEnvironment(workspace.workspaceFolders[0].uri, newEnv); - } - } - } else { - traceWarn( - `[resolveDefaultInterpreter] NativeFinder did not resolve an executable for path: ${userSetdefaultInterpreter}`, - ); - } - } catch (err) { - traceError(`[resolveDefaultInterpreter] Error resolving default interpreter: ${err}`); - } - } -} diff --git a/src/internal.api.ts b/src/internal.api.ts index ce258e31..2573a6ee 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -109,8 +109,28 @@ export interface EnvironmentManagers extends Disposable { clearCache(scope: EnvironmentManagerScope): Promise; - setEnvironment(scope: SetEnvironmentScope, environment?: PythonEnvironment): Promise; - setEnvironments(scope: Uri[] | string, environment?: PythonEnvironment): Promise; + /** + * Sets the environment for a scope. + * @param scope - The scope to set the environment for + * @param environment - The environment to set (optional) + * @param shouldPersistSettings - Whether to persist to settings.json (default: true) + */ + setEnvironment( + scope: SetEnvironmentScope, + environment?: PythonEnvironment, + shouldPersistSettings?: boolean, + ): Promise; + /** + * Sets environments for multiple scopes. + * @param scope - Array of URIs or 'global' + * @param environment - The environment to set (optional) + * @param shouldPersistSettings - Whether to persist to settings.json (default: true) + */ + setEnvironments( + scope: Uri[] | string, + environment?: PythonEnvironment, + shouldPersistSettings?: boolean, + ): Promise; setEnvironmentsIfUnset(scope: Uri[] | string, environment?: PythonEnvironment): Promise; getEnvironment(scope: GetEnvironmentScope): Promise; @@ -118,7 +138,10 @@ export interface EnvironmentManagers extends Disposable { } export class InternalEnvironmentManager implements EnvironmentManager { - public constructor(public readonly id: string, private readonly manager: EnvironmentManager) {} + public constructor( + public readonly id: string, + private readonly manager: EnvironmentManager, + ) {} public get name(): string { return this.manager.name; @@ -223,7 +246,10 @@ export class InternalEnvironmentManager implements EnvironmentManager { } export class InternalPackageManager implements PackageManager { - public constructor(public readonly id: string, private readonly manager: PackageManager) {} + public constructor( + public readonly id: string, + private readonly manager: PackageManager, + ) {} public get name(): string { return this.manager.name; @@ -313,7 +339,10 @@ export class PythonEnvironmentImpl implements PythonEnvironment { public readonly sysPrefix: string; public readonly group?: string | EnvironmentGroupInfo; - constructor(public readonly envId: PythonEnvironmentId, info: PythonEnvironmentInfo) { + constructor( + public readonly envId: PythonEnvironmentId, + info: PythonEnvironmentInfo, + ) { this.name = info.name; this.displayName = info.displayName ?? this.name; this.shortDisplayName = info.shortDisplayName; @@ -338,7 +367,10 @@ export class PythonPackageImpl implements Package { public readonly iconPath?: IconPath; public readonly uris?: readonly Uri[]; - constructor(public readonly pkgId: PackageId, info: PackageInfo) { + constructor( + public readonly pkgId: PackageId, + info: PackageInfo, + ) { this.name = info.name; this.displayName = info.displayName ?? this.name; this.version = info.version; diff --git a/src/test/features/interpreterSelection.unit.test.ts b/src/test/features/interpreterSelection.unit.test.ts new file mode 100644 index 00000000..9e264e58 --- /dev/null +++ b/src/test/features/interpreterSelection.unit.test.ts @@ -0,0 +1,1410 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { ConfigurationChangeEvent, Uri, workspace, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; +import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; +import * as workspaceApis from '../../common/workspace.apis'; +import { + applyInitialEnvironmentSelection, + registerInterpreterSettingsChangeListener, + resolveEnvironmentByPriority, + resolveGlobalEnvironmentByPriority, +} from '../../features/interpreterSelection'; +import * as helpers from '../../helpers'; +import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api'; +import { NativePythonFinder } from '../../managers/common/nativePythonFinder'; + +/** + * Creates a mock WorkspaceConfiguration for testing. + */ +function createMockConfig(pythonProjects: unknown[] = []): Partial { + return { + get: (key: string) => { + if (key === 'pythonProjects') { + return pythonProjects; + } + return undefined; + }, + }; +} + +suite('Interpreter Selection - Priority Chain', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockProjectManager: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + + const testUri = Uri.file('/test/workspace'); + const mockVenvEnv: PythonEnvironment = { + envId: { id: 'venv-env-1', managerId: 'ms-python.python:venv' }, + name: 'Test Venv', + displayName: 'Test Venv', + version: '3.11.0', + displayPath: '/test/workspace/.venv', + environmentPath: Uri.file('/test/workspace/.venv'), + sysPrefix: '/test/workspace/.venv', + execInfo: { run: { executable: '/test/workspace/.venv/bin/python' } }, + }; + const mockSystemEnv: PythonEnvironment = { + envId: { id: 'system-env-1', managerId: 'ms-python.python:system' }, + name: 'System Python', + displayName: 'System Python 3.11', + version: '3.11.0', + displayPath: '/usr/bin/python3.11', + environmentPath: Uri.file('/usr/bin/python3.11'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.11' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + // Create mock managers + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager], + } as unknown as sinon.SinonStubbedInstance; + + mockProjectManager = { + get: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + // Default: getEnvironmentManager returns the manager for a given ID + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + suite('Priority 1: pythonProjects[]', () => { + test('should use manager from pythonProjects[] when configured', async () => { + // Setup: pythonProjects[] has a venv manager configured + sandbox.stub(workspace, 'getConfiguration').returns( + createMockConfig([ + { + path: '/test/workspace', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ]) as WorkspaceConfiguration, + ); + const mockProject: Partial = { uri: testUri, name: 'test' }; + mockProjectManager.get.returns(mockProject as PythonProject); + const mockWorkspaceFolder: Partial = { uri: testUri }; + sandbox.stub(workspace, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'pythonProjects'); + assert.strictEqual(result.manager.id, 'ms-python.python:venv'); + }); + }); + + suite('Priority 2: User-configured defaultEnvManager', () => { + test('should use user-configured defaultEnvManager when set', async () => { + // Setup: No pythonProjects[], but user configured defaultEnvManager + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:venv'; + } + return undefined; + }); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'defaultEnvManager'); + assert.strictEqual(result.manager.id, 'ms-python.python:venv'); + }); + + test('should skip to Priority 3 when defaultEnvManager is not user-configured (only fallback)', async () => { + // Setup: No pythonProjects[], no user-configured defaultEnvManager (returns undefined) + // But there IS a user-configured defaultInterpreterPath + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/usr/bin/python3.11'; + } + return undefined; // defaultEnvManager NOT user-configured + }); + mockNativeFinder.resolve.resolves({ executable: '/usr/bin/python3.11', version: '3.11.0', prefix: '/usr' }); + mockApi.resolveEnvironment.resolves(mockSystemEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'defaultInterpreterPath'); + }); + }); + + suite('Priority 3: python.defaultInterpreterPath', () => { + test('should use defaultInterpreterPath when set and resolvable', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/usr/bin/python3.11'; + } + return undefined; + }); + mockNativeFinder.resolve.resolves({ executable: '/usr/bin/python3.11', version: '3.11.0', prefix: '/usr' }); + mockApi.resolveEnvironment.resolves(mockSystemEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'defaultInterpreterPath'); + assert.ok(result.environment); + assert.strictEqual(result.environment.displayPath, '/usr/bin/python3.11'); + }); + + test('should fall through to Priority 4 when defaultInterpreterPath cannot be resolved', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/nonexistent/python'; + } + return undefined; + }); + mockNativeFinder.resolve.rejects(new Error('Not found')); + mockVenvManager.get.resolves(mockVenvEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'autoDiscovery'); + }); + + test('should use original user path even when nativeFinder resolves to a different executable', async () => { + // This test covers the scenario where user configures a pyenv path but + // nativeFinder resolves to a different path (e.g., homebrew python due to symlinks) + const userPyenvPath = '/Users/test/.pyenv/versions/3.13.7/bin/python'; + const resolvedHomebrewPath = '/opt/homebrew/bin/python3'; + + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return userPyenvPath; + } + return undefined; + }); + + // Native finder resolves to a DIFFERENT path (e.g., following symlinks) + mockNativeFinder.resolve.resolves({ + executable: resolvedHomebrewPath, + version: '3.14.2', + prefix: '/opt/homebrew', + }); + + // API resolves the homebrew path to a homebrew environment + const homebrewEnv: PythonEnvironment = { + envId: { id: 'homebrew-env', managerId: 'ms-python.python:system' }, + name: 'Homebrew Python', + displayName: 'Python 3.14.2 (homebrew)', + version: '3.14.2', + displayPath: resolvedHomebrewPath, + environmentPath: Uri.file(resolvedHomebrewPath), + sysPrefix: '/opt/homebrew', + execInfo: { run: { executable: resolvedHomebrewPath } }, + }; + mockApi.resolveEnvironment.resolves(homebrewEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should still use defaultInterpreterPath source + assert.strictEqual(result.source, 'defaultInterpreterPath'); + assert.ok(result.environment); + + // The environment should use the USER's original path, not the resolved one + assert.strictEqual( + result.environment.displayPath, + userPyenvPath, + 'displayPath should use user configured path', + ); + assert.strictEqual( + result.environment.execInfo?.run?.executable, + userPyenvPath, + 'executable should use user configured path', + ); + assert.strictEqual( + result.environment.environmentPath?.fsPath, + userPyenvPath, + 'environmentPath should use user configured path', + ); + }); + }); + + suite('Priority 4: Auto-discovery', () => { + test('should use local venv when found', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + mockVenvManager.get.resolves(mockVenvEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'autoDiscovery'); + assert.strictEqual(result.manager.id, 'ms-python.python:venv'); + assert.strictEqual(result.environment, mockVenvEnv); + }); + + test('should fall back to system manager when no local venv', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + mockVenvManager.get.resolves(undefined); // No local venv + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'autoDiscovery'); + assert.strictEqual(result.manager.id, 'ms-python.python:system'); + }); + + test('should throw error when no managers are available', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // Create mock with no managers + const emptyEnvManagers = { + getEnvironmentManager: sandbox.stub().returns(undefined), + setEnvironment: sandbox.stub().resolves(), + managers: [], + } as unknown as sinon.SinonStubbedInstance; + + await assert.rejects( + async () => + resolveEnvironmentByPriority( + testUri, + emptyEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ), + /No environment managers available/, + ); + }); + }); + + suite('Edge Cases', () => { + test('should fall through when nativeFinder resolves but returns no executable', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/some/path/python'; + } + return undefined; + }); + // nativeFinder resolves but with undefined executable + mockNativeFinder.resolve.resolves({ executable: undefined, version: '3.11.0', prefix: '/usr' }); + mockVenvManager.get.resolves(mockVenvEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should fall through to auto-discovery since resolution failed + assert.strictEqual(result.source, 'autoDiscovery'); + }); + + test('should fall through when api.resolveEnvironment returns undefined', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/usr/bin/python3.11'; + } + return undefined; + }); + mockNativeFinder.resolve.resolves({ executable: '/usr/bin/python3.11', version: '3.11.0', prefix: '/usr' }); + mockApi.resolveEnvironment.resolves(undefined); // API returns undefined + mockVenvManager.get.resolves(mockVenvEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should fall through to auto-discovery + assert.strictEqual(result.source, 'autoDiscovery'); + }); + + test('should use first available manager when venv and system managers not found', async () => { + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const mockCondaManager = { + id: 'ms-python.python:conda', + name: 'conda', + displayName: 'Conda', + get: sandbox.stub().resolves(undefined), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + // Only conda manager available, no venv or system + const condaOnlyEnvManagers = { + getEnvironmentManager: sandbox.stub().returns(undefined), + setEnvironment: sandbox.stub().resolves(), + managers: [mockCondaManager], + } as unknown as sinon.SinonStubbedInstance; + + const result = await resolveEnvironmentByPriority( + testUri, + condaOnlyEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'autoDiscovery'); + assert.strictEqual(result.manager.id, 'ms-python.python:conda'); + }); + }); +}); + +suite('Interpreter Selection - applyInitialEnvironmentSelection', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockProjectManager: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + + const testUri = Uri.file('/test/workspace'); + const mockVenvEnv: PythonEnvironment = { + envId: { id: 'venv-env-1', managerId: 'ms-python.python:venv' }, + name: 'Test Venv', + displayName: 'Test Venv', + version: '3.11.0', + displayPath: '/test/workspace/.venv', + environmentPath: Uri.file('/test/workspace/.venv'), + sysPrefix: '/test/workspace/.venv', + execInfo: { run: { executable: '/test/workspace/.venv/bin/python' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub().resolves(mockVenvEnv), + set: sandbox.stub().resolves(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + setEnvironments: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager], + } as unknown as sinon.SinonStubbedInstance; + + mockProjectManager = { + get: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + test('should call setEnvironment with shouldPersistSettings=false', async () => { + sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Verify setEnvironment was called with shouldPersistSettings=false + assert.ok(mockEnvManagers.setEnvironment.called); + const callArgs = mockEnvManagers.setEnvironment.firstCall.args; + assert.strictEqual(callArgs[2], false, 'shouldPersistSettings should be false'); + }); + + test('should process all workspace folders', async () => { + const uri1 = Uri.file('/workspace1'); + const uri2 = Uri.file('/workspace2'); + sandbox.stub(workspace, 'workspaceFolders').value([ + { uri: uri1, name: 'workspace1', index: 0 }, + { uri: uri2, name: 'workspace2', index: 1 }, + ]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should be called once for each workspace folder + assert.strictEqual(mockEnvManagers.setEnvironment.callCount, 2); + }); + + test('should also set global environment when no workspace folders', async () => { + sandbox.stub(workspace, 'workspaceFolders').value([]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should call setEnvironments for global scope + assert.ok(mockEnvManagers.setEnvironments.called, 'setEnvironments should be called for global scope'); + const callArgs = mockEnvManagers.setEnvironments.firstCall.args; + assert.strictEqual(callArgs[0], 'global', 'First arg should be "global"'); + assert.strictEqual(callArgs[2], false, 'shouldPersistSettings should be false'); + }); +}); + +suite('Interpreter Selection - resolveGlobalEnvironmentByPriority', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + + const mockSystemEnv: PythonEnvironment = { + envId: { id: 'system-env-1', managerId: 'ms-python.python:system' }, + name: 'System Python', + displayName: 'System Python 3.11', + version: '3.11.0', + displayPath: '/usr/bin/python3.11', + environmentPath: Uri.file('/usr/bin/python3.11'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.11' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + setEnvironments: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager], + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + if (id === undefined) { + return mockSystemManager; // Default manager for global scope + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + test('should use user-configured defaultEnvManager for global scope', async () => { + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:venv'; + } + return undefined; + }); + + const result = await resolveGlobalEnvironmentByPriority( + mockEnvManagers as unknown as EnvironmentManagers, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'defaultEnvManager'); + assert.strictEqual(result.manager.id, 'ms-python.python:venv'); + }); + + test('should use defaultInterpreterPath for global scope when configured', async () => { + const userPyenvPath = '/Users/test/.pyenv/versions/3.13.7/bin/python'; + + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return userPyenvPath; + } + return undefined; + }); + + mockNativeFinder.resolve.resolves({ + executable: userPyenvPath, + version: '3.13.7', + prefix: '/Users/test/.pyenv/versions/3.13.7', + }); + mockApi.resolveEnvironment.resolves(mockSystemEnv); + + const result = await resolveGlobalEnvironmentByPriority( + mockEnvManagers as unknown as EnvironmentManagers, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'defaultInterpreterPath'); + assert.ok(result.environment); + assert.strictEqual(result.environment.displayPath, userPyenvPath); + assert.strictEqual(result.environment.execInfo?.run?.executable, userPyenvPath); + }); + + test('should use original user path for global scope even when nativeFinder resolves to different executable', async () => { + // This is the key bug fix test - user configures pyenv path, native finder returns homebrew + const userPyenvPath = '/Users/test/.pyenv/versions/3.13.7/bin/python'; + const resolvedHomebrewPath = '/opt/homebrew/bin/python3'; + + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return userPyenvPath; + } + return undefined; + }); + + // Native finder resolves to a DIFFERENT path (e.g., following symlinks) + mockNativeFinder.resolve.resolves({ + executable: resolvedHomebrewPath, + version: '3.14.2', + prefix: '/opt/homebrew', + }); + + // API resolves the homebrew path to a homebrew environment + const homebrewEnv: PythonEnvironment = { + envId: { id: 'homebrew-env', managerId: 'ms-python.python:system' }, + name: 'Homebrew Python', + displayName: 'Python 3.14.2 (homebrew)', + version: '3.14.2', + displayPath: resolvedHomebrewPath, + environmentPath: Uri.file(resolvedHomebrewPath), + sysPrefix: '/opt/homebrew', + execInfo: { run: { executable: resolvedHomebrewPath } }, + }; + mockApi.resolveEnvironment.resolves(homebrewEnv); + + const result = await resolveGlobalEnvironmentByPriority( + mockEnvManagers as unknown as EnvironmentManagers, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should still use defaultInterpreterPath source + assert.strictEqual(result.source, 'defaultInterpreterPath'); + assert.ok(result.environment); + + // The environment should use the USER's original path, not the resolved one + assert.strictEqual( + result.environment.displayPath, + userPyenvPath, + 'displayPath should use user configured path', + ); + assert.strictEqual( + result.environment.execInfo?.run?.executable, + userPyenvPath, + 'executable should use user configured path', + ); + }); + + test('should fall back to system manager when no user settings for global scope', async () => { + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const result = await resolveGlobalEnvironmentByPriority( + mockEnvManagers as unknown as EnvironmentManagers, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.strictEqual(result.source, 'autoDiscovery'); + assert.strictEqual(result.manager.id, 'ms-python.python:system'); + }); +}); + +suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockProjectManager: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + + const testUri = Uri.file('/test/workspace'); + const mockVenvEnv: PythonEnvironment = { + envId: { id: 'venv-env-1', managerId: 'ms-python.python:venv' }, + name: 'Test Venv', + displayName: 'Test Venv', + version: '3.11.0', + displayPath: '/test/workspace/.venv', + environmentPath: Uri.file('/test/workspace/.venv'), + sysPrefix: '/test/workspace/.venv', + execInfo: { run: { executable: '/test/workspace/.venv/bin/python' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub().resolves(mockVenvEnv), + set: sandbox.stub().resolves(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + setEnvironments: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager], + } as unknown as sinon.SinonStubbedInstance; + + mockProjectManager = { + get: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + test('should re-run priority chain when python.defaultInterpreterPath changes', async () => { + // Capture the callback registered with onDidChangeConfiguration + let configChangeCallback: ((e: ConfigurationChangeEvent) => void) | undefined; + sandbox.stub(workspaceApis, 'onDidChangeConfiguration').callsFake((callback) => { + configChangeCallback = callback; + return { dispose: () => {} }; + }); + + sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // Register the listener + const disposable = registerInterpreterSettingsChangeListener( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.ok(configChangeCallback, 'Config change callback should be registered'); + + // Simulate a configuration change for defaultInterpreterPath + const mockEvent: ConfigurationChangeEvent = { + affectsConfiguration: (section: string) => section === 'python.defaultInterpreterPath', + }; + + // Reset the call count before triggering the change + mockEnvManagers.setEnvironment.resetHistory(); + mockEnvManagers.setEnvironments.resetHistory(); + + // Trigger the configuration change + await configChangeCallback(mockEvent); + + // Verify that applyInitialEnvironmentSelection was called (which calls setEnvironment/setEnvironments) + assert.ok( + mockEnvManagers.setEnvironment.called || mockEnvManagers.setEnvironments.called, + 'Should re-run priority chain when defaultInterpreterPath changes', + ); + + disposable.dispose(); + }); + + test('should re-run priority chain when python-envs.defaultEnvManager changes', async () => { + let configChangeCallback: ((e: ConfigurationChangeEvent) => void) | undefined; + sandbox.stub(workspaceApis, 'onDidChangeConfiguration').callsFake((callback) => { + configChangeCallback = callback; + return { dispose: () => {} }; + }); + + sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const disposable = registerInterpreterSettingsChangeListener( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.ok(configChangeCallback, 'Config change callback should be registered'); + + const mockEvent: ConfigurationChangeEvent = { + affectsConfiguration: (section: string) => section === 'python-envs.defaultEnvManager', + }; + + mockEnvManagers.setEnvironment.resetHistory(); + mockEnvManagers.setEnvironments.resetHistory(); + + await configChangeCallback(mockEvent); + + assert.ok( + mockEnvManagers.setEnvironment.called || mockEnvManagers.setEnvironments.called, + 'Should re-run priority chain when defaultEnvManager changes', + ); + + disposable.dispose(); + }); + + test('should re-run priority chain when python-envs.pythonProjects changes', async () => { + let configChangeCallback: ((e: ConfigurationChangeEvent) => void) | undefined; + sandbox.stub(workspaceApis, 'onDidChangeConfiguration').callsFake((callback) => { + configChangeCallback = callback; + return { dispose: () => {} }; + }); + + sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const disposable = registerInterpreterSettingsChangeListener( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.ok(configChangeCallback, 'Config change callback should be registered'); + + const mockEvent: ConfigurationChangeEvent = { + affectsConfiguration: (section: string) => section === 'python-envs.pythonProjects', + }; + + mockEnvManagers.setEnvironment.resetHistory(); + mockEnvManagers.setEnvironments.resetHistory(); + + await configChangeCallback(mockEvent); + + assert.ok( + mockEnvManagers.setEnvironment.called || mockEnvManagers.setEnvironments.called, + 'Should re-run priority chain when pythonProjects changes', + ); + + disposable.dispose(); + }); + + test('should NOT re-run priority chain when unrelated configuration changes', async () => { + let configChangeCallback: ((e: ConfigurationChangeEvent) => void) | undefined; + sandbox.stub(workspaceApis, 'onDidChangeConfiguration').callsFake((callback) => { + configChangeCallback = callback; + return { dispose: () => {} }; + }); + + sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const disposable = registerInterpreterSettingsChangeListener( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.ok(configChangeCallback, 'Config change callback should be registered'); + + // Simulate a configuration change for an unrelated setting + const mockEvent: ConfigurationChangeEvent = { + affectsConfiguration: (section: string) => section === 'editor.fontSize', + }; + + mockEnvManagers.setEnvironment.resetHistory(); + mockEnvManagers.setEnvironments.resetHistory(); + + await configChangeCallback(mockEvent); + + assert.ok( + !mockEnvManagers.setEnvironment.called && !mockEnvManagers.setEnvironments.called, + 'Should NOT re-run priority chain when unrelated configuration changes', + ); + + disposable.dispose(); + }); +}); + +suite('Interpreter Selection - Settings over Cache Priority', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockProjectManager: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + let mockCondaManager: sinon.SinonStubbedInstance; + + const testUri = Uri.file('/test/workspace'); + const mockVenvEnv: PythonEnvironment = { + envId: { id: 'venv-env-1', managerId: 'ms-python.python:venv' }, + name: 'Test Venv', + displayName: 'Test Venv', + version: '3.11.0', + displayPath: '/test/workspace/.venv', + environmentPath: Uri.file('/test/workspace/.venv'), + sysPrefix: '/test/workspace/.venv', + execInfo: { run: { executable: '/test/workspace/.venv/bin/python' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub().resolves(mockVenvEnv), + set: sandbox.stub().resolves(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockCondaManager = { + id: 'ms-python.python:conda', + name: 'conda', + displayName: 'Conda', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + setEnvironments: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager, mockCondaManager], + } as unknown as sinon.SinonStubbedInstance; + + mockProjectManager = { + get: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + if (id === 'ms-python.python:conda') { + return mockCondaManager; + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + test('should use user-configured defaultEnvManager even when cache has different manager', async () => { + // This test verifies settings take priority over cache + // Setup: User has configured defaultEnvManager=conda, but cache has venv + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:conda'; // User wants conda + } + return undefined; + }); + + // Even though the cache might have venv, the user's setting for conda should be respected + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // The result should use the user's configured manager (conda), not the cached one + assert.strictEqual(result.source, 'defaultEnvManager'); + assert.strictEqual(result.manager.id, 'ms-python.python:conda'); + }); + + test('should use pythonProjects manager even when defaultEnvManager is set', async () => { + // This test verifies pythonProjects[] has highest priority (Priority 1 over Priority 2) + sandbox.stub(workspace, 'getConfiguration').returns( + createMockConfig([ + { + path: '/test/workspace', + envManager: 'ms-python.python:venv', // Project says venv + packageManager: 'ms-python.python:pip', + }, + ]) as WorkspaceConfiguration, + ); + const mockProject: Partial = { uri: testUri, name: 'test' }; + mockProjectManager.get.returns(mockProject as PythonProject); + const mockWorkspaceFolder: Partial = { uri: testUri }; + sandbox.stub(workspace, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); + + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:conda'; // User's default is conda + } + return undefined; + }); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // pythonProjects[] (venv) should take priority over defaultEnvManager (conda) + assert.strictEqual(result.source, 'pythonProjects'); + assert.strictEqual(result.manager.id, 'ms-python.python:venv'); + }); + + test('should fall back to auto-discovery when no user settings configured', async () => { + // When no settings are configured, cache from auto-discovery should be used + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should fall through to auto-discovery + assert.strictEqual(result.source, 'autoDiscovery'); + }); +}); + +suite('Interpreter Selection - Multi-Root Workspace', () => { + let sandbox: sinon.SinonSandbox; + let mockEnvManagers: sinon.SinonStubbedInstance; + let mockProjectManager: sinon.SinonStubbedInstance; + let mockNativeFinder: sinon.SinonStubbedInstance; + let mockApi: sinon.SinonStubbedInstance; + let mockVenvManager: sinon.SinonStubbedInstance; + let mockSystemManager: sinon.SinonStubbedInstance; + + const folder1Uri = Uri.file('/workspace/folder1'); + const folder2Uri = Uri.file('/workspace/folder2'); + + const mockVenvEnv1: PythonEnvironment = { + envId: { id: 'venv-env-folder1', managerId: 'ms-python.python:venv' }, + name: 'Folder1 Venv', + displayName: 'Folder1 Venv 3.11', + version: '3.11.0', + displayPath: '/workspace/folder1/.venv', + environmentPath: Uri.file('/workspace/folder1/.venv'), + sysPrefix: '/workspace/folder1/.venv', + execInfo: { run: { executable: '/workspace/folder1/.venv/bin/python' } }, + }; + + const mockVenvEnv2: PythonEnvironment = { + envId: { id: 'venv-env-folder2', managerId: 'ms-python.python:venv' }, + name: 'Folder2 Venv', + displayName: 'Folder2 Venv 3.12', + version: '3.12.0', + displayPath: '/workspace/folder2/.venv', + environmentPath: Uri.file('/workspace/folder2/.venv'), + sysPrefix: '/workspace/folder2/.venv', + execInfo: { run: { executable: '/workspace/folder2/.venv/bin/python' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + mockVenvManager = { + id: 'ms-python.python:venv', + name: 'venv', + displayName: 'Venv', + get: sandbox.stub(), + set: sandbox.stub().resolves(), + } as unknown as sinon.SinonStubbedInstance; + + mockSystemManager = { + id: 'ms-python.python:system', + name: 'system', + displayName: 'System', + get: sandbox.stub(), + set: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers = { + getEnvironmentManager: sandbox.stub(), + setEnvironment: sandbox.stub().resolves(), + setEnvironments: sandbox.stub().resolves(), + managers: [mockVenvManager, mockSystemManager], + } as unknown as sinon.SinonStubbedInstance; + + mockProjectManager = { + get: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockNativeFinder = { + resolve: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockApi = { + resolveEnvironment: sandbox.stub(), + } as unknown as sinon.SinonStubbedInstance; + + mockEnvManagers.getEnvironmentManager.callsFake((scope: unknown) => { + const id = typeof scope === 'string' ? scope : undefined; + if (id === 'ms-python.python:venv') { + return mockVenvManager; + } + if (id === 'ms-python.python:system') { + return mockSystemManager; + } + return undefined; + }); + }); + + teardown(() => { + sandbox.restore(); + }); + + test('each folder should get its own local venv in multi-root workspace', async () => { + // Setup: Two workspace folders, each with their own local venv + sandbox.stub(workspace, 'workspaceFolders').value([ + { uri: folder1Uri, name: 'folder1', index: 0 }, + { uri: folder2Uri, name: 'folder2', index: 1 }, + ]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // Venv manager returns different venvs for each folder + mockVenvManager.get.callsFake((scope: Uri | undefined) => { + if (scope?.fsPath === folder1Uri.fsPath) { + return Promise.resolve(mockVenvEnv1); + } + if (scope?.fsPath === folder2Uri.fsPath) { + return Promise.resolve(mockVenvEnv2); + } + return Promise.resolve(undefined); + }); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Verify setEnvironment was called twice - once for each folder + assert.strictEqual( + mockEnvManagers.setEnvironment.callCount, + 2, + 'setEnvironment should be called for each folder', + ); + + // Verify first folder got its own venv (not overwritten) + const firstCall = mockEnvManagers.setEnvironment.firstCall; + const firstUri = firstCall.args[0] as Uri; + assert.strictEqual(firstUri.fsPath, folder1Uri.fsPath, 'First call should be for folder1'); + assert.strictEqual(firstCall.args[1]?.envId.id, 'venv-env-folder1', 'Folder1 should get its own venv'); + + // Verify second folder got its own venv + const secondCall = mockEnvManagers.setEnvironment.secondCall; + const secondUri = secondCall.args[0] as Uri; + assert.strictEqual(secondUri.fsPath, folder2Uri.fsPath, 'Second call should be for folder2'); + assert.strictEqual(secondCall.args[1]?.envId.id, 'venv-env-folder2', 'Folder2 should get its own venv'); + + // Both should NOT persist to settings + assert.strictEqual(firstCall.args[2], false, 'First folder should not persist to settings'); + assert.strictEqual(secondCall.args[2], false, 'Second folder should not persist to settings'); + }); + + test('first folder venv should not be overwritten when second folder has no venv', async () => { + // This tests the scenario in issue #1145 where the first folder's venv gets overwritten + sandbox.stub(workspace, 'workspaceFolders').value([ + { uri: folder1Uri, name: 'folder1', index: 0 }, + { uri: folder2Uri, name: 'folder2', index: 1 }, + ]); + sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // First folder has a venv, second folder does not + mockVenvManager.get.callsFake((scope: Uri | undefined) => { + if (scope?.fsPath === folder1Uri.fsPath) { + return Promise.resolve(mockVenvEnv1); + } + // folder2 has no local venv + return Promise.resolve(undefined); + }); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Verify first folder still gets its own venv + const firstCall = mockEnvManagers.setEnvironment.firstCall; + const firstUri = firstCall.args[0] as Uri; + assert.strictEqual(firstUri.fsPath, folder1Uri.fsPath); + assert.strictEqual(firstCall.args[1]?.envId.id, 'venv-env-folder1', 'Folder1 venv should NOT be overwritten'); + + // Second folder falls back to system (no local venv) - this is correct behavior + const secondCall = mockEnvManagers.setEnvironment.secondCall; + const secondUri = secondCall.args[0] as Uri; + assert.strictEqual(secondUri.fsPath, folder2Uri.fsPath); + // The second folder should get undefined (system fallback) since it has no venv + }); + + test('multi-root workspace respects per-folder pythonProjects settings', async () => { + // Setup: Two folders with different pythonProjects[] settings + const mockProject1: Partial = { uri: folder1Uri, name: 'project1' }; + const mockProject2: Partial = { uri: folder2Uri, name: 'project2' }; + + sandbox.stub(workspace, 'workspaceFolders').value([ + { uri: folder1Uri, name: 'folder1', index: 0 }, + { uri: folder2Uri, name: 'folder2', index: 1 }, + ]); + + // Different pythonProjects settings for each folder + sandbox.stub(workspace, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const scopeUri = scope as Uri | undefined; + if (scopeUri?.fsPath === folder1Uri.fsPath) { + return createMockConfig([ + { path: '/workspace/folder1', envManager: 'ms-python.python:venv' }, + ]) as WorkspaceConfiguration; + } + if (scopeUri?.fsPath === folder2Uri.fsPath) { + return createMockConfig([ + { path: '/workspace/folder2', envManager: 'ms-python.python:system' }, + ]) as WorkspaceConfiguration; + } + return createMockConfig([]) as WorkspaceConfiguration; + }); + + mockProjectManager.get.callsFake((uri: Uri) => { + if (uri.fsPath === folder1Uri.fsPath) { + return mockProject1 as PythonProject; + } + if (uri.fsPath === folder2Uri.fsPath) { + return mockProject2 as PythonProject; + } + return undefined; + }); + + sandbox.stub(workspace, 'getWorkspaceFolder').callsFake((uri: Uri) => { + if (uri.fsPath === folder1Uri.fsPath) { + return { uri: folder1Uri, name: 'folder1', index: 0 } as WorkspaceFolder; + } + if (uri.fsPath === folder2Uri.fsPath) { + return { uri: folder2Uri, name: 'folder2', index: 1 } as WorkspaceFolder; + } + return undefined; + }); + + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + mockVenvManager.get.resolves(mockVenvEnv1); + mockSystemManager.get.resolves(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Both folders should be processed independently + assert.strictEqual(mockEnvManagers.setEnvironment.callCount, 2, 'Both folders should be processed'); + }); +}); diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 632c818b..fae18ddd 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -3,22 +3,22 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { ConfigurationTarget } from 'vscode'; import * as workspaceApis from '../../../common/workspace.apis'; -import { setAllManagerSettings, setEnvironmentManager, setPackageManager } from '../../../features/settings/settingHelpers'; +import { + setAllManagerSettings, + setEnvironmentManager, + setPackageManager, +} from '../../../features/settings/settingHelpers'; import { MockWorkspaceConfiguration } from '../../mocks/mockWorkspaceConfig'; /** - * These tests verify that settings are NOT written unnecessarily when: - * 1. Setting the system manager (which is the implicit default/fallback) - * 2. Setting pip package manager (the default) - * - * This prevents the bug where opening a non-Python repo with defaultInterpreterPath - * set would write unwanted settings like "defaultEnvManager: system" to global settings. + * These tests verify that settings ARE written when the value changes, + * regardless of whether it's the default/system manager or not. * * Note: These tests focus on the global settings path (project=undefined) because * workspace-scoped tests would require mocking workspace.getWorkspaceFolder which * cannot be easily stubbed in unit tests. */ -suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { +suite('Setting Helpers - Settings Write Behavior', () => { const SYSTEM_MANAGER_ID = 'ms-python.python:system'; const VENV_MANAGER_ID = 'ms-python.python:venv'; const PIP_MANAGER_ID = 'ms-python.python:pip'; @@ -40,6 +40,8 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { function createMockConfig(options: { defaultEnvManagerGlobalValue?: string; defaultPackageManagerGlobalValue?: string; + currentEnvManager?: string; + currentPkgManager?: string; }): MockWorkspaceConfiguration { const mockConfig = new MockWorkspaceConfiguration(); @@ -69,10 +71,10 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { // Override get to return effective values (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => { if (key === 'defaultEnvManager') { - return (options.defaultEnvManagerGlobalValue ?? VENV_MANAGER_ID) as T; + return (options.currentEnvManager ?? options.defaultEnvManagerGlobalValue ?? VENV_MANAGER_ID) as T; } if (key === 'defaultPackageManager') { - return (options.defaultPackageManagerGlobalValue ?? PIP_MANAGER_ID) as T; + return (options.currentPkgManager ?? options.defaultPackageManagerGlobalValue ?? PIP_MANAGER_ID) as T; } return defaultValue; }; @@ -95,33 +97,9 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { } suite('setAllManagerSettings - Global Settings', () => { - test('should NOT write global defaultEnvManager when setting system manager with no existing setting', async () => { - const mockConfig = createMockConfig({ - // No explicit global settings - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setAllManagerSettings([ - { - project: undefined, // Global scope - envManager: SYSTEM_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - }, - ]); - - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual( - envManagerUpdates.length, - 0, - 'Should NOT write global defaultEnvManager when setting system manager with no existing setting', - ); - }); - - test('should NOT write global defaultPackageManager when setting pip with no existing setting', async () => { + test('should write global defaultEnvManager when value differs from current', async () => { const mockConfig = createMockConfig({ - // No explicit global settings + currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); @@ -133,62 +111,23 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { }, ]); - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual( - pkgManagerUpdates.length, - 0, - 'Should NOT write global defaultPackageManager when setting pip with no existing setting', - ); - }); - - test('should write global defaultEnvManager when there is an existing global setting', async () => { - const mockConfig = createMockConfig({ - defaultEnvManagerGlobalValue: VENV_MANAGER_ID, // Existing global setting - defaultPackageManagerGlobalValue: PIP_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setAllManagerSettings([ - { - project: undefined, - envManager: SYSTEM_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - }, - ]); - const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual( - envManagerUpdates.length, - 1, - 'Should write global defaultEnvManager when there is an existing global setting', - ); + assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); assert.strictEqual(envManagerUpdates[0].value, SYSTEM_MANAGER_ID); }); - test('should write global defaultEnvManager when setting NON-system manager (venv) with no existing setting', async () => { + test('should NOT write global defaultEnvManager when value is same as current', async () => { const mockConfig = createMockConfig({ - // No explicit global settings, but mock get to return system + currentEnvManager: SYSTEM_MANAGER_ID, }); - // Override get to return system (so setting venv would be a change) - (mockConfig as any).get = (key: string): T | undefined => { - if (key === 'defaultEnvManager') { - return SYSTEM_MANAGER_ID as T; - } - if (key === 'defaultPackageManager') { - return PIP_MANAGER_ID as T; - } - return undefined; - }; sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); await setAllManagerSettings([ { project: undefined, - envManager: VENV_MANAGER_ID, // Non-system manager + envManager: SYSTEM_MANAGER_ID, packageManager: PIP_MANAGER_ID, }, ]); @@ -196,35 +135,21 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual( - envManagerUpdates.length, - 1, - 'Should write global defaultEnvManager when setting venv (non-system) manager', - ); - assert.strictEqual(envManagerUpdates[0].value, VENV_MANAGER_ID); + assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same as current'); }); - test('should write global defaultPackageManager when setting NON-pip manager (conda) with no existing setting', async () => { + test('should write global defaultPackageManager when value differs from current', async () => { const mockConfig = createMockConfig({ - // No explicit global settings + currentEnvManager: VENV_MANAGER_ID, + currentPkgManager: PIP_MANAGER_ID, }); - // Override get to return current pip value - (mockConfig as any).get = (key: string): T | undefined => { - if (key === 'defaultEnvManager') { - return VENV_MANAGER_ID as T; - } - if (key === 'defaultPackageManager') { - return PIP_MANAGER_ID as T; - } - return undefined; - }; sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); await setAllManagerSettings([ { project: undefined, - envManager: VENV_MANAGER_ID, // Non-system manager to trigger pkg manager write - packageManager: CONDA_MANAGER_ID, // Non-pip manager + envManager: VENV_MANAGER_ID, + packageManager: CONDA_MANAGER_ID, }, ]); @@ -234,16 +159,16 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { assert.strictEqual( pkgManagerUpdates.length, 1, - 'Should write global defaultPackageManager when setting non-pip manager', + 'Should write global defaultPackageManager when value differs', ); assert.strictEqual(pkgManagerUpdates[0].value, CONDA_MANAGER_ID); }); }); suite('setEnvironmentManager - Global Settings', () => { - test('should NOT write when setting system manager with no existing global setting', async () => { + test('should write when value differs from current', async () => { const mockConfig = createMockConfig({ - // No existing settings + currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); @@ -257,16 +182,12 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual( - envManagerUpdates.length, - 0, - 'Should NOT write global defaultEnvManager for system manager with no existing setting', - ); + assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); }); - test('should write when there is an existing global setting', async () => { + test('should NOT write when value is same as current', async () => { const mockConfig = createMockConfig({ - defaultEnvManagerGlobalValue: VENV_MANAGER_ID, + currentEnvManager: SYSTEM_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); @@ -280,21 +201,21 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual(envManagerUpdates.length, 1, 'Should write when updating existing global setting'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same'); }); }); suite('setPackageManager - Global Settings', () => { - test('should NOT write when setting pip manager with no existing global setting', async () => { + test('should write when value differs from current', async () => { const mockConfig = createMockConfig({ - // No existing settings + currentPkgManager: PIP_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); await setPackageManager([ { project: undefined, // Global scope - packageManager: PIP_MANAGER_ID, + packageManager: CONDA_MANAGER_ID, }, ]); @@ -303,14 +224,14 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { ); assert.strictEqual( pkgManagerUpdates.length, - 0, - 'Should NOT write global defaultPackageManager for pip manager with no existing setting', + 1, + 'Should write global defaultPackageManager when value differs', ); }); - test('should write when there is an existing global setting', async () => { + test('should NOT write when value is same as current', async () => { const mockConfig = createMockConfig({ - defaultPackageManagerGlobalValue: CONDA_MANAGER_ID, + currentPkgManager: PIP_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); @@ -324,8 +245,7 @@ suite('Setting Helpers - Avoid Unnecessary Settings Writes', () => { const pkgManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual(pkgManagerUpdates.length, 1, 'Should write when updating existing global setting'); + assert.strictEqual(pkgManagerUpdates.length, 0, 'Should NOT write when value is same'); }); }); }); - From 6355ebe144fb2d176d85e57468822c51fe7baba2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 20:48:23 -0800 Subject: [PATCH 2/6] improve logging --- src/features/envManagers.ts | 3 +- src/features/interpreterSelection.ts | 49 ++++++++++++++-------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 7f8cb352..f7549377 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -191,14 +191,13 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } - // Fall back to cached environment's manager if no settings + // Fall back to cached environment's manager if no user-configured settings const project = context ? this.pm.get(context) : undefined; const key = project ? project.uri.toString() : 'global'; const cachedEnv = this._previousEnvironments.get(key); if (cachedEnv) { const cachedManager = this._environmentManagers.get(cachedEnv.envId.managerId); if (cachedManager) { - traceVerbose(`[getEnvironmentManager] Using cached manager ${cachedManager.id} for scope ${key}`); return cachedManager; } } diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index b954669b..817690f9 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { commands, ConfigurationChangeEvent, Disposable, l10n, Uri, workspace } from 'vscode'; import { PythonEnvironment, PythonEnvironmentApi } from '../api'; import { SYSTEM_MANAGER_ID, VENV_MANAGER_ID } from '../common/constants'; -import { traceInfo, traceVerbose, traceWarn } from '../common/logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../common/logging'; import { showWarningMessage } from '../common/window.apis'; import { getConfiguration, onDidChangeConfiguration } from '../common/workspace.apis'; import { getUserConfiguredSetting } from '../helpers'; @@ -59,9 +59,7 @@ async function resolvePriorityChainCore( api: PythonEnvironmentApi, ): Promise<{ result: PriorityChainResult; errors: SettingResolutionError[] }> { const errors: SettingResolutionError[] = []; - const logPrefix = scope ? `[resolveEnvironmentByPriority] ${scope.fsPath}` : '[resolveGlobalEnvironmentByPriority]'; - - traceVerbose(`${logPrefix} Starting priority chain resolution`); + const logPrefix = scope ? `[priorityChain] ${scope.fsPath}` : '[priorityChain:global]'; // PRIORITY 1: Check pythonProjects[] for this workspace path (workspace scope only) if (scope && projectManager) { @@ -78,7 +76,7 @@ async function resolvePriorityChainCore( reason: `Environment manager '${projectManagerId}' is not registered`, }; errors.push(error); - traceWarn(`${logPrefix} ${error.reason}`); + traceWarn(`${logPrefix} pythonProjects[] manager '${projectManagerId}' not found, trying next priority`); } } @@ -96,7 +94,7 @@ async function resolvePriorityChainCore( reason: `Environment manager '${userConfiguredManager}' is not registered`, }; errors.push(error); - traceWarn(`${logPrefix} ${error.reason}`); + traceWarn(`${logPrefix} defaultEnvManager '${userConfiguredManager}' not found, trying next priority`); } // PRIORITY 3: User-configured python.defaultInterpreterPath @@ -113,11 +111,12 @@ async function resolvePriorityChainCore( reason: `Could not resolve interpreter path '${userInterpreterPath}'`, }; errors.push(error); - traceWarn(`${logPrefix} ${error.reason}, falling through to auto-discovery`); + traceWarn( + `${logPrefix} defaultInterpreterPath '${userInterpreterPath}' unresolvable, falling back to auto-discovery`, + ); } - // PRIORITY 4: Auto-discovery - traceVerbose(`${logPrefix} Priority 4: Falling through to auto-discovery`); + // PRIORITY 4: Auto-discovery (no user-configured settings matched) const autoDiscoverResult = await autoDiscoverEnvironment(scope, envManagers); return { result: autoDiscoverResult, errors }; } @@ -186,11 +185,10 @@ async function autoDiscoverEnvironment( try { const localEnv = await venvManager.get(scope); if (localEnv) { - traceVerbose(`[autoDiscover] Priority 4: Auto-discovered local venv: ${localEnv.displayName}`); return { manager: venvManager, environment: localEnv, source: 'autoDiscovery' }; } } catch (err) { - traceWarn(`[autoDiscover] Error checking venv manager: ${err}`); + traceError(`[autoDiscover] Failed to check venv manager: ${err}`); } } } @@ -198,14 +196,13 @@ async function autoDiscoverEnvironment( // Fall back to system manager const systemManager = envManagers.getEnvironmentManager(SYSTEM_MANAGER_ID); if (systemManager) { - traceVerbose('[autoDiscover] Priority 4: Using system Python (fallback)'); return { manager: systemManager, source: 'autoDiscovery' }; } // Last resort: use any available manager const anyManager = envManagers.managers[0]; if (anyManager) { - traceVerbose(`[autoDiscover] Priority 4: No venv or system manager, using first available: ${anyManager.id}`); + traceWarn(`[autoDiscover] No venv or system manager available, using fallback manager: ${anyManager.id}`); return { manager: anyManager, source: 'autoDiscovery' }; } @@ -238,7 +235,9 @@ export async function applyInitialEnvironmentSelection( api: PythonEnvironmentApi, ): Promise { const folders = workspace.workspaceFolders ?? []; - traceVerbose(`[applyInitialEnvironmentSelection] Starting - ${folders.length} workspace folder(s)`); + traceInfo( + `[interpreterSelection] Applying initial environment selection for ${folders.length} workspace folder(s)`, + ); const allErrors: SettingResolutionError[] = []; @@ -259,12 +258,11 @@ export async function applyInitialEnvironmentSelection( // Cache only β€” NO settings.json write (shouldPersistSettings = false) await envManagers.setEnvironment(folder.uri, env, false); - traceVerbose( - `[applyInitialEnvironmentSelection] Set environment for ${folder.uri.fsPath}: ` + - `manager=${result.manager.id}, source=${result.source}, env=${env?.displayName ?? 'none'}`, + traceInfo( + `[interpreterSelection] ${folder.name}: ${env?.displayName ?? 'none'} (source: ${result.source})`, ); } catch (err) { - traceWarn(`[applyInitialEnvironmentSelection] Error setting environment for ${folder.uri.fsPath}: ${err}`); + traceError(`[interpreterSelection] Failed to set environment for ${folder.uri.fsPath}: ${err}`); } } @@ -280,12 +278,9 @@ export async function applyInitialEnvironmentSelection( // Cache only β€” NO settings.json write (shouldPersistSettings = false) await envManagers.setEnvironments('global', env, false); - traceVerbose( - `[applyInitialEnvironmentSelection] Set global environment: ` + - `manager=${result.manager.id}, source=${result.source}, env=${env?.displayName ?? 'none'}`, - ); + traceInfo(`[interpreterSelection] global: ${env?.displayName ?? 'none'} (source: ${result.source})`); } catch (err) { - traceWarn(`[applyInitialEnvironmentSelection] Error setting global environment: ${err}`); + traceError(`[interpreterSelection] Failed to set global environment: ${err}`); } // Notify user if any settings could not be applied @@ -409,11 +404,17 @@ async function tryResolveInterpreterPath( }, }, }; + traceVerbose( + `[tryResolveInterpreterPath] Resolved '${interpreterPath}' to ${resolved.executable} (${resolved.version})`, + ); return { manager, environment: newEnv, source: 'defaultInterpreterPath' }; } } + traceVerbose( + `[tryResolveInterpreterPath] Could not resolve '${interpreterPath}' - no executable or manager found`, + ); } catch (err) { - traceWarn(`[tryResolveInterpreterPath] Error resolving interpreter path: ${err}`); + traceVerbose(`[tryResolveInterpreterPath] Resolution failed for '${interpreterPath}': ${err}`); } return undefined; } From e396f5b743f2463368ada98fc94ac254fe956919 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 20:52:07 -0800 Subject: [PATCH 3/6] test stub fix --- src/features/interpreterSelection.ts | 13 +++- .../interpreterSelection.unit.test.ts | 78 +++++++++---------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index 817690f9..85b31911 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -2,12 +2,17 @@ // Licensed under the MIT License. import * as path from 'path'; -import { commands, ConfigurationChangeEvent, Disposable, l10n, Uri, workspace } from 'vscode'; +import { commands, ConfigurationChangeEvent, Disposable, l10n, Uri } from 'vscode'; import { PythonEnvironment, PythonEnvironmentApi } from '../api'; import { SYSTEM_MANAGER_ID, VENV_MANAGER_ID } from '../common/constants'; import { traceError, traceInfo, traceVerbose, traceWarn } from '../common/logging'; import { showWarningMessage } from '../common/window.apis'; -import { getConfiguration, onDidChangeConfiguration } from '../common/workspace.apis'; +import { + getConfiguration, + getWorkspaceFolder, + getWorkspaceFolders, + onDidChangeConfiguration, +} from '../common/workspace.apis'; import { getUserConfiguredSetting } from '../helpers'; import { EnvironmentManagers, @@ -234,7 +239,7 @@ export async function applyInitialEnvironmentSelection( nativeFinder: NativePythonFinder, api: PythonEnvironmentApi, ): Promise { - const folders = workspace.workspaceFolders ?? []; + const folders = getWorkspaceFolders() ?? []; traceInfo( `[interpreterSelection] Applying initial environment selection for ${folders.length} workspace folder(s)`, ); @@ -351,7 +356,7 @@ function getProjectSpecificEnvManager(projectManager: PythonProjectManager, scop if (overrides.length > 0) { const pw = projectManager.get(scope); - const w = workspace.getWorkspaceFolder(scope); + const w = getWorkspaceFolder(scope); if (pw && w) { const pwPath = path.normalize(pw.uri.fsPath); const matching = overrides.find((s) => path.resolve(w.uri.fsPath, s.path) === pwPath); diff --git a/src/test/features/interpreterSelection.unit.test.ts b/src/test/features/interpreterSelection.unit.test.ts index 9e264e58..82d79e60 100644 --- a/src/test/features/interpreterSelection.unit.test.ts +++ b/src/test/features/interpreterSelection.unit.test.ts @@ -3,7 +3,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; -import { ConfigurationChangeEvent, Uri, workspace, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; +import { ConfigurationChangeEvent, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; import * as workspaceApis from '../../common/workspace.apis'; import { @@ -119,7 +119,7 @@ suite('Interpreter Selection - Priority Chain', () => { suite('Priority 1: pythonProjects[]', () => { test('should use manager from pythonProjects[] when configured', async () => { // Setup: pythonProjects[] has a venv manager configured - sandbox.stub(workspace, 'getConfiguration').returns( + sandbox.stub(workspaceApis, 'getConfiguration').returns( createMockConfig([ { path: '/test/workspace', @@ -131,7 +131,7 @@ suite('Interpreter Selection - Priority Chain', () => { const mockProject: Partial = { uri: testUri, name: 'test' }; mockProjectManager.get.returns(mockProject as PythonProject); const mockWorkspaceFolder: Partial = { uri: testUri }; - sandbox.stub(workspace, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); + sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const result = await resolveEnvironmentByPriority( @@ -150,7 +150,7 @@ suite('Interpreter Selection - Priority Chain', () => { suite('Priority 2: User-configured defaultEnvManager', () => { test('should use user-configured defaultEnvManager when set', async () => { // Setup: No pythonProjects[], but user configured defaultEnvManager - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python-envs' && key === 'defaultEnvManager') { return 'ms-python.python:venv'; @@ -173,7 +173,7 @@ suite('Interpreter Selection - Priority Chain', () => { test('should skip to Priority 3 when defaultEnvManager is not user-configured (only fallback)', async () => { // Setup: No pythonProjects[], no user-configured defaultEnvManager (returns undefined) // But there IS a user-configured defaultInterpreterPath - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return '/usr/bin/python3.11'; @@ -197,7 +197,7 @@ suite('Interpreter Selection - Priority Chain', () => { suite('Priority 3: python.defaultInterpreterPath', () => { test('should use defaultInterpreterPath when set and resolvable', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return '/usr/bin/python3.11'; @@ -221,7 +221,7 @@ suite('Interpreter Selection - Priority Chain', () => { }); test('should fall through to Priority 4 when defaultInterpreterPath cannot be resolved', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return '/nonexistent/python'; @@ -248,7 +248,7 @@ suite('Interpreter Selection - Priority Chain', () => { const userPyenvPath = '/Users/test/.pyenv/versions/3.13.7/bin/python'; const resolvedHomebrewPath = '/opt/homebrew/bin/python3'; - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return userPyenvPath; @@ -309,7 +309,7 @@ suite('Interpreter Selection - Priority Chain', () => { suite('Priority 4: Auto-discovery', () => { test('should use local venv when found', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); mockVenvManager.get.resolves(mockVenvEnv); @@ -327,7 +327,7 @@ suite('Interpreter Selection - Priority Chain', () => { }); test('should fall back to system manager when no local venv', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); mockVenvManager.get.resolves(undefined); // No local venv @@ -344,7 +344,7 @@ suite('Interpreter Selection - Priority Chain', () => { }); test('should throw error when no managers are available', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); // Create mock with no managers @@ -370,7 +370,7 @@ suite('Interpreter Selection - Priority Chain', () => { suite('Edge Cases', () => { test('should fall through when nativeFinder resolves but returns no executable', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return '/some/path/python'; @@ -394,7 +394,7 @@ suite('Interpreter Selection - Priority Chain', () => { }); test('should fall through when api.resolveEnvironment returns undefined', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python' && key === 'defaultInterpreterPath') { return '/usr/bin/python3.11'; @@ -418,7 +418,7 @@ suite('Interpreter Selection - Priority Chain', () => { }); test('should use first available manager when venv and system managers not found', async () => { - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const mockCondaManager = { @@ -526,8 +526,8 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => { }); test('should call setEnvironment with shouldPersistSettings=false', async () => { - sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); await applyInitialEnvironmentSelection( @@ -546,11 +546,11 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => { test('should process all workspace folders', async () => { const uri1 = Uri.file('/workspace1'); const uri2 = Uri.file('/workspace2'); - sandbox.stub(workspace, 'workspaceFolders').value([ + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([ { uri: uri1, name: 'workspace1', index: 0 }, { uri: uri2, name: 'workspace2', index: 1 }, ]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); await applyInitialEnvironmentSelection( @@ -565,8 +565,8 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => { }); test('should also set global environment when no workspace folders', async () => { - sandbox.stub(workspace, 'workspaceFolders').value([]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); await applyInitialEnvironmentSelection( @@ -855,8 +855,8 @@ suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => return { dispose: () => {} }; }); - sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); // Register the listener @@ -897,8 +897,8 @@ suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => return { dispose: () => {} }; }); - sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const disposable = registerInterpreterSettingsChangeListener( @@ -934,8 +934,8 @@ suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => return { dispose: () => {} }; }); - sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const disposable = registerInterpreterSettingsChangeListener( @@ -971,8 +971,8 @@ suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => return { dispose: () => {} }; }); - sandbox.stub(workspace, 'workspaceFolders').value([{ uri: testUri, name: 'test', index: 0 }]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const disposable = registerInterpreterSettingsChangeListener( @@ -1093,7 +1093,7 @@ suite('Interpreter Selection - Settings over Cache Priority', () => { test('should use user-configured defaultEnvManager even when cache has different manager', async () => { // This test verifies settings take priority over cache // Setup: User has configured defaultEnvManager=conda, but cache has venv - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python-envs' && key === 'defaultEnvManager') { return 'ms-python.python:conda'; // User wants conda @@ -1117,7 +1117,7 @@ suite('Interpreter Selection - Settings over Cache Priority', () => { test('should use pythonProjects manager even when defaultEnvManager is set', async () => { // This test verifies pythonProjects[] has highest priority (Priority 1 over Priority 2) - sandbox.stub(workspace, 'getConfiguration').returns( + sandbox.stub(workspaceApis, 'getConfiguration').returns( createMockConfig([ { path: '/test/workspace', @@ -1129,7 +1129,7 @@ suite('Interpreter Selection - Settings over Cache Priority', () => { const mockProject: Partial = { uri: testUri, name: 'test' }; mockProjectManager.get.returns(mockProject as PythonProject); const mockWorkspaceFolder: Partial = { uri: testUri }; - sandbox.stub(workspace, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); + sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(mockWorkspaceFolder as WorkspaceFolder); sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { if (section === 'python-envs' && key === 'defaultEnvManager') { @@ -1153,7 +1153,7 @@ suite('Interpreter Selection - Settings over Cache Priority', () => { test('should fall back to auto-discovery when no user settings configured', async () => { // When no settings are configured, cache from auto-discovery should be used - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); const result = await resolveEnvironmentByPriority( @@ -1259,11 +1259,11 @@ suite('Interpreter Selection - Multi-Root Workspace', () => { test('each folder should get its own local venv in multi-root workspace', async () => { // Setup: Two workspace folders, each with their own local venv - sandbox.stub(workspace, 'workspaceFolders').value([ + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([ { uri: folder1Uri, name: 'folder1', index: 0 }, { uri: folder2Uri, name: 'folder2', index: 1 }, ]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); // Venv manager returns different venvs for each folder @@ -1310,11 +1310,11 @@ suite('Interpreter Selection - Multi-Root Workspace', () => { test('first folder venv should not be overwritten when second folder has no venv', async () => { // This tests the scenario in issue #1145 where the first folder's venv gets overwritten - sandbox.stub(workspace, 'workspaceFolders').value([ + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([ { uri: folder1Uri, name: 'folder1', index: 0 }, { uri: folder2Uri, name: 'folder2', index: 1 }, ]); - sandbox.stub(workspace, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); // First folder has a venv, second folder does not @@ -1351,13 +1351,13 @@ suite('Interpreter Selection - Multi-Root Workspace', () => { const mockProject1: Partial = { uri: folder1Uri, name: 'project1' }; const mockProject2: Partial = { uri: folder2Uri, name: 'project2' }; - sandbox.stub(workspace, 'workspaceFolders').value([ + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([ { uri: folder1Uri, name: 'folder1', index: 0 }, { uri: folder2Uri, name: 'folder2', index: 1 }, ]); // Different pythonProjects settings for each folder - sandbox.stub(workspace, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + sandbox.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { const scopeUri = scope as Uri | undefined; if (scopeUri?.fsPath === folder1Uri.fsPath) { return createMockConfig([ @@ -1382,7 +1382,7 @@ suite('Interpreter Selection - Multi-Root Workspace', () => { return undefined; }); - sandbox.stub(workspace, 'getWorkspaceFolder').callsFake((uri: Uri) => { + sandbox.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri: Uri) => { if (uri.fsPath === folder1Uri.fsPath) { return { uri: folder1Uri, name: 'folder1', index: 0 } as WorkspaceFolder; } From c40036ed18f792bff8a9c3c71cea37e3fba131bd Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 20:52:42 -0800 Subject: [PATCH 4/6] learning --- .github/instructions/testing-workflow.instructions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index 88a3e656..f2bee581 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -579,3 +579,4 @@ envConfig.inspect - Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1) - Always compile tests (`npm run compile-tests`) before running them after adding new test cases - test counts will be wrong if running against stale compiled output (1) - Never create "documentation tests" that just `assert.ok(true)` β€” if mocking limitations prevent testing, either test a different layer that IS mockable, or skip the test entirely with a clear explanation (1) +- When stubbing vscode APIs in tests via wrapper modules (e.g., `workspaceApis`), the production code must also use those wrappers β€” sinon cannot stub properties directly on the vscode namespace like `workspace.workspaceFolders`, so both production and test code must reference the same stubbable wrapper functions (1) From 733f0058f3d8bea77062d0c11446e644070ed89a Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 21:03:03 -0800 Subject: [PATCH 5/6] test fixes and learning more --- .github/instructions/generic.instructions.md | 24 +++++++++++-------- .../testing-workflow.instructions.md | 20 +++++++++++++++- .../interpreterSelection.unit.test.ts | 14 +++++------ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/.github/instructions/generic.instructions.md b/.github/instructions/generic.instructions.md index c9e3c87e..cef58689 100644 --- a/.github/instructions/generic.instructions.md +++ b/.github/instructions/generic.instructions.md @@ -6,31 +6,35 @@ Provide project context and coding guidelines that AI should follow when generat ## Localization -- Localize all user-facing messages using VS Code’s `l10n` API. -- Internal log messages do not require localization. +- Localize all user-facing messages using VS Code’s `l10n` API. +- Internal log messages do not require localization. ## Logging -- Use the extension’s logging utilities (`traceLog`, `traceVerbose`) for internal logs. -- Do not use `console.log` or `console.warn` for logging. +- Use the extension’s logging utilities (`traceLog`, `traceVerbose`) for internal logs. +- Do not use `console.log` or `console.warn` for logging. ## Settings Precedence -- Always consider VS Code settings precedence: +- Always consider VS Code settings precedence: 1. Workspace folder 2. Workspace 3. User/global -- Remove or update settings from the highest precedence scope first. +- Remove or update settings from the highest precedence scope first. ## Error Handling & User Notifications -- Avoid showing the same error message multiple times in a session; track state with a module-level variable. -- Use clear, actionable error messages and offer relevant buttons (e.g., "Open settings", "Close"). +- Avoid showing the same error message multiple times in a session; track state with a module-level variable. +- Use clear, actionable error messages and offer relevant buttons (e.g., "Open settings", "Close"). ## Documentation -- Add clear docstrings to public functions, describing their purpose, parameters, and behavior. +- Add clear docstrings to public functions, describing their purpose, parameters, and behavior. + +## Cross-Platform Path Handling + +**CRITICAL**: This extension runs on Windows, macOS, and Linux. NEVER hardcode POSIX-style paths. ## Learnings -- When using `getConfiguration().inspect()`, always pass a scope/Uri to `getConfiguration(section, scope)` β€” otherwise `workspaceFolderValue` will be `undefined` because VS Code doesn't know which folder to inspect (1) +- When using `getConfiguration().inspect()`, always pass a scope/Uri to `getConfiguration(section, scope)` β€” otherwise `workspaceFolderValue` will be `undefined` because VS Code doesn't know which folder to inspect (1) diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index f2bee581..d1d63a19 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -33,6 +33,23 @@ This guide covers the full testing lifecycle: - Test failures or compilation errors - Coverage reports or test output analysis +## 🚨 CRITICAL: Common Mistakes (Read First!) + +These mistakes have occurred REPEATEDLY. Check this list BEFORE writing any test code: + +| Mistake | Fix | +| ---------------------------------------------- | ------------------------------------------------------------------ | +| Hardcoded POSIX paths like `'/test/workspace'` | Use `'.'` for relative paths, `Uri.file(x).fsPath` for comparisons | +| Stubbing `workspace.getConfiguration` directly | Stub the wrapper `workspaceApis.getConfiguration` instead | +| Stubbing `workspace.workspaceFolders` property | Stub wrapper function `workspaceApis.getWorkspaceFolders()` | +| Comparing `fsPath` to raw string | Compare `fsPath` to `Uri.file(expected).fsPath` | + +**Pre-flight checklist before completing test work:** + +- [ ] All paths use `Uri.file().fsPath` (no hardcoded `/path/to/x`) +- [ ] All VS Code API stubs use wrapper modules, not `vscode.*` directly +- [ ] Tests pass on both Windows and POSIX + ## Test Types When implementing tests as an AI agent, choose between two main types: @@ -579,4 +596,5 @@ envConfig.inspect - Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1) - Always compile tests (`npm run compile-tests`) before running them after adding new test cases - test counts will be wrong if running against stale compiled output (1) - Never create "documentation tests" that just `assert.ok(true)` β€” if mocking limitations prevent testing, either test a different layer that IS mockable, or skip the test entirely with a clear explanation (1) -- When stubbing vscode APIs in tests via wrapper modules (e.g., `workspaceApis`), the production code must also use those wrappers β€” sinon cannot stub properties directly on the vscode namespace like `workspace.workspaceFolders`, so both production and test code must reference the same stubbable wrapper functions (1) +- **REPEATED**: When stubbing vscode APIs in tests via wrapper modules (e.g., `workspaceApis`), the production code must also use those wrappers β€” sinon cannot stub properties directly on the vscode namespace like `workspace.workspaceFolders`, so both production and test code must reference the same stubbable wrapper functions (3) +- **REPEATED**: Use OS-agnostic path handling in tests: use `'.'` for relative paths in configs (NOT `/test/workspace`), compare `fsPath` to `Uri.file(expected).fsPath` (NOT raw strings). This breaks on Windows every time! (5) diff --git a/src/test/features/interpreterSelection.unit.test.ts b/src/test/features/interpreterSelection.unit.test.ts index 82d79e60..615bd167 100644 --- a/src/test/features/interpreterSelection.unit.test.ts +++ b/src/test/features/interpreterSelection.unit.test.ts @@ -122,7 +122,7 @@ suite('Interpreter Selection - Priority Chain', () => { sandbox.stub(workspaceApis, 'getConfiguration').returns( createMockConfig([ { - path: '/test/workspace', + path: '.', envManager: 'ms-python.python:venv', packageManager: 'ms-python.python:pip', }, @@ -301,7 +301,7 @@ suite('Interpreter Selection - Priority Chain', () => { ); assert.strictEqual( result.environment.environmentPath?.fsPath, - userPyenvPath, + Uri.file(userPyenvPath).fsPath, 'environmentPath should use user configured path', ); }); @@ -1117,10 +1117,11 @@ suite('Interpreter Selection - Settings over Cache Priority', () => { test('should use pythonProjects manager even when defaultEnvManager is set', async () => { // This test verifies pythonProjects[] has highest priority (Priority 1 over Priority 2) + // Use '.' as relative path - path.resolve(workspaceFolder, '.') equals workspaceFolder sandbox.stub(workspaceApis, 'getConfiguration').returns( createMockConfig([ { - path: '/test/workspace', + path: '.', envManager: 'ms-python.python:venv', // Project says venv packageManager: 'ms-python.python:pip', }, @@ -1357,16 +1358,15 @@ suite('Interpreter Selection - Multi-Root Workspace', () => { ]); // Different pythonProjects settings for each folder + // Use '.' as relative path - path.resolve(workspaceFolder, '.') equals workspaceFolder sandbox.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { const scopeUri = scope as Uri | undefined; if (scopeUri?.fsPath === folder1Uri.fsPath) { - return createMockConfig([ - { path: '/workspace/folder1', envManager: 'ms-python.python:venv' }, - ]) as WorkspaceConfiguration; + return createMockConfig([{ path: '.', envManager: 'ms-python.python:venv' }]) as WorkspaceConfiguration; } if (scopeUri?.fsPath === folder2Uri.fsPath) { return createMockConfig([ - { path: '/workspace/folder2', envManager: 'ms-python.python:system' }, + { path: '.', envManager: 'ms-python.python:system' }, ]) as WorkspaceConfiguration; } return createMockConfig([]) as WorkspaceConfiguration; From bc0a76c631b79634988922628741c685544f1c99 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 2 Feb 2026 21:12:01 -0800 Subject: [PATCH 6/6] windows yay --- .github/instructions/generic.instructions.md | 5 +++++ src/features/interpreterSelection.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/instructions/generic.instructions.md b/.github/instructions/generic.instructions.md index cef58689..cf8498c6 100644 --- a/.github/instructions/generic.instructions.md +++ b/.github/instructions/generic.instructions.md @@ -35,6 +35,11 @@ Provide project context and coding guidelines that AI should follow when generat **CRITICAL**: This extension runs on Windows, macOS, and Linux. NEVER hardcode POSIX-style paths. +- Use `path.join()` or `path.resolve()` instead of string concatenation with `/` +- Use `Uri.file(path).fsPath` when comparing paths (Windows uses `\`, POSIX uses `/`) +- When asserting path equality, compare `fsPath` to `fsPath`, not raw strings + ## Learnings - When using `getConfiguration().inspect()`, always pass a scope/Uri to `getConfiguration(section, scope)` β€” otherwise `workspaceFolderValue` will be `undefined` because VS Code doesn't know which folder to inspect (1) +- **path.normalize() vs path.resolve()**: On Windows, `path.normalize('\test')` keeps it as `\test`, but `path.resolve('\test')` adds the current drive β†’ `C:\test`. When comparing paths, use `path.resolve()` on BOTH sides or they won't match (2) diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index 85b31911..e2aee39e 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -358,7 +358,7 @@ function getProjectSpecificEnvManager(projectManager: PythonProjectManager, scop const pw = projectManager.get(scope); const w = getWorkspaceFolder(scope); if (pw && w) { - const pwPath = path.normalize(pw.uri.fsPath); + const pwPath = path.resolve(pw.uri.fsPath); const matching = overrides.find((s) => path.resolve(w.uri.fsPath, s.path) === pwPath); if (matching && matching.envManager && matching.envManager.length > 0) { return matching.envManager;