fix pipenv and poetry setting (#1244)

eleanorjboyd · web-flow · commit 7b0486fcea86 · 2026-02-18T09:12:28.000-08:00
ensure that pipenvPath and poetryPath are supported as settings fixes #912 and #918
diff --git a/src/managers/pipenv/pipenvUtils.ts b/src/managers/pipenv/pipenvUtils.ts
@@ -66,13 +66,25 @@ function getPipenvPathFromSettings(): string | undefined {
 }
 
 export async function getPipenv(native?: NativePythonFinder): Promise<string | undefined> {
+    // Priority 1: Settings (if explicitly set and valid)
+    const settingPath = getPipenvPathFromSettings();
+    if (settingPath) {
+        if (await fs.exists(untildify(settingPath))) {
+            traceInfo(`Using pipenv from settings: ${settingPath}`);
+            return untildify(settingPath);
+        }
+        traceInfo(`Pipenv path from settings does not exist: ${settingPath}`);
+    }
+
+    // Priority 2: In-memory cache
     if (pipenvPath) {
         if (await fs.exists(untildify(pipenvPath))) {
             return untildify(pipenvPath);
         }
         pipenvPath = undefined;
     }
 
+    // Priority 3: Persistent state
     const state = await getWorkspacePersistentState();
     const storedPath = await state.get<string>(PIPENV_PATH_KEY);
     if (storedPath) {
@@ -84,26 +96,15 @@ export async function getPipenv(native?: NativePythonFinder): Promise<string | u
         await state.set(PIPENV_PATH_KEY, undefined);
     }
 
-    // try to get from settings
-    const settingPath = getPipenvPathFromSettings();
-    if (settingPath) {
-        if (await fs.exists(untildify(settingPath))) {
-            pipenvPath = settingPath;
-            traceInfo(`Using pipenv from settings: ${settingPath}`);
-            return untildify(pipenvPath);
-        }
-        traceInfo(`Pipenv path from settings does not exist: ${settingPath}`);
-    }
-
-    // Try to find pipenv in PATH
+    // Priority 4: PATH lookup
     const foundPipenv = await findPipenv();
     if (foundPipenv) {
         pipenvPath = foundPipenv;
         traceInfo(`Found pipenv in PATH: ${foundPipenv}`);
         return foundPipenv;
     }
 
-    // Use native finder as fallback
+    // Priority 5: Native finder as fallback
     if (native) {
         const data = await native.refresh(false);
         const managers = data
diff --git a/src/managers/poetry/poetryUtils.ts b/src/managers/poetry/poetryUtils.ts
@@ -9,6 +9,7 @@ import { traceError, traceInfo } from '../../common/logging';
 import { getWorkspacePersistentState } from '../../common/persistentState';
 import { getUserHomeDir, untildify } from '../../common/utils/pathUtils';
 import { isWindows } from '../../common/utils/platformUtils';
+import { getSettingWorkspaceScope } from '../../features/settings/settingHelpers';
 import {
     isNativeEnvInfo,
     NativeEnvInfo,
@@ -50,6 +51,11 @@ export const POETRY_VIRTUALENVS_PATH_KEY = `${ENVS_EXTENSION_ID}:poetry:VIRTUALE
 let poetryPath: string | undefined;
 let poetryVirtualenvsPath: string | undefined;
 
+function getPoetryPathFromSettings(): string | undefined {
+    const poetryPath = getSettingWorkspaceScope<string>('python', 'poetryPath');
+    return poetryPath ? poetryPath : undefined;
+}
+
 export async function clearPoetryCache(): Promise<void> {
     // Reset in-memory cache
     poetryPath = undefined;
@@ -113,13 +119,25 @@ export async function setPoetryForWorkspaces(fsPath: string[], envPath: string |
 }
 
 export async function getPoetry(native?: NativePythonFinder): Promise<string | undefined> {
+    // Priority 1: Settings (if explicitly set and valid)
+    const settingPath = getPoetryPathFromSettings();
+    if (settingPath) {
+        if (await fs.exists(untildify(settingPath))) {
+            traceInfo(`Using poetry from settings: ${settingPath}`);
+            return untildify(settingPath);
+        }
+        traceInfo(`Poetry path from settings does not exist: ${settingPath}`);
+    }
+
+    // Priority 2: In-memory cache
     if (poetryPath) {
         if (await fs.exists(untildify(poetryPath))) {
             return untildify(poetryPath);
         }
         poetryPath = undefined;
     }
 
+    // Priority 3: Persistent state
     const state = await getWorkspacePersistentState();
     const storedPath = await state.get<string>(POETRY_PATH_KEY);
     if (storedPath) {
@@ -137,14 +155,14 @@ export async function getPoetry(native?: NativePythonFinder): Promise<string | u
         await state.set(POETRY_PATH_KEY, undefined);
     }
 
-    // Check in standard PATH locations
+    // Priority 4: PATH lookup
     poetryPath = await findPoetry();
     if (poetryPath) {
         await setPoetry(poetryPath);
         return poetryPath;
     }
 
-    // Check for user-installed poetry
+    // Priority 5: Known user-install locations
     const home = getUserHomeDir();
     if (home) {
         const poetryUserInstall = path.join(
@@ -158,6 +176,7 @@ export async function getPoetry(native?: NativePythonFinder): Promise<string | u
         }
     }
 
+    // Priority 6: Native finder as fallback
     if (native) {
         const data = await native.refresh(false);
         const managers = data
diff --git a/src/test/managers/conda/condaUtils.getConda.unit.test.ts b/src/test/managers/conda/condaUtils.getConda.unit.test.ts
@@ -0,0 +1,146 @@
+import assert from 'assert';
+import * as sinon from 'sinon';
+import * as logging from '../../../common/logging';
+import * as persistentState from '../../../common/persistentState';
+import * as workspaceApis from '../../../common/workspace.apis';
+import { clearCondaCache, CONDA_PATH_KEY, getConda } from '../../../managers/conda/condaUtils';
+
+/**
+ * Tests for getConda prioritization.
+ *
+ * The priority order should be:
+ * 1. Settings (python.condaPath) - if set (non-empty)
+ * 2. In-memory cache
+ * 3. Persistent state
+ * 4. PATH lookup (which)
+ * 5. Known locations
+ * 6. Native finder
+ *
+ * These tests verify the correct order by checking which functions are called and in what order.
+ */
+suite('Conda Utils - getConda prioritization', () => {
+    let getConfigurationStub: sinon.SinonStub;
+    let mockConfig: { get: sinon.SinonStub };
+    let mockState: { get: sinon.SinonStub; set: sinon.SinonStub };
+    let getWorkspacePersistentStateStub: sinon.SinonStub;
+
+    setup(async () => {
+        // Clear in-memory cache before each test
+        await clearCondaCache();
+
+        mockConfig = {
+            get: sinon.stub(),
+        };
+        getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
+        getConfigurationStub.withArgs('python').returns(mockConfig);
+        sinon.stub(logging, 'traceInfo');
+
+        mockState = {
+            get: sinon.stub(),
+            set: sinon.stub().resolves(),
+        };
+        getWorkspacePersistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState');
+        getWorkspacePersistentStateStub.resolves(mockState);
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('Priority 1: Settings path is used first when set', async () => {
+        // Arrange: Settings returns a valid path
+        const settingsPath = '/custom/path/to/conda';
+        mockConfig.get.withArgs('condaPath').returns(settingsPath);
+
+        // Act
+        const result = await getConda();
+
+        // Assert: Should use settings path immediately (no existence check for settings in getConda)
+        assert.strictEqual(result, settingsPath);
+        // Verify persistent state was NOT called (settings took priority)
+        assert.ok(!mockState.get.called, 'Persistent state should not be checked when settings is set');
+    });
+
+    test('Settings check happens before any other source', async () => {
+        // Arrange: Settings returns empty (no setting)
+        mockConfig.get.withArgs('condaPath').returns('');
+        mockState.get.withArgs(CONDA_PATH_KEY).resolves(undefined);
+
+        // Act
+        try {
+            await getConda();
+        } catch {
+            // Expected to throw when nothing found
+        }
+
+        // Assert: Configuration was accessed first
+        assert.ok(getConfigurationStub.calledWith('python'), 'Configuration should be checked');
+        assert.ok(mockConfig.get.calledWith('condaPath'), 'Settings should be checked');
+    });
+
+    test('Persistent state is checked when settings is empty', async () => {
+        // Arrange: No settings
+        mockConfig.get.withArgs('condaPath').returns('');
+
+        // Persistent state returns undefined too
+        mockState.get.withArgs(CONDA_PATH_KEY).resolves(undefined);
+
+        // Act
+        try {
+            await getConda();
+        } catch {
+            // Expected to throw when nothing found
+        }
+
+        // Assert: Both settings and persistent state were checked
+        assert.ok(mockConfig.get.calledWith('condaPath'), 'Settings should be checked first');
+        assert.ok(mockState.get.calledWith(CONDA_PATH_KEY), 'Persistent state should be checked');
+    });
+
+    test('Settings path takes priority over cache', async () => {
+        // Arrange: First set up so something would be cached
+        // We can't easily test the cache without fs stubs, but we can verify
+        // that settings is always checked first
+
+        // Now set a settings path
+        const settingsPath = '/custom/conda';
+        mockConfig.get.withArgs('condaPath').returns(settingsPath);
+
+        // Act
+        const result = await getConda();
+
+        // Assert: Should use settings
+        assert.strictEqual(result, settingsPath);
+    });
+
+    test('Settings with non-empty value is used regardless of validity', async () => {
+        // This is key behavior: getConda() returns settings immediately without checking existence
+        // The caller is responsible for validating the path if needed
+        const settingsPath = '/nonexistent/conda';
+        mockConfig.get.withArgs('condaPath').returns(settingsPath);
+
+        // Act
+        const result = await getConda();
+
+        // Assert: Should return settings path directly
+        assert.strictEqual(result, settingsPath);
+    });
+
+    test('Code checks settings first in the function body', () => {
+        // This is a structural test - verify the function checks settings at the top
+        // by inspecting that getConfiguration is called synchronously
+        // before any async operations
+
+        // Arrange
+        mockConfig.get.withArgs('condaPath').returns('/some/path');
+
+        // Start the call (don't await)
+        const promise = getConda();
+
+        // Assert: Settings was checked synchronously (before promise resolves)
+        assert.ok(getConfigurationStub.called, 'Configuration should be checked synchronously at function start');
+
+        // Clean up
+        return promise;
+    });
+});
diff --git a/src/test/managers/pipenv/pipenvUtils.getPipenv.unit.test.ts b/src/test/managers/pipenv/pipenvUtils.getPipenv.unit.test.ts
@@ -0,0 +1,112 @@
+import assert from 'assert';
+import * as sinon from 'sinon';
+import * as logging from '../../../common/logging';
+import * as persistentState from '../../../common/persistentState';
+import * as settingHelpers from '../../../features/settings/settingHelpers';
+import { clearPipenvCache, getPipenv, PIPENV_PATH_KEY } from '../../../managers/pipenv/pipenvUtils';
+
+/**
+ * Tests for getPipenv prioritization.
+ *
+ * The priority order should be:
+ * 1. Settings (python.pipenvPath) - if set and valid
+ * 2. In-memory cache
+ * 3. Persistent state
+ * 4. PATH lookup (which)
+ * 5. Native finder
+ *
+ * These tests verify the correct order by checking which functions are called and in what order.
+ */
+suite('Pipenv Utils - getPipenv prioritization', () => {
+    let getSettingStub: sinon.SinonStub;
+    let mockState: { get: sinon.SinonStub; set: sinon.SinonStub };
+    let getWorkspacePersistentStateStub: sinon.SinonStub;
+
+    setup(() => {
+        // Clear in-memory cache before each test
+        clearPipenvCache();
+
+        getSettingStub = sinon.stub(settingHelpers, 'getSettingWorkspaceScope');
+        sinon.stub(logging, 'traceInfo');
+
+        mockState = {
+            get: sinon.stub(),
+            set: sinon.stub().resolves(),
+        };
+        getWorkspacePersistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState');
+        getWorkspacePersistentStateStub.resolves(mockState);
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('Settings check happens before any other source', async () => {
+        // Arrange: Settings returns undefined (no setting)
+        getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
+        mockState.get.withArgs(PIPENV_PATH_KEY).resolves(undefined);
+
+        // Act
+        await getPipenv();
+
+        // Assert: Settings function was called
+        assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked');
+
+        // Settings should be checked BEFORE persistent state is accessed
+        // getPipenv() checks settings synchronously at the start, then does async work
+        // We verify by checking that settings was called before any persistent state access
+        assert.ok(getSettingStub.called, 'Settings should be checked');
+        // If persistent state was accessed, settings must have been checked first
+        if (mockState.get.called) {
+            assert.ok(
+                getSettingStub.calledBefore(getWorkspacePersistentStateStub),
+                'Settings should be checked before persistent state',
+            );
+        }
+    });
+
+    test('When settings returns a path, it is checked before cache', async () => {
+        // Arrange: Settings returns a path
+        const settingsPath = '/custom/path/to/pipenv';
+        getSettingStub.withArgs('python', 'pipenvPath').returns(settingsPath);
+
+        // Act
+        await getPipenv();
+
+        // Assert: Settings was checked first
+        assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked');
+    });
+
+    test('Persistent state is checked when settings returns undefined', async () => {
+        // Arrange: No settings
+        getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
+
+        // Persistent state returns undefined too
+        mockState.get.withArgs(PIPENV_PATH_KEY).resolves(undefined);
+
+        // Act
+        await getPipenv();
+
+        // Assert: Both settings and persistent state were checked
+        assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked first');
+        assert.ok(mockState.get.calledWith(PIPENV_PATH_KEY), 'Persistent state should be checked');
+    });
+
+    test('Code checks settings first in the function body', () => {
+        // This is a structural test - verify the function checks settings at the top
+        // by inspecting that getSettingWorkspaceScope is called synchronously
+        // before any async operations
+
+        // Arrange
+        getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
+
+        // Start the call (don't await)
+        const promise = getPipenv();
+
+        // Assert: Settings was checked synchronously (before promise resolves)
+        assert.ok(getSettingStub.called, 'Settings should be checked synchronously at function start');
+
+        // Clean up
+        return promise;
+    });
+});
diff --git a/src/test/managers/poetry/poetryUtils.getPoetry.unit.test.ts b/src/test/managers/poetry/poetryUtils.getPoetry.unit.test.ts
