diff --git a/src/extension.ts b/src/extension.ts index f45d46aa2..bd9f1aea0 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -107,6 +107,7 @@ import { registerSystemPythonFeatures } from './managers/builtin/main'; import { SysPythonManager } from './managers/builtin/sysPythonManager'; import { createNativePythonFinder, + clearCacheDirectory, getNativePythonToolsPathAndSource, getNativePythonToolsVersion, NativePythonFinder, @@ -246,6 +247,8 @@ export async function activate(context: ExtensionContext): Promise { await clearPersistentState(); await envManagers.clearCache(undefined); - await clearShellProfileCache(shellStartupProviders); + try { + if (sharedNativeFinder) { + await sharedNativeFinder.clearCache(); + } else { + await clearCacheDirectory(context); + } + } finally { + await clearShellProfileCache(shellStartupProviders); + } }), ...(isInlineScriptsFeatureEnabled() ? [ @@ -657,6 +668,7 @@ export async function activate(context: ExtensionContext): Promise; + /** + * Clears every discovery cache owned by the finder: the in-memory result map, the live PET `clear` + * request, and the on-disk cache directory. Rejects when the on-disk clear fails and no live server + * handled it. + */ + clearCache(): Promise; } interface NativeLog { level: string; @@ -342,15 +348,22 @@ async function sendRequestWithTimeout( } } -class NativePythonFinderImpl implements NativePythonFinder { +interface InFlightRefresh { + promise: Promise; + configuration: ConfigurationOptions; + generation: number; +} + +/** Concrete {@link NativePythonFinder} backed by the PET JSON-RPC server. Exported for unit tests. */ +export class NativePythonFinderImpl implements NativePythonFinder { private connection: rpc.MessageConnection; - private readonly pool: WorkerPool; - private cache: Map = new Map(); + private readonly pool: WorkerPool; + private readonly cache = new DiscoveryResultCache(); /** * Tracks in-flight hard refreshes by cache key so concurrent callers share a * single PET scan instead of queueing duplicate work. */ - private inFlightRefreshes: Map> = new Map(); + private inFlightRefreshes: Map = new Map(); private startDisposables: Disposable[] = []; private proc: ChildProcess | undefined; private processExited: boolean = false; @@ -374,8 +387,8 @@ class NativePythonFinderImpl implements NativePythonFinder { private readonly cacheDirectory?: Uri, ) { this.connection = this.start(); - this.pool = createRunningWorkerPool( - async (options) => await this.doRefresh(options), + this.pool = createRunningWorkerPool( + async (task) => await this.doRefresh(task.options, task.configuration), 1, 'NativeRefresh-task', ); @@ -580,13 +593,22 @@ class NativePythonFinderImpl implements NativePythonFinder { return 'all'; } - private async handleHardRefresh(options?: NativePythonEnvironmentKind | Uri[]): Promise { + private async handleHardRefresh( + options?: NativePythonEnvironmentKind | Uri[], + prebuiltConfiguration?: ConfigurationOptions, + ): Promise { const key = this.getKey(options); + const configuration = prebuiltConfiguration ?? (await this.buildConfigurationOptions()); + const generationAtStart = this.cache.generation; const inFlight = this.inFlightRefreshes.get(key); - if (inFlight) { + if ( + inFlight && + inFlight.generation === generationAtStart && + configurationEquals(inFlight.configuration, configuration) + ) { this.outputChannel.debug(`[Finder] Coalescing hard refresh with in-flight request for key: ${key}`); - return inFlight; + return inFlight.promise; } this.cache.delete(key); @@ -596,45 +618,81 @@ class NativePythonFinderImpl implements NativePythonFinder { this.outputChannel.debug(`[Finder] Hard refresh for key: ${key}`); } - // .finally clears the in-flight slot on both success AND failure paths so - // a rejected refresh does not poison the cache — the next call after a - // failure starts a fresh attempt, matching today's behavior. + let entry: InFlightRefresh; const refreshPromise = this.pool - .addToQueue(options) - .then((result) => { - if (!result || !Array.isArray(result)) { - this.outputChannel.warn(`[pet] Worker pool returned invalid result type: ${typeof result}`); + .addToQueue({ options, configuration }) + .then((refreshResult) => { + const results = refreshResult?.results; + if (!results || !Array.isArray(results)) { + this.outputChannel.warn( + `[pet] Worker pool returned invalid result type: ${typeof refreshResult}`, + ); return [] as NativeInfo[]; } - this.cache.set(key, result); - return result; + this.cache.set(key, refreshResult.configuration, results, generationAtStart); + return results; }) .finally(() => { - this.inFlightRefreshes.delete(key); + if (this.inFlightRefreshes.get(key) === entry) { + this.inFlightRefreshes.delete(key); + } }); - this.inFlightRefreshes.set(key, refreshPromise); + entry = { promise: refreshPromise, configuration, generation: generationAtStart }; + this.inFlightRefreshes.set(key, entry); return refreshPromise; } private async handleSoftRefresh(options?: NativePythonEnvironmentKind | Uri[]): Promise { const key = this.getKey(options); - const cacheResult = this.cache.get(key); - // Validate cache integrity - if cached value is not a valid array, do a hard refresh - if (!cacheResult || !Array.isArray(cacheResult)) { - if (cacheResult !== undefined) { - this.outputChannel.warn(`[pet] Cache contained invalid data type: ${typeof cacheResult}`); - this.cache.delete(key); + const configuration = await this.buildConfigurationOptions(); + const cacheResult = this.cache.getValid(key, configuration); + if (cacheResult) { + if (!options) { + this.outputChannel.debug('[Finder] Returning cached environments for all'); + } else { + this.outputChannel.debug(`[Finder] Returning cached environments for key: ${key}`); } - return this.handleHardRefresh(options); + return cacheResult; } - if (!options) { - this.outputChannel.debug('[Finder] Returning cached environments for all'); - } else { - this.outputChannel.debug(`[Finder] Returning cached environments for key: ${key}`); + return this.handleHardRefresh(options, configuration); + } + + public async clearCache(): Promise { + this.cache.clear(); + this.lastConfiguration = undefined; + + let liveClearSucceeded = false; + const serverLive = !this.startFailed && !this.processExited && this.proc !== undefined; + if (serverLive) { + try { + await sendRequestWithTimeout(this.connection, 'clear', {}, CLEAR_TIMEOUT_MS); + liveClearSucceeded = true; + this.outputChannel.info('[pet] Cleared native discovery cache via live server'); + } catch (ex) { + this.outputChannel.warn('[pet] Live `clear` request failed; relying on on-disk clear', ex); + } + } + + if (this.cacheDirectory) { + try { + await fs.emptyDir(this.cacheDirectory.fsPath); + this.outputChannel.info('[pet] Cleared on-disk PET cache directory'); + } catch (ex) { + if (liveClearSucceeded) { + this.outputChannel.warn( + '[pet] Redundant on-disk cache clear failed after a successful live clear; ignoring', + ex, + ); + } else { + this.outputChannel.error('[pet] Failed to clear on-disk PET cache directory', ex); + throw ex; + } + } + } else if (!liveClearSucceeded) { + this.outputChannel.warn('[pet] No live server and no cache directory configured; nothing to clear.'); } - return cacheResult; } public dispose() { @@ -843,12 +901,16 @@ class NativePythonFinderImpl implements NativePythonFinder { }; } - private async doRefresh(options?: NativePythonEnvironmentKind | Uri[]): Promise { + private async doRefresh( + options: NativePythonEnvironmentKind | Uri[] | undefined, + configuration: ConfigurationOptions, + ): Promise { let lastError: unknown; for (let attempt = 0; attempt <= MAX_REFRESH_RETRIES; attempt++) { try { - return await this.doRefreshAttempt(options, attempt); + const results = await this.doRefreshAttempt(options, attempt, configuration); + return { results, configuration }; } catch (ex) { lastError = ex; @@ -874,7 +936,7 @@ class NativePythonFinderImpl implements NativePythonFinder { // Non-timeout errors or final timeout — check if server is fully exhausted if (this.isServerExhausted()) { this.outputChannel.warn('[pet] Server mode exhausted, falling back to JSON CLI for refresh'); - return this.refreshViaJsonCli(options); + return { results: await this.refreshViaJsonCli(options, configuration), configuration }; } throw ex; } @@ -883,7 +945,7 @@ class NativePythonFinderImpl implements NativePythonFinder { // Should not reach here, but TypeScript needs this if (this.isServerExhausted()) { this.outputChannel.warn('[pet] Server mode exhausted, falling back to JSON CLI for refresh (final)'); - return this.refreshViaJsonCli(options); + return { results: await this.refreshViaJsonCli(options, configuration), configuration }; } throw lastError; } @@ -891,6 +953,7 @@ class NativePythonFinderImpl implements NativePythonFinder { private async doRefreshAttempt( options: NativePythonEnvironmentKind | Uri[] | undefined, attempt: number, + configuration: ConfigurationOptions, ): Promise { await this.ensureProcessRunning(); const disposables: Disposable[] = []; @@ -899,12 +962,9 @@ class NativePythonFinderImpl implements NativePythonFinder { const sw = new StopWatch(); let unresolvedCount = 0; let refreshPerf: RefreshPerformance | undefined; - let workspaceDirCount: number | undefined; - let searchPathCount: number | undefined; + const workspaceDirCount: number | undefined = configuration.workspaceDirectories.length; + const searchPathCount: number | undefined = configuration.environmentDirectories.length; try { - const configuration = await this.buildConfigurationOptions(); - workspaceDirCount = configuration.workspaceDirectories.length; - searchPathCount = configuration.environmentDirectories.length; await this.configure(configuration); const refreshOptions = this.getRefreshOptions(options); disposables.push( @@ -1023,11 +1083,12 @@ class NativePythonFinderImpl implements NativePythonFinder { * Must be invoked when ever there are changes to any data related to the configuration details. */ private async configure(options?: ConfigurationOptions) { + const generationAtStart = this.cache.generation; const configuration = options ?? (await this.buildConfigurationOptions()); const workspaceDirCount = configuration.workspaceDirectories.length; const envDirCount = configuration.environmentDirectories.length; // No need to send a configuration request if there are no changes. - if (this.lastConfiguration && this.configurationEquals(configuration, this.lastConfiguration)) { + if (this.lastConfiguration && configurationEquals(configuration, this.lastConfiguration)) { this.outputChannel.debug('[pet] configure: No changes detected, skipping configuration update.'); sendTelemetryEvent( EventNames.PET_CONFIGURE, @@ -1048,8 +1109,13 @@ class NativePythonFinderImpl implements NativePythonFinder { const retryCount = this.configureRetry.timeoutCount; try { await sendRequestWithTimeout(this.connection, 'configure', configuration, timeoutMs); - // Only cache after success so failed/timed-out calls will retry - this.lastConfiguration = configuration; + if (this.cache.generation === generationAtStart) { + this.lastConfiguration = configuration; + } else { + this.outputChannel.debug( + '[pet] configure: cache was cleared during configure; not caching lastConfiguration', + ); + } this.configureRetry.onSuccess(); sendTelemetryEvent( EventNames.PET_CONFIGURE, @@ -1203,10 +1269,13 @@ class NativePythonFinderImpl implements NativePythonFinder { * Spawns PET as a one-shot subprocess and parses the JSON output. * * @param options Optional kind filter or URI search paths (same semantics as refresh()). + * @param config The effective configuration for this refresh. * @returns NativeInfo[] containing managers and environments, same as server mode. */ - private async refreshViaJsonCli(options?: NativePythonEnvironmentKind | Uri[]): Promise { - const config = await this.buildConfigurationOptions(); + private async refreshViaJsonCli( + options: NativePythonEnvironmentKind | Uri[] | undefined, + config: ConfigurationOptions, + ): Promise { // venvFolders must be included explicitly as search paths when options is Uri[], // mirroring getRefreshOptions() server-mode behaviour (searchPaths may override environmentDirectories). const venvFolders = getPythonSettingAndUntildify('venvFolders') ?? []; @@ -1348,56 +1417,120 @@ class NativePythonFinderImpl implements NativePythonFinder { }); return parsed; } +} - /** - * Compares two ConfigurationOptions objects for equality. - * Uses property-by-property comparison to avoid issues with JSON.stringify - * (property order, undefined values serialization). - */ - private configurationEquals(a: ConfigurationOptions, b: ConfigurationOptions): boolean { - // Compare simple optional string properties - if (a.condaExecutable !== b.condaExecutable) { +export type ConfigurationOptions = { + workspaceDirectories: string[]; + environmentDirectories: string[]; + condaExecutable: string | undefined; + pipenvExecutable: string | undefined; + poetryExecutable: string | undefined; + cacheDirectory?: string; +}; + +export interface RefreshResult { + results: NativeInfo[]; + configuration: ConfigurationOptions; +} + +interface RefreshTask { + options?: NativePythonEnvironmentKind | Uri[]; + configuration: ConfigurationOptions; +} + +/** Property-by-property equality (arrays order-independent) that gates `configure` and soft hits. */ +export function configurationEquals(a: ConfigurationOptions, b: ConfigurationOptions): boolean { + if (a.condaExecutable !== b.condaExecutable) { + return false; + } + if (a.pipenvExecutable !== b.pipenvExecutable) { + return false; + } + if (a.poetryExecutable !== b.poetryExecutable) { + return false; + } + if (a.cacheDirectory !== b.cacheDirectory) { + return false; + } + + const arraysEqual = (arr1: string[], arr2: string[]): boolean => { + if (arr1.length !== arr2.length) { return false; } - if (a.pipenvExecutable !== b.pipenvExecutable) { - return false; + const sorted1 = [...arr1].sort(); + const sorted2 = [...arr2].sort(); + return sorted1.every((val, idx) => val === sorted2[idx]); + }; + + if (!arraysEqual(a.workspaceDirectories, b.workspaceDirectories)) { + return false; + } + if (!arraysEqual(a.environmentDirectories, b.environmentDirectories)) { + return false; + } + + return true; +} + +interface DiscoveryCacheEntry { + configuration: ConfigurationOptions; + results: NativeInfo[]; + generation: number; +} + +/** + * Result cache keyed by refresh scope: a soft hit is valid only when the key's saved configuration + * still matches, and a monotonic {@link generation} (advanced by {@link clear}) rejects pre-clear stores. + */ +export class DiscoveryResultCache { + private readonly entries = new Map(); + private currentGeneration = 0; + + get generation(): number { + return this.currentGeneration; + } + + get size(): number { + return this.entries.size; + } + + getValid(key: string, configuration: ConfigurationOptions): NativeInfo[] | undefined { + const entry = this.entries.get(key); + if (!entry) { + return undefined; } - if (a.poetryExecutable !== b.poetryExecutable) { - return false; + if (entry.generation !== this.currentGeneration) { + this.entries.delete(key); + return undefined; } - if (a.cacheDirectory !== b.cacheDirectory) { - return false; + if (!configurationEquals(entry.configuration, configuration)) { + return undefined; } + return entry.results; + } - // Compare array properties using sorted comparison to handle order differences - const arraysEqual = (arr1: string[], arr2: string[]): boolean => { - if (arr1.length !== arr2.length) { - return false; - } - const sorted1 = [...arr1].sort(); - const sorted2 = [...arr2].sort(); - return sorted1.every((val, idx) => val === sorted2[idx]); - }; - - if (!arraysEqual(a.workspaceDirectories, b.workspaceDirectories)) { + set( + key: string, + configuration: ConfigurationOptions, + results: NativeInfo[], + generationAtStart: number, + ): boolean { + if (generationAtStart !== this.currentGeneration) { return false; } - if (!arraysEqual(a.environmentDirectories, b.environmentDirectories)) { - return false; - } - + this.entries.set(key, { configuration, results, generation: this.currentGeneration }); return true; } -} -export type ConfigurationOptions = { - workspaceDirectories: string[]; - environmentDirectories: string[]; - condaExecutable: string | undefined; - pipenvExecutable: string | undefined; - poetryExecutable: string | undefined; - cacheDirectory?: string; -}; + delete(key: string): void { + this.entries.delete(key); + } + + clear(): void { + this.currentGeneration++; + this.entries.clear(); + } +} /** * Parses the stdout of `pet find --json` into a structured result. @@ -1684,7 +1817,13 @@ export function getCacheDirectory(context: ExtensionContext): Uri { export async function clearCacheDirectory(context: ExtensionContext): Promise { const cacheDirectory = getCacheDirectory(context); - await fs.emptyDir(cacheDirectory.fsPath).catch(noop); + try { + await fs.emptyDir(cacheDirectory.fsPath); + traceVerbose(`[pet] Cleared on-disk discovery cache directory: ${cacheDirectory.fsPath}`); + } catch (ex) { + traceError(`[pet] Failed to clear on-disk discovery cache directory: ${cacheDirectory.fsPath}`, ex); + throw ex; + } } export async function createNativePythonFinder( diff --git a/src/test/managers/common/nativePythonFinder.clearCache.unit.test.ts b/src/test/managers/common/nativePythonFinder.clearCache.unit.test.ts new file mode 100644 index 000000000..df458723e --- /dev/null +++ b/src/test/managers/common/nativePythonFinder.clearCache.unit.test.ts @@ -0,0 +1,482 @@ +import assert from 'node:assert'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import fsExtra from 'fs-extra'; +import * as sinon from 'sinon'; +import { ExtensionContext, LogOutputChannel, Uri } from 'vscode'; +import * as logging from '../../../common/logging'; +import * as telemetrySender from '../../../common/telemetry/sender'; +import { PythonProjectApi } from '../../../api'; +import { + ConfigurationOptions, + clearCacheDirectory, + NativePythonFinderImpl, + NativeInfo, + RefreshResult, +} from '../../../managers/common/nativePythonFinder'; + +interface FakeConnection { + sendRequest: sinon.SinonStub; + dispose: sinon.SinonStub; +} + +suite('NativePythonFinderImpl.clearCache', () => { + let startStub: sinon.SinonStub; + let emptyDirStub: sinon.SinonStub; + let outputChannel: LogOutputChannel; + const finders: NativePythonFinderImpl[] = []; + + setup(() => { + outputChannel = makeOutputChannel(); + emptyDirStub = sinon.stub(fsExtra, 'emptyDir').resolves(); + startStub = installStartStub(); + }); + + teardown(() => { + disposeAll(finders); + sinon.restore(); + }); + + test('sends the `clear` RPC and also empties the on-disk cache directory when a live server is available', async () => { + const cacheDir = Uri.file(path.join(os.tmpdir(), 'pet-clear-live')); + const finder = createFinder(cacheDir); + const sendRequest = connectionOf(finder).sendRequest; + + await finder.clearCache(); + + assert.strictEqual(sendRequest.callCount, 1, 'clear RPC should be sent once'); + assert.strictEqual(sendRequest.firstCall.args[0], 'clear'); + assert.deepStrictEqual(sendRequest.firstCall.args[1], {}); + assert.strictEqual(emptyDirStub.callCount, 1); + assert.strictEqual(emptyDirStub.firstCall.args[0], cacheDir.fsPath); + }); + + test('advances the cache generation and resets lastConfiguration', async () => { + const finder = createFinder(Uri.file(path.join(os.tmpdir(), 'pet-clear-gen'))); + const asAny = finder as unknown as { cache: { generation: number }; lastConfiguration?: unknown }; + asAny.lastConfiguration = { workspaceDirectories: [] }; + const generationBefore = asAny.cache.generation; + + await finder.clearCache(); + + assert.strictEqual(asAny.cache.generation, generationBefore + 1); + assert.strictEqual(asAny.lastConfiguration, undefined); + }); + + test('empties the on-disk cache directory when no live server is available', async () => { + const cacheDir = Uri.file(path.join(os.tmpdir(), 'pet-clear-nolive')); + const finder = createFinder(cacheDir); + (finder as unknown as { processExited: boolean }).processExited = true; + const sendRequest = connectionOf(finder).sendRequest; + + await finder.clearCache(); + + assert.strictEqual(sendRequest.called, false, 'no RPC should be sent without a live server'); + assert.strictEqual(emptyDirStub.callCount, 1); + assert.strictEqual(emptyDirStub.firstCall.args[0], cacheDir.fsPath); + }); + + test('still empties the on-disk cache directory when the live `clear` RPC fails', async () => { + const cacheDir = Uri.file(path.join(os.tmpdir(), 'pet-clear-rpcfail')); + const finder = createFinder(cacheDir); + connectionOf(finder).sendRequest.rejects(new Error('rpc boom')); + + await finder.clearCache(); + + assert.strictEqual(emptyDirStub.callCount, 1, 'disk clear should run even after RPC failure'); + assert.strictEqual(emptyDirStub.firstCall.args[0], cacheDir.fsPath); + }); + + test('propagates on-disk clear failures instead of swallowing them', async () => { + const cacheDir = Uri.file(path.join(os.tmpdir(), 'pet-clear-diskfail')); + const finder = createFinder(cacheDir); + (finder as unknown as { processExited: boolean }).processExited = true; + emptyDirStub.rejects(new Error('disk boom')); + + await assert.rejects(() => finder.clearCache(), /disk boom/); + }); + + test('does not fail the command when the redundant on-disk clear fails after a successful live clear', async () => { + const cacheDir = Uri.file(path.join(os.tmpdir(), 'pet-clear-redundant-fail')); + const finder = createFinder(cacheDir); + emptyDirStub.rejects(new Error('redundant disk boom')); + + await finder.clearCache(); + + assert.strictEqual(connectionOf(finder).sendRequest.callCount, 1, 'live clear should have run'); + assert.strictEqual(emptyDirStub.callCount, 1, 'redundant disk sweep should have been attempted'); + }); + + test('warns and does not throw when there is no live server and no cache directory', async () => { + const finder = createFinder(undefined); + (finder as unknown as { processExited: boolean }).processExited = true; + + await finder.clearCache(); + + assert.strictEqual(emptyDirStub.called, false); + }); + + suite('start-time bookkeeping', () => { + test('start() is invoked exactly once per finder construction', () => { + createFinder(Uri.file(path.join(os.tmpdir(), 'pet-clear-count'))); + assert.strictEqual(startStub.callCount, 1); + }); + }); + + function createFinder(cacheDirectory: Uri | undefined): NativePythonFinderImpl { + const finder = new NativePythonFinderImpl(outputChannel, path.join('tool', 'pet'), makeApi(), cacheDirectory); + finders.push(finder); + return finder; + } +}); + +suite('NativePythonFinderImpl.configure clear-race guard', () => { + let outputChannel: LogOutputChannel; + const finders: NativePythonFinderImpl[] = []; + + setup(() => { + outputChannel = makeOutputChannel(); + sinon.stub(fsExtra, 'emptyDir').resolves(); + sinon.stub(telemetrySender, 'sendTelemetryEvent'); + installStartStub(); + }); + + teardown(() => { + disposeAll(finders); + sinon.restore(); + }); + + type Internals = { + connection: FakeConnection; + configure: (options?: ConfigurationOptions) => Promise; + lastConfiguration?: ConfigurationOptions; + }; + + test('does not resurrect lastConfiguration when clearCache() lands mid-configure', async () => { + const finder = createFinder(); + const internals = finder as unknown as Internals & { clearCache: () => Promise }; + const sendRequest = connectionOf(finder).sendRequest; + + let resolveConfigure!: (value: unknown) => void; + sendRequest.withArgs('configure').returns(new Promise((resolve) => (resolveConfigure = resolve))); + sendRequest.withArgs('clear').resolves(null); + + const config = makeConfig(); + const configurePromise = internals.configure(config); + await Promise.resolve(); + + await finder.clearCache(); + + resolveConfigure(null); + await configurePromise; + + assert.strictEqual( + internals.lastConfiguration, + undefined, + 'a configure that raced a clear must not resurrect lastConfiguration', + ); + }); + + test('caches lastConfiguration normally when no clear intervenes', async () => { + const finder = createFinder(); + const internals = finder as unknown as Internals; + + const config = makeConfig(); + await internals.configure(config); + + assert.deepStrictEqual(internals.lastConfiguration, config); + }); + + function makeConfig(): ConfigurationOptions { + return { + workspaceDirectories: [Uri.file('/work/a').fsPath], + environmentDirectories: [], + condaExecutable: undefined, + pipenvExecutable: undefined, + poetryExecutable: undefined, + }; + } + + function createFinder(): NativePythonFinderImpl { + const finder = new NativePythonFinderImpl( + outputChannel, + path.join('tool', 'pet'), + makeApi(), + Uri.file(path.join(os.tmpdir(), 'pet-configure-race')), + ); + finders.push(finder); + return finder; + } +}); + +suite('NativePythonFinderImpl in-flight refresh coalescing', () => { + let outputChannel: LogOutputChannel; + const finders: NativePythonFinderImpl[] = []; + + setup(() => { + outputChannel = makeOutputChannel(); + sinon.stub(fsExtra, 'emptyDir').resolves(); + sinon.stub(telemetrySender, 'sendTelemetryEvent'); + installStartStub(); + }); + + teardown(() => { + disposeAll(finders); + sinon.restore(); + }); + + test('a hard refresh in flight when clearCache() runs is not reused by a later refresh', async () => { + const finder = createFinder(); + stubBuildConfiguration(finder).resolves(makeConfiguration()); + const addToQueue = stubAddToQueue(finder); + const first = createDeferred(); + const second = createDeferred(); + addToQueue.onCall(0).returns(first.promise); + addToQueue.onCall(1).returns(second.promise); + + const firstRefresh = finder.refresh(true); + await flushMicrotasks(); + assert.strictEqual(addToQueue.callCount, 1); + + await finder.clearCache(); + + const secondRefresh = finder.refresh(true); + await flushMicrotasks(); + assert.strictEqual(addToQueue.callCount, 2, 'a refresh after clear must not coalesce onto pre-clear work'); + + first.resolve({ results: makeNativeResult('stale'), configuration: makeConfiguration() }); + second.resolve({ results: makeNativeResult('fresh'), configuration: makeConfiguration() }); + const secondResults = await secondRefresh; + await firstRefresh; + + assert.strictEqual(secondResults.length, 1); + assert.strictEqual((secondResults[0] as { executable: string }).executable, '/py/fresh'); + }); + + test('a hard refresh in flight when the configuration changes is not reused by a later refresh', async () => { + const finder = createFinder(); + const buildConfiguration = stubBuildConfiguration(finder); + const configA = makeConfiguration(); + const configB = makeConfiguration({ condaExecutable: Uri.file('/tools/conda-b').fsPath }); + buildConfiguration.onCall(0).resolves(configA); + buildConfiguration.onCall(1).resolves(configB); + const addToQueue = stubAddToQueue(finder); + const first = createDeferred(); + const second = createDeferred(); + addToQueue.onCall(0).returns(first.promise); + addToQueue.onCall(1).returns(second.promise); + + const firstRefresh = finder.refresh(false); + await flushMicrotasks(); + assert.strictEqual(addToQueue.callCount, 1); + + const secondRefresh = finder.refresh(false); + await flushMicrotasks(); + assert.strictEqual(addToQueue.callCount, 2, 'a refresh under a changed configuration must not coalesce'); + + first.resolve({ results: makeNativeResult('a'), configuration: configA }); + second.resolve({ results: makeNativeResult('b'), configuration: configB }); + await Promise.all([firstRefresh, secondRefresh]); + }); + + test('a second refresh for the same key and configuration coalesces onto the in-flight request', async () => { + const finder = createFinder(); + stubBuildConfiguration(finder).resolves(makeConfiguration()); + const addToQueue = stubAddToQueue(finder); + const first = createDeferred(); + addToQueue.onCall(0).returns(first.promise); + + const firstRefresh = finder.refresh(true); + await flushMicrotasks(); + const secondRefresh = finder.refresh(true); + await flushMicrotasks(); + + assert.strictEqual(addToQueue.callCount, 1); + + first.resolve({ results: makeNativeResult('shared'), configuration: makeConfiguration() }); + const [firstResults, secondResults] = await Promise.all([firstRefresh, secondRefresh]); + assert.strictEqual(firstResults, secondResults); + }); + + test('queued refreshes execute under the configuration captured when requested (A→B→A)', async () => { + const finder = createFinder(); + const configA = makeConfiguration(); + const configB = makeConfiguration({ condaExecutable: Uri.file('/tools/conda-b').fsPath }); + const buildConfiguration = stubBuildConfiguration(finder); + buildConfiguration.onCall(0).resolves(configA); + buildConfiguration.onCall(1).resolves(configB); + buildConfiguration.onCall(2).resolves(configA); + + const gates = [createDeferred(), createDeferred(), createDeferred()]; + const executedConfigurations: ConfigurationOptions[] = []; + let dispatched = 0; + sinon + .stub( + finder as unknown as { + doRefreshAttempt: ( + options: unknown, + attempt: number, + configuration: ConfigurationOptions, + ) => Promise; + }, + 'doRefreshAttempt', + ) + .callsFake((_options: unknown, _attempt: number, configuration: ConfigurationOptions) => { + executedConfigurations.push(configuration); + return gates[dispatched++].promise; + }); + + const firstRefresh = finder.refresh(true); + await flushMicrotasks(); + const secondRefresh = finder.refresh(true); + await flushMicrotasks(); + const thirdRefresh = finder.refresh(true); + await flushMicrotasks(); + + assert.deepStrictEqual(executedConfigurations, [configA]); + + gates[0].resolve(makeNativeResult('a')); + await flushMicrotasks(); + gates[1].resolve(makeNativeResult('b')); + await flushMicrotasks(); + gates[2].resolve(makeNativeResult('a2')); + await Promise.all([firstRefresh, secondRefresh, thirdRefresh]); + + assert.deepStrictEqual(executedConfigurations, [configA, configB, configA]); + assert.strictEqual(buildConfiguration.callCount, 3); + }); + + function createFinder(): NativePythonFinderImpl { + const finder = new NativePythonFinderImpl( + outputChannel, + path.join('tool', 'pet'), + makeApi(), + Uri.file(path.join(os.tmpdir(), 'pet-inflight-race')), + ); + finders.push(finder); + return finder; + } + + function stubAddToQueue(finder: NativePythonFinderImpl): sinon.SinonStub { + return sinon.stub((finder as unknown as { pool: { addToQueue: () => unknown } }).pool, 'addToQueue'); + } + + function stubBuildConfiguration(finder: NativePythonFinderImpl): sinon.SinonStub { + return sinon.stub( + finder as unknown as { buildConfigurationOptions: () => Promise }, + 'buildConfigurationOptions', + ); + } + + function makeConfiguration(overrides: Partial = {}): ConfigurationOptions { + return { + workspaceDirectories: [Uri.file('/work/a').fsPath], + environmentDirectories: [], + condaExecutable: undefined, + pipenvExecutable: undefined, + poetryExecutable: undefined, + ...overrides, + }; + } + + function makeNativeResult(tag: string): NativeInfo[] { + return [{ executable: `/py/${tag}` } as unknown as NativeInfo]; + } + + function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; + } + + function flushMicrotasks(): Promise { + return new Promise((resolve) => setImmediate(resolve)); + } +}); + +suite('clearCacheDirectory (disk fallback before finder exists)', () => { + let emptyDirStub: sinon.SinonStub; + let traceVerboseStub: sinon.SinonStub; + let traceErrorStub: sinon.SinonStub; + + setup(() => { + emptyDirStub = sinon.stub(fsExtra, 'emptyDir').resolves(); + traceVerboseStub = sinon.stub(logging, 'traceVerbose'); + traceErrorStub = sinon.stub(logging, 'traceError'); + }); + + teardown(() => sinon.restore()); + + test('empties the pythonLocator directory under global storage', async () => { + const globalStorage = Uri.file(path.join(os.tmpdir(), 'global-storage')); + await clearCacheDirectory(makeContext(globalStorage)); + + assert.strictEqual(emptyDirStub.callCount, 1); + assert.strictEqual(emptyDirStub.firstCall.args[0], Uri.joinPath(globalStorage, 'pythonLocator').fsPath); + assert.strictEqual(traceVerboseStub.called, true); + }); + + test('logs and propagates filesystem failures', async () => { + const globalStorage = Uri.file(path.join(os.tmpdir(), 'global-storage-fail')); + emptyDirStub.rejects(new Error('emptyDir failed')); + + await assert.rejects(() => clearCacheDirectory(makeContext(globalStorage)), /emptyDir failed/); + assert.strictEqual(traceErrorStub.called, true); + }); + + function makeContext(globalStorage: Uri): ExtensionContext { + return { globalStorageUri: globalStorage } as unknown as ExtensionContext; + } +}); + +function makeOutputChannel(): LogOutputChannel { + const noop = sinon.stub(); + return { + info: noop, + warn: noop, + error: noop, + debug: noop, + trace: noop, + append: noop, + appendLine: noop, + replace: noop, + clear: noop, + show: noop, + hide: noop, + dispose: noop, + name: 'test', + logLevel: 0, + onDidChangeLogLevel: noop, + } as unknown as LogOutputChannel; +} + +function makeApi(): PythonProjectApi { + return { getPythonProjects: () => [] } as unknown as PythonProjectApi; +} + +function installStartStub(): sinon.SinonStub { + return sinon + .stub(NativePythonFinderImpl.prototype as unknown as { start: () => unknown }, 'start') + .callsFake(function (this: Record) { + this.proc = { exitCode: null, kill: sinon.stub() }; + const connection: FakeConnection = { sendRequest: sinon.stub().resolves(null), dispose: sinon.stub() }; + return connection; + }); +} + +function connectionOf(finder: NativePythonFinderImpl): FakeConnection { + return (finder as unknown as { connection: FakeConnection }).connection; +} + +function disposeAll(finders: NativePythonFinderImpl[]): void { + while (finders.length > 0) { + const finder = finders.pop(); + try { + finder?.dispose(); + } catch { + // ignore + } + } +} diff --git a/src/test/managers/common/nativePythonFinder.discoveryCache.unit.test.ts b/src/test/managers/common/nativePythonFinder.discoveryCache.unit.test.ts new file mode 100644 index 000000000..d20867dea --- /dev/null +++ b/src/test/managers/common/nativePythonFinder.discoveryCache.unit.test.ts @@ -0,0 +1,210 @@ +import assert from 'node:assert'; +import { Uri } from 'vscode'; +import { + ConfigurationOptions, + configurationEquals, + DiscoveryResultCache, + NativeInfo, +} from '../../../managers/common/nativePythonFinder'; + +suite('configurationEquals', () => { + test('returns true for two independently-built identical configurations', () => { + assert.strictEqual(configurationEquals(makeConfig(), makeConfig()), true); + }); + + test('is order-independent for workspaceDirectories', () => { + const a = makeConfig({ workspaceDirectories: [Uri.file('/work/a').fsPath, Uri.file('/work/b').fsPath] }); + const b = makeConfig({ workspaceDirectories: [Uri.file('/work/b').fsPath, Uri.file('/work/a').fsPath] }); + assert.strictEqual(configurationEquals(a, b), true); + }); + + test('is order-independent for environmentDirectories', () => { + const a = makeConfig({ environmentDirectories: [Uri.file('/envs/x').fsPath, Uri.file('/envs/y').fsPath] }); + const b = makeConfig({ environmentDirectories: [Uri.file('/envs/y').fsPath, Uri.file('/envs/x').fsPath] }); + assert.strictEqual(configurationEquals(a, b), true); + }); + + test('detects a changed workspaceDirectories entry', () => { + const a = makeConfig(); + const b = makeConfig({ workspaceDirectories: [Uri.file('/work/a').fsPath, Uri.file('/work/c').fsPath] }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a different workspaceDirectories length', () => { + const a = makeConfig(); + const b = makeConfig({ workspaceDirectories: [Uri.file('/work/a').fsPath] }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a changed environmentDirectories entry', () => { + const a = makeConfig(); + const b = makeConfig({ environmentDirectories: [Uri.file('/envs/x').fsPath, Uri.file('/envs/z').fsPath] }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a changed condaExecutable', () => { + const a = makeConfig(); + const b = makeConfig({ condaExecutable: Uri.file('/tools/conda2').fsPath }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects condaExecutable defined-vs-undefined', () => { + const a = makeConfig(); + const b = makeConfig({ condaExecutable: undefined }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a changed pipenvExecutable', () => { + const a = makeConfig(); + const b = makeConfig({ pipenvExecutable: Uri.file('/tools/pipenv2').fsPath }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a changed poetryExecutable', () => { + const a = makeConfig(); + const b = makeConfig({ poetryExecutable: Uri.file('/tools/poetry2').fsPath }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects a changed cacheDirectory', () => { + const a = makeConfig(); + const b = makeConfig({ cacheDirectory: Uri.file('/cache/other').fsPath }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('detects cacheDirectory defined-vs-undefined', () => { + const a = makeConfig(); + const b = makeConfig({ cacheDirectory: undefined }); + assert.strictEqual(configurationEquals(a, b), false); + }); + + test('treats identical fsPath workspace directories as equal', () => { + const p = Uri.file('/work/spaces and unicode проекты').fsPath; + const a = makeConfig({ workspaceDirectories: [p] }); + const b = makeConfig({ workspaceDirectories: [Uri.file('/work/spaces and unicode проекты').fsPath] }); + assert.strictEqual(configurationEquals(a, b), true); + }); +}); + +suite('DiscoveryResultCache', () => { + const ALL = 'all'; + const CONDA = 'Conda'; + const URI_KEY = [Uri.file('/some/project').fsPath].join('\0'); + + test('returns undefined for an unknown key', () => { + const cache = new DiscoveryResultCache(); + assert.strictEqual(cache.getValid(ALL, makeConfig()), undefined); + }); + + test('per-key same-config lookup is a hit', () => { + const cache = new DiscoveryResultCache(); + const config = makeConfig(); + const results = makeResults('all'); + cache.set(ALL, config, results, cache.generation); + assert.strictEqual(cache.getValid(ALL, config), results); + }); + + test('round-trips the exact configuration used to tag the entry', () => { + const cache = new DiscoveryResultCache(); + const configA = makeConfig(); + const configB = makeConfig({ condaExecutable: Uri.file('/tools/conda-changed').fsPath }); + const results = makeResults('all'); + cache.set(ALL, configA, results, cache.generation); + assert.strictEqual(cache.getValid(ALL, configA), results, 'same config should hit'); + assert.strictEqual(cache.getValid(ALL, configB), undefined, 'changed config should miss'); + }); + + test('a config change for one key does not invalidate other keys', () => { + const cache = new DiscoveryResultCache(); + const configA = makeConfig(); + const configB = makeConfig({ environmentDirectories: [Uri.file('/envs/new').fsPath] }); + const allResults = makeResults('all'); + const condaResults = makeResults('conda'); + cache.set(ALL, configA, allResults, cache.generation); + cache.set(CONDA, configA, condaResults, cache.generation); + + cache.set(ALL, configB, makeResults('all-2'), cache.generation); + + assert.deepStrictEqual(cache.getValid(CONDA, configA), condaResults); + assert.strictEqual(cache.getValid(ALL, configB)?.length, 1); + }); + + test('alternating all/kind/URI lookups with unrelated configs do not thrash', () => { + const cache = new DiscoveryResultCache(); + const configAll = makeConfig(); + const configConda = makeConfig({ poetryExecutable: Uri.file('/tools/poetry-b').fsPath }); + const configUri = makeConfig({ cacheDirectory: Uri.file('/cache/uri').fsPath }); + cache.set(ALL, configAll, makeResults('all'), cache.generation); + cache.set(CONDA, configConda, makeResults('conda'), cache.generation); + cache.set(URI_KEY, configUri, makeResults('uri'), cache.generation); + assert.strictEqual(cache.size, 3); + + for (let i = 0; i < 3; i++) { + assert.ok(cache.getValid(ALL, configAll), 'all should stay valid'); + assert.ok(cache.getValid(CONDA, configConda), 'conda should stay valid'); + assert.ok(cache.getValid(URI_KEY, configUri), 'uri should stay valid'); + } + assert.strictEqual(cache.size, 3); + }); + + test('delete() removes a single key', () => { + const cache = new DiscoveryResultCache(); + const config = makeConfig(); + cache.set(ALL, config, makeResults('all'), cache.generation); + cache.delete(ALL); + assert.strictEqual(cache.getValid(ALL, config), undefined); + assert.strictEqual(cache.size, 0); + }); + + test('clear() empties all entries and advances the generation', () => { + const cache = new DiscoveryResultCache(); + const config = makeConfig(); + cache.set(ALL, config, makeResults('all'), cache.generation); + cache.set(CONDA, config, makeResults('conda'), cache.generation); + const generationBefore = cache.generation; + + cache.clear(); + + assert.strictEqual(cache.size, 0); + assert.strictEqual(cache.generation, generationBefore + 1); + assert.strictEqual(cache.getValid(ALL, config), undefined); + assert.strictEqual(cache.getValid(CONDA, config), undefined); + }); + + test('a store from a refresh that began before clear() does not repopulate', () => { + const cache = new DiscoveryResultCache(); + const config = makeConfig(); + const generationAtStart = cache.generation; + cache.clear(); + const stored = cache.set(ALL, config, makeResults('stale'), generationAtStart); + + assert.strictEqual(stored, false, 'stale-generation store must be rejected'); + assert.strictEqual(cache.size, 0, 'cache must remain empty after clear'); + assert.strictEqual(cache.getValid(ALL, config), undefined); + }); + + test('a store with the current generation after clear() succeeds', () => { + const cache = new DiscoveryResultCache(); + const config = makeConfig(); + cache.clear(); + const stored = cache.set(ALL, config, makeResults('fresh'), cache.generation); + assert.strictEqual(stored, true); + assert.strictEqual(cache.getValid(ALL, config)?.length, 1); + }); +}); + +function makeConfig(overrides: Partial = {}): ConfigurationOptions { + return { + workspaceDirectories: [Uri.file('/work/a').fsPath, Uri.file('/work/b').fsPath], + environmentDirectories: [Uri.file('/envs/x').fsPath, Uri.file('/envs/y').fsPath], + condaExecutable: Uri.file('/tools/conda').fsPath, + pipenvExecutable: Uri.file('/tools/pipenv').fsPath, + poetryExecutable: Uri.file('/tools/poetry').fsPath, + cacheDirectory: Uri.file('/cache/poetry').fsPath, + ...overrides, + }; +} + +function makeResults(tag: string): NativeInfo[] { + return [{ executable: `/py/${tag}` } as unknown as NativeInfo]; +}