Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions src/managers/pipenv/pipenvUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down
23 changes: 21 additions & 2 deletions src/managers/poetry/poetryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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,
Expand Down Expand Up @@ -49,6 +50,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;
Expand Down Expand Up @@ -112,13 +118,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) {
Expand All @@ -136,14 +154,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(
Expand All @@ -157,6 +175,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
Expand Down
146 changes: 146 additions & 0 deletions src/test/managers/conda/condaUtils.getConda.unit.test.ts
Original file line number Diff line number Diff line change
@@ -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;
});
});
112 changes: 112 additions & 0 deletions src/test/managers/pipenv/pipenvUtils.getPipenv.unit.test.ts
Original file line number Diff line number Diff line change
@@ -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;
});
});
Loading
Loading