Skip to content

Commit 87f865e

Browse files
edvilmeCopilotCopilot
authored
Adopt package manager command classes (#1686)
## Summary Adopts the package manager command classes introduced in #1621 across Pip/UV, Conda, and Poetry while preserving existing behavior and error contracts. ## Changes - Replaces existing command construction and execution paths with the new command classes. - Uses JSON unconditionally for machine-readable commands when the installed tool supports it, including all required flags. - Uses a dedicated text parser for Pip 21.2-25.0; Pip 25.1+ always uses `pip index versions --json`. - Preserves operation-specific timeout behavior: install/uninstall mutations are unbounded, while list/direct-list operations retain 30-second timeouts. - Propagates malformed version output as an operational failure rather than treating it as a successful empty result. - Keeps destructive environment removal confirmation bypass explicit through `runHeadless`. ## Relationship - Targets `main` directly and includes the changes merged in #1717. - Replaces #1678, whose base branch could not be changed after GitHub registered it as part of a stack. - Deterministic command argument and parser tests are intentionally included here so the adopted implementations are covered in the same PR. The superseded network-dependent roundtrip test was removed in favor of the controlled package-manager integration fixtures. ## Testing - `npm run lint` - `npm run compile-tests` - `npm run unittest` (1,855 passing, 6 pending) - Focused package-manager command, selection, timeout, parsing, and removal-confirmation tests --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: 3f16397e-0917-4efb-8d75-566c71ebf9ba Copilot-Session: 6b2fe9b5-38ea-442f-b07a-b6c71134d480 Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e Copilot-Session: 5aba47cf-0b43-48b9-bc22-75e625da70e8
1 parent 8137950 commit 87f865e

33 files changed

Lines changed: 1517 additions & 989 deletions

.github/instructions/testing-workflow.instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ envConfig.inspect
603603
- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1)
604604
- Use `sinon.useFakeTimers()` with `clock.tickAsync()` instead of `await new Promise(resolve => setTimeout(resolve, ms))` for debounce/timeout handling - eliminates flakiness and speeds up tests significantly (1)
605605
- Always compile tests (`npm run compile-tests`) before running them after adding new test cases - test counts will be wrong if running against stale compiled output (1)
606+
- **Delete stale compiled test output after removing a test source file**: `compile-tests` does not remove the corresponding file under `out/`, so local unit or integration runs may execute a deleted test until that artifact is cleaned (1)
606607
- Never create "documentation tests" that just `assert.ok(true)` — if mocking limitations prevent testing, either test a different layer that IS mockable, or skip the test entirely with a clear explanation (1)
607608
- When stubbing vscode APIs in tests via wrapper modules (e.g., `workspaceApis`), the production code must also use those wrappers — sinon cannot stub properties directly on the vscode namespace like `workspace.workspaceFolders`, so both production and test code must reference the same stubbable wrapper functions (4)
608609
- **Before writing tests**, check if the function under test calls VS Code APIs directly (e.g., `commands.executeCommand`, `window.createTreeView`, `workspace.getConfiguration`). If so, FIRST update the production code to use wrapper functions from `src/common/*.apis.ts` (create the wrapper if it doesn't exist), THEN write tests that stub those wrappers. This prevents CI failures where sinon cannot stub the vscode namespace (4)
@@ -611,3 +612,4 @@ envConfig.inspect
611612
- **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)
612613
- **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)
613614
- **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)
615+
- **Use controlled fixture providers for package lifecycle integration tests**: Discovering an existing Conda environment does not guarantee API quick-create can succeed. Use `createEnvironmentFixture()` so creation prerequisites and cleanup are deterministic (1)

src/common/inlineScript/cacheKey.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33

44
import { createHash } from 'crypto';
5-
import { normalizePackageName } from '../../managers/builtin/utils';
5+
import { normalizePackageName } from '../../managers/common/packageUtils';
66
import { normalizePath } from '../utils/pathUtils';
77

88
/** Length, in hex chars, of the cache key returned by {@link computeCacheKey}. 16 = 64 bits of SHA-256; fixed-length and filesystem-safe. */

src/managers/base/commands/packageManagerCommand.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export abstract class PackageManagerCommand {
2828
protected pythonExecutable: string;
2929
protected cwd?: string;
3030
protected log?: LogOutputChannel;
31-
protected timeout: number = 300000;
31+
protected timeout: number | undefined;
3232
protected config?: WorkspaceConfiguration;
3333

3434
constructor(options: CommandConstructorOptions) {

src/managers/builtin/commands/availableVersions.ts

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,24 @@ import type { Pep440Version } from '@renovatebot/pep440';
22
import { AvailableVersionsCommand, type AvailableVersionsExecuteArgs } from '../../base/commands/index';
33
import { runPython, runUV } from '../helpers';
44

5+
function parseVersionsJson(output: string, tool: 'pip' | 'uv'): string[] {
6+
const parsed: unknown = JSON.parse(output);
7+
if (
8+
typeof parsed !== 'object' ||
9+
parsed === null ||
10+
!('versions' in parsed) ||
11+
!Array.isArray(parsed.versions) ||
12+
!parsed.versions.every((version) => typeof version === 'string')
13+
) {
14+
throw new Error(`Unexpected package version JSON from ${tool}.`);
15+
}
16+
17+
return parsed.versions;
18+
}
19+
520
/**
621
* Pip available versions command.
7-
* Parsed command: `python -m pip index versions <package> --json --python-version <version>`
22+
* Parsed command: `python -m pip index versions <package> --json --disable-pip-version-check --python-version <version>`
823
* Official documentation: https://pip.pypa.io/en/stable/cli/pip_index/
924
*/
1025
export class PipAvailableVersionsCommand extends AvailableVersionsCommand {
@@ -16,6 +31,7 @@ export class PipAvailableVersionsCommand extends AvailableVersionsCommand {
1631
'versions',
1732
executeArgs.packageName,
1833
'--json',
34+
'--disable-pip-version-check',
1935
'--python-version',
2036
executeArgs.pythonVersion,
2137
];
@@ -30,20 +46,41 @@ export class PipAvailableVersionsCommand extends AvailableVersionsCommand {
3046
executeArgs.cancellationToken,
3147
this.timeout,
3248
);
33-
const match = output.match(/{[\s\S]*}/);
34-
if (!match) {
35-
return [];
36-
}
49+
return this.parseVersions(parseVersionsJson(output, 'pip'), executeArgs.includePrerelease);
50+
}
51+
}
52+
53+
/**
54+
* Pip available versions command for Pip 21.2 through 25.0, before JSON output was supported.
55+
*/
56+
export class PipAvailableVersionsTextCommand extends AvailableVersionsCommand {
57+
protected buildCommand(executeArgs: AvailableVersionsExecuteArgs): string[] {
58+
return [
59+
'-m',
60+
'pip',
61+
'index',
62+
'versions',
63+
executeArgs.packageName,
64+
'--disable-pip-version-check',
65+
'--python-version',
66+
executeArgs.pythonVersion,
67+
];
68+
}
3769

38-
try {
39-
const parsed = JSON.parse(match[0]) as { versions?: string[] };
40-
return this.parseVersions(
41-
Array.isArray(parsed.versions) ? parsed.versions : [],
42-
executeArgs.includePrerelease,
43-
);
44-
} catch {
45-
return [];
70+
async execute(executeArgs: AvailableVersionsExecuteArgs): Promise<Pep440Version[]> {
71+
const output = await runPython(
72+
this.pythonExecutable,
73+
this.buildCommand(executeArgs),
74+
undefined,
75+
this.log,
76+
executeArgs.cancellationToken,
77+
this.timeout,
78+
);
79+
const match = output.match(/^Available versions:\s*(.+)$/im);
80+
if (!match) {
81+
throw new Error('Unable to parse available package versions from pip output.');
4682
}
83+
return this.parseVersions(match[1].split(','), executeArgs.includePrerelease);
4784
}
4885
}
4986

@@ -75,19 +112,6 @@ export class UvAvailableVersionsCommand extends AvailableVersionsCommand {
75112
executeArgs.cancellationToken,
76113
this.timeout,
77114
);
78-
const match = output.match(/{[\s\S]*}/);
79-
if (!match) {
80-
return [];
81-
}
82-
83-
try {
84-
const parsed = JSON.parse(match[0]) as { versions?: string[] };
85-
return this.parseVersions(
86-
Array.isArray(parsed.versions) ? parsed.versions : [],
87-
executeArgs.includePrerelease,
88-
);
89-
} catch {
90-
return [];
91-
}
115+
return this.parseVersions(parseVersionsJson(output, 'uv'), executeArgs.includePrerelease);
92116
}
93117
}

src/managers/builtin/commands/factory.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,24 @@ import { shouldUseUv } from '../helpers';
33

44
type CommandConstructor<T> = new (options: CommandConstructorOptions) => T;
55

6+
export type PipOrUvCommand<P, U> = { kind: 'pip'; command: P } | { kind: 'uv'; command: U };
7+
8+
export async function createPipOrUvCommandWithKind<P, U>(
9+
options: CommandConstructorOptions,
10+
environmentPath: string,
11+
PipCommand: CommandConstructor<P>,
12+
UvCommand: CommandConstructor<U>,
13+
): Promise<PipOrUvCommand<P, U>> {
14+
return (await shouldUseUv(options.log, environmentPath))
15+
? { kind: 'uv', command: new UvCommand(options) }
16+
: { kind: 'pip', command: new PipCommand(options) };
17+
}
18+
619
export async function createPipOrUvCommand<T, P extends T, U extends T>(
720
options: CommandConstructorOptions,
821
environmentPath: string,
922
PipCommand: CommandConstructor<P>,
1023
UvCommand: CommandConstructor<U>,
1124
): Promise<T> {
12-
return (await shouldUseUv(options.log, environmentPath)) ? new UvCommand(options) : new PipCommand(options);
25+
return (await createPipOrUvCommandWithKind(options, environmentPath, PipCommand, UvCommand)).command;
1326
}

src/managers/builtin/commands/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
export { PipAvailableVersionsCommand, UvAvailableVersionsCommand } from './availableVersions';
1+
export {
2+
PipAvailableVersionsCommand,
3+
PipAvailableVersionsTextCommand,
4+
UvAvailableVersionsCommand,
5+
} from './availableVersions';
26
export { PipInstallCommand, UvInstallCommand } from './install';
37
export { PipListCommand, UvListCommand } from './list';
48
export { PipListDirectNamesCommand, UvListDirectNamesCommand } from './listDirectNames';

src/managers/builtin/commands/listDirectNames.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { ListDirectNamesCommand, type BaseExecuteArgs } from '../../base/commands/index';
2+
import { normalizePackageName } from '../../common/packageUtils';
23
import { runPython, runUV } from '../helpers';
3-
import { normalizePackageName } from '../utils';
44

55
/**
66
* Pip list direct names command.
7-
* Parsed command: `python -m pip list --format=json --not-required`
7+
* Parsed command: `python -m pip list --format=json --not-required --disable-pip-version-check`
88
* Official documentation: https://pip.pypa.io/en/stable/cli/pip_list/
99
*/
1010
export class PipListDirectNamesCommand extends ListDirectNamesCommand {
1111
protected buildCommand(): string[] {
12-
return ['-m', 'pip', 'list', '--format=json', '--not-required'];
12+
return ['-m', 'pip', 'list', '--format=json', '--not-required', '--disable-pip-version-check'];
1313
}
1414

1515
async execute(executeArgs?: BaseExecuteArgs): Promise<Set<string>> {

src/managers/builtin/pipListUtils.ts

Lines changed: 0 additions & 34 deletions
This file was deleted.

0 commit comments

Comments
 (0)