Skip to content

Commit 79cd066

Browse files
Retry transient virtual environment removal failures
Retry recoverable filesystem errors inside the validated deletion operation without weakening integration assertions. Preserve error reporting and environment tracking on final failure, with deterministic regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent fbb46cf commit 79cd066

2 files changed

Lines changed: 41 additions & 4 deletions

File tree

‎src/managers/builtin/venvUtils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,7 +825,7 @@ export async function removeVenv(
825825
},
826826
async () => {
827827
try {
828-
await fsapi.remove(envPath);
828+
await fsapi.rm(envPath, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
829829
await backends.uv.untrackEnvironment(environment.environmentPath.fsPath);
830830
await backends.venv.untrackEnvironment(environment.environmentPath.fsPath);
831831
return true;

‎src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import * as assert from 'assert';
2-
import * as fse from 'fs-extra';
2+
import fse from 'fs-extra';
33
import * as os from 'os';
44
import * as path from 'path';
55
import * as sinon from 'sinon';
6+
import { Uri } from 'vscode';
67
import { Common } from '../../../common/localize';
78
import * as windowApis from '../../../common/window.apis';
89
import * as uvEnvironments from '../../../managers/builtin/uvEnvironments';
@@ -155,10 +156,11 @@ suite('venvUtils removeVenv validation integration', () => {
155156
);
156157
});
157158

158-
test('headless removal skips confirmation and removes the environment', async () => {
159+
test('headless removal skips confirmation and removes the environment with bounded filesystem retries', async () => {
159160
const tempRoot = await fse.mkdtemp(path.join(os.tmpdir(), 'remove-venv-'));
160161
const envPath = path.join(tempRoot, '.venv');
161162
await fse.outputFile(path.join(envPath, 'pyvenv.cfg'), 'home = base');
163+
const rmSpy = sinon.spy(fse, 'rm');
162164
const showWarningMessageStub = sinon.stub(windowApis, 'showWarningMessage');
163165
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never));
164166
sinon.stub(uvEnvironments, 'removeUvEnvironment').resolves();
@@ -172,10 +174,45 @@ suite('venvUtils removeVenv validation integration', () => {
172174

173175
assert.strictEqual(removed, true);
174176
assert.strictEqual(showWarningMessageStub.callCount, 0);
177+
sinon.assert.calledOnce(rmSpy);
178+
const [removedPath, rmOptions] = rmSpy.firstCall.args;
179+
assert.ok(typeof removedPath === 'string');
180+
assert.strictEqual(Uri.file(removedPath).fsPath, Uri.file(envPath).fsPath);
181+
assert.deepStrictEqual(rmOptions, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
175182
assert.strictEqual(await fse.pathExists(envPath), false);
176183
} finally {
177184
sinon.restore();
178-
await fse.remove(tempRoot);
185+
await fse.rm(tempRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
186+
}
187+
});
188+
189+
test('failed filesystem removal reports an error without untracking the environment', async () => {
190+
const tempRoot = await fse.mkdtemp(path.join(os.tmpdir(), 'remove-venv-failed-'));
191+
const envPath = path.join(tempRoot, '.venv');
192+
await fse.outputFile(path.join(envPath, 'pyvenv.cfg'), 'home = base');
193+
const error = Object.assign(new Error('Environment executable is still locked'), { code: 'EPERM' });
194+
const rmStub = sinon.stub(fse, 'rm').rejects(error);
195+
const untrackStub = sinon.stub(uvEnvironments, 'removeUvEnvironment').resolves();
196+
const showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage').resolves(undefined);
197+
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never));
198+
const log = createMockLogOutputChannel();
199+
200+
try {
201+
const removed = await removeVenv(
202+
createMockPythonEnvironment({ name: '.venv', envPath }),
203+
log,
204+
{ runHeadless: true },
205+
);
206+
207+
assert.strictEqual(removed, false);
208+
sinon.assert.calledOnce(rmStub);
209+
sinon.assert.notCalled(untrackStub);
210+
sinon.assert.calledOnce(log.error as sinon.SinonStub);
211+
sinon.assert.calledOnce(showErrorMessageStub);
212+
assert.strictEqual(await fse.pathExists(envPath), true);
213+
} finally {
214+
sinon.restore();
215+
await fse.rm(tempRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
179216
}
180217
});
181218

0 commit comments

Comments
 (0)