Add package manager command smoke tests

[edvilme]

Summary

Adds focused unit coverage for the concrete package manager command classes introduced in #1621, across Pip/UV, Conda, and Poetry. Also corrects the UvUninstallCommand doc comment.

What the tests cover

• Read commands (list, list-direct-names, version, available-versions, Poetry show): stub the runner with representative output and assert the parsed result — package lists (with rows missing a version dropped), direct names with normalization, uv pip tree indentation filtering, version-string parsing, and available-version prerelease filtering.
• Mutation commands (install / uninstall / add / remove): grouped as smoke tests that assert execute() resolves. They have no output to parse; their concrete command strings are exercised by integration tests and by the adoption PR Adopt package manager command classes #1686.

These deliberately avoid asserting the exact argument vector, which would merely restate buildCommand.

Doc fix

• UvUninstallCommand: uv pip uninstall has no -y/--yes flag and is non-interactive by default — corrected the misleading comment.

Relationship

• Depends only on the already-merged command classes (Introduce package manager command classes #1621); targets main directly.
• Independent of the adoption PR Adopt package manager command classes #1686. These two were previously a GitHub stack; the stack has been undone and both PRs now target main. Both currently add the command test files, so whichever merges first the other needs a trivial rebase on those files.

Testing

• npm run compile-tests
• npm run unittest (58 command tests passing)

[edvilme]

Contributes to #1622

[edvilme]

Note: These tests only verify that the commands are valid, a further PR will add integration tests with the package managers and round-trips such as: fetching available versions for a package, installing it, listing (direct) packages, uninstalling, etc...

[Copilot]

Pull request overview

This PR adds lightweight “smoke” unit tests for the concrete package manager command classes introduced in #1621, ensuring each command’s execute() method can run end-to-end under minimal mocked conditions without throwing/rejecting.

Changes:

• Adds smoke unit tests for Pip/UV command classes, stubbing runPython/runUV and providing minimal parseable outputs where needed.
• Adds smoke unit tests for Conda command classes, stubbing runCondaExecutable and providing minimal JSON/string outputs.
• Adds smoke unit tests for Poetry command classes, stubbing runPoetry / getPoetryVersion and providing minimal outputs for parsing commands.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/test/managers/builtin/commands.unit.test.ts	Smoke coverage for Pip/UV command classes with mocked execution helpers and minimal outputs.
src/test/managers/conda/commands.unit.test.ts	Smoke coverage for Conda command classes with mocked conda execution and minimal JSON/string outputs.
src/test/managers/poetry/commands.unit.test.ts	Smoke coverage for Poetry command classes with mocked runPoetry/getPoetryVersion and minimal outputs.

[StellaHuang95]

Finding: The tests pass even when the commands do nothing or produce incorrect results

Severity: Medium

Locations:

• src/test/managers/builtin/commands.unit.test.ts#L32-L109
• src/test/managers/conda/commands.unit.test.ts#L29-L71
• src/test/managers/poetry/commands.unit.test.ts#L31-L60

Every test uses this general assertion:

await assert.doesNotReject(() => command.execute(...));

In simple terms, this asks only:

“Did this function finish without throwing an error?”

It does not ask:

• Was the package-manager executable called?
• Was the correct subcommand used?
• Were the correct arguments passed?
• Was the correct environment selected?
• Was cancellation forwarded?
• Was the returned output parsed correctly?
• Did the command return the expected packages or versions?

Why that matters

For example, this test is named:

test('PipInstallCommand executes without error', async () => {
    const command = new PipInstallCommand(...);

    await assert.doesNotReject(() =>
        command.execute({ packages: [{ packageName: 'package' }] }),
    );
});

The real implementation should invoke something equivalent to:

python -m pip install package

But all of the following broken implementations would still pass:

async execute(): Promise<void> {
    // Do nothing.
}

async execute(): Promise<void> {
    await runPython('python', ['-m', 'pip', 'uninstall', 'package']);
}

async execute(): Promise<void> {
    await runPython('python', ['incorrect-command']);
}

Because runPython is stubbed to resolve successfully and the test never inspects its arguments, a completely wrong command looks identical to a correct one.

Concrete regressions these tests would miss

Pip and UV

The tests would not detect:

• Pip using uninstall instead of install.
• Pip omitting -m pip.
• UV omitting --python <interpreter>.
• UV using the wrong Python environment.
• Upgrade requests omitting --upgrade.
• Package versions being dropped.
• Editable-install arguments being incorrectly transformed.
• Uninstall omitting Pip’s noninteractive -y option.

For example, UvInstallCommand is expected to construct:

uv pip install --python python package

The test does not assert any part of that command.

Conda

The tests would not detect:

• Installing into the wrong environment.
• Omitting --prefix environment.
• Omitting --yes and causing an interactive hang.
• Using install instead of remove.
• Incorrect formatting of versioned packages.
• Upgrade operations omitting --update-all.

The environment path is particularly important:

condaEnvironmentPath: 'environment'

The test supplies it but never confirms it reaches the command runner. The implementation could silently install packages into Conda’s currently active environment and the test would still pass.

Poetry

The tests would not detect:

• Running Poetry in the wrong working directory.
• Omitting add, remove, show, or --top-level.
• Formatting package@version incorrectly.
• Dropping the project’s cwd.
• Returning the wrong package names.

Poetry operates against a project’s pyproject.toml, so the working directory is behaviorally important. This test:

new PoetryAddCommand({
    pythonExecutable: 'poetry',
    cwd: 'project',
    log: mockLog,
});

never verifies that 'project' is passed to runPoetry. If it were omitted, Poetry could modify a different project or fail to find pyproject.toml.

Recommended improvement

At minimum, each mutation command should verify the runner invocation.

For example:

await command.execute({
    packages: [{ packageName: 'package' }],
});

assert.ok(
    runPythonStub.calledOnceWithExactly(
        'python',
        ['-m', 'pip', 'install', 'package'],
        undefined,
        mockLog,
        undefined,
        sinon.match.number,
    ),
);

The exact assertion should match the helper’s contract, but the important point is to verify the executable, arguments, environment/cwd, cancellation token, and timeout.

[eleanorjboyd]

The added parsing assertions are a useful improvement. I left one inline comment on the remaining command-construction coverage gap and the inaccurate integration-test claim; this is the main issue I would address before treating these as meaningful command tests.

[eleanorjboyd]

This integration-coverage claim is not currently true. packageManagement.integration.test.ts only selects venv environments for install/uninstall, so it never exercises the Conda or Poetry command classes; that path also dispatches to Pip or UV dynamically, so it does not deterministically cover either concrete command string. Because these unit tests do not inspect the runner invocation either, subcommand, flag, interpreter, environment-prefix, and cwd regressions remain uncovered. Please assert the runner arguments in these suites (including the analogous Conda/Poetry tests), or remove this claim until deterministic integration coverage exists.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/test/managers/builtin/commands.unit.test.ts:66

• PR description says these command tests intentionally do not assert parsed values, but this test asserts parsing/filtering behavior (prerelease filtering). Either update the PR description to match the actual coverage, or reduce these to smoke-only tests that just ensure execute() resolves.

    test('PipAvailableVersionsCommand parses versions and filters prereleases', async () => {
        runPythonStub.resolves(JSON.stringify({ versions: ['2.0.0rc1', '1.0.0'] }));
        const command = new PipAvailableVersionsCommand({ pythonExecutable: 'python', log: mockLog });

        const result = await command.execute({
            packageName: 'package',
            pythonVersion: '3.13.1',
            includePrerelease: false,
        });

        assert.deepStrictEqual(result.map((v) => v.public), ['1.0.0']);

src/test/managers/poetry/commands.unit.test.ts:49

• PR description says these smoke tests "intentionally do not assert ... parsed values", but this test asserts parsing behavior (and the test title explicitly states it parses fields). Either update the PR description/scope accordingly, or adjust this to a pure smoke test (only assert execute() resolves).

    test('PoetryShowCommand parses name, version and description', async () => {
        runPoetryStub.resolves(['requests 2.31.0 Python HTTP for Humans.', ''].join('\n'));
        const command = new PoetryShowCommand({ pythonExecutable: 'poetry', cwd: 'project', log: mockLog });

        const result = await command.execute();

src/test/managers/conda/commands.unit.test.ts:57

• PR description states the new tests "verify only that each command executes without throwing or rejecting" and do not assert parsed values, but this test asserts parsed output (versions list). Consider either updating the PR description to reflect the added parser coverage, or converting this to a smoke-only doesNotReject test.

    test('CondaAvailableVersionsCommand parses versions keyed by package name', async () => {
        runCondaStub.resolves(JSON.stringify({ package: [{ version: '1.0.0' }, { version: '2.0.0' }] }));
        const command = new CondaAvailableVersionsCommand({ pythonExecutable: 'conda', log: mockLog });

        const result = await command.execute({ packageName: 'package', pythonVersion: '' });

        assert.deepStrictEqual(result.map((v) => v.public), ['1.0.0', '2.0.0']);

[edvilme]

The added parsing assertions are a useful improvement. I left one inline comment on the remaining command-construction coverage gap and the inaccurate integration-test claim; this is the main issue I would address before treating these as meaningful command tests.

I think you are right. I will try changing the order of these PRs. I think it would be better to have the proper integration first and then perform actual roundtrip integration tests; maybe even in the same PR.