Skip to content

Commit efe2f9c

Browse files
edvilmeedvilme
committed
Enforce headless package manager behavior
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
1 parent df6d1a2 commit efe2f9c

6 files changed

Lines changed: 221 additions & 82 deletions

File tree

‎.github/workflows/package-manager-network-check.yml‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
name: Package Manager Network Check
2+
3+
on:
4+
workflow_dispatch:
5+
6+
permissions:
7+
contents: read
8+
9+
env:
10+
NODE_VERSION: '22.21.1'
11+
12+
jobs:
13+
package-manager-network-tests:
14+
name: Package Manager Network Tests
15+
runs-on: ubuntu-latest
16+
timeout-minutes: 30
17+
18+
steps:
19+
- name: Checkout
20+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
21+
22+
- name: Checkout Python Environment Tools
23+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
24+
with:
25+
repository: 'microsoft/python-environment-tools'
26+
path: 'python-env-tools-src'
27+
sparse-checkout: |
28+
crates
29+
Cargo.toml
30+
Cargo.lock
31+
sparse-checkout-cone-mode: false
32+
33+
- name: Install Rust Toolchain
34+
uses: dtolnay/rust-toolchain@stable
35+
36+
- name: Cache Rust build
37+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
38+
with:
39+
path: |
40+
~/.cargo/registry
41+
~/.cargo/git
42+
python-env-tools-src/target
43+
key: ${{ runner.os }}-cargo-pet-${{ hashFiles('python-env-tools-src/Cargo.lock') }}
44+
restore-keys: |
45+
${{ runner.os }}-cargo-pet-
46+
47+
- name: Build Python Environment Tools
48+
run: cargo build --release --package pet
49+
working-directory: python-env-tools-src
50+
51+
- name: Copy pet binary
52+
run: |
53+
mkdir -p python-env-tools/bin
54+
cp python-env-tools-src/target/release/pet python-env-tools/bin/
55+
chmod +x python-env-tools/bin/pet
56+
57+
- name: Install Node
58+
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
59+
with:
60+
node-version: ${{ env.NODE_VERSION }}
61+
cache: 'npm'
62+
63+
- name: Install Python
64+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
65+
with:
66+
python-version: '3.12'
67+
68+
- name: Install Dependencies
69+
run: npm ci
70+
71+
- name: Compile Extension
72+
run: npm run compile
73+
74+
- name: Compile Tests
75+
run: npm run compile-tests
76+
77+
- name: Run Package Manager Network Integration Tests
78+
uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6
79+
env:
80+
VSC_PYTHON_PACKAGE_NETWORK_TEST: '1'
81+
with:
82+
run: npm run integration-test -- --grep "Package Manager"

‎src/managers/builtin/pipPackageManager.ts‎

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Pep440Version } from '@renovatebot/pep440';
22
import { compare, explain as parse, rcompare } from '@renovatebot/pep440';
33
import {
44
CancellationError,
5+
CancellationToken,
56
Disposable,
67
Event,
78
EventEmitter,
@@ -74,40 +75,47 @@ export class PipPackageManager implements PackageManager, Disposable {
7475
install: toInstall,
7576
uninstall: toUninstall,
7677
};
78+
const execute = async (token?: CancellationToken): Promise<void> => {
79+
try {
80+
await managePackages(environment, manageOptions, this, token);
81+
await updatePackagesAndNotify(
82+
this,
83+
environment,
84+
this.packages.get(environment.envId.id),
85+
(changes) => {
86+
this._onDidChangePackages.fire({ environment, manager: this, changes });
87+
},
88+
() => this.fetchPackages(environment, !manageOptions.runHeadless),
89+
);
90+
} catch (e) {
91+
if (e instanceof CancellationError) {
92+
throw e;
93+
}
94+
this.log.error('Error managing packages', e);
95+
if (!manageOptions.runHeadless) {
96+
setImmediate(async () => {
97+
const result = await showErrorMessage('Error managing packages', 'View Output');
98+
if (result === 'View Output') {
99+
this.log.show();
100+
}
101+
});
102+
}
103+
throw e;
104+
}
105+
};
106+
107+
if (manageOptions.runHeadless) {
108+
await execute();
109+
return;
110+
}
111+
77112
await withProgress(
78113
{
79114
location: ProgressLocation.Notification,
80115
title: 'Installing packages',
81116
cancellable: true,
82117
},
83-
async (_progress, token) => {
84-
try {
85-
await managePackages(environment, manageOptions, this, token);
86-
await updatePackagesAndNotify(
87-
this,
88-
environment,
89-
this.packages.get(environment.envId.id),
90-
(changes) => {
91-
this._onDidChangePackages.fire({ environment, manager: this, changes });
92-
},
93-
() => this.fetchPackages(environment, !manageOptions.runHeadless),
94-
);
95-
} catch (e) {
96-
if (e instanceof CancellationError) {
97-
throw e;
98-
}
99-
this.log.error('Error managing packages', e);
100-
if (!manageOptions.runHeadless) {
101-
setImmediate(async () => {
102-
const result = await showErrorMessage('Error managing packages', 'View Output');
103-
if (result === 'View Output') {
104-
this.log.show();
105-
}
106-
});
107-
}
108-
throw e;
109-
}
110-
},
118+
async (_progress, token) => execute(token),
111119
);
112120
}
113121

‎src/managers/conda/condaPackageManager.ts‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { explain as parse, rcompare } from '@renovatebot/pep440';
33
import * as path from 'path';
44
import {
55
CancellationError,
6+
CancellationToken,
67
Disposable,
78
Event,
89
EventEmitter,
@@ -72,37 +73,44 @@ export class CondaPackageManager implements PackageManager, Disposable {
7273
install: toInstall,
7374
uninstall: toUninstall,
7475
};
76+
const execute = async (token?: CancellationToken): Promise<void> => {
77+
try {
78+
await managePackages(environment, manageOptions, token, this.log);
79+
await updatePackagesAndNotify(
80+
this,
81+
environment,
82+
this.packages.get(environment.envId.id),
83+
(changes) => {
84+
this._onDidChangePackages.fire({ environment, manager: this, changes });
85+
},
86+
);
87+
} catch (e) {
88+
if (e instanceof CancellationError) {
89+
throw e;
90+
}
91+
92+
this.log.error('Error installing packages', e);
93+
if (!manageOptions.runHeadless) {
94+
setImmediate(async () => {
95+
await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log);
96+
});
97+
}
98+
throw e;
99+
}
100+
};
101+
102+
if (manageOptions.runHeadless) {
103+
await execute();
104+
return;
105+
}
106+
75107
await withProgress(
76108
{
77109
location: ProgressLocation.Notification,
78110
title: CondaStrings.condaInstallingPackages,
79111
cancellable: true,
80112
},
81-
async (_progress, token) => {
82-
try {
83-
await managePackages(environment, manageOptions, token, this.log);
84-
await updatePackagesAndNotify(
85-
this,
86-
environment,
87-
this.packages.get(environment.envId.id),
88-
(changes) => {
89-
this._onDidChangePackages.fire({ environment, manager: this, changes });
90-
},
91-
);
92-
} catch (e) {
93-
if (e instanceof CancellationError) {
94-
throw e;
95-
}
96-
97-
this.log.error('Error installing packages', e);
98-
if (!manageOptions.runHeadless) {
99-
setImmediate(async () => {
100-
await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log);
101-
});
102-
}
103-
throw e;
104-
}
105-
},
113+
async (_progress, token) => execute(token),
106114
);
107115
}
108116

‎src/managers/conda/condaUtils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ export async function deleteCondaEnvironment(environment: PythonEnvironment, log
12531253
export async function managePackages(
12541254
environment: PythonEnvironment,
12551255
options: PackageManagementOptions,
1256-
token: CancellationToken,
1256+
token: CancellationToken | undefined,
12571257
log: LogOutputChannel,
12581258
): Promise<void> {
12591259
if (options.uninstall && options.uninstall.length > 0) {

‎src/managers/poetry/poetryPackageManager.ts‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -81,39 +81,46 @@ export class PoetryPackageManager implements PackageManager, Disposable {
8181
}
8282
}
8383

84+
const execute = async (token?: CancellationToken): Promise<void> => {
85+
try {
86+
await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, token);
87+
await updatePackagesAndNotify(
88+
this,
89+
environment,
90+
this.packages.get(environment.envId.id),
91+
(changes) => {
92+
this._onDidChangePackages.fire({ environment, manager: this, changes });
93+
},
94+
);
95+
} catch (e) {
96+
if (e instanceof CancellationError) {
97+
throw e;
98+
}
99+
this.log.error('Error managing packages with Poetry', e);
100+
if (!options.runHeadless) {
101+
setImmediate(async () => {
102+
const result = await showErrorMessage('Error managing packages with Poetry', 'View Output');
103+
if (result === 'View Output') {
104+
this.log.show();
105+
}
106+
});
107+
}
108+
throw e;
109+
}
110+
};
111+
112+
if (options.runHeadless) {
113+
await execute();
114+
return;
115+
}
116+
84117
await withProgress(
85118
{
86119
location: ProgressLocation.Notification,
87120
title: 'Managing packages with Poetry',
88121
cancellable: true,
89122
},
90-
async (_progress, token) => {
91-
try {
92-
await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, token);
93-
await updatePackagesAndNotify(
94-
this,
95-
environment,
96-
this.packages.get(environment.envId.id),
97-
(changes) => {
98-
this._onDidChangePackages.fire({ environment, manager: this, changes });
99-
},
100-
);
101-
} catch (e) {
102-
if (e instanceof CancellationError) {
103-
throw e;
104-
}
105-
this.log.error('Error managing packages with Poetry', e);
106-
if (!options.runHeadless) {
107-
setImmediate(async () => {
108-
const result = await showErrorMessage('Error managing packages with Poetry', 'View Output');
109-
if (result === 'View Output') {
110-
this.log.show();
111-
}
112-
});
113-
}
114-
throw e;
115-
}
116-
},
123+
async (_progress, token) => execute(token),
117124
);
118125
}
119126

‎src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ suite('Package manager headless conformance', () => {
5757

5858
test('rejects failures without showing error notifications', async () => {
5959
const operationError = new Error('package operation failed');
60-
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never));
60+
const withProgress = sinon.stub(windowApis, 'withProgress');
6161
sinon.stub(builtinUtils, 'managePackages').rejects(operationError);
6262
sinon.stub(condaUtils, 'managePackages').rejects(operationError);
6363
sinon.stub(poetryUtils, 'getPoetry').resolves(undefined);
@@ -70,6 +70,40 @@ suite('Package manager headless conformance', () => {
7070
);
7171
}
7272

73+
assert.ok(withProgress.notCalled);
74+
assert.ok(showErrorMessage.notCalled);
75+
assert.ok(showErrorMessageWithLogs.notCalled);
76+
});
77+
78+
test('rejects refresh failures without showing progress or error notifications', async () => {
79+
const refreshError = new Error('package refresh failed');
80+
const withProgress = sinon.stub(windowApis, 'withProgress');
81+
sinon.stub(builtinUtils, 'managePackages').resolves();
82+
sinon.stub(condaUtils, 'managePackages').resolves();
83+
sinon
84+
.stub(
85+
PoetryPackageManager.prototype as unknown as {
86+
runPoetryManage: () => Promise<void>;
87+
},
88+
'runPoetryManage',
89+
)
90+
.resolves();
91+
sinon.stub(builtinUtils, 'refreshPipPackages').rejects(refreshError);
92+
sinon.stub(CondaPackageManager.prototype, 'getPackages').rejects(refreshError);
93+
sinon.stub(PoetryPackageManager.prototype, 'getPackages').rejects(refreshError);
94+
sinon.stub(PipPackageManager.prototype, 'getDirectPackageNames').resolves(undefined);
95+
sinon.stub(PoetryPackageManager.prototype, 'getDirectPackageNames').resolves(undefined);
96+
const showErrorMessage = sinon.stub(windowApis, 'showErrorMessage').resolves(undefined);
97+
const showErrorMessageWithLogs = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves();
98+
99+
for (const manager of createManagers()) {
100+
await assert.rejects(
101+
manager.manage(environment, { install: ['requests'], runHeadless: true }),
102+
(error: unknown) => error === refreshError,
103+
);
104+
}
105+
106+
assert.ok(withProgress.notCalled);
73107
assert.ok(showErrorMessage.notCalled);
74108
assert.ok(showErrorMessageWithLogs.notCalled);
75109
});

0 commit comments

Comments
 (0)