Skip to content

Commit

Permalink
Refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
karthiknadig committed Aug 17, 2023
1 parent 94f04b3 commit eb9569a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as fs from 'fs-extra';
import * as path from 'path';
import { WorkspaceFolder } from 'vscode';
import { traceError, traceInfo } from '../../../logging';
import { getVenvPath, showErrorMessageWithLogs } from '../common/commonUtils';
import { showErrorMessageWithLogs } from '../common/commonUtils';
import { CreateEnv } from '../../../common/utils/localize';
import { sleep } from '../../../common/utils/async';
import { switchSelectedPython } from './venvSwitchPython';
Expand Down Expand Up @@ -40,59 +40,59 @@ async function tryDeleteDir(dir: string): Promise<boolean> {
}
}

export async function deleteEnvironmentNonWindows(workspaceFolder: WorkspaceFolder): Promise<boolean> {
const venvPath = getVenvPath(workspaceFolder);
if (await tryDeleteDir(venvPath)) {
traceInfo(`Deleted venv dir: ${venvPath}`);
export async function deleteEnvironmentNonWindows(envDir: string): Promise<boolean> {
const name = path.basename(envDir);
if (await tryDeleteDir(envDir)) {
traceInfo(`Deleted "${name}" dir: ${envDir}`);
return true;
}
showErrorMessageWithLogs(CreateEnv.Venv.errorDeletingEnvironment);
return false;
}

export async function deleteEnvironmentWindows(
envDir: string,
envPythonBin: string,
workspaceFolder: WorkspaceFolder,
interpreter: string | undefined,
): Promise<boolean> {
const venvPath = getVenvPath(workspaceFolder);
const venvPythonPath = path.join(venvPath, 'Scripts', 'python.exe');

if (await tryDeleteFile(venvPythonPath)) {
traceInfo(`Deleted python executable: ${venvPythonPath}`);
if (await tryDeleteDir(venvPath)) {
traceInfo(`Deleted ".venv" dir: ${venvPath}`);
const name = path.basename(envDir);
if (await tryDeleteFile(envPythonBin)) {
traceInfo(`Deleted python executable: ${envPythonBin}`);
if (await tryDeleteDir(envDir)) {
traceInfo(`Deleted "${name}" dir: ${envDir}`);
return true;
}

traceError(`Failed to delete ".venv" dir: ${venvPath}`);
traceError(`Failed to delete "${name}" dir: ${envDir}`);
traceError(
'This happens if the virtual environment is still in use, or some binary in the venv is still running.',
'This happens if the virtual environment is still in use, or some binary in the "${name}" is still running.',
);
traceError(`Please delete the ".venv" manually: [${venvPath}]`);
traceError(`Please delete the "${name}" manually: [${envDir}]`);
showErrorMessageWithLogs(CreateEnv.Venv.errorDeletingEnvironment);
return false;
}
traceError(`Failed to delete python executable: ${venvPythonPath}`);
traceError(`Failed to delete python executable: ${envPythonBin}`);
traceError('This happens if the virtual environment is still in use.');

if (interpreter) {
traceError('We will attempt to switch python temporarily to delete the ".venv"');
traceError('We will attempt to switch python temporarily to delete the "${name}"');

await switchSelectedPython(interpreter, workspaceFolder.uri, 'temporarily to delete the ".venv"');
await switchSelectedPython(interpreter, workspaceFolder.uri, 'temporarily to delete the "${name}"');

traceInfo(`Attempting to delete ".venv" again: ${venvPath}`);
traceInfo(`Attempting to delete "${name}" again: ${envDir}`);
const ms = 500;
for (let i = 0; i < 5; i = i + 1) {
traceInfo(`Waiting for ${ms}ms to let processes exit, before a delete attempt.`);
await sleep(ms);
if (await tryDeleteDir(venvPath)) {
traceInfo(`Deleted ".venv" dir: ${venvPath}`);
if (await tryDeleteDir(envDir)) {
traceInfo(`Deleted "${name}" dir: ${envDir}`);
return true;
}
traceError(`Failed to delete ".venv" dir [${venvPath}] (attempt ${i + 1}/5).`);
traceError(`Failed to delete "${name}" dir [${envDir}] (attempt ${i + 1}/5).`);
}
} else {
traceError(`Please delete the ".venv" dir manually: [${venvPath}] manually.`);
traceError(`Please delete the "${name}" dir manually: [${envDir}] manually.`);
}
showErrorMessageWithLogs(CreateEnv.Venv.errorDeletingEnvironment);
return false;
Expand Down
7 changes: 4 additions & 3 deletions src/client/pythonEnvironments/creation/provider/venvUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { traceError, traceVerbose } from '../../../logging';
import { Commands } from '../../../common/constants';
import { isWindows } from '../../../common/platform/platformService';
import { getVenvPath, hasVenv } from '../common/commonUtils';
import { deleteEnvironmentNonWindows, deleteEnvironmentWindows } from './venvDeleteUtils';
import { deleteEnvironmentNonWindows, deleteEnvironmentWindows } from './envDeleteUtils';
import { sendTelemetryEvent } from '../../../telemetry';
import { EventName } from '../../../telemetry/constants';

Expand Down Expand Up @@ -248,9 +248,10 @@ async function deleteEnvironment(workspaceFolder: WorkspaceFolder, interpreter:
},
async () => {
if (isWindows()) {
return deleteEnvironmentWindows(workspaceFolder, interpreter);
const venvPythonPath = path.join(venvPath, 'Scripts', 'python.exe');
return deleteEnvironmentWindows(venvPath, venvPythonPath, workspaceFolder, interpreter);
}
return deleteEnvironmentNonWindows(workspaceFolder);
return deleteEnvironmentNonWindows(venvPath);
},
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as commonUtils from '../../../../client/pythonEnvironments/creation/com
import {
deleteEnvironmentNonWindows,
deleteEnvironmentWindows,
} from '../../../../client/pythonEnvironments/creation/provider/venvDeleteUtils';
} from '../../../../client/pythonEnvironments/creation/provider/envDeleteUtils';
import * as switchPython from '../../../../client/pythonEnvironments/creation/provider/venvSwitchPython';
import * as asyncApi from '../../../../client/common/utils/async';

Expand All @@ -27,6 +27,8 @@ suite('Test Delete environments (windows)', () => {
name: 'workspace1',
index: 0,
};
const venvPath = path.join(workspace1.uri.fsPath, 'venv');
const venvPythonPath = path.join(venvPath, 'scripts', 'python.exe');

setup(() => {
pathExistsStub = sinon.stub(fs, 'pathExists');
Expand All @@ -52,7 +54,7 @@ suite('Test Delete environments (windows)', () => {
test('Delete venv folder succeeded', async () => {
rmdirStub.resolves();
unlinkStub.resolves();
assert.ok(await deleteEnvironmentWindows(workspace1, 'python.exe'));
assert.ok(await deleteEnvironmentWindows(venvPath, venvPythonPath, workspace1, 'python.exe'));

assert.ok(rmdirStub.calledOnce);
assert.ok(unlinkStub.calledOnce);
Expand All @@ -62,7 +64,7 @@ suite('Test Delete environments (windows)', () => {
test('Delete python.exe succeeded but venv dir failed', async () => {
rmdirStub.rejects();
unlinkStub.resolves();
assert.notOk(await deleteEnvironmentWindows(workspace1, 'python.exe'));
assert.notOk(await deleteEnvironmentWindows(venvPath, venvPythonPath, workspace1, 'python.exe'));

assert.ok(rmdirStub.calledOnce);
assert.ok(unlinkStub.calledOnce);
Expand All @@ -72,7 +74,7 @@ suite('Test Delete environments (windows)', () => {
test('Delete python.exe failed first attempt', async () => {
unlinkStub.rejects();
rmdirStub.resolves();
assert.ok(await deleteEnvironmentWindows(workspace1, 'python.exe'));
assert.ok(await deleteEnvironmentWindows(venvPath, venvPythonPath, workspace1, 'python.exe'));

assert.ok(rmdirStub.calledOnce);
assert.ok(switchPythonStub.calledOnce);
Expand All @@ -82,15 +84,15 @@ suite('Test Delete environments (windows)', () => {
test('Delete python.exe failed all attempts', async () => {
unlinkStub.rejects();
rmdirStub.rejects();
assert.notOk(await deleteEnvironmentWindows(workspace1, 'python.exe'));
assert.notOk(await deleteEnvironmentWindows(venvPath, venvPythonPath, workspace1, 'python.exe'));
assert.ok(switchPythonStub.calledOnce);
assert.ok(showErrorMessageWithLogsStub.calledOnce);
});

test('Delete python.exe failed no interpreter', async () => {
unlinkStub.rejects();
rmdirStub.rejects();
assert.notOk(await deleteEnvironmentWindows(workspace1, undefined));
assert.notOk(await deleteEnvironmentWindows(venvPath, venvPythonPath, workspace1, undefined));
assert.ok(switchPythonStub.notCalled);
assert.ok(showErrorMessageWithLogsStub.calledOnce);
});
Expand All @@ -106,6 +108,7 @@ suite('Test Delete environments (linux/mac)', () => {
name: 'workspace1',
index: 0,
};
const venvPath = path.join(workspace1.uri.fsPath, 'venv');

setup(() => {
pathExistsStub = sinon.stub(fs, 'pathExists');
Expand All @@ -123,7 +126,7 @@ suite('Test Delete environments (linux/mac)', () => {
pathExistsStub.resolves(true);
rmdirStub.resolves();

assert.ok(await deleteEnvironmentNonWindows(workspace1));
assert.ok(await deleteEnvironmentNonWindows(venvPath));

assert.ok(pathExistsStub.calledOnce);
assert.ok(rmdirStub.calledOnce);
Expand All @@ -133,7 +136,7 @@ suite('Test Delete environments (linux/mac)', () => {
test('Delete venv folder failed', async () => {
pathExistsStub.resolves(true);
rmdirStub.rejects();
assert.notOk(await deleteEnvironmentNonWindows(workspace1));
assert.notOk(await deleteEnvironmentNonWindows(venvPath));

assert.ok(pathExistsStub.calledOnce);
assert.ok(rmdirStub.calledOnce);
Expand Down

0 comments on commit eb9569a

Please sign in to comment.