Skip to content

Commit

Permalink
Double-check for interpreters when running diagnostics (#12158)
Browse files Browse the repository at this point in the history
* Get interpreters if hasInterpreters returns false
* Undo ignoreErrors()
* Add unit tests
* Fixed tests
* Newline
  • Loading branch information
kimadeline committed Jun 8, 2020
1 parent e129bdf commit 8328187
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 3 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/11870.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Double-check for interpreters when running diagnostics before displaying the "Python is not installed" message.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,14 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
}

const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
const 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) {
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoPythonInterpretersDiagnostic, resource)];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -148,6 +152,34 @@ suite('Application Diagnostics - Checks Python Interpreter', () => {
settings.verifyAll();
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)
.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([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);

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)
Expand Down

0 comments on commit 8328187

Please sign in to comment.