Pass interpreter to shouldShowPrompt directly (microsoft#17042)

kimadeline · web-flow · commit 3e3c0dba2a0a · 2021-08-19T16:44:04.000-07:00
diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts
@@ -181,6 +181,6 @@ export type GetInterpreterOptions = { ignoreCache?: boolean; onSuggestion?: bool
 
 export const IPython27SupportPrompt = Symbol('IPython27SupportPrompt');
 export interface IPython27SupportPrompt {
-    shouldShowPrompt(resource?: Uri): Promise<boolean>;
+    shouldShowPrompt(interpreter: PythonEnvironment): Promise<boolean>;
     showPrompt(): Promise<void>;
 }
diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts
@@ -93,7 +93,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {
         const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder);
         this.currentlySelectedWorkspaceFolder = workspaceFolder;
         if (interpreter) {
-            if (await this.python27SupportPrompt.shouldShowPrompt(workspaceFolder)) {
+            if (await this.python27SupportPrompt.shouldShowPrompt(interpreter)) {
                 this.python27SupportPrompt.showPrompt();
             }
 
diff --git a/src/client/interpreter/display/python27Prompt.ts b/src/client/interpreter/display/python27Prompt.ts
@@ -4,13 +4,13 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
-import { Uri } from 'vscode';
 import { IApplicationShell } from '../../common/application/types';
 import { IPersistentStateFactory } from '../../common/types';
 import { Common, Python27Support } from '../../common/utils/localize';
-import { IInterpreterService, IPython27SupportPrompt } from '../contracts';
+import { IPython27SupportPrompt } from '../contracts';
 import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
+import { PythonEnvironment } from '../../pythonEnvironments/info';
 
 const doNotShowPromptStateKey = 'MESSAGE_KEY_FOR_27_SUPPORT_PROMPT';
 
@@ -21,11 +21,10 @@ export class Python27SupportPrompt implements IPython27SupportPrompt {
 
     constructor(
         @inject(IApplicationShell) private appShell: IApplicationShell,
-        @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
         @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory,
     ) {}
 
-    public async shouldShowPrompt(resource?: Uri): Promise<boolean> {
+    public async shouldShowPrompt(interpreter: PythonEnvironment): Promise<boolean> {
         // Check if "Do not show again" has been selected before.
         const doNotShowAgain = this.persistentState.createGlobalPersistentState<boolean>(
             doNotShowPromptStateKey,
@@ -36,10 +35,8 @@ export class Python27SupportPrompt implements IPython27SupportPrompt {
             return Promise.resolve(false);
         }
 
-        // Check if current env version is Python 2.7
-        const currentInterpreter = await this.interpreterService.getActiveInterpreter(resource);
-
-        if (currentInterpreter && currentInterpreter.version?.major === 2 && currentInterpreter.version?.minor === 7) {
+        // Check if current environment version is Python 2.7
+        if (interpreter.version?.major === 2 && interpreter.version?.minor === 7) {
             return Promise.resolve(true);
         }
 
diff --git a/src/test/interpreters/display/python27Prompt.unit.test.ts b/src/test/interpreters/display/python27Prompt.unit.test.ts
@@ -11,24 +11,20 @@ import { IApplicationShell } from '../../../client/common/application/types';
 import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState';
 import { IPersistentStateFactory } from '../../../client/common/types';
 import { Python27Support, Common } from '../../../client/common/utils/localize';
-import { IInterpreterService } from '../../../client/interpreter/contracts';
 import { Python27SupportPrompt } from '../../../client/interpreter/display/python27Prompt';
-import { InterpreterService } from '../../../client/interpreter/interpreterService';
 import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
 import * as Telemetry from '../../../client/telemetry';
 import { EventName } from '../../../client/telemetry/constants';
 
 suite('Python 2.7 support prompt', () => {
     let applicationShell: IApplicationShell;
-    let interpreterService: IInterpreterService;
     let persistentStateFactory: IPersistentStateFactory;
     let state: PersistentState<boolean>;
     let sendTelemetryEventStub: sinon.SinonStub;
     let telemetryEvents: { eventName: string; properties: Record<string, unknown> }[] = [];
 
     setup(() => {
         applicationShell = mock(ApplicationShell);
-        interpreterService = mock(InterpreterService);
         persistentStateFactory = mock(PersistentStateFactory);
         state = mock(PersistentState) as PersistentState<boolean>;
 
@@ -56,38 +52,33 @@ suite('Python 2.7 support prompt', () => {
     type TestCaseType = {
         doNotShow: boolean;
         interpreter: { version: { major: number; minor: number } };
-        getActiveInterpreterCalled: boolean;
         expected: boolean;
     };
 
     const testCases: TestCaseType[] = [
         {
             doNotShow: false,
             interpreter: { version: { major: 2, minor: 7 } },
-            getActiveInterpreterCalled: true,
             expected: true,
         },
         {
             doNotShow: false,
             interpreter: { version: { major: 3, minor: 9 } },
-            getActiveInterpreterCalled: true,
             expected: false,
         },
         {
             doNotShow: true,
             interpreter: { version: { major: 2, minor: 7 } },
-            getActiveInterpreterCalled: false,
             expected: false,
         },
         {
             doNotShow: true,
             interpreter: { version: { major: 3, minor: 9 } },
-            getActiveInterpreterCalled: false,
             expected: false,
         },
     ];
 
-    testCases.forEach(({ doNotShow, interpreter, getActiveInterpreterCalled, expected }) => {
+    testCases.forEach(({ doNotShow, interpreter, expected }) => {
         const testTitle = `Should${
             !expected ? ' not' : ''
         } show prompt if do not show is ${doNotShow} and interpreter is ${interpreter.version.major}.${
@@ -96,35 +87,29 @@ suite('Python 2.7 support prompt', () => {
 
         test(testTitle, async () => {
             when(state.value).thenReturn(doNotShow);
-            when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter as PythonEnvironment);
 
             const python27SupportPrompt = new Python27SupportPrompt(
                 instance(applicationShell),
-                instance(interpreterService),
                 instance(persistentStateFactory),
             );
 
-            const result = await python27SupportPrompt.shouldShowPrompt();
+            const result = await python27SupportPrompt.shouldShowPrompt(interpreter as PythonEnvironment);
 
             assert.strictEqual(result, expected);
-            if (getActiveInterpreterCalled) {
-                verify(interpreterService.getActiveInterpreter(anything())).once();
-            } else {
-                verify(interpreterService.getActiveInterpreter(anything())).never();
-            }
         });
     });
 
     test('Should not show prompt if it has been shown earlier in the session', async () => {
         const python27SupportPrompt = new Python27SupportPrompt(
             instance(applicationShell),
-            instance(interpreterService),
             instance(persistentStateFactory),
         );
 
         await python27SupportPrompt.showPrompt();
 
-        const result = await python27SupportPrompt.shouldShowPrompt();
+        const result = await python27SupportPrompt.shouldShowPrompt({
+            version: { major: 2, minor: 7 },
+        } as PythonEnvironment);
 
         assert.strictEqual(result, false);
     });
@@ -146,7 +131,6 @@ suite('Python 2.7 support prompt', () => {
 
             const python27SupportPrompt = new Python27SupportPrompt(
                 instance(appShell),
-                instance(interpreterService),
                 instance(persistentStateFactory),
             );
 
@@ -164,7 +148,6 @@ suite('Python 2.7 support prompt', () => {
 
             const python27SupportPrompt = new Python27SupportPrompt(
                 instance(appShell),
-                instance(interpreterService),
                 instance(persistentStateFactory),
             );
 
@@ -176,7 +159,6 @@ suite('Python 2.7 support prompt', () => {
         test('Telemetry event should be sent when prompt is shown', async () => {
             const python27SupportPrompt = new Python27SupportPrompt(
                 instance(applicationShell),
-                instance(interpreterService),
                 instance(persistentStateFactory),
             );
 

Original file line number	Diff line number	Diff line change
`@@ -181,6 +181,6 @@ export type GetInterpreterOptions = { ignoreCache?: boolean; onSuggestion?: bool`
`181`	`181`
`182`	`182`	`export const IPython27SupportPrompt = Symbol('IPython27SupportPrompt');`
`183`	`183`	`export interface IPython27SupportPrompt {`
`184`		`- shouldShowPrompt(resource?: Uri): Promise<boolean>;`
	`184`	`+ shouldShowPrompt(interpreter: PythonEnvironment): Promise<boolean>;`
`185`	`185`	`showPrompt(): Promise<void>;`
`186`	`186`	`}`
Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {`
`93`	`93`	`const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder);`
`94`	`94`	`this.currentlySelectedWorkspaceFolder = workspaceFolder;`
`95`	`95`	`if (interpreter) {`
`96`		`- if (await this.python27SupportPrompt.shouldShowPrompt(workspaceFolder)) {`
	`96`	`+ if (await this.python27SupportPrompt.shouldShowPrompt(interpreter)) {`
`97`	`97`	`this.python27SupportPrompt.showPrompt();`
`98`	`98`	`}`
`99`	`99`