Skip to content

Commit a044c35

Browse files
eleanorjboydeleanorjboyd
committed
fix pipenv and poetry setting
1 parent ded5557 commit a044c35

File tree

5 files changed

+406
-15
lines changed

5 files changed

+406
-15
lines changed

‎src/managers/pipenv/pipenvUtils.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,25 @@ function getPipenvPathFromSettings(): string | undefined {
6666
}
6767

6868
export async function getPipenv(native?: NativePythonFinder): Promise<string | undefined> {
69+
// Priority 1: Settings (if explicitly set and valid)
70+
const settingPath = getPipenvPathFromSettings();
71+
if (settingPath) {
72+
if (await fs.exists(untildify(settingPath))) {
73+
traceInfo(`Using pipenv from settings: ${settingPath}`);
74+
return untildify(settingPath);
75+
}
76+
traceInfo(`Pipenv path from settings does not exist: ${settingPath}`);
77+
}
78+
79+
// Priority 2: In-memory cache
6980
if (pipenvPath) {
7081
if (await fs.exists(untildify(pipenvPath))) {
7182
return untildify(pipenvPath);
7283
}
7384
pipenvPath = undefined;
7485
}
7586

87+
// Priority 3: Persistent state
7688
const state = await getWorkspacePersistentState();
7789
const storedPath = await state.get<string>(PIPENV_PATH_KEY);
7890
if (storedPath) {
@@ -84,26 +96,15 @@ export async function getPipenv(native?: NativePythonFinder): Promise<string | u
8496
await state.set(PIPENV_PATH_KEY, undefined);
8597
}
8698

87-
// try to get from settings
88-
const settingPath = getPipenvPathFromSettings();
89-
if (settingPath) {
90-
if (await fs.exists(untildify(settingPath))) {
91-
pipenvPath = settingPath;
92-
traceInfo(`Using pipenv from settings: ${settingPath}`);
93-
return untildify(pipenvPath);
94-
}
95-
traceInfo(`Pipenv path from settings does not exist: ${settingPath}`);
96-
}
97-
98-
// Try to find pipenv in PATH
99+
// Priority 4: PATH lookup
99100
const foundPipenv = await findPipenv();
100101
if (foundPipenv) {
101102
pipenvPath = foundPipenv;
102103
traceInfo(`Found pipenv in PATH: ${foundPipenv}`);
103104
return foundPipenv;
104105
}
105106

106-
// Use native finder as fallback
107+
// Priority 5: Native finder as fallback
107108
if (native) {
108109
const data = await native.refresh(false);
109110
const managers = data

‎src/managers/poetry/poetryUtils.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { traceError, traceInfo } from '../../common/logging';
88
import { getWorkspacePersistentState } from '../../common/persistentState';
99
import { getUserHomeDir, untildify } from '../../common/utils/pathUtils';
1010
import { isWindows } from '../../common/utils/platformUtils';
11+
import { getSettingWorkspaceScope } from '../../features/settings/settingHelpers';
1112
import {
1213
isNativeEnvInfo,
1314
NativeEnvInfo,
@@ -49,6 +50,11 @@ export const POETRY_VIRTUALENVS_PATH_KEY = `${ENVS_EXTENSION_ID}:poetry:VIRTUALE
4950
let poetryPath: string | undefined;
5051
let poetryVirtualenvsPath: string | undefined;
5152

53+
function getPoetryPathFromSettings(): string | undefined {
54+
const poetryPath = getSettingWorkspaceScope<string>('python', 'poetryPath');
55+
return poetryPath ? poetryPath : undefined;
56+
}
57+
5258
export async function clearPoetryCache(): Promise<void> {
5359
// Reset in-memory cache
5460
poetryPath = undefined;
@@ -112,13 +118,25 @@ export async function setPoetryForWorkspaces(fsPath: string[], envPath: string |
112118
}
113119

114120
export async function getPoetry(native?: NativePythonFinder): Promise<string | undefined> {
121+
// Priority 1: Settings (if explicitly set and valid)
122+
const settingPath = getPoetryPathFromSettings();
123+
if (settingPath) {
124+
if (await fs.exists(untildify(settingPath))) {
125+
traceInfo(`Using poetry from settings: ${settingPath}`);
126+
return untildify(settingPath);
127+
}
128+
traceInfo(`Poetry path from settings does not exist: ${settingPath}`);
129+
}
130+
131+
// Priority 2: In-memory cache
115132
if (poetryPath) {
116133
if (await fs.exists(untildify(poetryPath))) {
117134
return untildify(poetryPath);
118135
}
119136
poetryPath = undefined;
120137
}
121138

139+
// Priority 3: Persistent state
122140
const state = await getWorkspacePersistentState();
123141
const storedPath = await state.get<string>(POETRY_PATH_KEY);
124142
if (storedPath) {
@@ -136,14 +154,14 @@ export async function getPoetry(native?: NativePythonFinder): Promise<string | u
136154
await state.set(POETRY_PATH_KEY, undefined);
137155
}
138156

139-
// Check in standard PATH locations
157+
// Priority 4: PATH lookup
140158
poetryPath = await findPoetry();
141159
if (poetryPath) {
142160
await setPoetry(poetryPath);
143161
return poetryPath;
144162
}
145163

146-
// Check for user-installed poetry
164+
// Priority 5: Known user-install locations
147165
const home = getUserHomeDir();
148166
if (home) {
149167
const poetryUserInstall = path.join(
@@ -157,6 +175,7 @@ export async function getPoetry(native?: NativePythonFinder): Promise<string | u
157175
}
158176
}
159177

178+
// Priority 6: Native finder as fallback
160179
if (native) {
161180
const data = await native.refresh(false);
162181
const managers = data

‎src/test/managers/conda/condaUtils.getConda.unit.test.ts‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import assert from 'assert';
2+
import * as sinon from 'sinon';
3+
import * as logging from '../../../common/logging';
4+
import * as persistentState from '../../../common/persistentState';
5+
import * as workspaceApis from '../../../common/workspace.apis';
6+
import { clearCondaCache, CONDA_PATH_KEY, getConda } from '../../../managers/conda/condaUtils';
7+
8+
/**
9+
* Tests for getConda prioritization.
10+
*
11+
* The priority order should be:
12+
* 1. Settings (python.condaPath) - if set (non-empty)
13+
* 2. In-memory cache
14+
* 3. Persistent state
15+
* 4. PATH lookup (which)
16+
* 5. Known locations
17+
* 6. Native finder
18+
*
19+
* These tests verify the correct order by checking which functions are called and in what order.
20+
*/
21+
suite('Conda Utils - getConda prioritization', () => {
22+
let getConfigurationStub: sinon.SinonStub;
23+
let mockConfig: { get: sinon.SinonStub };
24+
let mockState: { get: sinon.SinonStub; set: sinon.SinonStub };
25+
let getWorkspacePersistentStateStub: sinon.SinonStub;
26+
27+
setup(async () => {
28+
// Clear in-memory cache before each test
29+
await clearCondaCache();
30+
31+
mockConfig = {
32+
get: sinon.stub(),
33+
};
34+
getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
35+
getConfigurationStub.withArgs('python').returns(mockConfig);
36+
sinon.stub(logging, 'traceInfo');
37+
38+
mockState = {
39+
get: sinon.stub(),
40+
set: sinon.stub().resolves(),
41+
};
42+
getWorkspacePersistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState');
43+
getWorkspacePersistentStateStub.resolves(mockState);
44+
});
45+
46+
teardown(() => {
47+
sinon.restore();
48+
});
49+
50+
test('Priority 1: Settings path is used first when set', async () => {
51+
// Arrange: Settings returns a valid path
52+
const settingsPath = '/custom/path/to/conda';
53+
mockConfig.get.withArgs('condaPath').returns(settingsPath);
54+
55+
// Act
56+
const result = await getConda();
57+
58+
// Assert: Should use settings path immediately (no existence check for settings in getConda)
59+
assert.strictEqual(result, settingsPath);
60+
// Verify persistent state was NOT called (settings took priority)
61+
assert.ok(!mockState.get.called, 'Persistent state should not be checked when settings is set');
62+
});
63+
64+
test('Settings check happens before any other source', async () => {
65+
// Arrange: Settings returns empty (no setting)
66+
mockConfig.get.withArgs('condaPath').returns('');
67+
mockState.get.withArgs(CONDA_PATH_KEY).resolves(undefined);
68+
69+
// Act
70+
try {
71+
await getConda();
72+
} catch {
73+
// Expected to throw when nothing found
74+
}
75+
76+
// Assert: Configuration was accessed first
77+
assert.ok(getConfigurationStub.calledWith('python'), 'Configuration should be checked');
78+
assert.ok(mockConfig.get.calledWith('condaPath'), 'Settings should be checked');
79+
});
80+
81+
test('Persistent state is checked when settings is empty', async () => {
82+
// Arrange: No settings
83+
mockConfig.get.withArgs('condaPath').returns('');
84+
85+
// Persistent state returns undefined too
86+
mockState.get.withArgs(CONDA_PATH_KEY).resolves(undefined);
87+
88+
// Act
89+
try {
90+
await getConda();
91+
} catch {
92+
// Expected to throw when nothing found
93+
}
94+
95+
// Assert: Both settings and persistent state were checked
96+
assert.ok(mockConfig.get.calledWith('condaPath'), 'Settings should be checked first');
97+
assert.ok(mockState.get.calledWith(CONDA_PATH_KEY), 'Persistent state should be checked');
98+
});
99+
100+
test('Settings path takes priority over cache', async () => {
101+
// Arrange: First set up so something would be cached
102+
// We can't easily test the cache without fs stubs, but we can verify
103+
// that settings is always checked first
104+
105+
// Now set a settings path
106+
const settingsPath = '/custom/conda';
107+
mockConfig.get.withArgs('condaPath').returns(settingsPath);
108+
109+
// Act
110+
const result = await getConda();
111+
112+
// Assert: Should use settings
113+
assert.strictEqual(result, settingsPath);
114+
});
115+
116+
test('Settings with non-empty value is used regardless of validity', async () => {
117+
// This is key behavior: getConda() returns settings immediately without checking existence
118+
// The caller is responsible for validating the path if needed
119+
const settingsPath = '/nonexistent/conda';
120+
mockConfig.get.withArgs('condaPath').returns(settingsPath);
121+
122+
// Act
123+
const result = await getConda();
124+
125+
// Assert: Should return settings path directly
126+
assert.strictEqual(result, settingsPath);
127+
});
128+
129+
test('Code checks settings first in the function body', () => {
130+
// This is a structural test - verify the function checks settings at the top
131+
// by inspecting that getConfiguration is called synchronously
132+
// before any async operations
133+
134+
// Arrange
135+
mockConfig.get.withArgs('condaPath').returns('/some/path');
136+
137+
// Start the call (don't await)
138+
const promise = getConda();
139+
140+
// Assert: Settings was checked synchronously (before promise resolves)
141+
assert.ok(getConfigurationStub.called, 'Configuration should be checked synchronously at function start');
142+
143+
// Clean up
144+
return promise;
145+
});
146+
});

‎src/test/managers/pipenv/pipenvUtils.getPipenv.unit.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import assert from 'assert';
2+
import * as sinon from 'sinon';
3+
import * as logging from '../../../common/logging';
4+
import * as persistentState from '../../../common/persistentState';
5+
import * as settingHelpers from '../../../features/settings/settingHelpers';
6+
import { clearPipenvCache, getPipenv, PIPENV_PATH_KEY } from '../../../managers/pipenv/pipenvUtils';
7+
8+
/**
9+
* Tests for getPipenv prioritization.
10+
*
11+
* The priority order should be:
12+
* 1. Settings (python.pipenvPath) - if set and valid
13+
* 2. In-memory cache
14+
* 3. Persistent state
15+
* 4. PATH lookup (which)
16+
* 5. Native finder
17+
*
18+
* These tests verify the correct order by checking which functions are called and in what order.
19+
*/
20+
suite('Pipenv Utils - getPipenv prioritization', () => {
21+
let getSettingStub: sinon.SinonStub;
22+
let mockState: { get: sinon.SinonStub; set: sinon.SinonStub };
23+
let getWorkspacePersistentStateStub: sinon.SinonStub;
24+
25+
setup(() => {
26+
// Clear in-memory cache before each test
27+
clearPipenvCache();
28+
29+
getSettingStub = sinon.stub(settingHelpers, 'getSettingWorkspaceScope');
30+
sinon.stub(logging, 'traceInfo');
31+
32+
mockState = {
33+
get: sinon.stub(),
34+
set: sinon.stub().resolves(),
35+
};
36+
getWorkspacePersistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState');
37+
getWorkspacePersistentStateStub.resolves(mockState);
38+
});
39+
40+
teardown(() => {
41+
sinon.restore();
42+
});
43+
44+
test('Settings check happens before any other source', async () => {
45+
// Arrange: Settings returns undefined (no setting)
46+
getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
47+
mockState.get.withArgs(PIPENV_PATH_KEY).resolves(undefined);
48+
49+
// Act
50+
await getPipenv();
51+
52+
// Assert: Settings function was called
53+
assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked');
54+
55+
// Settings should be checked BEFORE persistent state is accessed
56+
// getPipenv() checks settings synchronously at the start, then does async work
57+
// We verify by checking that settings was called before any persistent state access
58+
assert.ok(getSettingStub.called, 'Settings should be checked');
59+
// If persistent state was accessed, settings must have been checked first
60+
if (mockState.get.called) {
61+
assert.ok(
62+
getSettingStub.calledBefore(getWorkspacePersistentStateStub),
63+
'Settings should be checked before persistent state',
64+
);
65+
}
66+
});
67+
68+
test('When settings returns a path, it is checked before cache', async () => {
69+
// Arrange: Settings returns a path
70+
const settingsPath = '/custom/path/to/pipenv';
71+
getSettingStub.withArgs('python', 'pipenvPath').returns(settingsPath);
72+
73+
// Act
74+
await getPipenv();
75+
76+
// Assert: Settings was checked first
77+
assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked');
78+
});
79+
80+
test('Persistent state is checked when settings returns undefined', async () => {
81+
// Arrange: No settings
82+
getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
83+
84+
// Persistent state returns undefined too
85+
mockState.get.withArgs(PIPENV_PATH_KEY).resolves(undefined);
86+
87+
// Act
88+
await getPipenv();
89+
90+
// Assert: Both settings and persistent state were checked
91+
assert.ok(getSettingStub.calledWith('python', 'pipenvPath'), 'Settings should be checked first');
92+
assert.ok(mockState.get.calledWith(PIPENV_PATH_KEY), 'Persistent state should be checked');
93+
});
94+
95+
test('Code checks settings first in the function body', () => {
96+
// This is a structural test - verify the function checks settings at the top
97+
// by inspecting that getSettingWorkspaceScope is called synchronously
98+
// before any async operations
99+
100+
// Arrange
101+
getSettingStub.withArgs('python', 'pipenvPath').returns(undefined);
102+
103+
// Start the call (don't await)
104+
const promise = getPipenv();
105+
106+
// Assert: Settings was checked synchronously (before promise resolves)
107+
assert.ok(getSettingStub.called, 'Settings should be checked synchronously at function start');
108+
109+
// Clean up
110+
return promise;
111+
});
112+
});

0 commit comments

Comments
 (0)