chore: clean-up

Author: karthiknadig

src/managers/builtin/venvUtils.ts

@@ -422,39 +422,24 @@ export async function createPythonVenv(
     return createStepBasedVenvFlow(nativeFinder, api, log, manager, basePythons, venvRoot, options);
 }
 
-/**
- * Checks if a path is a drive root (e.g., "C:\" on Windows or "/" on Unix).
- */
 function isDriveRoot(fsPath: string): boolean {
     const normalized = path.normalize(fsPath);
     if (os.platform() === 'win32') {
-        // Windows drive root: "C:\" or "C:"
         return /^[a-zA-Z]:[\\/]?$/.test(normalized);
     }
-    // Unix root
     return normalized === '/';
 }
 
-/**
- * Checks if a path has a minimum number of directory components to be considered safe.
- * This helps prevent accidental deletion of high-level directories.
- */
 function hasMinimumPathDepth(fsPath: string, minDepth: number = 2): boolean {
     const normalized = path.normalize(fsPath);
     const parts = normalized.split(path.sep).filter((p) => p.length > 0 && p !== '.');
 
     if (os.platform() === 'win32') {
-        // On Windows, the drive letter counts as one component
-        // e.g., "C:\folder" has parts ["C:", "folder"] -> depth 2
         return parts.length >= minDepth;
     }
-    // On Unix, "/home/user" has parts ["home", "user"] -> depth 2
     return parts.length >= minDepth;
 }
 
-/**
- * Validates that a path appears to be a virtual environment by checking for pyvenv.cfg.
- */
 async function isValidVenvPath(fsPath: string): Promise<boolean> {
     try {
         const pyvenvCfgPath = path.join(fsPath, 'pyvenv.cfg');
@@ -464,24 +449,17 @@ async function isValidVenvPath(fsPath: string): Promise<boolean> {
     }
 }
 
-/**
- * Validates that a path is safe to remove as a virtual environment.
- * Returns an error message if unsafe, or undefined if safe.
- */
 async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel): Promise<string | undefined> {
-    // Check if it's a drive root
     if (isDriveRoot(envPath)) {
         log.error(`Refusing to remove drive root: ${envPath}`);
         return VenvManagerStrings.venvRemoveUnsafePath;
     }
 
-    // Check minimum path depth (at least 2 components to be safe)
     if (!hasMinimumPathDepth(envPath, 2)) {
         log.error(`Refusing to remove path with insufficient depth: ${envPath}`);
         return VenvManagerStrings.venvRemoveUnsafePath;
     }
 
-    // Check if it's actually a venv (has pyvenv.cfg)
     if (!(await isValidVenvPath(envPath))) {
         log.error(`Path does not appear to be a valid venv (no pyvenv.cfg): ${envPath}`);
         return VenvManagerStrings.venvRemoveInvalidPath;
@@ -493,11 +471,9 @@ async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel):
 export async function removeVenv(environment: PythonEnvironment, log: LogOutputChannel): Promise<boolean> {
     const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';
 
-    // Normalize path separators before checking endsWith
     const envFsPath = path.normalize(environment.environmentPath.fsPath);
     const envPath = envFsPath.endsWith(pythonPath) ? path.dirname(path.dirname(envFsPath)) : envFsPath;
 
-    // Safety validation before proceeding
     const validationError = await validateVenvRemovalPath(envPath, log);
     if (validationError) {
         showErrorMessage(validationError);

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

@@ -2,10 +2,6 @@ import * as assert from 'assert';
 import * as os from 'os';
 import * as path from 'path';
 
-// We need to test the helper functions that are now part of venvUtils
-// Since they are not exported, we'll test the behavior through removeVenv
-// For unit testing the validation logic, we can re-implement the pure logic tests here
-
 suite('venvUtils Path Validation', () => {
     suite('isDriveRoot behavior', () => {
         test('should identify Windows drive roots correctly', function () {
@@ -14,9 +10,6 @@ suite('venvUtils Path Validation', () => {
                 return;
             }
 
-            // Test cases that represent drive roots
-            // Note: path.normalize('C:') returns 'C:.' on Windows, so we exclude bare 'C:'
-            // The actual drive roots we care about are 'C:\' style paths
             const driveRoots = ['C:\\', 'D:\\', 'c:\\', 'C:/'];
 
             for (const root of driveRoots) {
@@ -100,10 +93,8 @@ suite('venvUtils Path Validation', () => {
 
     suite('Path normalization in removeVenv', () => {
         test('should normalize path separators before checking python.exe suffix', () => {
-            // This test verifies that paths with mixed separators are handled correctly
             const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';
 
-            // Example of a path that might have mixed separators on Windows
             const mixedPath =
                 os.platform() === 'win32' ? 'C:/Users/test/.venv/Scripts/python.exe' : '/home/user/.venv/bin/python';
 
@@ -112,7 +103,6 @@ suite('venvUtils Path Validation', () => {
 
             assert.strictEqual(endsWithPython, true, 'Normalized path should end with python executable');
 
-            // Verify the resulting env path after going up two directories
             const envPath = path.dirname(path.dirname(normalizedPath));
             const expectedEnvPath =
                 os.platform() === 'win32' ? path.normalize('C:/Users/test/.venv') : '/home/user/.venv';
@@ -123,7 +113,6 @@ suite('venvUtils Path Validation', () => {
         test('should correctly derive venv path from python executable path', () => {
             const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';
 
-            // Simulate what removeVenv does
             const testPaths =
                 os.platform() === 'win32'
                     ? [
@@ -151,7 +140,6 @@ suite('venvUtils removeVenv validation integration', () => {
 
         const expectedCfgPath = path.join(testEnvPath, 'pyvenv.cfg');
 
-        // Just verify the path is constructed correctly
         assert.strictEqual(
             expectedCfgPath,
             os.platform() === 'win32' ? 'C:\\Users\\test\\.venv\\pyvenv.cfg' : '/home/user/.venv/pyvenv.cfg',