diff --git a/src/extension.ts b/src/extension.ts index 04bcb3e8..f635874a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -508,13 +508,13 @@ export async function activate(context: ExtensionContext): Promise { updateViewsAndStatus(statusBar, workspaceView, managerView, api); }), - envManagers.onDidChangeEnvironment(async () => { + envManagers.onDidChangeManagerEnvironment(async () => { updateViewsAndStatus(statusBar, workspaceView, managerView, api); }), envManagers.onDidChangeEnvironments(async () => { updateViewsAndStatus(statusBar, workspaceView, managerView, api); }), - envManagers.onDidChangeEnvironmentFiltered(async (e) => { + envManagers.onDidChangeActiveEnvironment(async (e) => { managerView.environmentChanged(e); const location = e.uri?.fsPath ?? 'global'; traceInfo( diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 036194a0..36b63696 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -51,23 +51,42 @@ function generateId(name: string, extensionId?: string): string { export class PythonEnvironmentManagers implements EnvironmentManagers { private _environmentManagers: Map = new Map(); private _packageManagers: Map = new Map(); - private readonly _previousEnvironments = new Map(); + + /** + * The last environment announced as "active" for each scope. + * Keyed by project URI string or 'global'. + * + * Used for: (1) change detection before firing onDidChangeActiveEnvironment, + * (2) fallback manager routing when no explicit setting exists. + * + * Only mutated by setEnvironment() / setEnvironments() / refreshEnvironment(). + */ + private readonly _activeSelection = new Map(); private _onDidChangeEnvironmentManager = new EventEmitter(); private _onDidChangePackageManager = new EventEmitter(); private _onDidChangeEnvironments = new EventEmitter(); - private _onDidChangeEnvironment = new EventEmitter(); - private _onDidChangeEnvironmentFiltered = new EventEmitter(); + + /** Fires when ANY manager reports a selection change, regardless of whether that manager is selected. */ + private _onDidChangeManagerEnvironment = new EventEmitter(); + + /** Fires when the active (selected) environment for a scope actually changes. */ + private _onDidChangeActiveEnvironment = new EventEmitter(); private _onDidChangePackages = new EventEmitter(); public onDidChangeEnvironmentManager: Event = this._onDidChangeEnvironmentManager.event; public onDidChangePackageManager: Event = this._onDidChangePackageManager.event; public onDidChangeEnvironments: Event = this._onDidChangeEnvironments.event; - public onDidChangeEnvironment: Event = this._onDidChangeEnvironment.event; + + /** Fires when any registered manager reports a change — even if that manager is not the selected one. */ + public onDidChangeManagerEnvironment: Event = + this._onDidChangeManagerEnvironment.event; public onDidChangePackages: Event = this._onDidChangePackages.event; - public onDidChangeEnvironmentFiltered: Event = - this._onDidChangeEnvironmentFiltered.event; + + /** Fires only when the *selected* manager's environment for a scope actually changes. */ + public onDidChangeActiveEnvironment: Event = + this._onDidChangeActiveEnvironment.event; constructor(private readonly pm: PythonProjectManager) {} @@ -99,7 +118,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { return; } - setImmediate(() => this._onDidChangeEnvironment.fire(e)); + setImmediate(() => this._onDidChangeManagerEnvironment.fire(e)); }), ); @@ -165,6 +184,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { this._onDidChangeEnvironmentManager.dispose(); this._onDidChangePackageManager.dispose(); this._onDidChangeEnvironments.dispose(); + this._onDidChangeManagerEnvironment.dispose(); + this._onDidChangeActiveEnvironment.dispose(); this._onDidChangePackages.dispose(); } @@ -196,7 +217,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { // 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); + const cachedEnv = this._activeSelection.get(key); if (cachedEnv) { const cachedManager = this._environmentManagers.get(cachedEnv.envId.managerId); if (cachedManager) { @@ -339,11 +360,11 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } const key = project ? project.uri.toString() : 'global'; - const oldEnv = this._previousEnvironments.get(key); + const oldEnv = this._activeSelection.get(key); if (oldEnv?.envId.id !== environment?.envId.id) { - this._previousEnvironments.set(key, environment); + this._activeSelection.set(key, environment); setImmediate(() => - this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: environment, old: oldEnv }), + this._onDidChangeActiveEnvironment.fire({ uri: project?.uri, new: environment, old: oldEnv }), ); } } @@ -393,9 +414,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { const project = this.pm.get(uri); const key = project ? project.uri.toString() : 'global'; - const oldEnv = this._previousEnvironments.get(key); + const oldEnv = this._activeSelection.get(key); if (oldEnv?.envId.id !== environment?.envId.id) { - this._previousEnvironments.set(key, environment); + this._activeSelection.set(key, environment); events.push({ uri: project?.uri, new: environment, old: oldEnv }); } }); @@ -411,9 +432,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { }); } - const oldEnv = this._previousEnvironments.get('global'); + const oldEnv = this._activeSelection.get('global'); if (oldEnv?.envId.id !== environment?.envId.id) { - this._previousEnvironments.set('global', environment); + this._activeSelection.set('global', environment); events.push({ uri: undefined, new: environment, old: oldEnv }); } } @@ -422,7 +443,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (shouldPersistSettings) { await setAllManagerSettings(settings); } - setImmediate(() => events.forEach((e) => this._onDidChangeEnvironmentFiltered.fire(e))); + setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e))); } else { const promises: Promise[] = []; const events: DidChangeEnvironmentEventArgs[] = []; @@ -439,9 +460,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { // events. But it ensures that we always get the latest environment at the time of this call. const newEnv = await manager.get(uri); const key = project ? project.uri.toString() : 'global'; - const oldEnv = this._previousEnvironments.get(key); + const oldEnv = this._activeSelection.get(key); if (oldEnv?.envId.id !== newEnv?.envId.id) { - this._previousEnvironments.set(key, newEnv); + this._activeSelection.set(key, newEnv); events.push({ uri: project?.uri, new: newEnv, old: oldEnv }); } }; @@ -457,9 +478,9 @@ 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(undefined); - const oldEnv = this._previousEnvironments.get('global'); + const oldEnv = this._activeSelection.get('global'); if (oldEnv?.envId.id !== newEnv?.envId.id) { - this._previousEnvironments.set('global', newEnv); + this._activeSelection.set('global', newEnv); events.push({ uri: undefined, new: newEnv, old: oldEnv }); } }; @@ -467,7 +488,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } await Promise.all(promises); - setImmediate(() => events.forEach((e) => this._onDidChangeEnvironmentFiltered.fire(e))); + setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e))); } } @@ -503,8 +524,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { /** * Gets the current Python environment for the given scope URI or undefined for 'global'. * - * This method queries the appropriate environment manager for the latest environment for the scope. - * It also updates the internal cache and fires an event if the environment has changed since last check. + * This is a pure read: it queries the environment manager but does NOT update the + * internal selection cache or fire change events. Selection state is only mutated + * through setEnvironment() / setEnvironments() / refreshEnvironment(). * * @param scope The scope to get the environment. * @returns The current PythonEnvironment for the scope, or undefined if none is set. @@ -519,21 +541,35 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { return undefined; } - const project = scope ? this.pm.get(scope) : undefined; + return manager.get(scope); + } + + /** + * Refreshes the cached environment for the given scope by querying the selected manager. + * If the manager's current environment differs from the cache, updates the cache and + * fires onDidChangeActiveEnvironment. Use this when a manager signals that its + * selection has changed (e.g., via onDidChangeManagerEnvironment). + * + * Unlike getEnvironment(), this IS a mutation — it updates internal state. + * Unlike setEnvironment(), it does NOT call manager.set() or persist to settings. + */ + async refreshEnvironment(scope: GetEnvironmentScope): Promise { + const manager = this.getEnvironmentManager(scope); + if (!manager) { + return; + } - // 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 project = scope ? this.pm.get(scope) : undefined; const newEnv = await manager.get(scope); const key = project ? project.uri.toString() : 'global'; - const oldEnv = this._previousEnvironments.get(key); + const oldEnv = this._activeSelection.get(key); if (oldEnv?.envId.id !== newEnv?.envId.id) { - this._previousEnvironments.set(key, newEnv); + this._activeSelection.set(key, newEnv); setImmediate(() => - this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: newEnv, old: oldEnv }), + this._onDidChangeActiveEnvironment.fire({ uri: project?.uri, new: newEnv, old: oldEnv }), ); } - return newEnv; } getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[] { diff --git a/src/features/pythonApi.ts b/src/features/pythonApi.ts index 8b9dbc94..f071b0f5 100644 --- a/src/features/pythonApi.ts +++ b/src/features/pythonApi.ts @@ -1,35 +1,40 @@ -import { Uri, Disposable, Event, EventEmitter, Terminal, TaskExecution } from 'vscode'; +import { Disposable, Event, EventEmitter, TaskExecution, Terminal, Uri } from 'vscode'; import { - PythonEnvironmentApi, - PythonEnvironment, - EnvironmentManager, - PackageManager, + CreateEnvironmentOptions, + CreateEnvironmentScope, DidChangeEnvironmentEventArgs, DidChangeEnvironmentsEventArgs, + DidChangeEnvironmentVariablesEventArgs, + DidChangePackagesEventArgs, DidChangePythonProjectsEventArgs, + EnvironmentManager, + GetEnvironmentScope, GetEnvironmentsScope, Package, - PythonEnvironmentInfo, - PythonProject, - RefreshEnvironmentsScope, - DidChangePackagesEventArgs, - PythonEnvironmentId, - CreateEnvironmentScope, - SetEnvironmentScope, - GetEnvironmentScope, - PackageInfo, PackageId, - PythonProjectCreator, - ResolveEnvironmentContext, + PackageInfo, PackageManagementOptions, + PackageManager, + PythonBackgroundRunOptions, + PythonEnvironment, + PythonEnvironmentApi, + PythonEnvironmentId, + PythonEnvironmentInfo, PythonProcess, + PythonProject, + PythonProjectCreator, PythonTaskExecutionOptions, - PythonTerminalExecutionOptions, - PythonBackgroundRunOptions, PythonTerminalCreateOptions, - DidChangeEnvironmentVariablesEventArgs, - CreateEnvironmentOptions, + PythonTerminalExecutionOptions, + RefreshEnvironmentsScope, + ResolveEnvironmentContext, + SetEnvironmentScope, } from '../api'; +import { traceError, traceInfo } from '../common/logging'; +import { pickEnvironmentManager } from '../common/pickers/managers'; +import { createDeferred } from '../common/utils/deferred'; +import { checkUri } from '../common/utils/pathUtils'; +import { handlePythonPath } from '../common/utils/pythonPath'; import { EnvironmentManagers, InternalEnvironmentManager, @@ -38,17 +43,12 @@ import { PythonPackageImpl, PythonProjectManager, } from '../internal.api'; -import { createDeferred } from '../common/utils/deferred'; -import { traceInfo } from '../common/logging'; -import { pickEnvironmentManager } from '../common/pickers/managers'; -import { handlePythonPath } from '../common/utils/pythonPath'; -import { TerminalManager } from './terminal/terminalManager'; +import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './common/managerReady'; +import { EnvVarManager } from './execution/envVariableManager'; import { runAsTask } from './execution/runAsTask'; -import { runInTerminal } from './terminal/runInTerminal'; import { runInBackground } from './execution/runInBackground'; -import { EnvVarManager } from './execution/envVariableManager'; -import { checkUri } from '../common/utils/pathUtils'; -import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './common/managerReady'; +import { runInTerminal } from './terminal/runInTerminal'; +import { TerminalManager } from './terminal/terminalManager'; class PythonEnvironmentApiImpl implements PythonEnvironmentApi { private readonly _onDidChangeEnvironments = new EventEmitter(); @@ -71,7 +71,7 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi { this._onDidChangePythonProjects, this._onDidChangePackages, this._onDidChangeEnvironmentVariables, - this.envManagers.onDidChangeEnvironmentFiltered((e) => { + this.envManagers.onDidChangeActiveEnvironment((e) => { this._onDidChangeEnvironment.fire(e); const location = e.uri?.fsPath ?? 'global'; traceInfo( @@ -91,14 +91,14 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi { if (manager.onDidChangeEnvironment) { disposables.push( manager.onDidChangeEnvironment((e) => { - setImmediate(async () => { - // This will ensure that we use the right manager and only trigger the event - // if the user selected manager decided to change the environment. - // This ensures that if a unselected manager changes environment and raises events - // we don't trigger the Python API event which can cause issues with the consumers. - // This will trigger onDidChangeEnvironmentFiltered event in envManagers, which the Python - // API listens to, and re-triggers the onDidChangeEnvironment event. - await this.envManagers.getEnvironment(e.uri); + setImmediate(() => { + // Refresh the central cache for this scope. This ensures that only the + // *selected* manager's changes propagate (refreshEnvironment checks + // getEnvironmentManager(scope) internally). It updates the cache and + // fires onDidChangeActiveEnvironment, which the Python API listens to. + this.envManagers.refreshEnvironment(e.uri).catch((err) => + traceError('Failed to refresh environment on change:', err), + ); }); }), ); diff --git a/src/features/views/projectView.ts b/src/features/views/projectView.ts index 2c10b472..ba689ad9 100644 --- a/src/features/views/projectView.ts +++ b/src/features/views/projectView.ts @@ -59,7 +59,7 @@ export class ProjectView implements TreeDataProvider { this.projectManager.onDidChangeProjects(() => { this.debouncedUpdateProject.trigger(); }), - this.envManagers.onDidChangeEnvironment(() => { + this.envManagers.onDidChangeManagerEnvironment(() => { this.debouncedUpdateProject.trigger(); }), this.envManagers.onDidChangeEnvironments(() => { diff --git a/src/internal.api.ts b/src/internal.api.ts index 8637da75..3d896c0c 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -32,8 +32,8 @@ import { CreateEnvironmentNotSupported, RemoveEnvironmentNotSupported } from './ import { traceWarn } from './common/logging'; import { StopWatch } from './common/stopWatch'; import { EventNames } from './common/telemetry/constants'; -import { sendTelemetryEvent } from './common/telemetry/sender'; import { classifyError } from './common/telemetry/errorClassifier'; +import { sendTelemetryEvent } from './common/telemetry/sender'; export type EnvironmentManagerScope = undefined | string | Uri | PythonEnvironment; export type PackageManagerScope = undefined | string | Uri | PythonEnvironment | Package; @@ -86,20 +86,19 @@ export interface EnvironmentManagers extends Disposable { onDidChangeEnvironments: Event; /** - * This event is fired when an environment manager changes the environment for - * a particular scope (global, uri, workspace, etc). This can be any environment manager even if it is not the - * one selected by the user for the workspace. It is also fired if the change - * involves unselected to selected or selected to unselected. + * Fires when ANY registered environment manager reports a selection change for a scope, + * regardless of whether that manager is the one currently selected by the user. + * Use this for UI refresh (e.g., status bar updates) that should react to all manager activity. */ - onDidChangeEnvironment: Event; + onDidChangeManagerEnvironment: Event; /** - * This event is fired when a selected environment manager changes the environment - * for a particular scope (global, uri, workspace, etc). This is also only fired if - * the previous and current environments are different. It is also fired if the change - * involves unselected to selected or selected to unselected. + * Fires only when the *selected* (active) environment for a scope actually changes. + * This is the authoritative "the user's environment changed" event. Consumers that + * need to react to the effective interpreter (terminal activation, language server, + * Python API clients) should use this event. */ - onDidChangeEnvironmentFiltered: Event; + onDidChangeActiveEnvironment: Event; onDidChangePackages: Event; onDidChangeEnvironmentManager: Event; @@ -137,6 +136,7 @@ export interface EnvironmentManagers extends Disposable { ): Promise; setEnvironmentsIfUnset(scope: Uri[] | string, environment?: PythonEnvironment): Promise; getEnvironment(scope: GetEnvironmentScope): Promise; + refreshEnvironment(scope: GetEnvironmentScope): Promise; getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[]; } diff --git a/src/test/features/envManagers.unit.test.ts b/src/test/features/envManagers.unit.test.ts new file mode 100644 index 00000000..d642fa94 --- /dev/null +++ b/src/test/features/envManagers.unit.test.ts @@ -0,0 +1,338 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { Uri } from 'vscode'; +import { PythonEnvironment } from '../../api'; +import * as frameUtils from '../../common/utils/frameUtils'; +import * as workspaceApis from '../../common/workspace.apis'; +import { PythonEnvironmentManagers } from '../../features/envManagers'; +import { PythonProjectManager } from '../../internal.api'; + +suite('PythonEnvironmentManagers - getEnvironment', () => { + let sandbox: sinon.SinonSandbox; + let envManagers: PythonEnvironmentManagers; + let mockProjectManager: sinon.SinonStubbedInstance; + + const env311: PythonEnvironment = { + envId: { id: 'system-311', managerId: 'ms-python.python:system' }, + name: 'Python 3.11', + displayName: 'Python 3.11.15', + version: '3.11.15', + displayPath: '/usr/bin/python3.11', + environmentPath: Uri.file('/usr/bin/python3.11'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.11' } }, + }; + + const env314: PythonEnvironment = { + envId: { id: 'system-314', managerId: 'ms-python.python:system' }, + name: 'Python 3.14', + displayName: 'Python 3.14.4', + version: '3.14.4', + displayPath: '/usr/bin/python3.14', + environmentPath: Uri.file('/usr/bin/python3.14'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.14' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + + // Stub getCallingExtension to avoid stack-frame analysis issues in tests + sandbox.stub(frameUtils, 'getCallingExtension').returns('ms-python.python'); + + // Stub getConfiguration to return a minimal config that returns the system manager + sandbox.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string, defaultValue?: unknown) => { + if (key === 'defaultEnvManager') { + return 'ms-python.python:system'; + } + if (key === 'pythonProjects') { + return []; + } + return defaultValue; + }, + has: () => false, + inspect: () => undefined, + update: () => Promise.resolve(), + } as any); + + mockProjectManager = { + getProjects: sandbox.stub().returns([]), + get: sandbox.stub().returns(undefined), + } as unknown as sinon.SinonStubbedInstance; + + envManagers = new PythonEnvironmentManagers(mockProjectManager as unknown as PythonProjectManager); + }); + + teardown(() => { + sandbox.restore(); + }); + + /** + * Registers a fake environment manager that returns predefined environments. + */ + function registerFakeManager(managerId: string, getStub: sinon.SinonStub): void { + const fakeManager = { + name: managerId.split(':')[1], + displayName: managerId, + preferredPackageManagerId: 'ms-python.python:pip', + get: getStub, + set: sandbox.stub().resolves(), + resolve: sandbox.stub().resolves(undefined), + refresh: sandbox.stub().resolves(), + getEnvironments: sandbox.stub().resolves([]), + onDidChangeEnvironments: sandbox.stub().returns({ dispose: () => {} }), + onDidChangeEnvironment: sandbox.stub().returns({ dispose: () => {} }), + }; + envManagers.registerEnvironmentManager(fakeManager as any, { extensionId: 'ms-python.python' }); + } + + test('should NOT update cache when manager.get() returns a different env than what was set', async () => { + // Register a system manager whose get() returns env314 (the "latest") + const getStub = sandbox.stub().resolves(env314); + registerFakeManager('ms-python.python:system', getStub); + + // Simulate that initial selection set env311 via setEnvironment + await envManagers.setEnvironment(undefined, env311, false); + // Allow the setEnvironment change event to flush + await new Promise((resolve) => setImmediate(resolve)); + + // Now subscribe to change events AFTER the initial selection + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // Now getEnvironment() calls manager.get() which returns env314 + // but this should NOT update the cache or fire a change event + const result = await envManagers.getEnvironment(undefined); + + // getEnvironment returns what the manager reports (env314) + assert.strictEqual(result?.envId.id, 'system-314', 'Should return the manager result'); + + // But the internal cache should NOT have been updated + // (We verify by checking no change event was fired) + // Allow setImmediate callbacks to run + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(changeEvents.length, 0, 'Should NOT fire onDidChangeActiveEnvironment'); + }); + + test('should NOT fire change events on read', async () => { + const getStub = sandbox.stub().resolves(env311); + registerFakeManager('ms-python.python:system', getStub); + + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // Call getEnvironment multiple times + await envManagers.getEnvironment(undefined); + await envManagers.getEnvironment(undefined); + await envManagers.getEnvironment(undefined); + + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(changeEvents.length, 0, 'Pure read should never fire change events'); + }); + + test('should still return the correct env from manager.get()', async () => { + const getStub = sandbox.stub().resolves(env311); + registerFakeManager('ms-python.python:system', getStub); + + const result = await envManagers.getEnvironment(undefined); + assert.strictEqual(result?.envId.id, 'system-311'); + assert.ok(getStub.calledOnce, 'Should delegate to manager.get()'); + }); + + test('should return undefined when no managers are registered', async () => { + // No managers registered at all — size === 0 guard fires + const result = await envManagers.getEnvironment(Uri.file('/some/unknown/path')); + assert.strictEqual(result, undefined); + }); + + test('should return undefined when settings point to an unregistered manager', async () => { + // Register a 'conda' manager, but the config stub returns 'ms-python.python:system' + // as the defaultEnvManager. getEnvironmentManager will look up 'ms-python.python:system' + // in the map, find nothing, check the cache (empty), and return undefined. + // This exercises the fallback path in getEnvironmentManager beyond the size === 0 guard. + const getStub = sandbox.stub().resolves(env311); + registerFakeManager('ms-python.python:conda', getStub); + + const result = await envManagers.getEnvironment(Uri.file('/some/unrelated/path')); + assert.strictEqual( + result, + undefined, + 'Should return undefined when settings point to an unregistered manager and cache is empty', + ); + }); + + test('setEnvironment should still fire change events and update cache', async () => { + const getStub = sandbox.stub().resolves(env311); + registerFakeManager('ms-python.python:system', getStub); + + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // setEnvironment SHOULD update cache and fire event + await envManagers.setEnvironment(undefined, env311, false); + + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(changeEvents.length, 1, 'setEnvironment should fire change event'); + assert.strictEqual(changeEvents[0].new?.envId.id, 'system-311'); + }); + + test('subsequent getEnvironment does not overwrite setEnvironment selection', async () => { + // This is the core issue #1492 scenario: + // 1. setEnvironment selects env311 (from defaultInterpreterPath) + // 2. telemetry calls getEnvironment which triggers manager.get() returning env314 + // 3. The selection should NOT flip to env314 + + const getStub = sandbox.stub().resolves(env314); + registerFakeManager('ms-python.python:system', getStub); + + // Step 1: Initial selection picks env311 + await envManagers.setEnvironment(undefined, env311, false); + // Allow the setEnvironment change event to flush + await new Promise((resolve) => setImmediate(resolve)); + + // Subscribe AFTER initial selection + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // Step 2: Telemetry calls getEnvironment (which internally calls manager.get() → env314) + const result = await envManagers.getEnvironment(undefined); + + // It can return env314 (that's what the manager reports), but it must NOT fire a change event + assert.strictEqual(result?.envId.id, 'system-314'); + + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual( + changeEvents.length, + 0, + 'getEnvironment must not fire change events, preserving the initial selection', + ); + }); +}); + +suite('PythonEnvironmentManagers - refreshEnvironment', () => { + let sandbox: sinon.SinonSandbox; + let envManagers: PythonEnvironmentManagers; + let mockProjectManager: sinon.SinonStubbedInstance; + + const env311: PythonEnvironment = { + envId: { id: 'system-311', managerId: 'ms-python.python:system' }, + name: 'Python 3.11', + displayName: 'Python 3.11.15', + version: '3.11.15', + displayPath: '/usr/bin/python3.11', + environmentPath: Uri.file('/usr/bin/python3.11'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.11' } }, + }; + + const env314: PythonEnvironment = { + envId: { id: 'system-314', managerId: 'ms-python.python:system' }, + name: 'Python 3.14', + displayName: 'Python 3.14.4', + version: '3.14.4', + displayPath: '/usr/bin/python3.14', + environmentPath: Uri.file('/usr/bin/python3.14'), + sysPrefix: '/usr', + execInfo: { run: { executable: '/usr/bin/python3.14' } }, + }; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(frameUtils, 'getCallingExtension').returns('ms-python.python'); + sandbox.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string, defaultValue?: unknown) => { + if (key === 'defaultEnvManager') { + return 'ms-python.python:system'; + } + if (key === 'pythonProjects') { + return []; + } + return defaultValue; + }, + has: () => false, + inspect: () => undefined, + update: () => Promise.resolve(), + } as any); + + mockProjectManager = { + getProjects: sandbox.stub().returns([]), + get: sandbox.stub().returns(undefined), + } as unknown as sinon.SinonStubbedInstance; + + envManagers = new PythonEnvironmentManagers(mockProjectManager as unknown as PythonProjectManager); + }); + + teardown(() => { + sandbox.restore(); + }); + + function registerFakeManager(managerId: string, getStub: sinon.SinonStub): void { + const fakeManager = { + name: managerId.split(':')[1], + displayName: managerId, + preferredPackageManagerId: 'ms-python.python:pip', + get: getStub, + set: sandbox.stub().resolves(), + resolve: sandbox.stub().resolves(undefined), + refresh: sandbox.stub().resolves(), + getEnvironments: sandbox.stub().resolves([]), + onDidChangeEnvironments: sandbox.stub().returns({ dispose: () => {} }), + onDidChangeEnvironment: sandbox.stub().returns({ dispose: () => {} }), + }; + envManagers.registerEnvironmentManager(fakeManager as any, { extensionId: 'ms-python.python' }); + } + + test('should fire change event when manager reports a new environment', async () => { + const getStub = sandbox.stub().resolves(env314); + registerFakeManager('ms-python.python:system', getStub); + + // Set initial env + await envManagers.setEnvironment(undefined, env311, false); + await new Promise((resolve) => setImmediate(resolve)); + + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // refreshEnvironment should detect the difference and fire + await envManagers.refreshEnvironment(undefined); + await new Promise((resolve) => setImmediate(resolve)); + + assert.strictEqual(changeEvents.length, 1, 'refreshEnvironment should fire change event'); + assert.strictEqual(changeEvents[0].old?.envId.id, 'system-311'); + assert.strictEqual(changeEvents[0].new?.envId.id, 'system-314'); + + // Verify the cache was updated: a second refresh with the same env must NOT fire again + await envManagers.refreshEnvironment(undefined); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(changeEvents.length, 1, 'Second refresh with same env should NOT fire a second event'); + }); + + test('should NOT fire change event when manager reports same environment', async () => { + const getStub = sandbox.stub().resolves(env311); + registerFakeManager('ms-python.python:system', getStub); + + // Set initial env to env311 + await envManagers.setEnvironment(undefined, env311, false); + await new Promise((resolve) => setImmediate(resolve)); + + const changeEvents: any[] = []; + envManagers.onDidChangeActiveEnvironment((e) => changeEvents.push(e)); + + // refreshEnvironment sees no difference + await envManagers.refreshEnvironment(undefined); + await new Promise((resolve) => setImmediate(resolve)); + + assert.strictEqual(changeEvents.length, 0, 'No change means no event'); + }); + + test('should do nothing when no manager found for scope', async () => { + // No manager registered — should not throw + await envManagers.refreshEnvironment(Uri.file('/unknown/path')); + }); +});