Skip to content

Commit 8328187

Browse files
authored
Double-check for interpreters when running diagnostics (microsoft#12158)
* Get interpreters if hasInterpreters returns false * Undo ignoreErrors() * Add unit tests * Fixed tests * Newline
1 parent e129bdf commit 8328187

File tree

3 files changed

+43
-3
lines changed

3 files changed

+43
-3
lines changed

news/2 Fixes/11870.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Double-check for interpreters when running diagnostics before displaying the "Python is not installed" message.

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: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { IConfigurationService, IDisposableRegistry, IPythonSettings } from '../
3131
import { noop } from '../../../../client/common/utils/misc';
3232
import { IInterpreterHelper, IInterpreterService } from '../../../../client/interpreter/contracts';
3333
import { IServiceContainer } from '../../../../client/ioc/types';
34-
import { InterpreterType } from '../../../../client/pythonEnvironments/discovery/types';
34+
import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/discovery/types';
3535

3636
suite('Application Diagnostics - Checks Python Interpreter', () => {
3737
let diagnosticService: IDiagnosticsService;
@@ -130,7 +130,7 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
130130
expect(diagnostics).to.be.deep.equal([]);
131131
settings.verifyAll();
132132
});
133-
test('Should return diagnostics if there are no interpreters', async () => {
133+
test('Should return diagnostics if there are no interpreters after double-checking', async () => {
134134
settings
135135
.setup((s) => s.disableInstallationChecks)
136136
.returns(() => false)
@@ -139,6 +139,10 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
139139
.setup((i) => i.hasInterpreters)
140140
.returns(() => Promise.resolve(false))
141141
.verifiable(typemoq.Times.once());
142+
interpreterService
143+
.setup((i) => i.getInterpreters(undefined))
144+
.returns(() => Promise.resolve([]))
145+
.verifiable(typemoq.Times.once());
142146

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

0 commit comments

Comments
 (0)