Skip to content

Commit 94bc061

Browse files
Cherry pick fix for hasInterpreters and update change log (#12198)
* Double-check for interpreters when running diagnostics (#12158) * Get interpreters if hasInterpreters returns false * Undo ignoreErrors() * Add unit tests * Fixed tests * Newline * Fix merge issues. * Update change log Co-authored-by: Kim-Adeline Miguel <[email protected]>
1 parent 18316da commit 94bc061

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### Fixes
66

7+
1. Double-check for interpreters when running diagnostics before displaying the "Python is not installed" message.
8+
([#11870](https://github.com/Microsoft/vscode-python/issues/11870))
79
1. Ensure user cannot belong to all experiments in an experiment group.
810
([#11943](https://github.com/Microsoft/vscode-python/issues/11943))
911
1. Ensure extension features are started when in `Deprecate PythonPath` experiment and opening a file without any folder opened.

src/client/application/diagnostics/checks/pythonInterpreter.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,14 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
6767
}
6868

6969
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
70-
const hasInterpreters = await interpreterService.hasInterpreters;
70+
// hasInterpreters being false can mean one of 2 things:
71+
// 1. getInterpreters hasn't returned any interpreters;
72+
// 2. getInterpreters hasn't run yet.
73+
// We want to make sure that false comes from 1, so we're adding this fix until we refactor interpreter discovery.
74+
// Also see https://github.com/microsoft/vscode-python/issues/3023.
75+
const hasInterpreters =
76+
(await interpreterService.hasInterpreters) ||
77+
(await interpreterService.getInterpreters(resource)).length > 0;
7178

7279
if (!hasInterpreters) {
7380
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic, resource)];

src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ import { CommandsWithoutArgs } from '../../../../client/common/application/comma
2929
import { IPlatformService } from '../../../../client/common/platform/types';
3030
import { IConfigurationService, IDisposableRegistry, IPythonSettings } from '../../../../client/common/types';
3131
import { noop } from '../../../../client/common/utils/misc';
32-
import { IInterpreterHelper, IInterpreterService, InterpreterType } from '../../../../client/interpreter/contracts';
32+
import {
33+
IInterpreterHelper,
34+
IInterpreterService,
35+
InterpreterType,
36+
PythonInterpreter
37+
} from '../../../../client/interpreter/contracts';
3338
import { IServiceContainer } from '../../../../client/ioc/types';
3439

3540
suite('Application Diagnostics - Checks Python Interpreter', () => {
@@ -129,7 +134,7 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
129134
expect(diagnostics).to.be.deep.equal([]);
130135
settings.verifyAll();
131136
});
132-
test('Should return diagnostics if there are no interpreters', async () => {
137+
test('Should return diagnostics if there are no interpreters after double-checking', async () => {
133138
settings
134139
.setup((s) => s.disableInstallationChecks)
135140
.returns(() => false)
@@ -138,6 +143,10 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
138143
.setup((i) => i.hasInterpreters)
139144
.returns(() => Promise.resolve(false))
140145
.verifiable(typemoq.Times.once());
146+
interpreterService
147+
.setup((i) => i.getInterpreters(undefined))
148+
.returns(() => Promise.resolve([]))
149+
.verifiable(typemoq.Times.once());
141150

142151
const diagnostics = await diagnosticService.diagnose(undefined);
143152
expect(diagnostics).to.be.deep.equal(
@@ -147,6 +156,34 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
147156
settings.verifyAll();
148157
interpreterService.verifyAll();
149158
});
159+
test('Should return empty diagnostics if there are interpreters after double-checking', async () => {
160+
const interpreter: PythonInterpreter = { type: InterpreterType.Unknown } as any;
161+
162+
settings
163+
.setup((s) => s.disableInstallationChecks)
164+
.returns(() => false)
165+
.verifiable(typemoq.Times.once());
166+
interpreterService
167+
.setup((i) => i.hasInterpreters)
168+
.returns(() => Promise.resolve(false))
169+
.verifiable(typemoq.Times.once());
170+
interpreterService
171+
.setup((i) => i.getInterpreters(undefined))
172+
.returns(() => Promise.resolve([interpreter]))
173+
.verifiable(typemoq.Times.once());
174+
interpreterService
175+
.setup((i) => i.getActiveInterpreter(typemoq.It.isAny()))
176+
.returns(() => {
177+
return Promise.resolve(interpreter);
178+
})
179+
.verifiable(typemoq.Times.once());
180+
181+
const diagnostics = await diagnosticService.diagnose(undefined);
182+
183+
expect(diagnostics).to.be.deep.equal([], 'not the same');
184+
settings.verifyAll();
185+
interpreterService.verifyAll();
186+
});
150187
test('Should return invalid diagnostics if there are interpreters but no current interpreter', async () => {
151188
settings
152189
.setup((s) => s.disableInstallationChecks)

0 commit comments

Comments
 (0)