Skip to content

Commit 12033b0

Browse files
Copilotkarthiknadig
andcommitted
Address review feedback: use PYENV_ROOT, revert to 2-levels-up, fix tests
- Use PYENV_ROOT env var as primary source for pyenv root directory - Revert 3-levels-up Windows logic to 2-levels-up (pyenv-win versions are at ~/.pyenv/pyenv-win/versions/, not ~/.pyenv/versions/) - Add test for PYENV_ROOT preference - Fix POSIX test to use absolute paths with path.sep - Fix Windows test expectation to reflect correct 2-levels-up behavior Co-authored-by: karthiknadig <3840081+karthiknadig@users.noreply.github.com>
1 parent 9271724 commit 12033b0

File tree

2 files changed

+29
-15
lines changed

2 files changed

+29
-15
lines changed

src/managers/pyenv/pyenvUtils.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@ import { shortVersion, sortEnvironments } from '../common/utils';
2525

2626
/**
2727
* Returns the pyenv root directory from the pyenv executable path.
28-
* On Windows, pyenv-win is at `~/.pyenv/pyenv-win/bin/pyenv.bat` (3 levels up).
29-
* On POSIX, pyenv is at `~/.pyenv/bin/pyenv` (2 levels up).
28+
* Prefers `PYENV_ROOT` env var when set, otherwise goes up 2 levels from the binary.
29+
* On POSIX, pyenv binary is at `<root>/bin/pyenv` (e.g. `~/.pyenv/bin/pyenv`).
30+
* On Windows, pyenv-win binary is at `<root>/bin/pyenv.bat` (e.g. `~/.pyenv/pyenv-win/bin/pyenv.bat`,
31+
* where `<root>` is `~/.pyenv/pyenv-win`).
3032
*/
3133
export function getPyenvDir(pyenv: string): string {
32-
if (isWindows()) {
33-
return path.dirname(path.dirname(path.dirname(pyenv)));
34+
const pyenvRoot = process.env.PYENV_ROOT;
35+
if (pyenvRoot) {
36+
return pyenvRoot;
3437
}
3538
return path.dirname(path.dirname(pyenv));
3639
}
Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,44 @@
11
import assert from 'node:assert';
22
import * as path from 'path';
33
import * as sinon from 'sinon';
4-
import * as platformUtils from '../../../common/utils/platformUtils';
54
import { getPyenvDir } from '../../../managers/pyenv/pyenvUtils';
65

76
suite('pyenvUtils - getPyenvDir', () => {
8-
let isWindowsStub: sinon.SinonStub;
7+
let originalPyenvRoot: string | undefined;
98

109
setup(() => {
11-
isWindowsStub = sinon.stub(platformUtils, 'isWindows');
10+
originalPyenvRoot = process.env.PYENV_ROOT;
11+
delete process.env.PYENV_ROOT;
1212
});
1313

1414
teardown(() => {
1515
sinon.restore();
16+
if (originalPyenvRoot !== undefined) {
17+
process.env.PYENV_ROOT = originalPyenvRoot;
18+
} else {
19+
delete process.env.PYENV_ROOT;
20+
}
1621
});
1722

18-
test('should go up 2 levels on POSIX (bin/pyenv -> pyenv root)', () => {
19-
isWindowsStub.returns(false);
23+
test('should use PYENV_ROOT when set', () => {
24+
const pyenvRoot = path.join(path.sep, 'custom', 'pyenv', 'root');
25+
process.env.PYENV_ROOT = pyenvRoot;
26+
const pyenvBin = path.join(path.sep, 'other', 'bin', 'pyenv');
27+
const result = getPyenvDir(pyenvBin);
28+
assert.strictEqual(result, pyenvRoot);
29+
});
30+
31+
test('should go up 2 levels on POSIX when PYENV_ROOT is not set (bin/pyenv -> pyenv root)', () => {
2032
// e.g. /home/user/.pyenv/bin/pyenv
21-
const pyenvBin = path.join('home', 'user', '.pyenv', 'bin', 'pyenv');
33+
const pyenvBin = path.join(path.sep, 'home', 'user', '.pyenv', 'bin', 'pyenv');
2234
const result = getPyenvDir(pyenvBin);
23-
assert.strictEqual(result, path.join('home', 'user', '.pyenv'));
35+
assert.strictEqual(result, path.join(path.sep, 'home', 'user', '.pyenv'));
2436
});
2537

26-
test('should go up 3 levels on Windows (pyenv-win/bin/pyenv.bat -> pyenv root)', () => {
27-
isWindowsStub.returns(true);
28-
// e.g. C:\Users\user\.pyenv\pyenv-win\bin\pyenv.bat
38+
test('should go up 2 levels on Windows when PYENV_ROOT is not set (pyenv-win/bin/pyenv.bat -> pyenv-win)', () => {
39+
// e.g. C:\Users\user\.pyenv\pyenv-win\bin\pyenv.bat -> C:\Users\user\.pyenv\pyenv-win
2940
const pyenvBin = path.join('C:', 'Users', 'user', '.pyenv', 'pyenv-win', 'bin', 'pyenv.bat');
3041
const result = getPyenvDir(pyenvBin);
31-
assert.strictEqual(result, path.join('C:', 'Users', 'user', '.pyenv'));
42+
assert.strictEqual(result, path.join('C:', 'Users', 'user', '.pyenv', 'pyenv-win'));
3243
});
3344
});

0 commit comments

Comments
 (0)