Skip to content

test: add cross-platform package manager lifecycle coverage - #1717

Open
Eduardo Villalpando Mello (edvilme) wants to merge 20 commits into
mainfrom
package-manager-integration-coverage-feedback
Open

test: add cross-platform package manager lifecycle coverage#1717
Eduardo Villalpando Mello (edvilme) wants to merge 20 commits into
mainfrom
package-manager-integration-coverage-feedback

Conversation

@edvilme

@edvilme Eduardo Villalpando Mello (edvilme) commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #1704.

Summary

  • Adds shared conformance coverage for Pip, Conda, and Poetry headless package operations, ensuring failures reject without prompting or showing error UI.
  • Runs real Pip and Conda install, list, uninstall, and version-lookup coverage across the full OS/Python CI matrix.
  • Creates test-owned ephemeral venv and Conda environments in the integration suite, with ownership checks, bounded processes, and guaranteed cleanup; CI only provisions the required tools.
  • Distinguishes unsupported package version lookup with an internal PackageVersionLookupNotSupportedError. Supported profiles require a defined, non-empty result, while command, network, and parsing failures propagate.
  • Keeps Poetry lifecycle coverage explicitly deferred until CI has a controlled Poetry installation.

Validation

  • Lint, TypeScript compilation, and unit tests
  • Pip lifecycle and version lookup in a real VS Code integration host
  • Full GitHub Actions OS/Python matrix

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds unit-level “headless conformance” coverage to ensure package manager operations (Pip/Conda/Poetry) do not prompt for interactive input or display error UI when runHeadless: true, while still rejecting on failures. Also aligns Pip’s UI entry points with the existing VS Code wrapper APIs so they can be controlled/stubbed consistently in tests.

Changes:

  • Added a shared unit test suite that exercises headless package-management behavior across Pip, Conda, and Poetry.
  • Updated PipPackageManager to use withProgress / showErrorMessage from common/window.apis instead of vscode.window directly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts New conformance unit tests asserting headless runs do not prompt and do not show error UI, while failures still reject.
src/managers/builtin/pipPackageManager.ts Switched from vscode.window to window API wrappers for progress + error UI to support headless/test stubbing.
Suppressed comments (1)

src/managers/builtin/pipPackageManager.ts:82

  • New/updated user-facing strings in this hunk (progress titles like "Installing packages" and the error UI text/button) are not localized. The repo consistently localizes UI strings via l10n.t(...) or centralized string modules (e.g., CondaStrings from src/common/localize.ts). Please localize these strings so they’re translatable.
        await withProgress(
            {
                location: ProgressLocation.Notification,
                title: 'Installing packages',
                cancellable: true,
            },

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Eduardo Villalpando Mello (edvilme) added a commit that referenced this pull request Aug 18, 2026
Keep PR #1717 focused on deterministic headless package manager conformance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
@edvilme
Eduardo Villalpando Mello (edvilme) force-pushed the package-manager-integration-coverage-feedback branch from 9c1c12d to e1f3481 Compare August 18, 2026 22:46
Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@edvilme

Copy link
Copy Markdown
Contributor Author

CI diagnosis and fix pushed in df6d1a2. The opt-in live-network suite introduced by #1704 fails identically on #1704, current main, and this PR: Pip version lookup returns undefined, then Conda quick-create hangs until the shared 300s Mocha timeout. Because PR #1717's intended coverage is deterministic headless conformance, the fix removes only the automatic VSC_PYTHON_PACKAGE_NETWORK_TEST invocation from PR/push workflows; it retains the tests behind their explicit opt-in gate, regular integration profile coverage, and the new deterministic unit conformance. Local validation: lint, compile-tests, 1628 unit tests, workflow YAML/Prettier, and targeted Package Manager profile coverage integration test all pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

.github/workflows/pr-check.yml:337

  • The workflow no longer sets VSC_PYTHON_PACKAGE_NETWORK_TEST=1, so src/test/integration/packageManager.integration.test.ts will now always this.skip() (see line ~108) and the package-manager network integration suite won’t run in CI at all. If these tests are still intended to protect behavior, reintroduce a gated step that sets the env var (or remove/adjust the env guard and run them via an explicit job).
        if: runner.os == 'Linux'
        uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6
        with:
          run: npm run integration-test

      - name: Run Integration Tests (non-Linux)
        if: runner.os != 'Linux'
        run: npm run integration-test

.github/workflows/push-check.yml:336

  • The workflow no longer sets VSC_PYTHON_PACKAGE_NETWORK_TEST=1, so src/test/integration/packageManager.integration.test.ts will now always this.skip() (see line ~108) and the package-manager network integration suite won’t run on pushes either. If the suite is still required, add back a gated step that exports the env var (or migrate the tests away from env gating).

      - name: Run Integration Tests (Linux)
        if: runner.os == 'Linux'
        uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6
        with:
          run: npm run integration-test

      - name: Run Integration Tests (non-Linux)
        if: runner.os != 'Linux'

@StellaHuang95

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — Stella Huang (@StellaHuang95) is auto-reviewing this PR.

Comment thread .github/workflows/pr-check.yml
@StellaHuang95 Stella Huang (StellaHuang95) added the review-auto:changes-requested Automated review: posted blocking findings to address. label Aug 18, 2026
Bypass progress UI for headless Pip, Conda, and Poetry operations while preserving refresh and error propagation. Cover post-operation refresh failures and keep live network lifecycle tests available through a manual workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@StellaHuang95 Stella Huang (StellaHuang95) added review-auto:approved Automated review: no blocking findings (approval posted). and removed review-auto:changes-requested Automated review: posted blocking findings to address. labels Aug 19, 2026
Enable the guarded package-manager network suites in the existing integration step for every OS and Python version, and remove the separate manual workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Comment thread src/test/integration/packageManager.integration.test.ts
Comment thread src/test/integration/environmentFixture.ts Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Comment thread src/test/integration/packageManager.integration.test.ts Outdated
Comment thread src/test/integration/packageManager.integration.test.ts
Comment thread .github/workflows/pr-check.yml
Comment thread src/test/integration/environmentFixture.ts
Comment thread src/test/integration/environmentFixture.ts
@StellaHuang95 Stella Huang (StellaHuang95) added review-auto:changes-requested Automated review: posted blocking findings to address. and removed review-auto:approved Automated review: no blocking findings (approval posted). labels Aug 19, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Comment thread src/test/integration/packageManager.integration.test.ts
Comment thread src/test/integration/environmentFixture.ts
Comment thread src/test/integration/packageManager.integration.test.ts Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Comment thread src/test/integration/packageManager.integration.test.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
Comment thread src/features/pythonApi.ts
`No package manager supports version lookup for: ${context.envId.id}`,
);
}
return manager.getPackageAvailableVersions(context, packageName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue · Please address or respond

This changes the public API from resolving undefined to rejecting when no package manager supports lookup, breaking existing consumers despite the compatibility claim in api.ts. Preserve undefined at the existing API boundary or introduce the rejecting behavior through a versioned API.

[verified]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed in #1728

@edvilme Eduardo Villalpando Mello (edvilme) changed the title test: expand package manager integration coverage test: add cross-platform package manager lifecycle coverage Aug 19, 2026
Comment thread src/features/pythonApi.ts
throw new PackageVersionLookupNotSupportedError(
`No package manager supports version lookup for: ${context.envId.id}`,
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue · Please address or respond

This still changes the public contract from resolving undefined to rejecting when lookup is unavailable, so existing consumers can receive unexpected unhandled rejections. Preserve undefined at the public boundary, or formally update and export the public error contract and adjust all consumers; make the corresponding correction in InternalPackageManager.

[verified]

@StellaHuang95

Copy link
Copy Markdown
Contributor

The public package-version lookup API now rejects where it previously resolved undefined, creating a breaking consumer contract change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debt Code quality issues review-auto:changes-requested Automated review: posted blocking findings to address.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants