Ensure full refresh icon removes any environment which isn't relevant and remove hard refresh icon in quickpick (microsoft#19806)

Author: Kartik Raj

src/client/apiTypes.ts

@@ -121,10 +121,6 @@ export interface ActiveEnvironmentChangedParams {
     resource?: Uri;
 }
 
-export interface RefreshEnvironmentsOptions {
-    clearCache?: boolean;
-}
-
 export interface IProposedExtensionAPI {
     environment: {
         /**
@@ -203,7 +199,7 @@ export interface IProposedExtensionAPI {
          *     * clearCache : When true, this will clear the cache before environment refresh
          *                    is triggered.
          */
-        refreshEnvironment(options?: RefreshEnvironmentsOptions): Promise<EnvPathType[] | undefined>;
+        refreshEnvironment(): Promise<EnvPathType[] | undefined>;
         /**
          * Tracks discovery progress for current list of known environments, i.e when it starts, finishes or any other relevant
          * stage. Note the progress for a particular query is currently not tracked or reported, this only indicates progress of

src/client/common/constants.ts

@@ -85,7 +85,6 @@ export namespace Octicons {
  * to change the icons.
  */
 export namespace ThemeIcons {
-    export const ClearAll = 'clear-all';
     export const Refresh = 'refresh';
     export const SpinningLoader = 'loading~spin';
 }

src/client/common/utils/localize.ts

@@ -317,10 +317,6 @@ export namespace InterpreterQuickPickList {
         'InterpreterQuickPickList.refreshInterpreterList',
         'Refresh Interpreter list',
     );
-    export const clearAllAndRefreshInterpreterList = localize(
-        'InterpreterQuickPickList.clearAllAndRefreshInterpreterList',
-        'Clear all and Refresh Interpreter list',
-    );
     export const refreshingInterpreterList = localize(
         'InterpreterQuickPickList.refreshingInterpreterList',
         'Refreshing Interpreter list...',

src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts

@@ -80,11 +80,6 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
         tooltip: InterpreterQuickPickList.refreshInterpreterList,
     };
 
-    private readonly hardRefreshButton = {
-        iconPath: new ThemeIcon(ThemeIcons.ClearAll),
-        tooltip: InterpreterQuickPickList.clearAllAndRefreshInterpreterList,
-    };
-
     private readonly noPythonInstalled: ISpecialQuickPickItem = {
         label: `${Octicons.Error} ${InterpreterQuickPickList.noPythonInstalled}`,
         detail: InterpreterQuickPickList.clickForInstructions,
@@ -155,16 +150,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
             matchOnDescription: true,
             title: InterpreterQuickPickList.browsePath.openButtonLabel,
             customButtonSetups: [
-                {
-                    button: this.hardRefreshButton,
-                    callback: (quickpickInput) => {
-                        this.refreshButtonCallback(quickpickInput, true);
-                    },
-                },
                 {
                     button: this.refreshButton,
                     callback: (quickpickInput) => {
-                        this.refreshButtonCallback(quickpickInput, false);
+                        this.refreshButtonCallback(quickpickInput);
                     },
                 },
             ],
@@ -418,17 +407,17 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
         }
     }
 
-    private refreshButtonCallback(input: QuickPick<QuickPickItem>, clearCache: boolean) {
+    private refreshButtonCallback(input: QuickPick<QuickPickItem>) {
         input.buttons = [
             {
                 iconPath: new ThemeIcon(ThemeIcons.SpinningLoader),
                 tooltip: InterpreterQuickPickList.refreshingInterpreterList,
             },
         ];
         this.interpreterService
-            .triggerRefresh(undefined, { clearCache })
+            .triggerRefresh()
             .finally(() => {
-                input.buttons = [this.hardRefreshButton, this.refreshButton];
+                input.buttons = [this.refreshButton];
             })
             .ignoreErrors();
     }

src/client/interpreter/interpreterService.ts

@@ -244,7 +244,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
             traceLog('Conda envs without Python are known to not work well; fixing conda environment...');
             const promise = installer.install(Product.python, await this.getInterpreterDetails(pythonPath));
             shell.withProgress(progressOptions, () => promise);
-            promise.then(() => this.triggerRefresh(undefined, { clearCache: true }).ignoreErrors());
+            promise.then(() => this.triggerRefresh().ignoreErrors());
         }
     }
 }

src/client/proposedApi.ts

@@ -8,7 +8,6 @@ import {
     EnvironmentDetailsOptions,
     EnvironmentsChangedParams,
     IProposedExtensionAPI,
-    RefreshEnvironmentsOptions,
 } from './apiTypes';
 import { arePathsSame } from './common/platform/fs-paths';
 import { IInterpreterPathService, Resource } from './common/types';
@@ -101,8 +100,8 @@ export function buildProposedApi(
             setActiveEnvironment(path: string, resource?: Resource): Promise<void> {
                 return interpreterPathService.update(resource, ConfigurationTarget.WorkspaceFolder, path);
             },
-            async refreshEnvironment(options?: RefreshEnvironmentsOptions) {
-                await discoveryApi.triggerRefresh(undefined, options ? { clearCache: options.clearCache } : undefined);
+            async refreshEnvironment() {
+                await discoveryApi.triggerRefresh();
                 const paths = discoveryApi.getEnvs().map((e) => getEnvPath(e.executable.filename, e.location));
                 return Promise.resolve(paths);
             },

src/client/pythonEnvironments/base/locator.ts

@@ -194,10 +194,6 @@ export interface GetRefreshEnvironmentsOptions {
 }
 
 export type TriggerRefreshOptions = {
-    /**
-     * Trigger a fresh refresh.
-     */
-    clearCache?: boolean;
     /**
      * Only trigger a refresh if it hasn't already been triggered for this session.
      */

src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import { Event } from 'vscode';
+import { isTestExecution } from '../../../../common/constants';
 import { traceInfo } from '../../../../logging';
 import { reportInterpretersChanged } from '../../../../proposedApi';
 import { arePathsSame, getFileInfo, pathExists } from '../../../common/externalDependencies';
@@ -52,13 +53,9 @@ export interface IEnvsCollectionCache {
     /**
      * Removes invalid envs from cache. Note this does not check for outdated info when
      * validating cache.
+     * @param latestListOfEnvs Carries list of latest envs for this workspace session if known.
      */
-    validateCache(): Promise<void>;
-
-    /**
-     * Clears the in-memory cache. This can be used for hard refresh.
-     */
-    clearCache(): Promise<void>;
+    validateCache(latestListOfEnvs?: PythonEnvInfo[]): Promise<void>;
 }
 
 export type PythonEnvLatestInfo = { hasLatestInfo?: boolean } & PythonEnvInfo;
@@ -79,15 +76,35 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher<PythonEnvCollectionCha
         super();
     }
 
-    public async validateCache(): Promise<void> {
+    public async validateCache(latestListOfEnvs?: PythonEnvInfo[]): Promise<void> {
         /**
          * We do check if an env has updated as we already run discovery in background
          * which means env cache will have up-to-date envs eventually. This also means
-         * we avoid the cost of running lstat. So simply remove envs which no longer
-         * exist.
+         * we avoid the cost of running lstat. So simply remove envs which are no longer
+         * valid.
          */
         const areEnvsValid = await Promise.all(
-            this.envs.map((e) => (e.executable.filename === 'python' ? true : pathExists(e.executable.filename))),
+            this.envs.map(async (cachedEnv) => {
+                const { path } = getEnvPath(cachedEnv.executable.filename, cachedEnv.location);
+                if (await pathExists(path)) {
+                    if (latestListOfEnvs) {
+                        /**
+                         * Only consider a cached env to be valid if it's relevant. That means:
+                         * * It is either reported in the latest complete refresh for this session.
+                         * * Or it is relevant for some other workspace folder which is not opened currently.
+                         */
+                        if (cachedEnv.searchLocation) {
+                            return true;
+                        }
+                        if (latestListOfEnvs.some((env) => cachedEnv.id === env.id)) {
+                            return true;
+                        }
+                    } else {
+                        return true;
+                    }
+                }
+                return false;
+            }),
         );
         const invalidIndexes = areEnvsValid
             .map((isValid, index) => (isValid ? -1 : index))
@@ -194,6 +211,15 @@ async function validateInfo(env: PythonEnvInfo) {
 export async function createCollectionCache(storage: IPersistentStorage): Promise<PythonEnvInfoCache> {
     const cache = new PythonEnvInfoCache(storage);
     await cache.clearAndReloadFromStorage();
-    await cache.validateCache();
+    await validateCache(cache);
     return cache;
 }
+
+async function validateCache(cache: PythonEnvInfoCache) {
+    if (isTestExecution()) {
+        // For purposes for test execution, block on validation so that we can determinally know when it finishes.
+        return cache.validateCache();
+    }
+    // Validate in background so it doesn't block on returning the API object.
+    return cache.validateCache().ignoreErrors();
+}

src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts

@@ -108,15 +108,12 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
                 // Do not trigger another refresh if a refresh has previously finished.
                 return Promise.resolve();
             }
-            refreshPromise = this.startRefresh(query, options);
+            refreshPromise = this.startRefresh(query);
         }
         return refreshPromise.then(() => this.sendTelemetry(query, stopWatch));
     }
 
-    private startRefresh(query: PythonLocatorQuery | undefined, options?: TriggerRefreshOptions): Promise<void> {
-        if (options?.clearCache) {
-            this.cache.clearCache();
-        }
+    private startRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
         this.createProgressStates(query);
         const promise = this.addEnvsToCacheForQuery(query);
         return promise
@@ -176,7 +173,8 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
             this.cache.addEnv(env);
         }
         await updatesDone.promise;
-        await this.cache.validateCache();
+        // If query for all envs is done, `seen` should contain the list of all envs.
+        await this.cache.validateCache(query === undefined ? seen : undefined);
         this.cache.flush().ignoreErrors();
     }
 

src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts

@@ -16,7 +16,7 @@ import {
     WorkspaceFolder,
 } from 'vscode';
 import { cloneDeep } from 'lodash';
-import { anything, instance, mock, when } from 'ts-mockito';
+import { anything, instance, mock, when, verify } from 'ts-mockito';
 import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types';
 import { PathUtils } from '../../../../client/common/platform/pathUtils';
 import { IPlatformService } from '../../../../client/common/platform/types';
@@ -80,6 +80,7 @@ suite('Set Interpreter Command', () => {
         workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
         interpreterService = mock<IInterpreterService>();
         when(interpreterService.refreshPromise).thenReturn(undefined);
+        when(interpreterService.triggerRefresh()).thenResolve();
         when(interpreterService.triggerRefresh(anything(), anything())).thenResolve();
         workspace.setup((w) => w.rootPath).returns(() => 'rootPath');
 
@@ -574,17 +575,11 @@ suite('Set Interpreter Command', () => {
             expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
             const refreshButtons = actualParameters!.customButtonSetups;
             expect(refreshButtons).to.not.equal(undefined, 'Callback not set');
+            expect(refreshButtons?.length).to.equal(1);
 
-            expect(refreshButtons?.length).to.equal(2);
-            let arg;
-            when(interpreterService.triggerRefresh(undefined, anything())).thenCall((_, _arg) => {
-                arg = _arg;
-                return Promise.resolve();
-            });
             await refreshButtons![0].callback!({} as QuickPick<QuickPickItem>); // Invoke callback, meaning that the refresh button is clicked.
-            expect(arg).to.deep.equal({ clearCache: true });
-            await refreshButtons![1].callback!({} as QuickPick<QuickPickItem>); // Invoke callback, meaning that the refresh button is clicked.
-            expect(arg).to.deep.equal({ clearCache: false });
+
+            verify(interpreterService.triggerRefresh()).once();
         });
 
         test('Events to update quickpick updates the quickpick accordingly', async () => {