From c3cb0231e33828ce8811b2960be2b67340c93812 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Mon, 17 Aug 2026 19:37:13 -0400 Subject: [PATCH 1/5] test: cover registered package manager lifecycles (#1704) ## Summary Adds a package-manager-centric integration baseline that intentionally precedes and de-risks #1686, so the package-manager command refactor is exercised against behavior established on `main`. - drives one stateful install/list/direct-package/uninstall lifecycle per active profile - uses unique disposable projects and manager-owned disposable environments - exercises the live registered manager instances through a runtime-gated integration-test bridge - guards registry completeness so every registered package-manager ID has an active fixture or explicit deferral - covers normal Pip execution and Conda when their runtime prerequisites are available - records an uncached baseline instead of assuming a newly created environment is empty - restores workspace-scoped configuration from `inspect()` snapshots and performs guarded failure-safe cleanup - defers Poetry pending a Poetry-owned project/lockfile lifecycle - defers uv-backed Pip because changing the machine-scoped selection reliably within one extension host was not stable on `main`, while available-version lookup would also introduce `uv tool run pip` network seeding - pins the disposable integration-test user profile to normal Pip execution ## Validation - `npm run compile` - `npm run compile-tests` - `npm run lint` - `npm run unittest` - targeted `packageManagement.integration.test.js`: 3 passing, 2 prerequisite skips locally - Pip skipped because quick create selected Python 3.15.0 alpha, whose bundled Pip metadata is incomplete - Conda skipped because Conda is not installed - reviewer specialist: clean, no Critical or Important findings The active Pip and Conda fixtures require package-index/network access when their runtime prerequisites are present. Fixes #1701 --------- Copilot-Session: 6b2fe9b5-38ea-442f-b07a-b6c71134d480 Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4 --- .../testing-workflow.instructions.md | 1 + .github/workflows/pr-check.yml | 8 + .github/workflows/push-check.yml | 8 + api/CHANGELOG.md | 7 + api/package-lock.json | 4 +- api/package.json | 2 +- examples/sample1/src/api.ts | 111 ++++--- src/api.ts | 112 +++++--- src/extension.ts | 15 + src/features/pythonApi.ts | 9 +- src/internal.api.ts | 11 +- src/managers/builtin/pipPackageManager.ts | 73 +++-- src/managers/builtin/utils.ts | 6 +- src/managers/builtin/venvManager.ts | 15 +- src/managers/builtin/venvUtils.ts | 36 ++- src/managers/common/packageChanges.ts | 15 +- src/managers/conda/condaPackageManager.ts | 13 +- src/managers/poetry/poetryPackageManager.ts | 18 +- .../packageManagement.integration.test.ts | 8 +- .../packageManager.integration.test.ts | 271 ++++++++++++++++++ .../builtin/pipPackageManager.unit.test.ts | 20 ++ .../builtin/pipPackageRefresh.unit.test.ts | 53 ++++ .../managers/builtin/pipVersions.unit.test.ts | 35 ++- .../venvManager.createRemove.unit.test.ts | 22 +- .../builtin/venvUtils.removeVenv.unit.test.ts | 31 ++ .../common/packageChanges.unit.test.ts | 29 ++ .../conda/condaPackageManager.unit.test.ts | 41 +++ 27 files changed, 813 insertions(+), 161 deletions(-) create mode 100644 src/test/integration/packageManager.integration.test.ts create mode 100644 src/test/managers/builtin/pipPackageRefresh.unit.test.ts create mode 100644 src/test/managers/conda/condaPackageManager.unit.test.ts diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index b374c38d1..68958773b 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -606,3 +606,4 @@ envConfig.inspect - **Never skip tests to hide infrastructure problems**: If tests require native binaries (like `pet`), the CI workflow must build/download them. Skipping tests when infrastructure is missing gives false confidence. Build from source (like vscode-python does) rather than skipping. Tests should fail clearly when something is wrong (2) - **No retries for masking flakiness**: Mocha `retries` should not be used to mask test flakiness. If a test is flaky, fix the root cause. Retries hide real issues and slow down CI (1) - **pet binary is required for environment manager registration**: The smoke/E2E/integration tests require the `pet` binary from `microsoft/python-environment-tools` to be built and placed in `python-env-tools/bin/`. Without it, `waitForApiReady()` will timeout because managers never register. CI must build pet from source using `cargo build --release --package pet` (2) +- **Check exact project registration with `getPythonProjects()`**: `getPythonProject(uri)` can return a containing parent project, so it cannot prove that a nested project was registered or unregistered (1) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 1298ffa0b..9297f7df6 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -335,6 +335,14 @@ jobs: if: runner.os != 'Linux' run: npm run integration-test + - name: Run Package Manager Network Integration Tests + if: runner.os == 'Linux' && matrix.python-version == '3.12' + uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6 + env: + VSC_PYTHON_PACKAGE_NETWORK_TEST: '1' + with: + run: npm run integration-test -- --grep "Package Manager" + integration-tests-multiroot: name: Integration Tests (Multi-Root) runs-on: ${{ matrix.os }} diff --git a/.github/workflows/push-check.yml b/.github/workflows/push-check.yml index 96867be26..23db9b117 100644 --- a/.github/workflows/push-check.yml +++ b/.github/workflows/push-check.yml @@ -335,3 +335,11 @@ jobs: - name: Run Integration Tests (non-Linux) if: runner.os != 'Linux' run: npm run integration-test + + - name: Run Package Manager Network Integration Tests + if: runner.os == 'Linux' && matrix.python-version == '3.12' + uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6 + env: + VSC_PYTHON_PACKAGE_NETWORK_TEST: '1' + with: + run: npm run integration-test -- --grep "Package Manager" diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 082eac300..616edca22 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to the `@vscode/python-environments` API package are documen The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] + +### Added + +- Added `PackageManagementInteractionOptions` with an optional `runHeadless?: boolean` property, mixed into `PackageManagementOptions`. When `true`, package management operations run without any user prompts or interaction — steps that would normally require input, such as selecting packages to install when none are specified, are skipped instead of prompting — for automated or headless scenarios such as integration tests. +- Added `RemoveEnvironmentOptions` with an optional `runHeadless?: boolean` property to remove environments without a confirmation prompt in automated or headless scenarios. + ## [1.1.0] ### Added diff --git a/api/package-lock.json b/api/package-lock.json index 8363de4f9..7745eab9a 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vscode/python-environments", - "version": "1.1.0", + "version": "1.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vscode/python-environments", - "version": "1.1.0", + "version": "1.2.0", "license": "MIT", "dependencies": { "@renovatebot/pep440": "^3.1.0" diff --git a/api/package.json b/api/package.json index 7f68cf0b2..6b1c6e70e 100644 --- a/api/package.json +++ b/api/package.json @@ -1,7 +1,7 @@ { "name": "@vscode/python-environments", "description": "An API facade for the Python Environments extension in VS Code", - "version": "1.1.0", + "version": "1.2.0", "author": { "name": "Microsoft Corporation" }, diff --git a/examples/sample1/src/api.ts b/examples/sample1/src/api.ts index c45ae1cbd..00512e001 100644 --- a/examples/sample1/src/api.ts +++ b/examples/sample1/src/api.ts @@ -329,6 +329,17 @@ export interface QuickCreateConfig { readonly detail?: string; } +/** + * Options controlling environment removal. + */ +export interface RemoveEnvironmentOptions { + /** + * When `true`, removes the environment without prompting for confirmation. + * Intended for automated or headless scenarios. Defaults to `false`. + */ + runHeadless?: boolean; +} + /** * Interface representing an environment manager. */ @@ -392,7 +403,7 @@ export interface EnvironmentManager { * @param environment - The Python environment to remove. * @returns A promise that resolves when the environment is removed. */ - remove?(environment: PythonEnvironment): Promise; + remove?(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; /** * Refreshes the list of Python environments within the specified scope. @@ -739,49 +750,62 @@ export interface GetPackagesOptions { } /** - * Options for package management. + * Options controlling user interaction during package management operations. */ -export type PackageManagementOptions = - | { - /** - * Upgrade the packages if it is already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install: string[]; - - /** - * The list of packages to uninstall. - */ - uninstall?: string[]; - } - | { - /** - * Upgrade the packages if it is already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install?: string[]; +export interface PackageManagementInteractionOptions { + /** + * When `true`, the package management operation runs without any user prompts or + * interaction and relies solely on the packages provided in the options. Any step + * that would normally require user input — such as selecting packages to install + * when none are specified — is skipped instead of prompting the user. Intended for + * automated or headless scenarios such as integration tests. Defaults to `false`. + */ + runHeadless?: boolean; +} - /** - * The list of packages to uninstall. - */ - uninstall: string[]; - }; +export type PackageManagementOptions = PackageManagementInteractionOptions & + ( + | { + /** + * Upgrade the packages if it is already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall?: string[]; + } + | { + /** + * Upgrade the packages if it is already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install?: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall: string[]; + } + ); /** * Options for creating a Python environment. @@ -881,9 +905,10 @@ export interface PythonEnvironmentManagementApi { * Remove a Python environment. * * @param environment The Python environment to remove. + * @param options Optional parameters controlling environment removal. * @returns A promise that resolves when the environment has been removed. */ - removeEnvironment(environment: PythonEnvironment): Promise; + removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; } export interface PythonEnvironmentsApi { diff --git a/src/api.ts b/src/api.ts index 5d63a3aef..2779d27b0 100644 --- a/src/api.ts +++ b/src/api.ts @@ -345,6 +345,17 @@ export interface QuickCreateConfig { readonly detail?: string; } +/** + * Options controlling environment removal. + */ +export interface RemoveEnvironmentOptions { + /** + * When `true`, removes the environment without prompting for confirmation. + * Intended for automated or headless scenarios. Defaults to `false`. + */ + runHeadless?: boolean; +} + /** * Interface representing an environment manager. * @@ -425,7 +436,7 @@ export interface EnvironmentManager { * Invoked to delete the given environment. Typical triggers include an explicit user * action (such as a "Delete Environment" command) and programmatic removal via the API. */ - remove?(environment: PythonEnvironment): Promise; + remove?(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; /** * Refreshes the list of Python environments within the specified scope. @@ -872,47 +883,63 @@ export interface GetPackagesOptions { skipCache?: boolean; } -export type PackageManagementOptions = - | { - /** - * Upgrade the packages if they are already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation or uninstallation. - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install: string[]; - - /** - * The list of packages to uninstall. - */ - uninstall?: string[]; - } - | { - /** - * Upgrade the packages if they are already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation or uninstallation. - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install?: string[]; +/** + * Options controlling user interaction during package management operations. + */ +export interface PackageManagementInteractionOptions { + /** + * When `true`, the package management operation runs without any user prompts or + * interaction and relies solely on the packages provided in the options. Any step + * that would normally require user input — such as selecting packages to install + * when none are specified — is skipped instead of prompting the user. Intended for + * automated or headless scenarios such as integration tests. Defaults to `false`. + */ + runHeadless?: boolean; +} - /** - * The list of packages to uninstall. - */ - uninstall: string[]; - }; +export type PackageManagementOptions = PackageManagementInteractionOptions & + ( + | { + /** + * Upgrade the packages if they are already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall?: string[]; + } + | { + /** + * Upgrade the packages if they are already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install?: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall: string[]; + } + ); /** * Options for creating a Python environment. @@ -1011,9 +1038,10 @@ export interface PythonEnvironmentManagementApi { * Remove a Python environment. * * @param environment The Python environment to remove. + * @param options Optional parameters controlling environment removal. * @returns A promise that resolves when the environment has been removed. */ - removeEnvironment(environment: PythonEnvironment): Promise; + removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; } export interface PythonEnvironmentsApi { diff --git a/src/extension.ts b/src/extension.ts index 1d2d98aee..46f89009b 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -258,6 +258,21 @@ export async function activate(context: ExtensionContext): Promise + envManagers.packageManagers.map((manager) => manager.id), + ), + commands.registerCommand( + 'python-envs.test.getDirectPackageNames', + async (environment: PythonEnvironment) => { + const manager = envManagers.getPackageManager(environment); + const names = await manager?.getDirectPackageNames?.(environment); + return names ? Array.from(names) : undefined; + }, + ), + ] + : []), commands.registerCommand('python-envs.searchSettings', async () => { await openSearchSettings(); }), diff --git a/src/features/pythonApi.ts b/src/features/pythonApi.ts index 9c494b9eb..e93ed0cdb 100644 --- a/src/features/pythonApi.ts +++ b/src/features/pythonApi.ts @@ -29,6 +29,7 @@ import { PythonTerminalCreateOptions, PythonTerminalExecutionOptions, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from '../api'; @@ -107,9 +108,7 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { this.previousProjects = current; if (added.length > 0 || removed.length > 0) { - traceInfo( - `Python API: Projects changed. Added: ${added.length}, Removed: ${removed.length}`, - ); + traceInfo(`Python API: Projects changed. Added: ${added.length}, Removed: ${removed.length}`); this._onDidChangePythonProjects.fire({ added, removed }); } }), @@ -197,13 +196,13 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { return result; } } - async removeEnvironment(environment: PythonEnvironment): Promise { + async removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { await waitForEnvManagerId([environment.envId.managerId]); const manager = this.envManagers.getEnvironmentManager(environment); if (!manager) { return Promise.reject(new Error('No environment manager found')); } - return manager.remove(environment); + return manager.remove(environment, options); } async refreshEnvironments(scope: RefreshEnvironmentsScope): Promise { const currentScope = checkUri(scope) as RefreshEnvironmentsScope; diff --git a/src/internal.api.ts b/src/internal.api.ts index 9b09d5cf8..6d41cb5c3 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -26,6 +26,7 @@ import { PythonProjectCreator, QuickCreateConfig, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from './api'; @@ -208,9 +209,9 @@ export class InternalEnvironmentManager implements EnvironmentManager { return this.manager.remove !== undefined; } - remove(scope: PythonEnvironment): Promise { + remove(scope: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { return this.manager.remove - ? this.manager.remove(scope) + ? this.manager.remove(scope, options) : Promise.reject(new RemoveEnvironmentNotSupported(`Remove Environment not supported by: ${this.id}`)); } @@ -405,6 +406,12 @@ export class InternalPackageManager implements PackageManager { : Promise.resolve(undefined); } + getDirectPackageNames(environment: PythonEnvironment): Promise | undefined> { + return this.manager.getDirectPackageNames + ? this.manager.getDirectPackageNames(environment) + : Promise.resolve(undefined); + } + formatInstallSpec(packageName: string, version: string): string { return this.manager.formatInstallSpec ? this.manager.formatInstallSpec(packageName, version) diff --git a/src/managers/builtin/pipPackageManager.ts b/src/managers/builtin/pipPackageManager.ts index bd3bbb761..836244bb7 100644 --- a/src/managers/builtin/pipPackageManager.ts +++ b/src/managers/builtin/pipPackageManager.ts @@ -55,6 +55,10 @@ export class PipPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package picker. + return; + } const projects = this.venv.getProjectsByEnvironment(environment); const result = await getWorkspacePackagesToInstall(this.api, options, projects, environment, this.log); if (result) { @@ -86,18 +90,21 @@ export class PipPackageManager implements PackageManager, Disposable { (changes) => { this._onDidChangePackages.fire({ environment, manager: this, changes }); }, + () => this.fetchPackages(environment, !manageOptions.runHeadless), ); } catch (e) { if (e instanceof CancellationError) { throw e; } this.log.error('Error managing packages', e); - setImmediate(async () => { - const result = await window.showErrorMessage('Error managing packages', 'View Output'); - if (result === 'View Output') { - this.log.show(); - } - }); + if (!manageOptions.runHeadless) { + setImmediate(async () => { + const result = await window.showErrorMessage('Error managing packages', 'View Output'); + if (result === 'View Output') { + this.log.show(); + } + }); + } throw e; } }, @@ -119,25 +126,31 @@ export class PipPackageManager implements PackageManager, Disposable { this._onDidChangePackages.fire({ environment, manager: this, changes }); }, ); - this.packages.set(environment.envId.id, packages ?? []); + if (packages !== undefined) { + this.packages.set(environment.envId.id, packages); + } }, ); } async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { if (options?.skipCache || !this.packages.has(environment.envId.id)) { - const data = await refreshPipPackages(environment, this.log); - if (data === undefined) { - return this.packages.get(environment.envId.id); - } - - const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this)); - this.packages.set(environment.envId.id, packages); - return packages; + return this.fetchPackages(environment); } return this.packages.get(environment.envId.id); } + private async fetchPackages(environment: PythonEnvironment, showErrors = true): Promise { + const data = await refreshPipPackages(environment, this.log, { showErrors }); + if (data === undefined) { + return this.packages.get(environment.envId.id); + } + + const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this)); + this.packages.set(environment.envId.id, packages); + return packages; + } + async getVersion(environment: PythonEnvironment): Promise { try { const useUv = await shouldUseUv(this.log, environment.environmentPath.fsPath); @@ -186,9 +199,9 @@ export class PipPackageManager implements PackageManager, Disposable { return parsePipIndexVersionsJson(output); } - // pip >= 21.2.0 - use `pip index versions --json` to get available versions in a machine readable format. + // pip >= 25.1 - use `pip index versions --json` to get available versions in a machine readable format. const pipVersion = await this.getVersion(environment); - if (pipVersion && compare(pipVersion.public, '21.2.0') >= 0) { + if (pipVersion && compare(pipVersion.public, '25.1') >= 0) { const output = await runPython( python, ['-m', 'pip', 'index', 'versions', packageName, '--json', '--python-version', baseVersion], @@ -198,7 +211,17 @@ export class PipPackageManager implements PackageManager, Disposable { return parsePipIndexVersionsJson(output); } - // pip <= 20.3.4 - version picking is undefined; no reliable machine-readable API exists. + if (pipVersion && compare(pipVersion.public, '21.2') >= 0) { + const output = await runPython( + python, + ['-m', 'pip', 'index', 'versions', packageName, '--python-version', baseVersion], + undefined, + this.log, + ); + return parsePipIndexVersionsText(output); + } + + // pip < 21.2 - version picking is undefined; `pip index versions` is unavailable. } catch { return undefined; } @@ -245,3 +268,17 @@ export function parsePipIndexVersionsJson(output: string): Pep440Version[] | und return undefined; } } + +/** Parses the legacy text output from `pip index versions `. */ +export function parsePipIndexVersionsText(output: string): Pep440Version[] | undefined { + const match = output.match(/^Available versions:\s*(.+)$/im); + if (!match) { + return undefined; + } + const versions = match[1] + .split(',') + .map((version) => parse(version.trim())) + .filter((version): version is Pep440Version => version !== null) + .sort((a, b) => rcompare(a.public, b.public)); + return versions.length > 0 ? versions : undefined; +} diff --git a/src/managers/builtin/utils.ts b/src/managers/builtin/utils.ts index dc44fe759..f6ff2903a 100644 --- a/src/managers/builtin/utils.ts +++ b/src/managers/builtin/utils.ts @@ -218,7 +218,7 @@ async function execPipList(environment: PythonEnvironment, log?: LogOutputChanne export async function refreshPipPackages( environment: PythonEnvironment, log?: LogOutputChannel, - options?: { showProgress: boolean }, + options?: { showProgress?: boolean; showErrors?: boolean }, ): Promise { let data: string; try { @@ -238,7 +238,9 @@ export async function refreshPipPackages( return parsePipListJson(data, log); } catch (e) { log?.error('Error refreshing packages', e); - showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log); + if (options?.showErrors !== false) { + showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log); + } return undefined; } } diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 7af0f450a..6dcda4df8 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -1,14 +1,6 @@ import * as fs from 'fs/promises'; import * as path from 'path'; -import { - EventEmitter, - l10n, - LogOutputChannel, - MarkdownString, - ProgressLocation, - ThemeIcon, - Uri, -} from 'vscode'; +import { EventEmitter, l10n, LogOutputChannel, MarkdownString, ProgressLocation, ThemeIcon, Uri } from 'vscode'; import { CreateEnvironmentOptions, CreateEnvironmentScope, @@ -24,6 +16,7 @@ import { PythonProject, QuickCreateConfig, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from '../../api'; @@ -265,11 +258,11 @@ export class VenvManager implements EnvironmentManager { /** * Removes the specified Python environment, updates internal collections, and fires change events as needed. */ - async remove(environment: PythonEnvironment): Promise { + async remove(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { try { this.skipWatcherRefresh = true; - const isRemoved = await removeVenv(environment, this.log); + const isRemoved = await removeVenv(environment, this.log, options); if (!isRemoved) { return; } diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index c06146999..2962235e1 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -11,7 +11,13 @@ import { ThemeIcon, Uri, } from 'vscode'; -import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../api'; +import { + EnvironmentManager, + PythonEnvironment, + PythonEnvironmentApi, + PythonEnvironmentInfo, + RemoveEnvironmentOptions, +} from '../../api'; import { ENVS_EXTENSION_ID } from '../../common/constants'; import { Common, VenvManagerStrings } from '../../common/localize'; import { traceInfo, traceVerbose } from '../../common/logging'; @@ -553,7 +559,11 @@ async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel): return undefined; } -export async function removeVenv(environment: PythonEnvironment, log: LogOutputChannel): Promise { +export async function removeVenv( + environment: PythonEnvironment, + log: LogOutputChannel, + options?: RemoveEnvironmentOptions, +): Promise { const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python'; const envFsPath = path.normalize(environment.environmentPath.fsPath); @@ -568,15 +578,19 @@ export async function removeVenv(environment: PythonEnvironment, log: LogOutputC // Normalize path for UI display - ensure forward slashes on Windows const displayPath = normalizePath(envPath); - const confirm = await showWarningMessage( - l10n.t('Are you sure you want to remove {0}?', displayPath), - { - modal: true, - }, - { title: Common.yes }, - { title: Common.no, isCloseAffordance: true }, - ); - if (confirm?.title === Common.yes) { + const confirmed = + options?.runHeadless === true || + ( + await showWarningMessage( + l10n.t('Are you sure you want to remove {0}?', displayPath), + { + modal: true, + }, + { title: Common.yes }, + { title: Common.no, isCloseAffordance: true }, + ) + )?.title === Common.yes; + if (confirmed) { const result = await withProgress( { location: ProgressLocation.Notification, diff --git a/src/managers/common/packageChanges.ts b/src/managers/common/packageChanges.ts index 3e16ae361..6c484fccd 100644 --- a/src/managers/common/packageChanges.ts +++ b/src/managers/common/packageChanges.ts @@ -9,6 +9,8 @@ import { normalizePackageName } from '../builtin/utils'; */ export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: Package }[]) => void; +type PackageFetcher = () => Promise; + /** * Computes the list of package changes between a before and after snapshot. * @param before - The previous list of packages. @@ -41,19 +43,30 @@ export function getPackageChanges(before: Package[], after: Package[]): { kind: * This function calls {@link PackageManager.getPackages} with `skipCache` to fetch * the latest snapshot. The caller should pass the previously cached packages * so changes can be computed against the pre-refresh state. + * + * @param packageManager The package manager whose packages changed. + * @param environment The environment whose packages should be refreshed. + * @param before The package snapshot from before the operation. + * @param onChanges Callback invoked when package changes are detected. + * @param fetchPackages Optional internal fetcher for operation-specific refresh behavior. */ export async function updatePackagesAndNotify( packageManager: PackageManager, environment: PythonEnvironment, before: Package[] | undefined, onChanges: PackageChangesCallback, + fetchPackages?: PackageFetcher, ): Promise { const [after, afterDirectDependenciesNames] = await Promise.all([ - packageManager.getPackages(environment, { skipCache: true }).then((pkgs) => pkgs ?? []), + fetchPackages?.() ?? packageManager.getPackages(environment, { skipCache: true }), // Handle transitive dependencies (best-effort, don't break package refresh on failure) packageManager.getDirectPackageNames?.(environment).catch(() => undefined), ]); + if (after === undefined) { + return undefined; + } + // Enrich packages with transitive dependency info (best-effort, creates new objects to respect readonly) const enriched = afterDirectDependenciesNames && afterDirectDependenciesNames.size > 0 ? after.map((pkg) => ({ diff --git a/src/managers/conda/condaPackageManager.ts b/src/managers/conda/condaPackageManager.ts index d4fb44be3..d395d0ce6 100644 --- a/src/managers/conda/condaPackageManager.ts +++ b/src/managers/conda/condaPackageManager.ts @@ -54,6 +54,10 @@ export class CondaPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package picker. + return; + } const result = await getCommonCondaPackagesToInstall(environment, options, this.api); if (result) { toInstall = result.install; @@ -91,9 +95,12 @@ export class CondaPackageManager implements PackageManager, Disposable { } this.log.error('Error installing packages', e); - setImmediate(async () => { - await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log); - }); + if (!manageOptions.runHeadless) { + setImmediate(async () => { + await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log); + }); + } + throw e; } }, ); diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 9525254cb..e946f0452 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -59,6 +59,10 @@ export class PoetryPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package input prompt. + return; + } // Show package input UI if no packages are specified const installInput = await showInputBox({ prompt: 'Enter packages to install (comma separated)', @@ -99,12 +103,14 @@ export class PoetryPackageManager implements PackageManager, Disposable { throw e; } this.log.error('Error managing packages with Poetry', e); - setImmediate(async () => { - const result = await showErrorMessage('Error managing packages with Poetry', 'View Output'); - if (result === 'View Output') { - this.log.show(); - } - }); + if (!options.runHeadless) { + setImmediate(async () => { + const result = await showErrorMessage('Error managing packages with Poetry', 'View Output'); + if (result === 'View Output') { + this.log.show(); + } + }); + } throw e; } }, diff --git a/src/test/integration/packageManagement.integration.test.ts b/src/test/integration/packageManagement.integration.test.ts index 5998b6a17..7eb2a75c2 100644 --- a/src/test/integration/packageManagement.integration.test.ts +++ b/src/test/integration/packageManagement.integration.test.ts @@ -282,13 +282,13 @@ suite('Integration: Package Management', function () { try { if (wasInstalled) { // Uninstall first - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); packageInstalled = false; await sleep(2000); } // Install package - await api.managePackages(targetEnv, { install: [testPackage] }); + await api.managePackages(targetEnv, { install: [testPackage], runHeadless: true }); packageInstalled = true; // Refresh and verify @@ -299,7 +299,7 @@ suite('Integration: Package Management', function () { assert.ok(isNowInstalled, `${testPackage} should be installed after managePackages install`); // Uninstall - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); packageInstalled = false; // Refresh and verify @@ -312,7 +312,7 @@ suite('Integration: Package Management', function () { // Ensure cleanup even if assertions fail if (packageInstalled) { try { - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); } catch { console.log('Cleanup: failed to uninstall test package'); } diff --git a/src/test/integration/packageManager.integration.test.ts b/src/test/integration/packageManager.integration.test.ts new file mode 100644 index 000000000..f42df8539 --- /dev/null +++ b/src/test/integration/packageManager.integration.test.ts @@ -0,0 +1,271 @@ +import * as vscode from 'vscode'; + +import { compare } from '@renovatebot/pep440'; +import assert from 'assert'; +import * as path from 'path'; +import { Package, PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; +import { CONDA_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID, VENV_MANAGER_ID } from '../../common/constants'; +import { PythonProjectSettings } from '../../internal.api'; +import { getConda } from '../../managers/conda/condaUtils'; +import { ENVS_EXTENSION_ID } from '../constants'; +import { waitForCondition } from '../testUtils'; + +type PackageManagerId = `${string}:${string}`; + +interface PackageManagerProfile { + environmentManagerId: string; + name: string; + packageManagerId: PackageManagerId; + projectDirectory: string; + prerequisite(api: PythonEnvironmentApi): Promise; + supportsVersionLookup(packages: Package[]): boolean; +} + +const profiles: PackageManagerProfile[] = [ + { + environmentManagerId: VENV_MANAGER_ID, + name: 'Pip', + packageManagerId: DEFAULT_PACKAGE_MANAGER_ID, + projectDirectory: 'pip', + prerequisite: async (api) => + (await api.getEnvironments('global')).some((environment) => environment.version.startsWith('3.')), + supportsVersionLookup: (packages) => { + const pipVersion = packages.find((pkg) => pkg.name.toLowerCase() === 'pip')?.version; + return pipVersion !== undefined && compare(pipVersion, '21.2') >= 0; + }, + }, + { + environmentManagerId: CONDA_MANAGER_ID, + name: 'Conda', + packageManagerId: CONDA_MANAGER_ID, + projectDirectory: 'conda', + prerequisite: async () => { + try { + await getConda(); + return true; + } catch { + return false; + } + }, + supportsVersionLookup: () => true, + }, +]; + +const deferredPackageManagers: Readonly> = { + 'ms-python.python:poetry': 'Poetry lifecycle coverage requires a controlled Poetry installation.', +}; + +const deferredProfiles = { + pipWithUv: 'uv-backed Pip selection uses a machine-scoped setting and is unstable within one extension host.', +} as const; + +suite('Package Manager profile coverage', function () { + this.timeout(60_000); + + test('covers or explicitly defers every registered package manager', async () => { + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, 'Extension not found'); + const api: PythonEnvironmentApi = extension.isActive ? extension.exports : await extension.activate(); + await api.getEnvironments('global'); + + const registeredIds = await vscode.commands.executeCommand( + 'python-envs.test.getPackageManagerIds', + ); + assert.ok(registeredIds, 'Registered package-manager IDs are unavailable'); + + const coveredIds = new Set(profiles.map((profile) => profile.packageManagerId)); + const uncoveredIds = registeredIds.filter( + (managerId) => + !coveredIds.has(managerId as PackageManagerId) && + deferredPackageManagers[managerId as PackageManagerId] === undefined, + ); + assert.deepStrictEqual(uncoveredIds, [], `Package managers lack lifecycle coverage: ${uncoveredIds.join(', ')}`); + + for (const profile of profiles) { + assert.ok( + registeredIds.includes(profile.packageManagerId), + `Profile references an unregistered package manager: ${profile.packageManagerId}`, + ); + } + + for (const [profileName, reason] of Object.entries(deferredProfiles)) { + assert.ok(reason.length > 0, `Deferred profile lacks a reason: ${profileName}`); + } + }); +}); + +for (const profile of profiles) { + suite(`${profile.name} Package Manager`, function () { + this.timeout(300_000); + + let api: PythonEnvironmentApi; + let environment: PythonEnvironment | undefined; + let project: PythonProject | undefined; + let workspaceUri: vscode.Uri; + let previousPythonProjects: PythonProjectSettings[] | undefined; + let pythonProjectsUpdated = false; + suiteSetup(async function () { + if (process.env.VSC_PYTHON_PACKAGE_NETWORK_TEST !== '1') { + this.skip(); + return; + } + + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, 'Extension not found'); + if (!extension.isActive) { + await extension.activate(); + await waitForCondition(() => extension.isActive, 20_000, 'Extension did not activate in time'); + } + api = extension.exports; + assert.ok(api, 'API not available'); + + const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; + assert.ok(workspaceFolder, 'Integration test workspace not found'); + workspaceUri = workspaceFolder.uri; + const config = vscode.workspace.getConfiguration('python-envs', workspaceUri); + + if (!(await profile.prerequisite(api))) { + this.skip(); + return; + } + + const projectUri = vscode.Uri.joinPath( + workspaceUri, + `.package-manager-test-${profile.projectDirectory}-${process.pid}`, + ); + await vscode.workspace.fs.createDirectory(projectUri); + project = { + name: `${profile.name} Package Manager Test`, + uri: projectUri, + }; + previousPythonProjects = config.inspect('pythonProjects')?.workspaceFolderValue; + const pythonProjects = config.get('pythonProjects', []); + const projectSetting: PythonProjectSettings = { + path: path.relative(workspaceUri.fsPath, projectUri.fsPath).replace(/\\/g, '/'), + envManager: profile.environmentManagerId, + packageManager: profile.packageManagerId, + workspace: workspaceFolder.name, + }; + await config.update( + 'pythonProjects', + [...pythonProjects, projectSetting], + vscode.ConfigurationTarget.WorkspaceFolder, + ); + pythonProjectsUpdated = true; + await waitForCondition( + () => + api + .getPythonProjects() + .some((registeredProject) => registeredProject.uri.toString() === projectUri.toString()), + 10_000, + `Python project was not registered: ${projectUri.fsPath}`, + ); + + await api.refreshEnvironments(projectUri); + + environment = await api.createEnvironment(projectUri, { quickCreate: true }); + assert.ok(environment, `${profile.name} failed to create an environment after prerequisites passed`); + assert.strictEqual( + environment.envId.managerId, + profile.environmentManagerId, + `Expected an environment created by ${profile.environmentManagerId}`, + ); + }); + + test(`${profile.name} Package Manager should install, list, and uninstall a package`, async () => { + const packageName = 'requests'; + const baseline = await api.getPackages(environment!, { skipCache: true }); + assert.ok(baseline, 'Unable to list packages before installation'); + const wasInstalled = baseline.some((pkg) => pkg.name.toLowerCase() === packageName); + + if (!wasInstalled) { + await api.managePackages(environment!, { install: [packageName], runHeadless: true }); + } + let packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages after installation'); + assert.ok( + packages.some((pkg) => pkg.name.toLowerCase() === packageName), + 'Package not installed', + ); + + const directPackageNames = await vscode.commands.executeCommand( + 'python-envs.test.getDirectPackageNames', + environment!, + ); + if (directPackageNames !== undefined) { + assert.ok(directPackageNames.includes(packageName), 'Installed package was not reported as direct'); + } + + if (!wasInstalled) { + await api.managePackages(environment!, { uninstall: [packageName], runHeadless: true }); + packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages after uninstallation'); + assert.ok( + !packages.some((pkg) => pkg.name.toLowerCase() === packageName), + 'Package not uninstalled', + ); + } + }); + + test(`${profile.name} Package Manager should list available package versions`, async function () { + const packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages before version lookup'); + if (!profile.supportsVersionLookup(packages)) { + this.skip(); + return; + } + + const versions = await api.getPackageAvailableVersions(environment!, 'requests'); + assert.ok(versions, `${profile.name} unexpectedly failed to retrieve package versions`); + assert.ok(versions.length > 0, 'No package versions available'); + }); + + suiteTeardown(async () => { + try { + if (environment) { + const environmentPath = environment.environmentPath; + await api.removeEnvironment(environment, { runHeadless: true }); + await assert.rejects( + async () => vscode.workspace.fs.stat(environmentPath), + (error: unknown) => + error instanceof vscode.FileSystemError && error.code === 'FileNotFound', + `Environment was not removed: ${environmentPath.fsPath}`, + ); + } + } finally { + const config = vscode.workspace.getConfiguration('python-envs', workspaceUri); + if (project) { + try { + await api.setEnvironment(project.uri, undefined); + } finally { + try { + if (pythonProjectsUpdated) { + await config.update( + 'pythonProjects', + previousPythonProjects, + vscode.ConfigurationTarget.WorkspaceFolder, + ); + await waitForCondition( + () => + !api + .getPythonProjects() + .some( + (registeredProject) => + registeredProject.uri.toString() === project!.uri.toString(), + ), + 10_000, + `Python project was not unregistered: ${project.uri.fsPath}`, + ); + } + } finally { + await vscode.workspace.fs.delete(project.uri, { + recursive: true, + useTrash: false, + }); + } + } + } + } + }); + }); +} diff --git a/src/test/managers/builtin/pipPackageManager.unit.test.ts b/src/test/managers/builtin/pipPackageManager.unit.test.ts index 8a64abf1b..549bdd2de 100644 --- a/src/test/managers/builtin/pipPackageManager.unit.test.ts +++ b/src/test/managers/builtin/pipPackageManager.unit.test.ts @@ -40,4 +40,24 @@ suite('PipPackageManager', () => { assert.deepStrictEqual(initial, [cachedPackage]); assert.deepStrictEqual(afterFailedRefresh, [cachedPackage]); }); + + test('preserves undefined when an uncached refresh fails', async () => { + const environment = { + envId: { id: 'test-environment', managerId: 'test-manager' }, + environmentPath: Uri.file('/path/to/environment'), + } as PythonEnvironment; + const manager = new PipPackageManager( + { createPackageItem: sinon.stub() } as unknown as PythonEnvironmentApi, + { error: sinon.stub(), info: sinon.stub() } as unknown as LogOutputChannel, + {} as VenvManager, + ); + const refreshPackages = sinon.stub(builtinUtils, 'refreshPipPackages').resolves(undefined); + + const firstResult = await manager.getPackages(environment); + const secondResult = await manager.getPackages(environment); + + assert.strictEqual(firstResult, undefined); + assert.strictEqual(secondResult, undefined); + assert.strictEqual(refreshPackages.callCount, 2, 'A failed refresh should not populate the package cache'); + }); }); diff --git a/src/test/managers/builtin/pipPackageRefresh.unit.test.ts b/src/test/managers/builtin/pipPackageRefresh.unit.test.ts new file mode 100644 index 000000000..dff10003c --- /dev/null +++ b/src/test/managers/builtin/pipPackageRefresh.unit.test.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { LogOutputChannel, Uri } from 'vscode'; +import { PythonEnvironment } from '../../../api'; +import * as errorUtils from '../../../common/errors/utils'; +import * as helpers from '../../../managers/builtin/helpers'; +import { refreshPipPackages } from '../../../managers/builtin/utils'; + +suite('Pip package refresh', () => { + let environment: PythonEnvironment; + let log: LogOutputChannel; + let showErrorMessageWithLogsStub: sinon.SinonStub; + + setup(() => { + environment = { + environmentPath: Uri.file('.'), + execInfo: { + run: { + executable: 'python', + }, + }, + } as PythonEnvironment; + log = { + error: sinon.stub(), + info: sinon.stub(), + } as unknown as LogOutputChannel; + + sinon.stub(helpers, 'shouldUseUv').resolves(false); + sinon.stub(helpers, 'runPython').rejects(new Error('pip list failed')); + showErrorMessageWithLogsStub = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves(); + }); + + teardown(() => { + sinon.restore(); + }); + + test('shows an error when an interactive refresh fails', async () => { + const result = await refreshPipPackages(environment, log); + + assert.strictEqual(result, undefined); + assert.ok(showErrorMessageWithLogsStub.calledOnce); + }); + + test('does not show an error when a headless refresh fails', async () => { + const result = await refreshPipPackages(environment, log, { showErrors: false }); + + assert.strictEqual(result, undefined); + assert.ok(showErrorMessageWithLogsStub.notCalled); + }); +}); diff --git a/src/test/managers/builtin/pipVersions.unit.test.ts b/src/test/managers/builtin/pipVersions.unit.test.ts index 5c06c394b..b2bd15f6b 100644 --- a/src/test/managers/builtin/pipVersions.unit.test.ts +++ b/src/test/managers/builtin/pipVersions.unit.test.ts @@ -1,13 +1,16 @@ -import assert from 'assert'; import { explain } from '@renovatebot/pep440'; -import { parsePipIndexVersionsJson } from '../../../managers/builtin/pipPackageManager'; +import assert from 'assert'; +import { parsePipIndexVersionsJson, parsePipIndexVersionsText } from '../../../managers/builtin/pipPackageManager'; suite('Pip Version Parsing', () => { suite('parsePipIndexVersionsJson', () => { test('parses valid JSON with versions array', () => { const output = JSON.stringify({ name: 'requests', versions: ['2.31.0', '2.30.0', '2.29.0'] }); const versions = parsePipIndexVersionsJson(output); - assert.deepStrictEqual(versions, ['2.31.0', '2.30.0', '2.29.0'].map((v) => explain(v))); + assert.deepStrictEqual( + versions, + ['2.31.0', '2.30.0', '2.29.0'].map((v) => explain(v)), + ); }); test('parses output with a single version', () => { @@ -33,5 +36,29 @@ suite('Pip Version Parsing', () => { assert.strictEqual(versions, undefined); }); }); -}); + suite('parsePipIndexVersionsText', () => { + test('parses and sorts the available versions line', () => { + const output = [ + 'requests (2.32.5)', + 'Available versions: 2.31.0, 2.32.5, 2.30.0', + ' INSTALLED: 2.31.0', + ' LATEST: 2.32.5', + ].join('\n'); + const versions = parsePipIndexVersionsText(output); + assert.deepStrictEqual( + versions, + ['2.32.5', '2.31.0', '2.30.0'].map((version) => explain(version)), + ); + }); + + test('returns undefined when the available versions line is missing', () => { + assert.strictEqual(parsePipIndexVersionsText('ERROR: No matching distribution found'), undefined); + }); + + test('ignores invalid versions', () => { + const versions = parsePipIndexVersionsText('Available versions: invalid, 1.2.3'); + assert.deepStrictEqual(versions, [explain('1.2.3')]); + }); + }); +}); diff --git a/src/test/managers/builtin/venvManager.createRemove.unit.test.ts b/src/test/managers/builtin/venvManager.createRemove.unit.test.ts index 7e1e202be..12bf1c7b3 100644 --- a/src/test/managers/builtin/venvManager.createRemove.unit.test.ts +++ b/src/test/managers/builtin/venvManager.createRemove.unit.test.ts @@ -47,12 +47,11 @@ function createManager( const baseManager = { getEnvironments: sinon.stub().resolves(baseEnvironments), } as any as EnvironmentManager; - const manager = new VenvManager( - {} as NativePythonFinder, - api, - baseManager, - { info: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } as any, - ); + const manager = new VenvManager({} as NativePythonFinder, api, baseManager, { + info: sinon.stub(), + error: sinon.stub(), + warn: sinon.stub(), + } as any); (manager as any)._initialized = { completed: true, promise: Promise.resolve() }; (manager as any).collection = []; return manager; @@ -221,6 +220,17 @@ suite('VenvManager.remove - orchestration', () => { assert.strictEqual(events[0][0].environment, env); }); + test('forwards headless removal options to the removal helper', async () => { + const manager = createManager(); + const env = environment(); + removeVenvStub.resolves(true); + + await manager.remove(env, { runHeadless: true }); + + assert.strictEqual(removeVenvStub.firstCall.args[0], env); + assert.deepStrictEqual(removeVenvStub.firstCall.args[2], { runHeadless: true }); + }); + test('does not mutate state when the removal helper returns false', async () => { const manager = createManager(); const env = environment(); diff --git a/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts b/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts index 068eb5dca..b1fae91bf 100644 --- a/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts +++ b/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts @@ -1,6 +1,13 @@ import * as assert from 'assert'; +import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; +import * as sinon from 'sinon'; +import * as windowApis from '../../../common/window.apis'; +import * as uvEnvironments from '../../../managers/builtin/uvEnvironments'; +import { removeVenv } from '../../../managers/builtin/venvUtils'; +import { createMockLogOutputChannel } from '../../mocks/helper'; +import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment'; suite('venvUtils Path Validation', () => { suite('isDriveRoot behavior', () => { @@ -146,4 +153,28 @@ suite('venvUtils removeVenv validation integration', () => { 'Should check for pyvenv.cfg in the environment root', ); }); + + test('headless removal skips confirmation and removes the environment', async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'remove-venv-')); + const envPath = path.join(tempRoot, '.venv'); + await fs.outputFile(path.join(envPath, 'pyvenv.cfg'), 'home = base'); + const showWarningMessageStub = sinon.stub(windowApis, 'showWarningMessage'); + sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never)); + sinon.stub(uvEnvironments, 'removeUvEnvironment').resolves(); + + try { + const removed = await removeVenv( + createMockPythonEnvironment({ name: '.venv', envPath }), + createMockLogOutputChannel(), + { runHeadless: true }, + ); + + assert.strictEqual(removed, true); + assert.strictEqual(showWarningMessageStub.callCount, 0); + assert.strictEqual(await fs.pathExists(envPath), false); + } finally { + sinon.restore(); + await fs.remove(tempRoot); + } + }); }); diff --git a/src/test/managers/common/packageChanges.unit.test.ts b/src/test/managers/common/packageChanges.unit.test.ts index 1f65b3c75..8f6f77402 100644 --- a/src/test/managers/common/packageChanges.unit.test.ts +++ b/src/test/managers/common/packageChanges.unit.test.ts @@ -127,6 +127,35 @@ suite('packageChanges', () => { assert.strictEqual(changes[0].kind, PackageChangeKind.add); }); + test('uses an operation-specific package fetcher when provided', async () => { + const fetched = [{ name: 'requests', version: '2.31.0' } as Package]; + const fetchPackages = sinon.stub().resolves(fetched); + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify( + packageManager, + environment, + undefined, + onChanges, + fetchPackages, + ); + + assert.deepStrictEqual(result, fetched); + assert.ok(fetchPackages.calledOnce); + assert.ok(getPackagesStub.notCalled); + }); + + test('preserves undefined and does not report removals when fetching fails', async () => { + const before = [{ name: 'requests', version: '2.31.0' } as Package]; + getPackagesStub.resolves(undefined); + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, before, onChanges); + + assert.strictEqual(result, undefined); + assert.ok(onChanges.notCalled); + }); + test('does not fire callback when nothing changed', async () => { const pkgs = [{ name: 'requests', version: '2.31.0' } as Package]; getPackagesStub.resolves(pkgs); diff --git a/src/test/managers/conda/condaPackageManager.unit.test.ts b/src/test/managers/conda/condaPackageManager.unit.test.ts new file mode 100644 index 000000000..ea6614daf --- /dev/null +++ b/src/test/managers/conda/condaPackageManager.unit.test.ts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { LogOutputChannel, Uri } from 'vscode'; +import { PythonEnvironment, PythonEnvironmentApi } from '../../../api'; +import * as errorUtils from '../../../common/errors/utils'; +import * as windowApis from '../../../common/window.apis'; +import { CondaPackageManager } from '../../../managers/conda/condaPackageManager'; +import * as condaUtils from '../../../managers/conda/condaUtils'; + +suite('CondaPackageManager', () => { + teardown(() => { + sinon.restore(); + }); + + test('headless package failures reject without showing error UI', async () => { + const environment = { + envId: { id: 'test-environment', managerId: 'test-manager' }, + environmentPath: Uri.file('/path/to/environment'), + } as PythonEnvironment; + const logError = sinon.stub(); + const log = { + error: logError, + } as unknown as LogOutputChannel; + const manager = new CondaPackageManager({} as PythonEnvironmentApi, log); + const operationError = new Error('conda install failed'); + sinon.stub(condaUtils, 'managePackages').rejects(operationError); + sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never)); + const showErrorMessageWithLogs = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves(); + + await assert.rejects( + manager.manage(environment, { install: ['requests'], runHeadless: true }), + (error: unknown) => error === operationError, + ); + + assert.ok(logError.calledOnce); + assert.ok(showErrorMessageWithLogs.notCalled); + }); +}); From 8666b65eec9366f9e1ca36994abad71d9ca8b8f3 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Tue, 18 Aug 2026 12:07:31 +0100 Subject: [PATCH 2/5] Update logo.svg with new design and optimized dimensions (#1719) Replace the existing activity bar icon with a new design more aligned with the wider codicon design language. ![image.png](https://github.com/user-attachments/assets/65b0c66b-0e1a-4610-89d7-94206fafa044) Co-authored-by: mrleemurray --- files/logo.svg | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/files/logo.svg b/files/logo.svg index a999dbeac..f849d01e2 100644 --- a/files/logo.svg +++ b/files/logo.svg @@ -1,14 +1,12 @@ - - - + + + + + + - - - - - - - - + + + From 1c03107c5c2f4edc47fffb691eb330b9dc23125b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 12:59:13 -0700 Subject: [PATCH 3/5] Add inline script environment lifecycle telemetry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895 --- src/common/telemetry/constants.ts | 55 +++ .../builtin/inlineScript/envManager.ts | 221 ++++++++--- src/managers/builtin/uvPythonInstaller.ts | 61 ++- .../inlineScript/envManager.unit.test.ts | 365 ++++++++++++++++-- 4 files changed, 609 insertions(+), 93 deletions(-) diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts index 6b3432eb3..c718e6a88 100644 --- a/src/common/telemetry/constants.ts +++ b/src/common/telemetry/constants.ts @@ -221,6 +221,27 @@ export enum EventNames { * - dependencyCount: number (number of entries in the `dependencies` list) */ INLINE_SCRIPT_DETECTED = 'inlineScript.detected', + /** + * Telemetry event fired when inline-script environment creation completes + * successfully with a newly-built cache entry that passed verification and + * metadata persistence. + * Measures: + * - duration: number (ms spent in the underlying create/rebuild operation) + * - dependencyCount: number (normalized dependency count in the cache key) + */ + INLINE_SCRIPT_ENV_CREATED = 'inlineScript.envCreated', + /** + * Telemetry event fired when inline-script environment creation validates + * and reuses an existing cache entry without rebuilding it. + */ + INLINE_SCRIPT_ENV_REUSE_HIT = 'inlineScript.envReuseHit', + /** + * Telemetry event fired when inline-script environment creation cannot + * complete. + * Properties: + * - category: stable low-cardinality failure category + */ + INLINE_SCRIPT_ENV_ERROR = 'inlineScript.envError', /** * Telemetry event fired once per session, per URI, the first time a `.py` * file that previously raised an `inlineScript.detected` event receives a @@ -232,6 +253,15 @@ export enum EventNames { INLINE_SCRIPT_EDITED = 'inlineScript.edited', } +export type InlineScriptEnvErrorCategory = + | 'compatible-python-declined' + | 'discovery-failure' + | 'no-compatible-python' + | 'package-install-cancelled' + | 'install-failure' + | 'lock-timeout' + | 'lock-unavailable'; + // Map all events to their properties export interface IEventNamePropertyMapping { /* __GDPR__ @@ -695,6 +725,31 @@ export interface IEventNamePropertyMapping { errorType?: string; }; + /* __GDPR__ + "inlineScript.envCreated": { + "dependencyCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" }, + "": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" } + } + */ + [EventNames.INLINE_SCRIPT_ENV_CREATED]: { + // Goes through the measures payload (numeric); listed here for GDPR only. + dependencyCount?: number; + }; + + /* __GDPR__ + "inlineScript.envReuseHit": {"owner": "StellaHuang95" } + */ + [EventNames.INLINE_SCRIPT_ENV_REUSE_HIT]: never | undefined; + + /* __GDPR__ + "inlineScript.envError": { + "category": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" } + } + */ + [EventNames.INLINE_SCRIPT_ENV_ERROR]: { + category: InlineScriptEnvErrorCategory; + }; + /* __GDPR__ "inlineScript.detected": { "trigger": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" }, diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index d68d9dda6..00e5883bc 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -44,6 +44,8 @@ import { } from '../../../common/constants'; import { acquireFileLock, AcquiredFileLock } from '../../../common/lockfile.apis'; import { getWorkspacePersistentState, PersistentState } from '../../../common/persistentState'; +import { EventNames, InlineScriptEnvErrorCategory } from '../../../common/telemetry/constants'; +import { sendTelemetryEvent } from '../../../common/telemetry/sender'; import { isFileNotFoundError } from '../../../common/utils/filesystem'; import { normalizePath } from '../../../common/utils/pathUtils'; import { compareReleaseSegments, parseReleaseSegments } from '../../../common/utils/pep440Release'; @@ -80,8 +82,24 @@ interface CreateOrReuseEnvironmentOptions { interface BuildCacheEntryResult { readonly environment?: PythonEnvironment; readonly retainLock?: boolean; + readonly errorCategory?: InlineScriptEnvErrorCategory; } +interface BaseInterpreterSelectionResult { + readonly selectedBase?: SelectedBaseInterpreter; + readonly errorCategory?: InlineScriptEnvErrorCategory; +} + +interface SelectBaseInterpreterResult { + readonly selectedBase?: SelectedBaseInterpreter; + readonly discoveryFailed: boolean; +} + +type InstallPythonAndRefreshResult = + | { readonly kind: 'installed'; readonly installedPath: string } + | { readonly kind: 'declined' } + | { readonly kind: 'failed' }; + type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; @@ -167,6 +185,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } } catch (error) { + this.sendInlineScriptEnvErrorTelemetry('install-failure'); this.log.error(`Failed to set up inline-script environment: ${getErrorMessage(error)}`); return undefined; } @@ -178,14 +197,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { packages: readonly string[], options?: CreateEnvironmentOptions, ): Promise { - let selectedBase = await this.selectBaseInterpreter(metadata); - if (!selectedBase && options?.quickCreate !== true) { - selectedBase = await this.installAndSelectBaseInterpreter(metadata); - } - if (!selectedBase) { + const baseSelection = await this.selectOrInstallBaseInterpreter(metadata, options?.quickCreate === true); + if (!baseSelection.selectedBase) { + if (baseSelection.errorCategory) { + this.sendInlineScriptEnvErrorTelemetry(baseSelection.errorCategory); + } this.log.warn(`No compatible Python is available for inline-script environment creation: ${scriptUri.fsPath}.`); return undefined; } + const selectedBase = baseSelection.selectedBase; const cacheKey = computeCacheKey({ dependencies: packages, @@ -227,6 +247,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ]); } + private async selectOrInstallBaseInterpreter( + metadata: InlineScriptMetadata, + quickCreate: boolean, + ): Promise { + const selection = await this.selectBaseInterpreter(metadata); + if (selection.selectedBase) { + return { selectedBase: selection.selectedBase }; + } + if (quickCreate) { + return { + errorCategory: this.getBaseInterpreterErrorCategory(selection.discoveryFailed, 'no-compatible-python'), + }; + } + return this.installAndSelectBaseInterpreter(metadata, selection.discoveryFailed); + } + async refresh(_scope: RefreshEnvironmentsScope): Promise { return; } @@ -800,11 +836,13 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ); } - private async selectBaseInterpreter(metadata: InlineScriptMetadata): Promise { + private async selectBaseInterpreter(metadata: InlineScriptMetadata): Promise { let globalEnvironments: readonly PythonEnvironment[] = []; + let discoveryFailed = false; try { globalEnvironments = await this.api.getEnvironments('global'); } catch (error) { + discoveryFailed = true; this.log.warn(`Unable to query discovered base interpreters: ${getErrorMessage(error)}`); } const reported = [ @@ -845,7 +883,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { while (candidates.length > 0) { const environment = pickCompatibleInterpreter(candidates, undefined); if (!environment) { - return undefined; + return { discoveryFailed }; } candidates = candidates.filter((candidate) => candidate !== environment); @@ -854,7 +892,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { continue; } try { - return { environment, canonicalPath: await fs.realpath(executable) }; + return { + selectedBase: { environment, canonicalPath: await fs.realpath(executable) }, + discoveryFailed, + }; } catch (error) { this.log.warn( `Skipping base interpreter that cannot be resolved at ${executable}: ${getErrorMessage(error)}`, @@ -862,14 +903,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } - return undefined; + return { discoveryFailed }; } private async installAndSelectBaseInterpreter( metadata: InlineScriptMetadata, - ): Promise { + priorDiscoveryFailed = false, + ): Promise { const run = this.baseInterpreterInstallationQueue.then(() => - this.installAndSelectBaseInterpreterSerially(metadata), + this.installAndSelectBaseInterpreterSerially(metadata, priorDiscoveryFailed), ); // Keep the stored queue tail fulfilled so one failed request does not block later attempts; // the caller still observes the original result through `run`. @@ -882,35 +924,43 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private async installAndSelectBaseInterpreterSerially( metadata: InlineScriptMetadata, - ): Promise { + priorDiscoveryFailed: boolean, + ): Promise { const existing = await this.selectBaseInterpreter(metadata); - if (existing) { - return existing; + const discoveryFailed = priorDiscoveryFailed || existing.discoveryFailed; + if (existing.selectedBase) { + return { selectedBase: existing.selectedBase }; } const requiresPython = metadata.requiresPython?.trim() || undefined; const lowerBound = extractLowerBoundVersion(requiresPython); - const version = await this.selectInstallablePythonVersion(requiresPython, lowerBound); - if (requiresPython && !version) { + const versionSelection = await this.selectInstallablePythonVersion(requiresPython, lowerBound); + if (requiresPython && !versionSelection.version) { this.log.warn( 'Cannot install a Python for this inline script because no compatible install version could be selected.', ); - return undefined; + return { + errorCategory: this.getBaseInterpreterErrorCategory( + discoveryFailed, + versionSelection.errorCategory ?? 'no-compatible-python', + ), + }; } - const installedPath = await this.installPythonAndRefresh(requiresPython, version); - if (!installedPath) { - return undefined; + const installResult = await this.installPythonAndRefresh(requiresPython, versionSelection.version); + if (installResult.kind !== 'installed') { + return { + errorCategory: this.getBaseInterpreterErrorCategory( + discoveryFailed, + installResult.kind === 'declined' ? 'compatible-python-declined' : 'install-failure', + ), + }; } + const installedPath = installResult.installedPath; - let selected: SelectedBaseInterpreter | undefined; - try { - selected = await this.selectBaseInterpreter(metadata); - } catch (error) { - this.log.warn( - `Unable to refresh base-interpreter discovery after installing Python: ${getErrorMessage(error)}`, - ); - } + const refreshedSelection = await this.selectBaseInterpreter(metadata); + const discoveryFailedAfterInstall = discoveryFailed || refreshedSelection.discoveryFailed; + let selected = refreshedSelection.selectedBase; if (!selected) { const resolved = await resolveSystemPythonEnvironmentPath( installedPath, @@ -940,42 +990,55 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( 'Python was installed for an inline script, but no compatible base interpreter was discovered after refreshing environments.', ); + return { + errorCategory: this.getBaseInterpreterErrorCategory(discoveryFailedAfterInstall, 'install-failure'), + }; } - return selected; + return { selectedBase: selected }; } private async selectInstallablePythonVersion( requiresPython: string | undefined, lowerBound: string | undefined, - ): Promise { + ): Promise<{ readonly version?: string; readonly errorCategory?: InlineScriptEnvErrorCategory }> { if (!requiresPython) { - return lowerBound; + return { version: lowerBound }; } const prereleaseLowerBound = this.extractPrereleaseLowerBound(requiresPython); if (prereleaseLowerBound) { - return prereleaseLowerBound; + return { version: prereleaseLowerBound }; } const lowerBoundRelease = lowerBound ? parseReleaseSegments(lowerBound) : undefined; if (lowerBound && lowerBoundRelease?.[0] === 3) { if (/^>=\s*[^,]+$/.test(requiresPython) && this.matchesInstallConstraint(requiresPython, lowerBound)) { - return lowerBound; + return { version: lowerBound }; } if (/^==\s*[^,*]+$/.test(requiresPython) && this.matchesInstallConstraint(requiresPython, lowerBound)) { - return lowerBound; + return { version: lowerBound }; } } let available: uvPythonInstaller.UvPythonVersion[]; try { - if (!(await uvPythonInstaller.ensureUvForInlineScriptVersionLookup(requiresPython, this.log))) { - return undefined; + const uvLookupResult = await uvPythonInstaller.ensureUvForInlineScriptVersionLookupDetailed( + requiresPython, + this.log, + ); + if (uvLookupResult !== 'available') { + return { + errorCategory: + uvLookupResult === 'declined' ? 'compatible-python-declined' : 'install-failure', + }; } available = await uvPythonInstaller.getAvailablePythonVersions(); } catch (error) { this.log.warn(`Unable to query Python versions available from uv: ${getErrorMessage(error)}`); - return undefined; + return { errorCategory: 'install-failure' }; } - return available + if (available.length === 0) { + return { errorCategory: 'install-failure' }; + } + const version = available .filter( (candidate) => candidate.implementation === 'cpython' && @@ -991,6 +1054,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } return compareReleaseSegments(rightRelease, leftRelease); })[0]?.version; + return version ? { version } : { errorCategory: 'no-compatible-python' }; } private matchesInstallConstraint(requiresPython: string, version: string): boolean { @@ -1024,22 +1088,26 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private async installPythonAndRefresh( requiresPython: string | undefined, version: string | undefined, - ): Promise { - let installedPath: string | undefined; + ): Promise { + let promptResult: uvPythonInstaller.PromptInstallPythonViaUvResult; try { - installedPath = await uvPythonInstaller.promptInstallPythonViaUv('inlineScript', this.log, { + promptResult = await uvPythonInstaller.promptInstallPythonViaUvDetailed('inlineScript', this.log, { requiresPython, version, }); - if (!installedPath) { + if (promptResult.kind === 'declined') { this.log.warn( 'Python installation for inline-script environment creation was declined or did not complete.', ); - return undefined; + return { kind: 'declined' }; + } + if (promptResult.kind === 'failed') { + this.log.error('Failed to install Python for an inline script.'); + return { kind: 'failed' }; } } catch (error) { this.log.error(`Failed to install Python for an inline script: ${getErrorMessage(error)}`); - return undefined; + return { kind: 'failed' }; } try { @@ -1049,7 +1117,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { `Python was installed for an inline script, but environment discovery could not be refreshed: ${getErrorMessage(error)}`, ); } - return installedPath; + return { kind: 'installed', installedPath: promptResult.pythonPath }; } private async createOrReuseEnvironment({ @@ -1058,6 +1126,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadata, selectedBase, }: CreateOrReuseEnvironmentOptions): Promise { + const dependencyCount = this.getTelemetryDependencyCount(packages); const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); const envDir = getScriptEnvDir(this.globalStorageUri, cacheKey); await fs.ensureDir(cacheRoot.fsPath); @@ -1071,20 +1140,24 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { const cached = await this.inspectCacheEntry(cacheRoot, envDir, metadata, selectedBase); if (cached.kind === 'reusable') { + this.sendInlineScriptEnvReuseHitTelemetry(); return cached.environment; } if (cached.kind === 'uncertain') { this.log.warn( `Preserving an inline-script cache entry that could not be safely inspected: ${envDir.fsPath}`, ); + this.sendInlineScriptEnvErrorTelemetry('install-failure'); return undefined; } if (cached.kind === 'stale') { if (!(await this.removeCacheEntry(envDir))) { + this.sendInlineScriptEnvErrorTelemetry('install-failure'); return undefined; } } + const buildStartAtMs = Date.now(); const build = await this.buildCacheEntry(envDir, cacheRoot, packages, selectedBase); if (build.retainLock) { try { @@ -1095,8 +1168,16 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ); } } - return build.environment; + if (build.environment) { + this.sendInlineScriptEnvCreatedTelemetry(buildStartAtMs, dependencyCount); + return build.environment; + } + if (build.errorCategory) { + this.sendInlineScriptEnvErrorTelemetry(build.errorCategory); + } + return undefined; } catch (error) { + this.sendInlineScriptEnvErrorTelemetry(this.getCreateOrReuseErrorCategory(error)); this.log.error(`Failed to create or reuse inline-script cache entry: ${getErrorMessage(error)}`); return undefined; } finally { @@ -1210,21 +1291,21 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } catch (error) { this.log.error(`Failed to build inline-script environment: ${getErrorMessage(error)}`); await this.removeCacheEntry(envDir); - return {}; + return { errorCategory: 'install-failure' }; } if (result?.pkgInstallationCancelled) { this.log.warn( 'Inline-script package installation was cancelled; retaining the cache lock until explicit cleanup.', ); - return { retainLock: true }; + return { retainLock: true, errorCategory: 'package-install-cancelled' }; } if (!result?.environment || result.envCreationErr || result.pkgInstallationErr) { const error = result?.envCreationErr ?? result?.pkgInstallationErr ?? 'environment creation returned no result'; this.log.error(`Failed to build inline-script environment: ${error}`); await this.removeCacheEntry(envDir); - return {}; + return { errorCategory: 'install-failure' }; } if ( !this.areEqualPythonReleases(result.environment.version, selectedBase.environment.version) || @@ -1232,7 +1313,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ) { this.log.error('Created inline-script environment does not match the requested cache entry.'); await this.removeCacheEntry(envDir); - return {}; + return { errorCategory: 'install-failure' }; } try { @@ -1245,7 +1326,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } catch (error) { this.log.error(`Failed to record inline-script cache metadata: ${getErrorMessage(error)}`); await this.removeCacheEntry(envDir); - return {}; + return { errorCategory: 'install-failure' }; } return { environment: result.environment }; @@ -1282,6 +1363,44 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return compareReleaseSegments(actualRelease, expectedRelease) === 0; } + private getTelemetryDependencyCount(packages: ReadonlyArray): number { + return new Set(packages.map(normalizeDependency)).size; + } + + private sendInlineScriptEnvCreatedTelemetry(startAtMs: number, dependencyCount: number): void { + sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_CREATED, { + duration: Date.now() - startAtMs, + dependencyCount, + }); + } + + private sendInlineScriptEnvReuseHitTelemetry(): void { + sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT); + } + + private sendInlineScriptEnvErrorTelemetry(category: InlineScriptEnvErrorCategory): void { + sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category }); + } + + private getBaseInterpreterErrorCategory( + discoveryFailed: boolean, + fallbackCategory: InlineScriptEnvErrorCategory, + ): InlineScriptEnvErrorCategory { + return discoveryFailed ? 'discovery-failure' : fallbackCategory; + } + + private getCreateOrReuseErrorCategory(error: unknown): InlineScriptEnvErrorCategory { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + ['ELOCKED', 'ELOCKRETAINED', 'ELOCKORPHANED'].includes((error as NodeJS.ErrnoException).code ?? '') + ) { + return (error as NodeJS.ErrnoException).code === 'ELOCKED' ? 'lock-timeout' : 'lock-unavailable'; + } + return 'install-failure'; + } + dispose(): void { this._onDidChangeEnvironments.dispose(); this._onDidChangeEnvironment.dispose(); diff --git a/src/managers/builtin/uvPythonInstaller.ts b/src/managers/builtin/uvPythonInstaller.ts index c0fc90e64..7fabe4b58 100644 --- a/src/managers/builtin/uvPythonInstaller.ts +++ b/src/managers/builtin/uvPythonInstaller.ts @@ -40,6 +40,13 @@ export interface UvPythonInstallPromptOptions { readonly requiresPython?: string; } +export type EnsureUvForInlineScriptVersionLookupResult = 'available' | 'declined' | 'failed'; + +export type PromptInstallPythonViaUvResult = + | { readonly kind: 'installed'; readonly pythonPath: string } + | { readonly kind: 'declined' } + | { readonly kind: 'failed' }; + function sanitizePromptDetail(value: string | undefined): string | undefined { const normalized = value?.replace(PROMPT_CONTROL_CHARACTERS, ' ').replace(/\s+/g, ' ').trim(); if (!normalized) { @@ -185,30 +192,40 @@ export async function installUv(_log?: LogOutputChannel): Promise { return success; } -export async function ensureUvForInlineScriptVersionLookup( +export async function ensureUvForInlineScriptVersionLookupDetailed( requiresPython: string, log?: LogOutputChannel, -): Promise { +): Promise { if (await isUvInstalled(log)) { - return true; + return 'available'; } const displayedRequirement = sanitizePromptDetail(requiresPython); if (!displayedRequirement) { - return false; + return 'failed'; } const selection = await showInformationMessage( UvInstallStrings.inlineScriptInstallUvForVersionLookupPrompt(displayedRequirement), { modal: true }, UvInstallStrings.installUv, ); - if (selection !== UvInstallStrings.installUv || !(await installUv(log))) { - return false; + if (selection !== UvInstallStrings.installUv) { + return 'declined'; + } + if (!(await installUv(log))) { + return 'failed'; } if (await isUvInstalled(log)) { - return true; + return 'available'; } showErrorMessage(UvInstallStrings.uvInstallRestartRequired); - return false; + return 'failed'; +} + +export async function ensureUvForInlineScriptVersionLookup( + requiresPython: string, + log?: LogOutputChannel, +): Promise { + return (await ensureUvForInlineScriptVersionLookupDetailed(requiresPython, log)) === 'available'; } /** @@ -379,19 +396,19 @@ export async function installPythonViaUv(_log?: LogOutputChannel, version?: stri * @param trigger What triggered this prompt * @param log Optional log output channel * @param options Optional version and script requirement shown to the user and passed to uv after consent - * @returns Promise that resolves to the installed Python path, or undefined if not installed + * @returns Promise that resolves to a structured installed / declined / failed outcome */ -export async function promptInstallPythonViaUv( +export async function promptInstallPythonViaUvDetailed( trigger: UvPythonInstallTrigger, log?: LogOutputChannel, options?: UvPythonInstallPromptOptions, -): Promise { +): Promise { const state = trigger === 'inlineScript' ? undefined : await getGlobalPersistentState(); const dontAsk = await state?.get(UV_INSTALL_PYTHON_DONT_ASK_KEY); if (dontAsk) { traceLog('Skipping Python install prompt: user selected "Don\'t ask again"'); - return undefined; + return { kind: 'declined' }; } const version = sanitizePromptDetail(options?.version); @@ -399,11 +416,11 @@ export async function promptInstallPythonViaUv( if (trigger === 'inlineScript' && version && !INSTALLABLE_PYTHON_VERSION.test(version)) { traceWarn(`Skipping inline-script Python install prompt: invalid install version ${JSON.stringify(version)}`); - return undefined; + return { kind: 'failed' }; } if (trigger === 'inlineScript' && requiresPython && !version) { traceWarn('Skipping inline-script Python install prompt: no compatible install version was selected'); - return undefined; + return { kind: 'failed' }; } sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_PROMPTED, undefined, { trigger }); @@ -434,14 +451,24 @@ export async function promptInstallPythonViaUv( if (result === Common.dontAskAgain && state) { await state.set(UV_INSTALL_PYTHON_DONT_ASK_KEY, true); traceLog('User selected "Don\'t ask again" for Python install prompt'); - return undefined; + return { kind: 'declined' }; } if (result === installAction) { - return await installPythonWithUv(log, version); + const pythonPath = await installPythonWithUv(log, version); + return pythonPath ? { kind: 'installed', pythonPath } : { kind: 'failed' }; } - return undefined; + return { kind: 'declined' }; +} + +export async function promptInstallPythonViaUv( + trigger: UvPythonInstallTrigger, + log?: LogOutputChannel, + options?: UvPythonInstallPromptOptions, +): Promise { + const result = await promptInstallPythonViaUvDetailed(trigger, log, options); + return result.kind === 'installed' ? result.pythonPath : undefined; } /** diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 3d0488cae..ff06d9d00 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -13,6 +13,8 @@ import * as cacheLayout from '../../../../common/inlineScript/cacheLayout'; import * as metadataReader from '../../../../common/inlineScript/metadata'; import * as lockfileApis from '../../../../common/lockfile.apis'; import * as persistentState from '../../../../common/persistentState'; +import { EventNames } from '../../../../common/telemetry/constants'; +import * as telemetrySender from '../../../../common/telemetry/sender'; import { isWindows } from '../../../../common/utils/platformUtils'; import { normalizePath } from '../../../../common/utils/pathUtils'; import { getVenvPythonPath } from '../../../../common/utils/virtualEnvironment'; @@ -104,6 +106,7 @@ suite('InlineScriptEnvManager', () => { let nativeFinder: NativePythonFinder; let promptInstallPythonViaUvStub: sinon.SinonStub; let readMetadataStub: sinon.SinonStub; + let sendTelemetryStub: sinon.SinonStub; let inspectMetaStub: sinon.SinonStub; let retainLockStub: sinon.SinonStub; let releaseLockStub: sinon.SinonStub; @@ -152,9 +155,12 @@ suite('InlineScriptEnvManager', () => { computeCacheKeyStub = sinon.stub(cacheKey, 'computeCacheKey').returns(CACHE_KEY); getAvailablePythonVersionsStub = sinon.stub(uvPythonInstaller, 'getAvailablePythonVersions').resolves([]); ensureUvForVersionLookupStub = sinon - .stub(uvPythonInstaller, 'ensureUvForInlineScriptVersionLookup') - .resolves(true); - promptInstallPythonViaUvStub = sinon.stub(uvPythonInstaller, 'promptInstallPythonViaUv'); + .stub(uvPythonInstaller, 'ensureUvForInlineScriptVersionLookupDetailed') + .resolves('available'); + promptInstallPythonViaUvStub = sinon + .stub(uvPythonInstaller, 'promptInstallPythonViaUvDetailed') + .resolves({ kind: 'declined' }); + sendTelemetryStub = sinon.stub(telemetrySender, 'sendTelemetryEvent'); inspectMetaStub = sinon.stub(cacheLayout, 'inspectMetaJson').resolves({ kind: 'missing' }); baseInterpreterStatusStub = sinon.stub(cacheLayout, 'getBaseInterpreterStatus').resolves('available'); writeMetaStub = sinon.stub(cacheLayout, 'writeMetaJson').resolves(); @@ -228,6 +234,16 @@ suite('InlineScriptEnvManager', () => { return new Promise((resolve) => setImmediate(resolve)); } + function telemetryCalls(eventName: EventNames): sinon.SinonSpyCall[] { + return sendTelemetryStub.getCalls().filter((call) => call.args[0] === eventName); + } + + function assertNoInlineScriptLifecycleTelemetry(): void { + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + } + suite('static metadata and deferred methods', () => { test('exposes creation but leaves later-phase methods empty', async () => { const asInterface: EnvironmentManager = manager; @@ -386,7 +402,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -407,7 +423,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([]); apiGetEnvironmentsStub.onSecondCall().resolves([]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -422,7 +438,7 @@ suite('InlineScriptEnvManager', () => { test('does not mutate the cache when the user declines installation', async () => { readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); - promptInstallPythonViaUvStub.resolves(undefined); + promptInstallPythonViaUvStub.resolves({ kind: 'declined' }); assert.strictEqual(await manager.create(scriptUri()), undefined); @@ -449,7 +465,7 @@ suite('InlineScriptEnvManager', () => { const uvBase = makeEnvironment('ms-python.python:system', '3.13.2', uvExecutable); readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); apiGetEnvironmentsStub.resolves([baseEnvironment]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); apiRefreshEnvironmentsStub.rejects(new Error('discovery failed')); resolveSystemPythonStub.resolves(uvBase); @@ -474,7 +490,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().rejects(new Error('discovery failed')); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); resolveSystemPythonStub.resolves(uvBase); assert.ok(await manager.create(scriptUri())); @@ -524,7 +540,7 @@ suite('InlineScriptEnvManager', () => { arch: 'x86_64', }, ]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -552,7 +568,7 @@ suite('InlineScriptEnvManager', () => { makeUvPythonVersion('3.13.3'), makeUvPythonVersion('3.13.0'), ]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -571,7 +587,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); getAvailablePythonVersionsStub.resolves([makeUvPythonVersion('3.11.14')]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -590,7 +606,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); getAvailablePythonVersionsStub.resolves([]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -613,7 +629,7 @@ suite('InlineScriptEnvManager', () => { makeUvPythonVersion('3.15.0a6'), makeUvPythonVersion('3.14.2'), ]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -631,7 +647,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -649,7 +665,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves([uvBase]); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri())); @@ -682,7 +698,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onFirstCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onSecondCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onThirdCall().resolves(refreshedEnvironments); - promptInstallPythonViaUvStub.resolves(baseExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: baseExecutable }); assert.strictEqual(await manager.create(scriptUri()), undefined); @@ -730,7 +746,7 @@ suite('InlineScriptEnvManager', () => { signalPrompt!(); await installGate; installed = true; - return uvExecutable; + return { kind: 'installed', pythonPath: uvExecutable }; }); const first = manager.create(uri); @@ -750,23 +766,23 @@ suite('InlineScriptEnvManager', () => { const uri = scriptUri(); readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); apiGetEnvironmentsStub.resolves([baseEnvironment]); - let finishPrompt: ((value: undefined) => void) | undefined; + let finishPrompt: (() => void) | undefined; let signalPrompt: (() => void) | undefined; const promptShown = new Promise((resolve) => { signalPrompt = resolve; }); promptInstallPythonViaUvStub.callsFake( () => - new Promise((resolve) => { + new Promise<{ kind: 'declined' }>((resolve) => { signalPrompt!(); - finishPrompt = resolve; + finishPrompt = () => resolve({ kind: 'declined' }); }), ); const first = manager.create(uri); await promptShown; const second = manager.create(uri); - finishPrompt!(undefined); + finishPrompt!(); assert.deepStrictEqual(await Promise.all([first, second]), [undefined, undefined]); assert.deepStrictEqual(await Promise.all([first, second]), [undefined, undefined]); assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1); @@ -809,7 +825,7 @@ suite('InlineScriptEnvManager', () => { signalPrompt!(); await installGate; isInstalled = true; - return uvExecutable; + return { kind: 'installed', pythonPath: uvExecutable }; }); const first = manager.create(scriptUri('a.py')); @@ -862,7 +878,7 @@ suite('InlineScriptEnvManager', () => { signalPrompt!(); await installGate; installed = true; - return uvExecutable; + return { kind: 'installed', pythonPath: uvExecutable }; }); const first = manager.create(scriptUri('lower-bound.py')); @@ -898,7 +914,7 @@ suite('InlineScriptEnvManager', () => { promptInstallPythonViaUvStub.callsFake(async () => { signalPrompt!(); await installGate; - return uvExecutable; + return { kind: 'installed', pythonPath: uvExecutable }; }); const first = manager.create(scriptUri('first.py')); @@ -925,7 +941,7 @@ suite('InlineScriptEnvManager', () => { apiGetEnvironmentsStub.onThirdCall().resolves([baseEnvironment]); apiGetEnvironmentsStub.onCall(3).rejects(new Error('discovery unavailable')); resolveSystemPythonStub.resolves(uvBase); - promptInstallPythonViaUvStub.resolves(uvExecutable); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); assert.ok(await manager.create(scriptUri('first.py'))); assert.ok(await manager.create(scriptUri('second.py'))); @@ -1485,6 +1501,305 @@ suite('InlineScriptEnvManager', () => { }); }); + suite('telemetry', () => { + test('does not emit lifecycle telemetry for non-applicable create calls', async () => { + readMetadataStub.resolves(undefined); + + assert.strictEqual(await manager.create('global'), undefined); + assert.strictEqual(await manager.create(scriptUri()), undefined); + assertNoInlineScriptLifecycleTelemetry(); + }); + + test('emits envCreated with only duration and dependencyCount after verified creation', async () => { + assert.ok(await manager.create(scriptUri())); + + const createdCalls = telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED); + assert.strictEqual(createdCalls.length, 1); + assert.deepStrictEqual(createdCalls[0].args, [ + EventNames.INLINE_SCRIPT_ENV_CREATED, + { duration: 0, dependencyCount: 1 }, + ]); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + }); + + test('emits envReuseHit only for validated cache hits', async () => { + await fs.ensureDir(envDir().fsPath); + setSidecar({ + schemaVersion: cacheLayout.META_SCHEMA_VERSION, + baseInterpreterPath: baseExecutable, + baseInterpreterVersion: baseEnvironment.version, + lastUsedAt: NOW.toISOString(), + }); + const cached = makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(envDir().fsPath), + envDir().fsPath, + ); + await fs.outputFile(venvPythonPath(envDir().fsPath), ''); + resolveVenvStub.resolves(cached); + + assert.strictEqual(await manager.create(scriptUri()), cached); + + const reuseCalls = telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT); + assert.strictEqual(reuseCalls.length, 1); + assert.deepStrictEqual(reuseCalls[0].args, [EventNames.INLINE_SCRIPT_ENV_REUSE_HIT]); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + }); + + test('emits a single compatible-python-declined error for coalesced same-script requests', async () => { + const uri = scriptUri(); + readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); + apiGetEnvironmentsStub.resolves([baseEnvironment]); + let finishPrompt: (() => void) | undefined; + let signalPrompt: (() => void) | undefined; + const promptShown = new Promise((resolve) => { + signalPrompt = resolve; + }); + promptInstallPythonViaUvStub.callsFake( + () => + new Promise<{ kind: 'declined' }>((resolve) => { + signalPrompt!(); + finishPrompt = () => resolve({ kind: 'declined' }); + }), + ); + + const first = manager.create(uri); + await promptShown; + const second = manager.create(uri); + finishPrompt!(); + + assert.deepStrictEqual(await Promise.all([first, second]), [undefined, undefined]); + assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1); + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'compatible-python-declined' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + }); + + test('emits no-compatible-python when quick create cannot prompt for a compatible interpreter', async () => { + readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); + apiGetEnvironmentsStub.resolves([baseEnvironment]); + + assert.strictEqual(await manager.create(scriptUri(), { quickCreate: true }), undefined); + + assert.strictEqual(promptInstallPythonViaUvStub.callCount, 0); + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'no-compatible-python' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + }); + + test('emits discovery-failure when quick create cannot inspect discovered interpreters', async () => { + readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); + apiGetEnvironmentsStub.rejects(new Error('discovery unavailable')); + + assert.strictEqual(await manager.create(scriptUri(), { quickCreate: true }), undefined); + + assert.strictEqual(promptInstallPythonViaUvStub.callCount, 0); + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'discovery-failure' }]], + ); + }); + + test('emits discovery-failure instead of compatible-python-declined when discovery is unavailable', async () => { + const uri = scriptUri(); + readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); + apiGetEnvironmentsStub.rejects(new Error('discovery unavailable')); + + assert.strictEqual(await manager.create(uri), undefined); + + assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1); + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'discovery-failure' }]], + ); + }); + + test('emits discovery-failure instead of install-failure when discovery never recovers', async () => { + const uvExecutable = path.join(tempRoot, 'uv-python', isWindows() ? 'python.exe' : 'python'); + await fs.outputFile(uvExecutable, ''); + readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' }); + apiGetEnvironmentsStub.rejects(new Error('discovery unavailable')); + promptInstallPythonViaUvStub.resolves({ kind: 'installed', pythonPath: uvExecutable }); + resolveSystemPythonStub.resolves(undefined); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + + assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1); + assert.strictEqual(resolveSystemPythonStub.callCount, 1); + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'discovery-failure' }]], + ); + }); + + test('emits a single envCreated event for coalesced same-key creation', async () => { + let continueCreation: (() => void) | undefined; + let creationStarted: (() => void) | undefined; + const started = new Promise((resolve) => { + creationStarted = resolve; + }); + const gate = new Promise((resolve) => { + continueCreation = resolve; + }); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + await fs.outputFile(venvPythonPath(target), ''); + creationStarted!(); + await gate; + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + }; + }); + + const first = manager.create(scriptUri('a.py')); + await started; + const second = manager.create(scriptUri('b.py')); + continueCreation!(); + + const [firstResult, secondResult] = await Promise.all([first, second]); + assert.ok(firstResult); + assert.strictEqual(firstResult, secondResult); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 1); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + }); + + test('excludes lock and cache inspection time from envCreated duration', async () => { + await fs.ensureDir(envDir().fsPath); + lockStub.callsFake(async () => { + clock.tick(3_000); + return { release: releaseLockStub, retain: retainLockStub }; + }); + inspectMetaStub.callsFake(async () => { + clock.tick(2_000); + return { kind: 'missing' }; + }); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + clock.tick(25); + await fs.outputFile(venvPythonPath(target), ''); + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + }; + }); + + assert.ok(await manager.create(scriptUri())); + + const createdCalls = telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED); + assert.strictEqual(createdCalls.length, 1); + assert.deepStrictEqual(createdCalls[0].args, [ + EventNames.INLINE_SCRIPT_ENV_CREATED, + { duration: 25, dependencyCount: 1 }, + ]); + }); + + test('emits lock-timeout when the cache lock cannot be acquired', async () => { + lockStub.rejects(Object.assign(new Error('already locked'), { code: 'ELOCKED' })); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'lock-timeout' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + }); + + for (const code of ['ELOCKRETAINED', 'ELOCKORPHANED'] as const) { + test(`emits lock-unavailable when cache lock acquisition fails with ${code}`, async () => { + lockStub.rejects(Object.assign(new Error('lock unavailable'), { code })); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'lock-unavailable' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + }); + } + + test('emits package-install-cancelled and no success event on rollback', async () => { + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + await fs.outputFile(venvPythonPath(target), ''); + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + pkgInstallationErr: 'Canceled', + pkgInstallationCancelled: true, + }; + }); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'package-install-cancelled' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + }); + + test('emits install-failure when sidecar persistence rollback removes the new environment', async () => { + writeMetaStub.rejects(new Error('disk full')); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + + assert.deepStrictEqual( + telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args), + [[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'install-failure' }]], + ); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + }); + + test('rebuilds a failed reuse validation as creation without counting a reuse hit', async () => { + await fs.ensureDir(envDir().fsPath); + setSidecar({ + schemaVersion: cacheLayout.META_SCHEMA_VERSION, + baseInterpreterPath: baseExecutable, + baseInterpreterVersion: baseEnvironment.version, + lastUsedAt: NOW.toISOString(), + }); + await fs.outputFile(venvPythonPath(envDir().fsPath), ''); + resolveVenvStub.resolves( + makeEnvironment( + 'ms-python.python:inline-script', + '3.10.0', + venvPythonPath(envDir().fsPath), + envDir().fsPath, + ), + ); + + assert.ok(await manager.create(scriptUri())); + + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 1); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + }); + }); + suite('script association persistence', () => { test('sets, gets, unsets, persists, and reports only actual selection changes', async () => { const uri = scriptUri(); From aeeaeb26287b92484e0070443f4c806f1001e21b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 17:22:21 -0700 Subject: [PATCH 4/5] Expand inline script telemetry coverage Cover detailed uv outcomes and normalized dependency counts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- .../inlineScript/envManager.unit.test.ts | 13 +++++ .../builtin/uvPythonInstaller.unit.test.ts | 56 ++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index ff06d9d00..8b9ca2af6 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -1523,6 +1523,19 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); }); + test('deduplicates normalized dependencies for envCreated dependencyCount', async () => { + readMetadataStub.resolves({ ...VALID_METADATA, dependencies: ['Requests', 'requests'] }); + + assert.ok(await manager.create(scriptUri())); + + const createdCalls = telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED); + assert.strictEqual(createdCalls.length, 1); + assert.deepStrictEqual(createdCalls[0].args, [ + EventNames.INLINE_SCRIPT_ENV_CREATED, + { duration: 0, dependencyCount: 1 }, + ]); + }); + test('emits envReuseHit only for validated cache hits', async () => { await fs.ensureDir(envDir().fsPath); setSidecar({ diff --git a/src/test/managers/builtin/uvPythonInstaller.unit.test.ts b/src/test/managers/builtin/uvPythonInstaller.unit.test.ts index cf99c84ff..b2d556584 100644 --- a/src/test/managers/builtin/uvPythonInstaller.unit.test.ts +++ b/src/test/managers/builtin/uvPythonInstaller.unit.test.ts @@ -11,10 +11,12 @@ import * as windowApis from '../../../common/window.apis'; import * as helpers from '../../../managers/builtin/helpers'; import { clearDontAskAgain, + ensureUvForInlineScriptVersionLookupDetailed, ensureUvForInlineScriptVersionLookup, getAvailablePythonVersions, getUvPythonPath, isDontAskAgainSet, + promptInstallPythonViaUvDetailed, promptInstallPythonViaUv, UV_INSTALL_PYTHON_DONT_ASK_KEY, UvPythonVersion, @@ -68,6 +70,15 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { return executeTaskStub; } + test('should report available from the detailed uv lookup API', async () => { + isUvInstalledStub.resolves(true); + + const result = await ensureUvForInlineScriptVersionLookupDetailed('>=3.13,<3.14', mockLog); + + assert.strictEqual(result, 'available'); + assert(showInformationMessageStub.notCalled, 'Should not prompt when uv is already available'); + }); + test('should return undefined when "Don\'t ask again" is set', async () => { mockState.get.resolves(true); @@ -96,6 +107,15 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { ); }); + test('should report a declined detailed uv lookup distinctly from the boolean wrapper', async () => { + isUvInstalledStub.resolves(false); + showInformationMessageStub.resolves(undefined); + + const result = await ensureUvForInlineScriptVersionLookupDetailed('>=3.13,<3.14', mockLog); + + assert.strictEqual(result, 'declined'); + }); + test('should show correct prompt when uv is NOT installed', async () => { mockState.get.resolves(false); isUvInstalledStub.resolves(false); @@ -214,6 +234,28 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { assert(isUvInstalledStub.notCalled, 'Should stop before checking or installing uv'); }); + test('should report a failed detailed Python install prompt distinctly from the undefined wrapper', async () => { + mockState.get.resolves(false); + + const result = await promptInstallPythonViaUvDetailed('inlineScript', mockLog, { + requiresPython: '>=3.13', + version: 'latest\nInstall anyway', + }); + + assert.deepStrictEqual(result, { kind: 'failed' }); + assert(showInformationMessageStub.notCalled, 'Should not display an invalid install version'); + }); + + test('should report a declined detailed Python install prompt distinctly from the undefined wrapper', async () => { + mockState.get.resolves(false); + isUvInstalledStub.resolves(true); + showInformationMessageStub.resolves(undefined); + + const result = await promptInstallPythonViaUvDetailed('activation', mockLog); + + assert.deepStrictEqual(result, { kind: 'declined' }); + }); + test('should allow a validated prerelease install version', async () => { mockState.get.resolves(false); isUvInstalledStub.resolves(true); @@ -252,6 +294,16 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { assert.strictEqual(showErrorMessageStub.callCount, 0); }); + test('should report a failed detailed uv lookup distinctly from the boolean wrapper', async () => { + isUvInstalledStub.resolves(false); + showInformationMessageStub.resolves(UvInstallStrings.installUv); + stubUvInstallTask(1); + + const result = await ensureUvForInlineScriptVersionLookupDetailed('>=3.13,<3.14', mockLog); + + assert.strictEqual(result, 'failed'); + }); + test('should stop version lookup when uv installation fails', async () => { isUvInstalledStub.resolves(false); showInformationMessageStub.resolves(UvInstallStrings.installUv); @@ -402,7 +454,7 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { const spawnStub: sinon.SinonStub = sinon.stub(childProcessApis, 'spawnProcess'); spawnStub.returns(mockProcess); - const resultPromise = promptInstallPythonViaUv('inlineScript', mockLog, { + const resultPromise = promptInstallPythonViaUvDetailed('inlineScript', mockLog, { requiresPython: '>=3.13', version: '3.13', }); @@ -411,7 +463,7 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => { mockProcess.emit('exit', 0, null); }, 10); - assert.strictEqual(await resultPromise, '/usr/bin/python3.13'); + assert.deepStrictEqual(await resultPromise, { kind: 'installed', pythonPath: '/usr/bin/python3.13' }); const installTask = executeTaskStub.firstCall.args[0]; const execution = installTask.execution as ShellExecution; assert.strictEqual(execution.command, 'uv'); From f2c5987a54ac4861fbfee7f019d872c8be4f6a4a Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 20:00:01 -0700 Subject: [PATCH 5/5] Localize inline telemetry test helpers Keep telemetry-only helpers scoped to their consuming test suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- .../inlineScript/envManager.unit.test.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 8b9ca2af6..127f3795b 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -234,16 +234,6 @@ suite('InlineScriptEnvManager', () => { return new Promise((resolve) => setImmediate(resolve)); } - function telemetryCalls(eventName: EventNames): sinon.SinonSpyCall[] { - return sendTelemetryStub.getCalls().filter((call) => call.args[0] === eventName); - } - - function assertNoInlineScriptLifecycleTelemetry(): void { - assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); - assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); - assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); - } - suite('static metadata and deferred methods', () => { test('exposes creation but leaves later-phase methods empty', async () => { const asInterface: EnvironmentManager = manager; @@ -1502,6 +1492,16 @@ suite('InlineScriptEnvManager', () => { }); suite('telemetry', () => { + function telemetryCalls(eventName: EventNames): sinon.SinonSpyCall[] { + return sendTelemetryStub.getCalls().filter((call) => call.args[0] === eventName); + } + + function assertNoInlineScriptLifecycleTelemetry(): void { + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0); + assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0); + } + test('does not emit lifecycle telemetry for non-applicable create calls', async () => { readMetadataStub.resolves(undefined);