From a044c35075a64383986118c76fcec8cafd080a34 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 18 Feb 2026 08:02:12 -0800 Subject: [PATCH] fix pipenv and poetry setting --- src/managers/pipenv/pipenvUtils.ts | 27 ++-- src/managers/poetry/poetryUtils.ts | 23 ++- .../conda/condaUtils.getConda.unit.test.ts | 146 ++++++++++++++++++ .../pipenv/pipenvUtils.getPipenv.unit.test.ts | 112 ++++++++++++++ .../poetry/poetryUtils.getPoetry.unit.test.ts | 113 ++++++++++++++ 5 files changed, 406 insertions(+), 15 deletions(-) create mode 100644 src/test/managers/conda/condaUtils.getConda.unit.test.ts create mode 100644 src/test/managers/pipenv/pipenvUtils.getPipenv.unit.test.ts create mode 100644 src/test/managers/poetry/poetryUtils.getPoetry.unit.test.ts diff --git a/src/managers/pipenv/pipenvUtils.ts b/src/managers/pipenv/pipenvUtils.ts index 659cbff3..f6875c29 100644 --- a/src/managers/pipenv/pipenvUtils.ts +++ b/src/managers/pipenv/pipenvUtils.ts @@ -66,6 +66,17 @@ function getPipenvPathFromSettings(): string | undefined { } export async function getPipenv(native?: NativePythonFinder): Promise { + // 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); @@ -73,6 +84,7 @@ export async function getPipenv(native?: NativePythonFinder): Promise(PIPENV_PATH_KEY); if (storedPath) { @@ -84,18 +96,7 @@ export async function getPipenv(native?: NativePythonFinder): Promise('python', 'poetryPath'); + return poetryPath ? poetryPath : undefined; +} + export async function clearPoetryCache(): Promise { // Reset in-memory cache poetryPath = undefined; @@ -112,6 +118,17 @@ export async function setPoetryForWorkspaces(fsPath: string[], envPath: string | } export async function getPoetry(native?: NativePythonFinder): Promise { + // 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); @@ -119,6 +136,7 @@ export async function getPoetry(native?: NativePythonFinder): Promise(POETRY_PATH_KEY); if (storedPath) { @@ -136,14 +154,14 @@ export async function getPoetry(native?: NativePythonFinder): Promise { + 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 new file mode 100644 index 00000000..1ffa934e --- /dev/null +++ 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 new file mode 100644 index 00000000..7ae865ac --- /dev/null +++ b/src/test/managers/poetry/poetryUtils.getPoetry.unit.test.ts @@ -0,0 +1,113 @@ +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 { clearPoetryCache, getPoetry, POETRY_PATH_KEY } from '../../../managers/poetry/poetryUtils'; + +/** + * Tests for getPoetry prioritization. + * + * The priority order should be: + * 1. Settings (python.poetryPath) - if set and valid + * 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('Poetry Utils - getPoetry 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 + clearPoetryCache(); + + 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', 'poetryPath').returns(undefined); + mockState.get.withArgs(POETRY_PATH_KEY).resolves(undefined); + + // Act + await getPoetry(); + + // Assert: Settings function was called + assert.ok(getSettingStub.calledWith('python', 'poetryPath'), 'Settings should be checked'); + + // Settings should be checked BEFORE persistent state is accessed + // getPoetry() 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/poetry'; + getSettingStub.withArgs('python', 'poetryPath').returns(settingsPath); + + // Act + await getPoetry(); + + // Assert: Settings was checked first + assert.ok(getSettingStub.calledWith('python', 'poetryPath'), 'Settings should be checked'); + }); + + test('Persistent state is checked when settings returns undefined', async () => { + // Arrange: No settings + getSettingStub.withArgs('python', 'poetryPath').returns(undefined); + + // Persistent state returns undefined too + mockState.get.withArgs(POETRY_PATH_KEY).resolves(undefined); + + // Act + await getPoetry(); + + // Assert: Both settings and persistent state were checked + assert.ok(getSettingStub.calledWith('python', 'poetryPath'), 'Settings should be checked first'); + assert.ok(mockState.get.calledWith(POETRY_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', 'poetryPath').returns(undefined); + + // Start the call (don't await) + const promise = getPoetry(); + + // Assert: Settings was checked synchronously (before promise resolves) + assert.ok(getSettingStub.called, 'Settings should be checked synchronously at function start'); + + // Clean up + return promise; + }); +});