refactor: update resolveVirtualenvsPath to handle {cache-dir} placeholder and improve path resolution

Author: karthiknadig

src/managers/poetry/poetryUtils.ts

@@ -1,15 +1,13 @@
-import * as cp from 'child_process';
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import { promisify } from 'util';
 import { Uri } from 'vscode';
 import which from 'which';
 import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../api';
 import { execProcess } from '../../common/childProcess.apis';
 import { ENVS_EXTENSION_ID } from '../../common/constants';
 import { traceError, traceInfo } from '../../common/logging';
 import { getWorkspacePersistentState } from '../../common/persistentState';
-import { getUserHomeDir, untildify } from '../../common/utils/pathUtils';
+import { getUserHomeDir, normalizePath, untildify } from '../../common/utils/pathUtils';
 import { isMac, isWindows } from '../../common/utils/platformUtils';
 import { getSettingWorkspaceScope } from '../../features/settings/settingHelpers';
 import {
@@ -21,8 +19,6 @@ import {
 } from '../common/nativePythonFinder';
 import { getShellActivationCommands, shortVersion, sortEnvironments } from '../common/utils';
 
-const exec = promisify(cp.exec);
-
 /**
  * Checks if the POETRY_VIRTUALENVS_IN_PROJECT environment variable is set to a truthy value.
  * When true, Poetry creates virtualenvs in the project's `.venv` directory.
@@ -299,33 +295,36 @@ export function getDefaultPoetryVirtualenvsPath(): string | undefined {
  * First tries to query Poetry's cache-dir config, then falls back to platform-specific default.
  * @param poetry Path to the poetry executable
  * @param virtualenvsPath The path possibly containing {cache-dir} placeholder
+ * @returns The resolved path, or undefined if the placeholder cannot be resolved
  */
-async function resolveVirtualenvsPath(poetry: string, virtualenvsPath: string): Promise<string> {
+async function resolveVirtualenvsPath(poetry: string, virtualenvsPath: string): Promise<string | undefined> {
     if (!virtualenvsPath.includes('{cache-dir}')) {
         return virtualenvsPath;
     }
 
     // Try to get the actual cache-dir from Poetry
     try {
-        const { stdout } = await exec(`"${poetry}" config cache-dir`);
+        const { stdout } = await execProcess(`"${poetry}" config cache-dir`);
         if (stdout) {
             const cacheDir = stdout.trim();
             if (cacheDir && path.isAbsolute(cacheDir)) {
-                return virtualenvsPath.replace('{cache-dir}', cacheDir);
+                const resolved = virtualenvsPath.replace('{cache-dir}', cacheDir);
+                return path.normalize(resolved);
             }
         }
     } catch (e) {
-        traceError(`Error getting Poetry cache-dir config: ${e}`);
+        traceError('Error getting Poetry cache-dir config', e);
     }
 
     // Fall back to platform-specific default cache dir
     const defaultCacheDir = getDefaultPoetryCacheDir();
     if (defaultCacheDir) {
-        return virtualenvsPath.replace('{cache-dir}', defaultCacheDir);
+        const resolved = virtualenvsPath.replace('{cache-dir}', defaultCacheDir);
+        return path.normalize(resolved);
     }
 
-    // Last resort: return the original path (will likely not be valid)
-    return virtualenvsPath;
+    // Cannot resolve the placeholder - return undefined instead of unresolved path
+    return undefined;
 }
 
 export async function getPoetryVersion(poetry: string): Promise<string | undefined> {
@@ -357,8 +356,8 @@ export async function nativeToPythonEnv(
     const displayName = info.displayName || `poetry (${sv})`;
 
     // Check if this is a global Poetry virtualenv by checking if it's in Poetry's virtualenvs directory
-    // We need to use path.normalize() to ensure consistent path format comparison
-    const normalizedPrefix = path.normalize(info.prefix);
+    // We use normalizePath() for case-insensitive path comparison on Windows
+    const normalizedPrefix = normalizePath(info.prefix);
 
     // Determine if the environment is in Poetry's global virtualenvs directory
     let isGlobalPoetryEnv = false;
@@ -367,19 +366,17 @@ export async function nativeToPythonEnv(
     if (!isPoetryVirtualenvsInProject() || !info.project) {
         const virtualenvsPath = poetryVirtualenvsPath; // Use the cached value if available
         if (virtualenvsPath) {
-            const normalizedVirtualenvsPath = path.normalize(virtualenvsPath);
+            const normalizedVirtualenvsPath = normalizePath(virtualenvsPath);
             isGlobalPoetryEnv = normalizedPrefix.startsWith(normalizedVirtualenvsPath);
         } else {
             // Fall back to checking the platform-specific default location if we haven't cached the path yet
             const defaultPath = getDefaultPoetryVirtualenvsPath();
             if (defaultPath) {
-                const normalizedDefaultPath = path.normalize(defaultPath);
+                const normalizedDefaultPath = normalizePath(defaultPath);
                 isGlobalPoetryEnv = normalizedPrefix.startsWith(normalizedDefaultPath);
 
                 // Try to get the actual path asynchronously for next time
-                getPoetryVirtualenvsPath(_poetry).catch((e) =>
-                    traceError(`Error getting Poetry virtualenvs path: ${e}`),
-                );
+                getPoetryVirtualenvsPath(_poetry).catch((e) => traceError('Error getting Poetry virtualenvs path', e));
             }
         }
     }

src/test/constants.ts

@@ -26,7 +26,7 @@ export function isMultiRootTest(): boolean {
         return false;
     }
     try {
-        // eslint-disable-next-line @typescript-eslint/no-require-imports
+         
         const vscode = require('vscode');
         return Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 1;
     } catch {

src/test/features/projectManager.initialize.unit.test.ts

@@ -310,8 +310,8 @@ suite('Project Manager Initialization - Settings Preservation', () => {
         test('adding a workspace folder should NOT write project settings', async () => {
             const mockConfig = new MockWorkspaceConfiguration();
             (mockConfig as any).get = <T>(key: string, defaultValue?: T): T | undefined => {
-                if (key === 'pythonProjects') return [] as unknown as T;
-                if (key === 'defaultEnvManager') return 'ms-python.python:venv' as T;
+                if (key === 'pythonProjects') {return [] as unknown as T;}
+                if (key === 'defaultEnvManager') {return 'ms-python.python:venv' as T;}
                 return defaultValue;
             };
             mockConfig.update = () => Promise.resolve();
@@ -347,8 +347,8 @@ suite('Project Manager Initialization - Settings Preservation', () => {
         test('removing a workspace folder should NOT write additional settings', async () => {
             const mockConfig = new MockWorkspaceConfiguration();
             (mockConfig as any).get = <T>(key: string, defaultValue?: T): T | undefined => {
-                if (key === 'pythonProjects') return [] as unknown as T;
-                if (key === 'defaultEnvManager') return 'ms-python.python:venv' as T;
+                if (key === 'pythonProjects') {return [] as unknown as T;}
+                if (key === 'defaultEnvManager') {return 'ms-python.python:venv' as T;}
                 return defaultValue;
             };
             mockConfig.update = () => Promise.resolve();

src/test/managers/poetry/poetryUtils.unit.test.ts

@@ -3,16 +3,20 @@ import path from 'node:path';
 import * as sinon from 'sinon';
 import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../../api';
 import * as childProcessApis from '../../../common/childProcess.apis';
+import * as persistentState from '../../../common/persistentState';
 import * as pathUtils from '../../../common/utils/pathUtils';
 import * as platformUtils from '../../../common/utils/platformUtils';
 import { NativeEnvInfo } from '../../../managers/common/nativePythonFinder';
 import * as utils from '../../../managers/common/utils';
 import {
+    clearPoetryCache,
     getDefaultPoetryCacheDir,
     getDefaultPoetryVirtualenvsPath,
     getPoetryVersion,
+    getPoetryVirtualenvsPath,
     isPoetryVirtualenvsInProject,
     nativeToPythonEnv,
+    POETRY_VIRTUALENVS_PATH_KEY,
 } from '../../../managers/poetry/poetryUtils';
 
 suite('isPoetryVirtualenvsInProject', () => {
@@ -365,3 +369,181 @@ suite('getDefaultPoetryVirtualenvsPath', () => {
         assert.strictEqual(result, undefined);
     });
 });
+
+suite('getPoetryVirtualenvsPath - {cache-dir} placeholder resolution', () => {
+    let execProcessStub: sinon.SinonStub;
+    let isWindowsStub: sinon.SinonStub;
+    let isMacStub: sinon.SinonStub;
+    let getUserHomeDirStub: sinon.SinonStub;
+    let getWorkspacePersistentStateStub: sinon.SinonStub;
+    let mockState: { get: sinon.SinonStub; set: sinon.SinonStub };
+
+    setup(async () => {
+        execProcessStub = sinon.stub(childProcessApis, 'execProcess');
+        isWindowsStub = sinon.stub(platformUtils, 'isWindows');
+        isMacStub = sinon.stub(platformUtils, 'isMac');
+        getUserHomeDirStub = sinon.stub(pathUtils, 'getUserHomeDir');
+
+        // Create a mock state object to track persistence
+        mockState = {
+            get: sinon.stub(),
+            set: sinon.stub().resolves(),
+        };
+        getWorkspacePersistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState');
+        getWorkspacePersistentStateStub.resolves(mockState);
+
+        // Clear Poetry cache before each test
+        await clearPoetryCache();
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('resolves {cache-dir} placeholder when poetry config cache-dir succeeds', async () => {
+        // First call: virtualenvs.path returns a path with {cache-dir}
+        execProcessStub.onFirstCall().resolves({ stdout: '{cache-dir}/virtualenvs\n', stderr: '' });
+        // Second call: cache-dir config returns the actual path
+        execProcessStub.onSecondCall().resolves({ stdout: '/home/test/.cache/pypoetry\n', stderr: '' });
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        assert.strictEqual(result, path.join('/home/test/.cache/pypoetry', 'virtualenvs'));
+        // Verify the resolved path was persisted
+        assert.ok(
+            mockState.set.calledWith(
+                POETRY_VIRTUALENVS_PATH_KEY,
+                path.join('/home/test/.cache/pypoetry', 'virtualenvs'),
+            ),
+        );
+    });
+
+    test('falls back to platform default when poetry config cache-dir fails', async () => {
+        isWindowsStub.returns(false);
+        isMacStub.returns(false);
+        getUserHomeDirStub.returns('/home/test');
+
+        // First call: virtualenvs.path returns a path with {cache-dir}
+        execProcessStub.onFirstCall().resolves({ stdout: '{cache-dir}/virtualenvs\n', stderr: '' });
+        // Second call: cache-dir config fails
+        execProcessStub.onSecondCall().rejects(new Error('Command failed'));
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        // Should fall back to platform default cache dir
+        const expectedPath = path.join('/home/test', '.cache', 'pypoetry', 'virtualenvs');
+        assert.strictEqual(result, expectedPath);
+        // The resolved path should still be persisted
+        assert.ok(mockState.set.calledWith(POETRY_VIRTUALENVS_PATH_KEY, expectedPath));
+    });
+
+    test('falls back to platform default when poetry config cache-dir returns non-absolute path', async () => {
+        isWindowsStub.returns(false);
+        isMacStub.returns(false);
+        getUserHomeDirStub.returns('/home/test');
+
+        // First call: virtualenvs.path returns a path with {cache-dir}
+        execProcessStub.onFirstCall().resolves({ stdout: '{cache-dir}/virtualenvs\n', stderr: '' });
+        // Second call: cache-dir returns a relative path (invalid)
+        execProcessStub.onSecondCall().resolves({ stdout: 'relative/path\n', stderr: '' });
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        // Should fall back to platform default cache dir
+        const expectedPath = path.join('/home/test', '.cache', 'pypoetry', 'virtualenvs');
+        assert.strictEqual(result, expectedPath);
+    });
+
+    test('does not persist path when placeholder cannot be resolved and no platform default', async () => {
+        isWindowsStub.returns(false);
+        isMacStub.returns(false);
+        getUserHomeDirStub.returns(undefined); // No home dir available
+
+        // First call: virtualenvs.path returns a path with {cache-dir}
+        execProcessStub.onFirstCall().resolves({ stdout: '{cache-dir}/virtualenvs\n', stderr: '' });
+        // Second call: cache-dir config fails
+        execProcessStub.onSecondCall().rejects(new Error('Command failed'));
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        // Should fall back to platform default (which returns undefined when home is not available)
+        assert.strictEqual(result, undefined);
+        // Path should NOT be persisted when unresolved
+        assert.ok(!mockState.set.calledWith(POETRY_VIRTUALENVS_PATH_KEY, sinon.match.any));
+    });
+
+    test('handles virtualenvs.path without {cache-dir} placeholder (absolute path)', async () => {
+        // virtualenvs.path returns an absolute path directly
+        execProcessStub.onFirstCall().resolves({ stdout: '/custom/virtualenvs/path\n', stderr: '' });
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        assert.strictEqual(result, '/custom/virtualenvs/path');
+        // Should be persisted
+        assert.ok(mockState.set.calledWith(POETRY_VIRTUALENVS_PATH_KEY, '/custom/virtualenvs/path'));
+    });
+
+    test('falls back to platform default when virtualenvs.path returns non-absolute path without placeholder', async () => {
+        isWindowsStub.returns(false);
+        isMacStub.returns(false);
+        getUserHomeDirStub.returns('/home/test');
+
+        // virtualenvs.path returns a relative path (not valid)
+        execProcessStub.onFirstCall().resolves({ stdout: 'relative/path\n', stderr: '' });
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        // Should fall back to platform default
+        const expectedPath = path.join('/home/test', '.cache', 'pypoetry', 'virtualenvs');
+        assert.strictEqual(result, expectedPath);
+    });
+
+    test('uses cached value from persistent state', async () => {
+        mockState.get.resolves('/cached/virtualenvs/path');
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        assert.strictEqual(result, '/cached/virtualenvs/path');
+        // Should not call exec since we have a cached value
+        assert.ok(!execProcessStub.called);
+    });
+
+    test('handles virtualenvs.path config command failure', async () => {
+        isWindowsStub.returns(false);
+        isMacStub.returns(false);
+        getUserHomeDirStub.returns('/home/test');
+
+        // virtualenvs.path config fails
+        execProcessStub.onFirstCall().rejects(new Error('Config command failed'));
+
+        const result = await getPoetryVirtualenvsPath('/usr/bin/poetry');
+
+        // Should fall back to platform default
+        const expectedPath = path.join('/home/test', '.cache', 'pypoetry', 'virtualenvs');
+        assert.strictEqual(result, expectedPath);
+    });
+
+    test('Windows: resolves {cache-dir} with platform default when cache-dir query fails', async () => {
+        const originalLocalAppData = process.env.LOCALAPPDATA;
+        try {
+            isWindowsStub.returns(true);
+            process.env.LOCALAPPDATA = 'C:\\Users\\test\\AppData\\Local';
+
+            // First call: virtualenvs.path returns a path with {cache-dir}
+            execProcessStub.onFirstCall().resolves({ stdout: '{cache-dir}/virtualenvs\n', stderr: '' });
+            // Second call: cache-dir config fails
+            execProcessStub.onSecondCall().rejects(new Error('Command failed'));
+
+            const result = await getPoetryVirtualenvsPath('C:\\poetry\\poetry.exe');
+
+            const expectedPath = path.join('C:\\Users\\test\\AppData\\Local', 'pypoetry', 'Cache', 'virtualenvs');
+            assert.strictEqual(result, expectedPath);
+        } finally {
+            if (originalLocalAppData === undefined) {
+                delete process.env.LOCALAPPDATA;
+            } else {
+                process.env.LOCALAPPDATA = originalLocalAppData;
+            }
+        }
+    });
+});