Skip to content

Commit 091c402

Browse files
author
Kartik Raj
authored
Ensure we set display accordingly for envs lacking an interpreter (#18773)
* Display env folder path in interpreter display for environments lacking an interpreter * Fix tests * Ensure we set display accordingly for envs lacking an interpreter * Fix unit tests
1 parent ad3fb29 commit 091c402

File tree

9 files changed

+74
-20
lines changed

9 files changed

+74
-20
lines changed

src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
156156
return this._enterOrBrowseInterpreterPath(input, state, suggestions);
157157
} else {
158158
sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'selected' });
159-
const { interpreter } = selection as IInterpreterQuickPickItem;
160-
state.path = interpreter.envType === EnvironmentType.Conda ? interpreter.envPath : interpreter.path;
159+
state.path = (selection as IInterpreterQuickPickItem).path;
161160
}
162161

163162
return undefined;

src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { inject, injectable } from 'inversify';
77
import { Disposable, Uri } from 'vscode';
88
import { arePathsSame } from '../../../common/platform/fs-paths';
99
import { IPathUtils, Resource } from '../../../common/types';
10-
import { EnvironmentType, PythonEnvironment } from '../../../pythonEnvironments/info';
10+
import { getEnvPath } from '../../../pythonEnvironments/base/info/env';
11+
import { PythonEnvironment } from '../../../pythonEnvironments/info';
1112
import { IInterpreterService } from '../../contracts';
1213
import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector } from '../types';
1314

@@ -44,17 +45,16 @@ export class InterpreterSelector implements IInterpreterSelector {
4445
workspaceUri?: Uri,
4546
useDetailedName = false,
4647
): IInterpreterQuickPickItem {
47-
const detail = this.pathUtils.getDisplayName(
48-
interpreter.envType === EnvironmentType.Conda && interpreter.envPath
48+
const path =
49+
interpreter.envPath && getEnvPath(interpreter.path, interpreter.envPath).pathType === 'envFolderPath'
4950
? interpreter.envPath
50-
: interpreter.path,
51-
workspaceUri ? workspaceUri.fsPath : undefined,
52-
);
51+
: interpreter.path;
52+
const detail = this.pathUtils.getDisplayName(path, workspaceUri ? workspaceUri.fsPath : undefined);
5353
const cachedPrefix = interpreter.cachedEntry ? '(cached) ' : '';
5454
return {
5555
label: (useDetailedName ? interpreter.detailedDisplayName : interpreter.displayName) || 'Python',
5656
description: `${cachedPrefix}${detail}`,
57-
path: interpreter.path,
57+
path,
5858
interpreter,
5959
};
6060
}

src/client/pythonEnvironments/base/locators/composite/envsResolver.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Event, EventEmitter } from 'vscode';
66
import { identifyEnvironment } from '../../../common/environmentIdentifier';
77
import { IEnvironmentInfoService } from '../../info/environmentInfoService';
88
import { PythonEnvInfo } from '../../info';
9-
import { setEnvDisplayString } from '../../info/env';
9+
import { getEnvPath, setEnvDisplayString } from '../../info/env';
1010
import { InterpreterInformation } from '../../info/interpreter';
1111
import {
1212
BasicEnvInfo,
@@ -20,6 +20,7 @@ import { PythonEnvsChangedEvent } from '../../watcher';
2020
import { resolveBasicEnv } from './resolverUtils';
2121
import { traceVerbose } from '../../../../logging';
2222
import { getEnvironmentDirFromPath, getInterpreterPathFromDir, isPythonExecutable } from '../../../common/commonUtils';
23+
import { getEmptyVersion } from '../../info/pythonVersion';
2324

2425
/**
2526
* Calls environment info service which runs `interpreterInfo.py` script on environments received
@@ -145,9 +146,17 @@ function checkIfFinishedAndNotify(
145146
function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: PythonEnvInfo) {
146147
// Deep copy into a new object
147148
const resolvedEnv = cloneDeep(environment);
148-
resolvedEnv.version = interpreterInfo.version;
149149
resolvedEnv.executable.filename = interpreterInfo.executable.filename;
150150
resolvedEnv.executable.sysPrefix = interpreterInfo.executable.sysPrefix;
151+
const isEnvLackingPython =
152+
getEnvPath(resolvedEnv.executable.filename, resolvedEnv.location).pathType === 'envFolderPath';
153+
if (isEnvLackingPython) {
154+
// Install python later into these envs might change the version, which can be confusing for users.
155+
// So avoid displaying any version until it is installed.
156+
resolvedEnv.version = getEmptyVersion();
157+
} else {
158+
resolvedEnv.version = interpreterInfo.version;
159+
}
151160
resolvedEnv.arch = interpreterInfo.arch;
152161
// Display name should be set after all the properties as we need other properties to build display name.
153162
setEnvDisplayString(resolvedEnv);

src/client/pythonEnvironments/common/environmentManagers/conda.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ export class Conda {
416416
const parentDir = path.dirname(prefix);
417417
if (info.envs_dirs !== undefined) {
418418
for (const envsDir of info.envs_dirs) {
419-
if (parentDir === envsDir) {
419+
if (arePathsSame(parentDir, envsDir)) {
420420
return path.basename(prefix);
421421
}
422422
}

src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ suite('Set Interpreter Command', () => {
112112
description: interpreterPath,
113113
detail: '',
114114
label: 'This is the selected Python path',
115-
path: path.dirname(interpreterPath),
115+
path: `path/to/envFolder`,
116116
interpreter: {
117117
path: interpreterPath,
118118
id: interpreterPath,
119119
envType: EnvironmentType.Conda,
120-
envPath: path.dirname(interpreterPath),
120+
envPath: `path/to/envFolder`,
121121
} as PythonEnvironment,
122122
};
123123
const defaultInterpreterPath = 'defaultInterpreterPath';
@@ -132,12 +132,12 @@ suite('Set Interpreter Command', () => {
132132
description: interpreterPath,
133133
detail: '',
134134
label: 'Refreshed path',
135-
path: path.dirname(interpreterPath),
135+
path: `path/to/envFolder`,
136136
interpreter: {
137137
path: interpreterPath,
138138
id: interpreterPath,
139139
envType: EnvironmentType.Conda,
140-
envPath: path.dirname(interpreterPath),
140+
envPath: `path/to/envFolder`,
141141
} as PythonEnvironment,
142142
};
143143
const expectedEnterInterpreterPathSuggestion = {
@@ -624,7 +624,7 @@ suite('Set Interpreter Command', () => {
624624

625625
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);
626626

627-
expect(state.path).to.equal(item.path, '');
627+
expect(state.path).to.equal(item.interpreter.envPath, '');
628628
});
629629

630630
test('If an item is selected, send SELECT_INTERPRETER_SELECTED telemetry with the "selected" property value', async () => {

src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ suite('Interpreters - selector', () => {
8686
{ displayName: '2 (virtualenv)', path: 'c:/path2/path2', envType: EnvironmentType.VirtualEnv },
8787
{ displayName: '3', path: 'c:/path2/path2', envType: EnvironmentType.Unknown },
8888
{ displayName: '4', path: 'c:/path4/path4', envType: EnvironmentType.Conda },
89-
{ displayName: '5', path: 'c:/path5/path', envPath: 'c:/path5', envType: EnvironmentType.Conda },
89+
{
90+
displayName: '5',
91+
path: 'c:/path5/path',
92+
envPath: 'c:/path5/path/to/env',
93+
envType: EnvironmentType.Conda,
94+
},
9095
].map((item) => ({ ...info, ...item }));
9196
interpreterService
9297
.setup((x) => x.getAllInterpreters(TypeMoq.It.isAny()))
@@ -101,7 +106,7 @@ suite('Interpreters - selector', () => {
101106
new InterpreterQuickPickItem('2 (virtualenv)', 'c:/path2/path2'),
102107
new InterpreterQuickPickItem('3', 'c:/path2/path2'),
103108
new InterpreterQuickPickItem('4', 'c:/path4/path4'),
104-
new InterpreterQuickPickItem('5', 'c:/path5/path', 'c:/path5'),
109+
new InterpreterQuickPickItem('5', 'c:/path5/path/to/env', 'c:/path5/path/to/env'),
105110
];
106111

107112
assert.strictEqual(actual.length, expected.length, 'Suggestion lengths are different.');

src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
PythonVersion,
1717
UNKNOWN_PYTHON_VERSION,
1818
} from '../../../../../client/pythonEnvironments/base/info';
19-
import { parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion';
19+
import { getEmptyVersion, parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion';
2020
import { BasicEnvInfo, PythonEnvUpdatedEvent } from '../../../../../client/pythonEnvironments/base/locator';
2121
import { PythonEnvsResolver } from '../../../../../client/pythonEnvironments/base/locators/composite/envsResolver';
2222
import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher';
@@ -29,6 +29,7 @@ import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants';
2929
import { assertEnvEqual, assertEnvsEqual } from '../envTestUtils';
3030
import { createBasicEnv, getEnvs, getEnvsWithUpdates, SimpleLocator } from '../../common';
3131
import { getOSType, OSType } from '../../../../common';
32+
import { CondaInfo } from '../../../../../client/pythonEnvironments/common/environmentManagers/conda';
3233

3334
suite('Python envs locator - Environments Resolver', () => {
3435
let envInfoService: IEnvironmentInfoService;
@@ -240,6 +241,18 @@ suite('Python envs locator - Environments Resolver', () => {
240241

241242
suite('resolveEnv()', () => {
242243
let stubShellExec: sinon.SinonStub;
244+
const envsWithoutPython = path.join(TEST_LAYOUT_ROOT, 'envsWithoutPython');
245+
function condaInfo(condaPrefix: string): CondaInfo {
246+
return {
247+
conda_version: '4.8.0',
248+
python_version: '3.9.0',
249+
'sys.version': '3.9.0',
250+
'sys.prefix': '/some/env',
251+
root_prefix: '/some/prefix',
252+
envs: [condaPrefix],
253+
envs_dirs: [path.dirname(condaPrefix)],
254+
};
255+
}
243256
setup(() => {
244257
sinon.stub(platformApis, 'getOSType').callsFake(() => platformApis.OSType.Windows);
245258
stubShellExec = sinon.stub(externalDependencies, 'shellExecute');
@@ -280,6 +293,32 @@ suite('Python envs locator - Environments Resolver', () => {
280293
);
281294
});
282295

296+
test('Resolver should return empty version info for envs lacking an interpreter', async function () {
297+
if (getOSType() !== OSType.Windows) {
298+
this.skip();
299+
}
300+
stubShellExec.restore();
301+
sinon.stub(externalDependencies, 'getPythonSetting').withArgs('condaPath').returns('conda');
302+
sinon.stub(externalDependencies, 'shellExecute').callsFake(async (quoted: string) => {
303+
const [command, ...args] = quoted.split(' ');
304+
if (command === 'conda' && args[0] === 'info' && args[1] === '--json') {
305+
return { stdout: JSON.stringify(condaInfo(path.join(envsWithoutPython, 'condaLackingPython'))) };
306+
}
307+
return {
308+
stdout:
309+
'{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
310+
};
311+
});
312+
const parentLocator = new SimpleLocator([]);
313+
const resolver = new PythonEnvsResolver(parentLocator, envInfoService);
314+
315+
const expected = await resolver.resolveEnv(path.join(envsWithoutPython, 'condaLackingPython'));
316+
317+
assert.deepEqual(expected?.version, getEmptyVersion());
318+
assert.equal(expected?.display, "Python ('condaLackingPython')");
319+
assert.equal(expected?.detailedDisplayName, "Python ('condaLackingPython': conda)");
320+
});
321+
283322
test('If running interpreter info throws error, return undefined', async () => {
284323
stubShellExec.returns(
285324
new Promise<ExecutionResult<string>>((_resolve, reject) => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Usually contains command that was used to create or update the conda environment with time stamps.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Not real python exe

0 commit comments

Comments
 (0)