diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index 8fed2b50b266..81a1070ddbd2 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -3,10 +3,7 @@ import { inject, injectable } from 'inversify'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; -import { - IInterpreterAutoSelectionProxyService, - IInterpreterSecurityService, -} from '../../interpreter/autoSelection/types'; +import { IInterpreterAutoSelectionService, IInterpreterSecurityService } from '../../interpreter/autoSelection/types'; import { IServiceContainer } from '../../ioc/types'; import { IWorkspaceService } from '../application/types'; import { PythonSettings } from '../configSettings'; @@ -29,8 +26,8 @@ export class ConfigurationService implements IConfigurationService { } public getSettings(resource?: Uri): IPythonSettings { - const InterpreterAutoSelectionService = this.serviceContainer.get( - IInterpreterAutoSelectionProxyService, + const InterpreterAutoSelectionService = this.serviceContainer.get( + IInterpreterAutoSelectionService, ); const interpreterPathService = this.serviceContainer.get(IInterpreterPathService); const experiments = this.serviceContainer.get(IExperimentsManager); diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index b8187c923886..b1d8fbb876f0 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -88,6 +88,7 @@ interface IRawFSExtra { readFileSync(path: string, encoding: string): string; createReadStream(filename: string): ReadStream; createWriteStream(filename: string): WriteStream; + pathExists(filename: string): Promise; } interface IRawPath { @@ -125,6 +126,10 @@ export class RawFileSystem implements IRawFileSystem { ); } + public async pathExists(filename: string): Promise { + return this.fsExtra.pathExists(filename); + } + public async stat(filename: string): Promise { // Note that, prior to the November release of VS Code, // stat.ctime was always 0. @@ -355,6 +360,10 @@ export class FileSystemUtils implements IFileSystemUtils { // matches; otherwise a mismatch results in a "false" value fileType?: FileType, ): Promise { + if (fileType === undefined) { + // Do not need to run stat if not asking for file type. + return this.raw.pathExists(filename); + } let stat: FileStat; try { // Note that we are using stat() rather than lstat(). This @@ -368,9 +377,6 @@ export class FileSystemUtils implements IFileSystemUtils { return false; } - if (fileType === undefined) { - return true; - } if (fileType === FileType.Unknown) { // FileType.Unknown == 0, hence do not use bitwise operations. return stat.type === FileType.Unknown; @@ -563,6 +569,10 @@ export class FileSystem implements IFileSystem { return this.utils.fileExists(filename); } + public pathExists(filename: string): Promise { + return this.utils.pathExists(filename); + } + public fileExistsSync(filename: string): boolean { return this.utils.fileExistsSync(filename); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index c82f92f61658..58dbefaa795b 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -95,6 +95,7 @@ export type WriteStream = fs.WriteStream; // The low-level filesystem operations on which the extension depends. export interface IRawFileSystem { + pathExists(filename: string): Promise; // Get information about a file (resolve symlinks). stat(filename: string): Promise; // Get information about a file (do not resolve synlinks). @@ -216,6 +217,7 @@ export interface IFileSystem { createWriteStream(path: string): fs.WriteStream; // utils + pathExists(path: string): Promise; fileExists(path: string): Promise; fileExistsSync(path: string): boolean; directoryExists(path: string): Promise; diff --git a/src/client/common/process/pythonEnvironment.ts b/src/client/common/process/pythonEnvironment.ts index ffb7790f132b..a039c3eca12e 100644 --- a/src/client/common/process/pythonEnvironment.ts +++ b/src/client/common/process/pythonEnvironment.ts @@ -12,6 +12,7 @@ import * as internalPython from './internal/python'; import { ExecutionResult, IProcessService, ShellOptions, SpawnOptions } from './types'; class PythonEnvironment { + private cachedExecutablePath: Map> = new Map>(); private cachedInterpreterInformation: InterpreterInformation | undefined | null = null; constructor( @@ -49,8 +50,15 @@ class PythonEnvironment { if (await this.deps.isValidExecutable(this.pythonPath)) { return this.pythonPath; } + const result = this.cachedExecutablePath.get(this.pythonPath); + if (result !== undefined) { + // Another call for this environment has already been made, return its result + return result; + } const python = this.getExecutionInfo(); - return getExecutablePath(python, this.deps.exec); + const promise = getExecutablePath(python, this.deps.exec); + this.cachedExecutablePath.set(this.pythonPath, promise); + return promise; } public async getModuleVersion(moduleName: string): Promise { @@ -113,7 +121,7 @@ export function createPythonEnv( fs: IFileSystem, ): PythonEnvironment { const deps = createDeps( - async (filename) => fs.fileExists(filename), + async (filename) => fs.pathExists(filename), // We use the default: [pythonPath]. undefined, undefined, @@ -139,7 +147,7 @@ export function createCondaEnv( } const pythonArgv = [condaFile, ...runArgs, 'python']; const deps = createDeps( - async (filename) => fs.fileExists(filename), + async (filename) => fs.pathExists(filename), pythonArgv, // TODO: Use pythonArgv here once 'conda run' can be diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index a36bfcbf5469..c08ce6285b3d 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; +import { traceError } from '../logger'; import { IFileSystem } from '../platform/types'; import { IPathUtils } from '../types'; import { EnvironmentVariables, IEnvironmentVariablesService } from './types'; @@ -22,10 +23,17 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService filePath?: string, baseVars?: EnvironmentVariables, ): Promise { - if (!filePath || !(await this.fs.fileExists(filePath))) { + if (!filePath || !(await this.fs.pathExists(filePath))) { return; } - return parseEnvFile(await this.fs.readFile(filePath), baseVars); + const contents = await this.fs.readFile(filePath).catch((ex) => { + traceError('Custom .env is likely not pointing to a valid file', ex); + return undefined; + }); + if (!contents) { + return; + } + return parseEnvFile(contents, baseVars); } public mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables) { diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index d6da4616520d..0292da87c1bb 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -181,6 +181,11 @@ async function activateLegacy(ext: ExtensionState): Promise { const manager = serviceContainer.get(IExtensionActivationManager); context.subscriptions.push(manager); + + await interpreterManager + .refresh(workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders![0].uri : undefined) + .catch((ex) => traceError('Python Extension: interpreterManager.refresh', ex)); + const activationPromise = manager.activate(); serviceManager.get(ITerminalAutoActivation).register(); @@ -193,10 +198,6 @@ async function activateLegacy(ext: ExtensionState): Promise { serviceManager.get(ICodeExecutionManager).registerCommands(); - interpreterManager - .refresh(workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders![0].uri : undefined) - .catch((ex) => traceError('Python Extension: interpreterManager.refresh', ex)); - context.subscriptions.push(new LinterCommands(serviceManager)); languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration()); diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 9b58f0de9fe5..be5b57d21391 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -6,9 +6,10 @@ import { inject, injectable, named } from 'inversify'; import { Event, EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; +import { DeprecatePythonPath } from '../../common/experiments/groups'; import '../../common/extensions'; import { IFileSystem } from '../../common/platform/types'; -import { IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; +import { IExperimentsManager, IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/pythonVersion'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -34,7 +35,12 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio private readonly autoSelectedInterpreterByWorkspace = new Map(); - private globallyPreferredInterpreter!: IPersistentState; + private globallyPreferredInterpreter: IPersistentState< + PythonEnvironment | undefined + > = this.stateFactory.createGlobalPersistentState( + preferredGlobalInterpreter, + undefined, + ); private readonly rules: IInterpreterAutoSelectionRule[] = []; @@ -62,6 +68,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio workspaceInterpreter: IInterpreterAutoSelectionRule, @inject(IInterpreterAutoSelectionProxyService) proxy: IInterpreterAutoSelectionProxyService, @inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper, + @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, @inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService, ) { // It is possible we area always opening the same workspace folder, but we still need to determine and cache @@ -126,6 +133,9 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio // This method gets invoked from settings class, and this class in turn uses classes that relies on settings. // I.e. we can end up in a recursive loop. const interpreter = this._getAutoSelectedInterpreter(resource); + if (!this.experiments.inExperiment(DeprecatePythonPath.experiment)) { + return interpreter; // We do not about security service when not in experiment. + } // Unless the interpreter is marked as unsafe, return interpreter. return interpreter && this.interpreterSecurityService.isSafe(interpreter) === false ? undefined : interpreter; } diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index c52e11509057..4e381031fad8 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -3,7 +3,7 @@ import { Disposable, OutputChannel, StatusBarAlignment, StatusBarItem, Uri } fro import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; import '../../common/extensions'; -import { IDisposableRegistry, IOutputChannel, IPathUtils, Resource } from '../../common/types'; +import { IConfigurationService, IDisposableRegistry, IOutputChannel, IPathUtils, Resource } from '../../common/types'; import { Interpreters } from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -22,6 +22,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { private readonly workspaceService: IWorkspaceService; private readonly pathUtils: IPathUtils; private readonly interpreterService: IInterpreterService; + private readonly configService: IConfigurationService; private currentlySelectedInterpreterPath?: string; private currentlySelectedWorkspaceFolder: Resource; private readonly autoSelection: IInterpreterAutoSelectionService; @@ -38,6 +39,7 @@ export class InterpreterDisplay implements IInterpreterDisplay { const application = serviceContainer.get(IApplicationShell); const disposableRegistry = serviceContainer.get(IDisposableRegistry); + this.configService = serviceContainer.get(IConfigurationService); this.statusBar = application.createStatusBarItem(StatusBarAlignment.Left, 100); this.statusBar.command = 'python.setInterpreter'; @@ -73,7 +75,10 @@ export class InterpreterDisplay implements IInterpreterDisplay { } } private async updateDisplay(workspaceFolder?: Uri) { - await this.autoSelection.autoSelectInterpreter(workspaceFolder); + const interpreterPath = this.configService.getSettings(workspaceFolder)?.pythonPath; + if (!interpreterPath || interpreterPath === 'python') { + await this.autoSelection.autoSelectInterpreter(workspaceFolder); // Block on this only if no interpreter selected. + } const interpreter = await this.interpreterService.getActiveInterpreter(workspaceFolder); this.currentlySelectedWorkspaceFolder = workspaceFolder; if (interpreter) { diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index 4c8773befd3f..9321071c4ad9 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -1,5 +1,4 @@ import { inject, injectable } from 'inversify'; -import * as md5 from 'md5'; import * as path from 'path'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; import '../common/extensions'; @@ -283,16 +282,14 @@ export class InterpreterService implements Disposable, IInterpreterService { if (!info.cachedEntry && info.path && this.inMemoryCacheOfDisplayNames.has(info.path)) { return this.inMemoryCacheOfDisplayNames.get(info.path)!; } - const fileHash = (info.path ? await getInterpreterHash(info.path).catch(() => '') : '') || ''; - // Do not include display name into hash as that changes. - const interpreterHash = `${fileHash}-${md5(JSON.stringify({ ...info, displayName: '' }))}`; + const interpreterKey = info.path ?? ''; const store = this.persistentStateFactory.createGlobalPersistentState<{ hash: string; displayName: string }>( `${info.path}.interpreter.displayName.v7`, undefined, EXPIRY_DURATION, ); - if (store.value && store.value.hash === interpreterHash && store.value.displayName) { + if (store.value && store.value.hash === interpreterKey && store.value.displayName) { this.inMemoryCacheOfDisplayNames.set(info.path!, store.value.displayName); return store.value.displayName; } @@ -301,7 +298,7 @@ export class InterpreterService implements Disposable, IInterpreterService { // If dealing with cached entry, then do not store the display name in cache. if (!info.cachedEntry) { - await store.updateValue({ displayName, hash: interpreterHash }); + await store.updateValue({ displayName, hash: interpreterKey }); this.inMemoryCacheOfDisplayNames.set(info.path!, displayName); } diff --git a/src/test/common/configuration/service.unit.test.ts b/src/test/common/configuration/service.unit.test.ts index 2f6e59ef1107..241c3212862e 100644 --- a/src/test/common/configuration/service.unit.test.ts +++ b/src/test/common/configuration/service.unit.test.ts @@ -12,7 +12,7 @@ import { ConfigurationService } from '../../../client/common/configuration/servi import { DeprecatePythonPath } from '../../../client/common/experiments/groups'; import { IExperimentsManager, IInterpreterPathService } from '../../../client/common/types'; import { - IInterpreterAutoSelectionProxyService, + IInterpreterAutoSelectionService, IInterpreterSecurityService, } from '../../../client/interpreter/autoSelection/types'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -56,13 +56,13 @@ suite('Configuration Service', () => { } test('Fetching settings goes as expected', () => { - const interpreterAutoSelectionProxyService = TypeMoq.Mock.ofType(); + const interpreterAutoSelectionProxyService = TypeMoq.Mock.ofType(); serviceContainer .setup((s) => s.get(IInterpreterSecurityService)) .returns(() => interpreterSecurityService.object) .verifiable(TypeMoq.Times.once()); serviceContainer - .setup((s) => s.get(IInterpreterAutoSelectionProxyService)) + .setup((s) => s.get(IInterpreterAutoSelectionService)) .returns(() => interpreterAutoSelectionProxyService.object) .verifiable(TypeMoq.Times.once()); const settings = configService.getSettings(); diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 7c54bdd3e3f0..0578439132cf 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -56,6 +56,7 @@ interface IRawFS extends IPaths { writeFile(uri: vscode.Uri, content: Uint8Array): Thenable; // "fs-extra" + pathExists(filename: string): Promise; lstat(filename: string): Promise; chmod(filePath: string, mode: string | number): Promise; appendFile(filename: string, data: {}): Promise; @@ -926,9 +927,8 @@ suite('FileSystemUtils', () => { suite('pathExists', () => { test('exists (without type)', async () => { const filename = 'x/y/z/spam.py'; - const stat = createMockStat(); - deps.setup((d) => d.stat(filename)) // The "file" exists. - .returns(() => Promise.resolve(stat.object)); + deps.setup((d) => d.pathExists(filename)) // The "file" exists. + .returns(() => Promise.resolve(true)); const exists = await utils.pathExists(filename); @@ -936,11 +936,10 @@ suite('FileSystemUtils', () => { verifyAll(); }); - test('does not exist', async () => { + test('does not exist (without type)', async () => { const filename = 'x/y/z/spam.py'; - const err = vscode.FileSystemError.FileNotFound(filename); - deps.setup((d) => d.stat(filename)) // The file does not exist. - .throws(err); + deps.setup((d) => d.pathExists(filename)) // The "file" exists. + .returns(() => Promise.resolve(false)); const exists = await utils.pathExists(filename); @@ -948,29 +947,6 @@ suite('FileSystemUtils', () => { verifyAll(); }); - test('ignores errors from stat()', async () => { - const filename = 'x/y/z/spam.py'; - deps.setup((d) => d.stat(filename)) // It's broken. - .returns(() => Promise.reject(new Error('oops!'))); - - const exists = await utils.pathExists(filename); - - expect(exists).to.equal(false); - verifyAll(); - }); - - test('matches (type: undefined)', async () => { - const filename = 'x/y/z/spam.py'; - const stat = createMockStat(); - deps.setup((d) => d.stat(filename)) // The "file" exists. - .returns(() => Promise.resolve(stat.object)); - - const exists = await utils.pathExists(filename); - - expect(exists).to.equal(true); - verifyAll(); - }); - test('matches (type: file)', async () => { const filename = 'x/y/z/spam.py'; const stat = createMockStat(); @@ -1231,8 +1207,8 @@ suite('FileSystemUtils', () => { const err = vscode.FileSystemError.FileNotFound(dirname); deps.setup((d) => d.listdir(dirname)) // The "file" does not exist. .returns(() => Promise.reject(err)); - deps.setup((d) => d.stat(dirname)) // The "file" does not exist. - .returns(() => Promise.reject(err)); + deps.setup((d) => d.pathExists(dirname)) // The "file" does not exist. + .returns(() => Promise.resolve(false)); const entries = await utils.listdir(dirname); @@ -1245,9 +1221,7 @@ suite('FileSystemUtils', () => { const err = vscode.FileSystemError.FileNotADirectory(dirname); deps.setup((d) => d.listdir(dirname)) // Fail (async) with not-a-directory. .returns(() => Promise.reject(err)); - const stat = createMockStat(); - deps.setup((d) => d.stat(dirname)) // The "file" exists. - .returns(() => Promise.resolve(stat.object)); + deps.setup((d) => d.pathExists(dirname)).returns(() => Promise.resolve(true)); // The "file" exists. const promise = utils.listdir(dirname); @@ -1260,29 +1234,13 @@ suite('FileSystemUtils', () => { const err = new Error('oops!'); deps.setup((d) => d.listdir(dirname)) // Fail (async) with an arbitrary error. .returns(() => Promise.reject(err)); - deps.setup((d) => d.stat(dirname)) // Fail with file-not-found. - .throws(vscode.FileSystemError.FileNotFound(dirname)); + deps.setup((d) => d.pathExists(dirname)).returns(() => Promise.resolve(false)); const entries = await utils.listdir(dirname); expect(entries).to.deep.equal([]); verifyAll(); }); - - test('fails if the raw call fails', async () => { - const dirname = 'x/y/z/spam'; - const err = new Error('oops!'); - deps.setup((d) => d.listdir(dirname)) // Fail with an arbirary error. - .throws(err); - const stat = createMockStat(); - deps.setup((d) => d.stat(dirname)) // The "file" exists. - .returns(() => Promise.resolve(stat.object)); - - const promise = utils.listdir(dirname); - - await expect(promise).to.eventually.be.rejected; - verifyAll(); - }); }); suite('getSubDirectories', () => { diff --git a/src/test/common/process/pythonEnvironment.unit.test.ts b/src/test/common/process/pythonEnvironment.unit.test.ts index 0b9f48ee99ca..3abc2a87c646 100644 --- a/src/test/common/process/pythonEnvironment.unit.test.ts +++ b/src/test/common/process/pythonEnvironment.unit.test.ts @@ -166,7 +166,7 @@ suite('PythonEnvironment', () => { }); test('getExecutablePath should return pythonPath if pythonPath is a file', async () => { - fileSystem.setup((f) => f.fileExists(pythonPath)).returns(() => Promise.resolve(true)); + fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(true)); const env = createPythonEnv(pythonPath, processService.object, fileSystem.object); const result = await env.getExecutablePath(); @@ -176,7 +176,7 @@ suite('PythonEnvironment', () => { test('getExecutablePath should not return pythonPath if pythonPath is not a file', async () => { const executablePath = 'path/to/dummy/executable'; - fileSystem.setup((f) => f.fileExists(pythonPath)).returns(() => Promise.resolve(false)); + fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false)); const argv = [isolated, '-c', 'import sys;print(sys.executable)']; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) @@ -190,7 +190,7 @@ suite('PythonEnvironment', () => { test('getExecutablePath should throw if the result of exec() writes to stderr', async () => { const stderr = 'bar'; - fileSystem.setup((f) => f.fileExists(pythonPath)).returns(() => Promise.resolve(false)); + fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false)); const argv = [isolated, '-c', 'import sys;print(sys.executable)']; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index e1316be62209..38a4f678c2da 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -38,7 +38,7 @@ suite('Environment Variables Service', () => { fs.verifyAll(); } function setFile(fileName: string, text: string) { - fs.setup((f) => f.fileExists(fileName)) // Handle the specific file. + fs.setup((f) => f.pathExists(fileName)) // Handle the specific file. .returns(() => Promise.resolve(true)); // The file exists. fs.setup((f) => f.readFile(fileName)) // Handle the specific file. .returns(() => Promise.resolve(text)); // Pretend to read from the file. @@ -53,7 +53,7 @@ suite('Environment Variables Service', () => { }); test('Custom variables should be undefined with non-existent files', async () => { - fs.setup((f) => f.fileExists(filename)) // Handle the specific file. + fs.setup((f) => f.pathExists(filename)) // Handle the specific file. .returns(() => Promise.resolve(false)); // The file is missing. const vars = await variablesService.parseFile(filename); @@ -64,7 +64,7 @@ suite('Environment Variables Service', () => { test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { const dirname = 'x/y/z'; - fs.setup((f) => f.fileExists(dirname)) // Handle the specific "file". + fs.setup((f) => f.pathExists(dirname)) // Handle the specific "file". .returns(() => Promise.resolve(false)); // It isn't a "regular" file. const vars = await variablesService.parseFile(dirname); diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index f26559cf54c6..e5785726264f 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -9,10 +9,11 @@ import { anything, instance, mock, reset, verify, when } from 'ts-mockito'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; +import { ExperimentsManager } from '../../../client/common/experiments/manager'; import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IPersistentStateFactory, Resource } from '../../../client/common/types'; +import { IExperimentsManager, IPersistentStateFactory, Resource } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; import { InterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection'; import { InterpreterSecurityService } from '../../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService'; @@ -51,6 +52,7 @@ suite('Interpreters - Auto Selection', () => { let helper: IInterpreterHelper; let proxy: IInterpreterAutoSelectionProxyService; let interpreterSecurityService: IInterpreterSecurityService; + let experiments: IExperimentsManager; class InterpreterAutoSelectionServiceTest extends InterpreterAutoSelectionService { public initializeStore(resource: Resource): Promise { return super.initializeStore(resource); @@ -78,7 +80,8 @@ suite('Interpreters - Auto Selection', () => { workspaceInterpreter = mock(WorkspaceVirtualEnvInterpretersAutoSelectionRule); helper = mock(InterpreterHelper); proxy = mock(InterpreterAutoSelectionProxyService); - when(interpreterSecurityService.isSafe(anything())).thenReturn(undefined); + experiments = mock(ExperimentsManager); + when(experiments.inExperiment(anything())).thenReturn(false); autoSelectionService = new InterpreterAutoSelectionServiceTest( instance(workspaceService), @@ -92,6 +95,7 @@ suite('Interpreters - Auto Selection', () => { instance(workspaceInterpreter), instance(proxy), instance(helper), + instance(experiments), instance(interpreterSecurityService), ); }); @@ -172,7 +176,7 @@ suite('Interpreters - Auto Selection', () => { await autoSelectionService.initializeStore(undefined); await autoSelectionService.initializeStore(undefined); - verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).once(); + verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).twice(); }); test("Clear file stored in cache if it doesn't exist", async () => { const pythonPath = 'Hello World'; @@ -188,7 +192,7 @@ suite('Interpreters - Auto Selection', () => { await autoSelectionService.initializeStore(undefined); - verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).once(); + verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).twice(); verify(state.value).atLeast(1); verify(fs.fileExists(pythonPath)).once(); verify(state.updateValue(undefined)).once(); @@ -207,7 +211,7 @@ suite('Interpreters - Auto Selection', () => { await autoSelectionService.initializeStore(undefined); - verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).once(); + verify(stateFactory.createGlobalPersistentState(preferredGlobalInterpreter, undefined)).twice(); verify(state.value).atLeast(1); verify(fs.fileExists(pythonPath)).once(); verify(state.updateValue(undefined)).never(); @@ -235,7 +239,8 @@ suite('Interpreters - Auto Selection', () => { expect(selectedInterpreter).to.deep.equal(interpreterInfo); expect(eventFired).to.deep.equal(false, 'event fired'); }); - test('If interpreter chosen is unsafe, return `undefined` as the auto-selected interpreter', async () => { + test('When in experiment, if interpreter chosen is unsafe, return `undefined` as the auto-selected interpreter', async () => { + when(experiments.inExperiment(anything())).thenReturn(true); const interpreterInfo = { path: 'pythonPath' } as any; autoSelectionService._getAutoSelectedInterpreter = () => interpreterInfo; reset(interpreterSecurityService); diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index cd115bcdf999..9d919e4b8111 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -6,7 +6,6 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import { Container } from 'inversify'; -import * as md5 from 'md5'; import * as path from 'path'; import * as sinon from 'sinon'; import { SemVer } from 'semver'; @@ -394,7 +393,6 @@ suite('Interpreters service', () => { test('Return cached display name', async () => { const pythonPath = '1234'; const interpreterInfo: Partial = { path: pythonPath }; - const hash = `-${md5(JSON.stringify({ ...interpreterInfo, displayName: '' }))}`; const expectedDisplayName = 'Formatted display name'; getInterpreterHashStub.rejects({}); @@ -403,7 +401,7 @@ suite('Interpreters service', () => { .returns(() => { const state = { updateValue: () => Promise.resolve(), - value: { hash, displayName: expectedDisplayName }, + value: { hash: pythonPath, displayName: expectedDisplayName }, }; return state as any; }) @@ -415,31 +413,6 @@ suite('Interpreters service', () => { expect(displayName).to.equal(expectedDisplayName); persistentStateFactory.verifyAll(); }); - test('Cached display name is not used if file hashes differ', async () => { - const pythonPath = '1234'; - const interpreterInfo: Partial = { path: pythonPath }; - const fileHash = 'File_Hash'; - - getInterpreterHashStub.withArgs(pythonPath).resolves(fileHash); - const expectedDisplayName = 'Formatted display name'; - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => { - const state = { - updateValue: () => Promise.resolve(), - value: { fileHash: 'something else', displayName: expectedDisplayName }, - }; - return state as any; - }) - .verifiable(TypeMoq.Times.once()); - - const service = new InterpreterService(serviceContainer, pyenvs.object, experimentService.object); - const displayName = await service.getDisplayName(interpreterInfo, undefined).catch(() => ''); - - expect(displayName).to.not.equal(expectedDisplayName); - expect(getInterpreterHashStub.calledOnce).to.equal(true); - persistentStateFactory.verifyAll(); - }); }); suite('Display name with incomplete interpreter versions', () => {