Ensure command to get executable is run only once per session (microsoft#19606)

Kartik Raj · web-flow · commit 069bced1489a · 2022-08-02T17:37:13.000-07:00
diff --git a/src/client/common/process/pythonEnvironment.ts b/src/client/common/process/pythonEnvironment.ts
@@ -8,12 +8,14 @@ import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/ex
 import { InterpreterInformation } from '../../pythonEnvironments/info';
 import { getExecutablePath } from '../../pythonEnvironments/info/executable';
 import { getInterpreterInfo } from '../../pythonEnvironments/info/interpreter';
+import { isTestExecution } from '../constants';
 import { IFileSystem } from '../platform/types';
 import * as internalPython from './internal/python';
 import { ExecutionResult, IProcessService, IPythonEnvironment, ShellOptions, SpawnOptions } from './types';
 
+const cachedExecutablePath: Map<string, Promise<string | undefined>> = new Map<string, Promise<string | undefined>>();
+
 class PythonEnvironment implements IPythonEnvironment {
-    private cachedExecutablePath: Map<string, Promise<string>> = new Map<string, Promise<string>>();
     private cachedInterpreterInformation: InterpreterInformation | undefined | null = null;
 
     constructor(
@@ -45,20 +47,20 @@ class PythonEnvironment implements IPythonEnvironment {
         return this.cachedInterpreterInformation;
     }
 
-    public async getExecutablePath(): Promise<string> {
+    public async getExecutablePath(): Promise<string | undefined> {
         // If we've passed the python file, then return the file.
         // This is because on mac if using the interpreter /usr/bin/python2.7 we can get a different value for the path
         if (await this.deps.isValidExecutable(this.pythonPath)) {
             return this.pythonPath;
         }
-        const result = this.cachedExecutablePath.get(this.pythonPath);
-        if (result !== undefined) {
+        const result = cachedExecutablePath.get(this.pythonPath);
+        if (result !== undefined && !isTestExecution()) {
             // Another call for this environment has already been made, return its result
             return result;
         }
         const python = this.getExecutionInfo();
         const promise = getExecutablePath(python, this.deps.shellExec);
-        this.cachedExecutablePath.set(this.pythonPath, promise);
+        cachedExecutablePath.set(this.pythonPath, promise);
         return promise;
     }
 
diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts
@@ -89,7 +89,7 @@ export const IPythonExecutionService = Symbol('IPythonExecutionService');
 
 export interface IPythonExecutionService {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
-    getExecutablePath(): Promise<string>;
+    getExecutablePath(): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[]): PythonExecInfo;
@@ -105,7 +105,7 @@ export interface IPythonExecutionService {
 export interface IPythonEnvironment {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
     getExecutionObservableInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
-    getExecutablePath(): Promise<string>;
+    getExecutablePath(): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
diff --git a/src/client/pythonEnvironments/info/executable.ts b/src/client/pythonEnvironments/info/executable.ts
@@ -3,6 +3,7 @@
 
 import { getExecutable } from '../../common/process/internal/python';
 import { ShellExecFunc } from '../../common/process/types';
+import { traceError } from '../../logging';
 import { copyPythonExecInfo, PythonExecInfo } from '../exec';
 
 /**
@@ -17,19 +18,24 @@ export async function getExecutablePath(
     python: PythonExecInfo,
     shellExec: ShellExecFunc,
     timeout?: number,
-): Promise<string> {
-    const [args, parse] = getExecutable();
-    const info = copyPythonExecInfo(python, args);
-    const argv = [info.command, ...info.args];
-    // Concat these together to make a set of quoted strings
-    const quoted = argv.reduce(
-        (p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
-        '',
-    );
-    const result = await shellExec(quoted, { timeout: timeout ?? 15000 });
-    const executable = parse(result.stdout.trim());
-    if (executable === '') {
-        throw new Error(`${quoted} resulted in empty stdout`);
+): Promise<string | undefined> {
+    try {
+        const [args, parse] = getExecutable();
+        const info = copyPythonExecInfo(python, args);
+        const argv = [info.command, ...info.args];
+        // Concat these together to make a set of quoted strings
+        const quoted = argv.reduce(
+            (p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
+            '',
+        );
+        const result = await shellExec(quoted, { timeout: timeout ?? 15000 });
+        const executable = parse(result.stdout.trim());
+        if (executable === '') {
+            throw new Error(`${quoted} resulted in empty stdout`);
+        }
+        return executable;
+    } catch (ex) {
+        traceError(ex);
+        return undefined;
     }
-    return executable;
 }
diff --git a/src/test/common/process/pythonEnvironment.unit.test.ts b/src/test/common/process/pythonEnvironment.unit.test.ts
@@ -202,17 +202,17 @@ suite('PythonEnvironment', () => {
         expect(result).to.equal(executablePath, "getExecutablePath() sbould not return pythonPath if it's not a file");
     });
 
-    test('getExecutablePath should throw if the result of exec() writes to stderr', async () => {
+    test('getExecutablePath should return `undefined` if the result of exec() writes to stderr', async () => {
         const stderr = 'bar';
         fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false));
         processService
             .setup((p) => p.shellExec(`${pythonPath} -c "import sys;print(sys.executable)"`, TypeMoq.It.isAny()))
             .returns(() => Promise.reject(new StdErrError(stderr)));
         const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
 
-        const result = env.getExecutablePath();
+        const result = await env.getExecutablePath();
 
-        await expect(result).to.eventually.be.rejectedWith(stderr);
+        expect(result).to.be.equal(undefined);
     });
 
     test('isModuleInstalled should call processService.exec()', async () => {
