From 48b6ff25614288234d83b51117a8c19db63c338a Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 4 Jun 2020 16:55:04 -0700 Subject: [PATCH 1/5] Get interpreters if hasInterpreters returns false --- news/2 Fixes/11870.md | 1 + .../diagnostics/checks/pythonInterpreter.ts | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 news/2 Fixes/11870.md diff --git a/news/2 Fixes/11870.md b/news/2 Fixes/11870.md new file mode 100644 index 000000000000..05ada1f0b016 --- /dev/null +++ b/news/2 Fixes/11870.md @@ -0,0 +1 @@ +Double-check for interpreters when running diagnostics before displaying the "Python is not installed" message. diff --git a/src/client/application/diagnostics/checks/pythonInterpreter.ts b/src/client/application/diagnostics/checks/pythonInterpreter.ts index f4219f0c8c4b..4e1db14e2824 100644 --- a/src/client/application/diagnostics/checks/pythonInterpreter.ts +++ b/src/client/application/diagnostics/checks/pythonInterpreter.ts @@ -67,9 +67,17 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService { } const interpreterService = this.serviceContainer.get(IInterpreterService); - const hasInterpreters = await interpreterService.hasInterpreters; + let hasInterpreters = await interpreterService.hasInterpreters; if (!hasInterpreters) { + // hasInterpreters being false can mean one of 2 things: + // 1. getInterpreters hasn't returned any interpreters; + // 2. getInterpreters hasn't run yet. + // We want to make sure that false comes from 1, so we're adding this fix until we refactor interpreter discovery. + // Also see https://github.com/microsoft/vscode-python/issues/3023. + interpreterService.getInterpreters(resource).ignoreErrors(); + hasInterpreters = await interpreterService.hasInterpreters; + return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic, resource)]; } From 0b8f44cd35e8fb15d1aa3fc723255d9c6e73d793 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 4 Jun 2020 20:01:50 -0700 Subject: [PATCH 2/5] Undo ignoreErrors() --- .../diagnostics/checks/pythonInterpreter.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/client/application/diagnostics/checks/pythonInterpreter.ts b/src/client/application/diagnostics/checks/pythonInterpreter.ts index 4e1db14e2824..6040234f7f2a 100644 --- a/src/client/application/diagnostics/checks/pythonInterpreter.ts +++ b/src/client/application/diagnostics/checks/pythonInterpreter.ts @@ -67,17 +67,16 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService { } const interpreterService = this.serviceContainer.get(IInterpreterService); - let hasInterpreters = await interpreterService.hasInterpreters; + // hasInterpreters being false can mean one of 2 things: + // 1. getInterpreters hasn't returned any interpreters; + // 2. getInterpreters hasn't run yet. + // We want to make sure that false comes from 1, so we're adding this fix until we refactor interpreter discovery. + // Also see https://github.com/microsoft/vscode-python/issues/3023. + const hasInterpreters = + (await interpreterService.hasInterpreters) || + (await interpreterService.getInterpreters(resource)).length > 0; if (!hasInterpreters) { - // hasInterpreters being false can mean one of 2 things: - // 1. getInterpreters hasn't returned any interpreters; - // 2. getInterpreters hasn't run yet. - // We want to make sure that false comes from 1, so we're adding this fix until we refactor interpreter discovery. - // Also see https://github.com/microsoft/vscode-python/issues/3023. - interpreterService.getInterpreters(resource).ignoreErrors(); - hasInterpreters = await interpreterService.hasInterpreters; - return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic, resource)]; } From 63519486a71ffbcc06da7ed03d9c1e1f35d82746 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 4 Jun 2020 20:01:57 -0700 Subject: [PATCH 3/5] Add unit tests --- .../checks/pythonInterpreter.unit.test.ts | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts index 0520b9d85bb1..6d3ba23af1aa 100644 --- a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts @@ -130,7 +130,7 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { expect(diagnostics).to.be.deep.equal([]); settings.verifyAll(); }); - test('Should return diagnostics if there are no interpreters', async () => { + test('Should return diagnostics if there are no interpreters after double-checking', async () => { settings .setup((s) => s.disableInstallationChecks) .returns(() => false) @@ -139,6 +139,10 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { .setup((i) => i.hasInterpreters) .returns(() => Promise.resolve(false)) .verifiable(typemoq.Times.once()); + interpreterService + .setup((i) => i.getInterpreters(undefined)) + .returns(() => Promise.resolve([])) + .verifiable(typemoq.Times.once()); const diagnostics = await diagnosticService.diagnose(undefined); expect(diagnostics).to.be.deep.equal( @@ -148,6 +152,25 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { settings.verifyAll(); interpreterService.verifyAll(); }); + test('Should return empty diagnostics if there are interpreters after double-checking', async () => { + settings + .setup((s) => s.disableInstallationChecks) + .returns(() => false) + .verifiable(typemoq.Times.once()); + interpreterService + .setup((i) => i.hasInterpreters) + .returns(() => Promise.resolve(false)) + .verifiable(typemoq.Times.once()); + interpreterService + .setup((i) => i.getInterpreters(undefined)) + .returns(() => Promise.resolve([{ type: InterpreterType.Unknown } as any])) + .verifiable(typemoq.Times.once()); + + const diagnostics = await diagnosticService.diagnose(undefined); + expect(diagnostics).to.be.deep.equal([], 'not the same'); + settings.verifyAll(); + interpreterService.verifyAll(); + }); test('Should return invalid diagnostics if there are interpreters but no current interpreter', async () => { settings .setup((s) => s.disableInstallationChecks) From 899c50c5ac87902c79beecc61e1ff36ce995a74e Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 5 Jun 2020 09:19:16 -0700 Subject: [PATCH 4/5] Fixed tests --- .../checks/pythonInterpreter.unit.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts index 6d3ba23af1aa..9fca871a1923 100644 --- a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts @@ -31,7 +31,7 @@ import { IConfigurationService, IDisposableRegistry, IPythonSettings } from '../ import { noop } from '../../../../client/common/utils/misc'; import { IInterpreterHelper, IInterpreterService } from '../../../../client/interpreter/contracts'; import { IServiceContainer } from '../../../../client/ioc/types'; -import { InterpreterType } from '../../../../client/pythonEnvironments/discovery/types'; +import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/discovery/types'; suite('Application Diagnostics - Checks Python Interpreter', () => { let diagnosticService: IDiagnosticsService; @@ -153,6 +153,8 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { interpreterService.verifyAll(); }); test('Should return empty diagnostics if there are interpreters after double-checking', async () => { + const interpreter: PythonInterpreter = { type: InterpreterType.Unknown } as any; + settings .setup((s) => s.disableInstallationChecks) .returns(() => false) @@ -163,7 +165,13 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { .verifiable(typemoq.Times.once()); interpreterService .setup((i) => i.getInterpreters(undefined)) - .returns(() => Promise.resolve([{ type: InterpreterType.Unknown } as any])) + .returns(() => Promise.resolve([interpreter])) + .verifiable(typemoq.Times.once()); + interpreterService + .setup((i) => i.getActiveInterpreter(typemoq.It.isAny())) + .returns(() => { + return Promise.resolve(interpreter); + }) .verifiable(typemoq.Times.once()); const diagnostics = await diagnosticService.diagnose(undefined); From 3c466464f7fd3caba670e364eddbd59777f3f0e0 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 5 Jun 2020 11:04:31 -0700 Subject: [PATCH 5/5] Newline --- .../diagnostics/checks/pythonInterpreter.unit.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts index 9fca871a1923..e0dfed705c65 100644 --- a/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts @@ -175,6 +175,7 @@ suite('Application Diagnostics - Checks Python Interpreter', () => { .verifiable(typemoq.Times.once()); const diagnostics = await diagnosticService.diagnose(undefined); + expect(diagnostics).to.be.deep.equal([], 'not the same'); settings.verifyAll(); interpreterService.verifyAll();