diff --git a/.eslintignore b/.eslintignore index 18ef6e6d6d8e..8ca365adb514 100644 --- a/.eslintignore +++ b/.eslintignore @@ -41,8 +41,6 @@ src/test/interpreters/mocks.ts src/test/interpreters/interpreterVersion.unit.test.ts src/test/interpreters/virtualEnvs/index.unit.test.ts src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts -src/test/interpreters/autoSelection/interpreterSecurity/interpreterEvaluation.unit.test.ts -src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityService.unit.test.ts src/test/interpreters/autoSelection/rules/settings.unit.test.ts src/test/interpreters/autoSelection/rules/cached.unit.test.ts src/test/interpreters/autoSelection/rules/winRegistry.unit.test.ts @@ -56,8 +54,6 @@ src/test/interpreters/helpers.unit.test.ts src/test/interpreters/currentPathService.unit.test.ts src/test/interpreters/display.unit.test.ts -src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts - src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts src/test/terminals/codeExecution/codeExecutionManager.unit.test.ts src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts @@ -93,7 +89,6 @@ src/test/activation/activeResource.unit.test.ts src/test/activation/node/languageServerChangeHandler.unit.test.ts src/test/activation/node/activator.unit.test.ts src/test/activation/extensionSurvey.unit.test.ts -src/test/activation/activationManager.unit.test.ts src/test/utils/fs.ts @@ -111,7 +106,6 @@ src/test/api.functional.test.ts src/test/testing/argsService.test.ts src/test/testing/mocks.ts src/test/testing/debugger.test.ts -src/test/testing/serviceRegistry.ts src/test/testing/unittest/unittest.test.ts src/test/testing/unittest/unittest.discovery.unit.test.ts src/test/testing/unittest/unittest.discovery.test.ts @@ -180,7 +174,6 @@ src/test/common/utils/cacheUtils.unit.test.ts src/test/common/utils/decorators.unit.test.ts src/test/common/utils/localize.functional.test.ts src/test/common/utils/version.unit.test.ts -src/test/common/configSettings/configSettings.pythonPath.unit.test.ts src/test/common/configSettings/configSettings.unit.test.ts src/test/common/featureDeprecationManager.unit.test.ts src/test/common/serviceRegistry.unit.test.ts @@ -287,7 +280,6 @@ src/client/interpreter/configuration/interpreterComparer.ts src/client/interpreter/configuration/interpreterSelector/commands/base.ts src/client/interpreter/configuration/interpreterSelector/commands/resetInterpreter.ts src/client/interpreter/configuration/interpreterSelector/commands/setShebangInterpreter.ts -src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts src/client/interpreter/configuration/pythonPathUpdaterServiceFactory.ts src/client/interpreter/configuration/services/globalUpdaterService.ts src/client/interpreter/configuration/services/workspaceUpdaterService.ts @@ -296,9 +288,6 @@ src/client/interpreter/helpers.ts src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts src/client/interpreter/virtualEnvs/types.ts src/client/interpreter/virtualEnvs/index.ts -src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage.ts -src/client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation.ts -src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService.ts src/client/interpreter/autoSelection/rules/baseRule.ts src/client/interpreter/autoSelection/rules/winRegistry.ts src/client/interpreter/autoSelection/rules/settings.ts @@ -353,7 +342,6 @@ src/client/activation/languageServer/languageServerPackageService.ts src/client/activation/languageServer/analysisOptions.ts src/client/activation/languageServer/activator.ts src/client/activation/commands.ts -src/client/activation/activationManager.ts src/client/activation/progress.ts src/client/activation/extensionSurvey.ts src/client/activation/common/languageServerChangeHandler.ts @@ -509,7 +497,6 @@ src/client/common/dotnet/services/macCompatibilityService.ts src/client/common/dotnet/services/linuxCompatibilityService.ts src/client/common/dotnet/services/windowsCompatibilityService.ts src/client/common/logger.ts -src/client/common/constants.ts src/client/common/variables/serviceRegistry.ts src/client/common/variables/environment.ts src/client/common/variables/types.ts @@ -523,7 +510,6 @@ src/client/common/cancellation.ts src/client/common/interpreterPathService.ts src/client/common/startPage/startPageMessageListener.ts src/client/common/application/customEditorService.ts -src/client/common/application/commands.ts src/client/common/application/applicationShell.ts src/client/common/application/languageService.ts src/client/common/application/notebook.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 43905ff343f4..18dd2b77e599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,18 @@ ([#16141](https://github.com/Microsoft/vscode-python/issues/16141)) 1. Ensure users upgrade to v0.2.0 of the torch-tb-profiler TensorBoard plugin to access jump-to-source functionality. ([#16330](https://github.com/Microsoft/vscode-python/issues/16330)) +1. Added `python.defaultInterpreterPath` setting at workspace level when in `pythonDeprecatePythonPath` experiment. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) +1. Added default Interpreter path entry at the bottom of the interpreter list. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) +1. Remove execution isolation script used to run tools. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) +1. Show `python.pythonPath` deprecation prompt when in `pythonDeprecatePythonPath` experiment. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) +1. Do not show safety prompt before autoselecting a workspace interpreter. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) +1. Assume workspace interpreters are safe to execute for discovery. + ([#16485](https://github.com/Microsoft/vscode-python/issues/16485)) ### Fixes 1. Fixes a bug in the bandit linter where messages weren't being propagated to the editor. diff --git a/package.json b/package.json index 352217ad4d54..6085bcc47a9f 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.54.0" + "vscode": "^1.57.0" }, "keywords": [ "python", @@ -88,7 +88,6 @@ "onCommand:python.switchToDailyChannel", "onCommand:python.switchToWeeklyChannel", "onCommand:python.clearWorkspaceInterpreter", - "onCommand:python.resetInterpreterSecurityStorage", "onCommand:python.startPage.open", "onCommand:python.enableSourceMapSupport", "onCommand:python.launchTensorBoard", @@ -235,11 +234,6 @@ "title": "%python.command.python.clearWorkspaceInterpreter.title%", "category": "Python" }, - { - "command": "python.resetInterpreterSecurityStorage", - "title": "%python.command.python.resetInterpreterSecurityStorage.title%", - "category": "Python" - }, { "command": "python.refactorExtractVariable", "title": "%python.command.python.refactorExtractVariable.title%", @@ -488,11 +482,6 @@ "title": "%python.command.python.clearWorkspaceInterpreter.title%", "category": "Python" }, - { - "command": "python.resetInterpreterSecurityStorage", - "title": "%python.command.python.resetInterpreterSecurityStorage.title%", - "category": "Python" - }, { "command": "python.viewOutput", "title": "%python.command.python.viewOutput.title%", @@ -1126,15 +1115,15 @@ "python.defaultInterpreterPath": { "type": "string", "default": "python", - "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.", - "scope": "machine" + "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path. This default setting is used as a fallback if no interpreter is selected for the workspace. The extension will also not set nor change the value of this setting, it will only read from it.", + "scope": "resource" }, "python.experiments.optInto": { "type": "array", "default": [], "items": { "enum": [ - "DeprecatePythonPath - experiment", + "pythonDeprecatePythonPath", "pythonTensorboardExperiment", "pythonDiscoveryModule", "pythonDiscoveryModuleWithoutWatcher", @@ -1151,7 +1140,7 @@ "default": [], "items": { "enum": [ - "DeprecatePythonPath - experiment", + "pythonDeprecatePythonPath", "pythonTensorboardExperiment", "pythonDiscoveryModule", "pythonDiscoveryModuleWithoutWatcher", @@ -1713,7 +1702,7 @@ "python.pythonPath": { "type": "string", "default": "python", - "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.", + "description": "(DEPRECATED: Note this setting is not used when in pythonDeprecatePythonPath experiment) Path to Python, you can use a custom version of Python by modifying this setting to include the full path.", "scope": "resource" }, "python.condaPath": { @@ -1865,12 +1854,6 @@ "description": "Enable auto run test discovery when saving a test file.", "scope": "resource" }, - "python.useIsolation": { - "type": "boolean", - "default": true, - "description": "Execute tools in isolation from the workspace.", - "scope": "machine" - }, "python.venvFolders": { "type": "array", "default": [], diff --git a/package.nls.json b/package.nls.json index d09b730f5f72..96ee3df8c1e9 100644 --- a/package.nls.json +++ b/package.nls.json @@ -11,7 +11,6 @@ "python.command.python.switchToDailyChannel.title": "Switch to Insiders Daily Channel", "python.command.python.switchToWeeklyChannel.title": "Switch to Insiders Weekly Channel", "python.command.python.clearWorkspaceInterpreter.title": "Clear Workspace Interpreter Setting", - "python.command.python.resetInterpreterSecurityStorage.title": "Reset Stored Info for Untrusted Interpreters", "python.command.python.refactorExtractVariable.title": "Extract Variable", "python.command.python.refactorExtractMethod.title": "Extract Method", "python.command.python.viewOutput.title": "Show Output", @@ -61,7 +60,6 @@ "Interpreters.entireWorkspace": "Entire workspace", "Interpreters.pythonInterpreterPath": "Python interpreter path: {0}", "Interpreters.LoadingInterpreters": "Loading Python Interpreters", - "Interpreters.unsafeInterpreterMessage": "We found a Python environment in this workspace. Do you want to select it to start up the features in the Python extension? Only accept if you trust this environment.", "Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.", "Logging.CurrentWorkingDirectory": "cwd:", "InterpreterQuickPickList.quickPickListPlaceholder": "Current: {0}", @@ -74,6 +72,7 @@ "InterpreterQuickPickList.browsePath.label": "Find...", "InterpreterQuickPickList.browsePath.detail": "Browse your file system to find a Python interpreter.", "InterpreterQuickPickList.browsePath.title": "Select Python interpreter", + "InterpreterQuickPickList.defaultInterpreterPath.label": "Use default Python interpreter path", "diagnostics.upgradeCodeRunner": "Please update the Code Runner extension for it to be compatible with the Python extension.", "Common.bannerLabelYes": "Yes", "Common.bannerLabelNo": "No", @@ -119,7 +118,7 @@ "Installer.searchForHelp": "Search for help", "Installer.couldNotInstallLibrary": "Could not install {0}. If pip is not available, please use the package manager of your choice to manually install this library into your Python environment.", "Installer.dataScienceInstallPrompt": "Data Science library {0} is not installed. Install?", - "diagnostics.removedPythonPathFromSettings": "We removed the \"python.pythonPath\" setting from your settings.json file as the setting is no longer used by the Python extension. You can get the path of your selected interpreter in the Python output channel. [Learn more](https://aka.ms/AA7jfor).", + "diagnostics.removedPythonPathFromSettings": "The \"python.pythonPath\" setting in your settings.json is no longer used by the Python extension. If you want, you can use a new setting called \"python.defaultInterpreterPath\" instead. Keep in mind that you need to change the value of this setting manually as the Python extension doesn’t modify it when you change interpreters. [Learn more](https://aka.ms/AA7jfor).", "diagnostics.warnSourceMaps": "Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.", "diagnostics.disableSourceMaps": "Disable Source Map Support", "diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.", diff --git a/package.nls.zh-cn.json b/package.nls.zh-cn.json index 27f1c01a1e64..df3c5ccdf158 100644 --- a/package.nls.zh-cn.json +++ b/package.nls.zh-cn.json @@ -11,7 +11,6 @@ "python.command.python.switchToDailyChannel.title": "切换到每日预览版本", "python.command.python.switchToWeeklyChannel.title": "切换到每周预览版本", "python.command.python.clearWorkspaceInterpreter.title": "清除工作区解释器设置", - "python.command.python.resetInterpreterSecurityStorage.title": "重置已存储的未受信任解释器的信息", "python.command.python.refactorExtractVariable.title": "提取变量", "python.command.python.refactorExtractMethod.title": "提取方法", "python.command.python.viewOutput.title": "显示输出", @@ -54,7 +53,6 @@ "Interpreters.entireWorkspace": "完整工作区", "Interpreters.pythonInterpreterPath": "Python 解释器路径: {0}", "Interpreters.LoadingInterpreters": "正在加载 Python 解释器", - "Interpreters.unsafeInterpreterMessage": "此工作区中有一个 Python 环境，是否启用它来激活扩展中的功能？请在信任此环境时启用。", "Interpreters.condaInheritEnvMessage": "您正在使用 conda 环境，如果您在集成终端中遇到相关问题，建议您允许 Python 扩展将用户设置中的 \"terminal.integrated.inheritEnv\" 改为 false。", "Logging.CurrentWorkingDirectory": "cwd:", "InterpreterQuickPickList.quickPickListPlaceholder": "当前: {0}", @@ -108,7 +106,6 @@ "Installer.searchForHelp": "搜索帮助", "Installer.couldNotInstallLibrary": "无法安装 {0} 。如果 pip 不可用，请使用选择的包管理器手动将此库安装到您的 Python 环境中。", "Installer.dataScienceInstallPrompt": "数据科学库 {0} 未安装，是否安装？", - "diagnostics.removedPythonPathFromSettings": "已在 settings.json 文件中删除了 \"python.pythonPath\" ，因为 Python 扩展不再使用该设置。您可以在 Python 输出中获取所选解释器的路径。[了解更多](https://aka.ms/AA7jfor).", "diagnostics.warnSourceMaps": "已启用 Source Map 支持，这会影响 Python 扩展的性能。", "diagnostics.disableSourceMaps": "禁用 Source Map 支持", "diagnostics.warnBeforeEnablingSourceMaps": "启用 Source Map 支持将影响 Python 扩展的性能。", diff --git a/pythonFiles/pyvsc-run-isolated.py b/pythonFiles/pyvsc-run-isolated.py deleted file mode 100644 index 18c2a61872f0..000000000000 --- a/pythonFiles/pyvsc-run-isolated.py +++ /dev/null @@ -1,32 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -if __name__ != "__main__": - raise Exception("{} cannot be imported".format(__name__)) - -import os -import os.path -import runpy -import sys - - -def normalize(path): - return os.path.normcase(os.path.normpath(path)) - - -# We "isolate" the script/module (sys.argv[1]) by removing current working -# directory or '' in sys.path and then sending the target on to runpy. -cwd = normalize(os.getcwd()) -sys.path[:] = [p for p in sys.path if p != "" and normalize(p) != cwd] -del sys.argv[0] -module = sys.argv[0] -if module == "-c": - ns = {} - for code in sys.argv[1:]: - exec(code, ns, ns) -elif module.startswith("-"): - raise NotImplementedError(sys.argv) -elif module.endswith(".py"): - runpy.run_path(module, run_name="__main__") -else: - runpy.run_module(module, run_name="__main__", alter_sys=True) diff --git a/src/client/activation/activationManager.ts b/src/client/activation/activationManager.ts index ca2276838036..bd7fc45f6517 100644 --- a/src/client/activation/activationManager.ts +++ b/src/client/activation/activationManager.ts @@ -7,13 +7,13 @@ import { inject, injectable, multiInject } from 'inversify'; import { TextDocument } from 'vscode'; import { IApplicationDiagnostics } from '../application/types'; import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types'; -import { DEFAULT_INTERPRETER_SETTING, PYTHON_LANGUAGE } from '../common/constants'; +import { PYTHON_LANGUAGE } from '../common/constants'; import { DeprecatePythonPath } from '../common/experiments/groups'; import { traceDecorators } from '../common/logger'; import { IFileSystem } from '../common/platform/types'; -import { IDisposable, IExperimentsManager, IInterpreterPathService, Resource } from '../common/types'; -import { createDeferred, Deferred } from '../common/utils/async'; -import { IInterpreterAutoSelectionService, IInterpreterSecurityService } from '../interpreter/autoSelection/types'; +import { IDisposable, IExperimentService, IInterpreterPathService, Resource } from '../common/types'; +import { Deferred } from '../common/utils/async'; +import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types'; import { IInterpreterService } from '../interpreter/contracts'; import { sendActivationTelemetry } from '../telemetry/envFileTelemetry'; import { IExtensionActivationManager, IExtensionActivationService, IExtensionSingleActivationService } from './types'; @@ -21,9 +21,13 @@ import { IExtensionActivationManager, IExtensionActivationService, IExtensionSin @injectable() export class ExtensionActivationManager implements IExtensionActivationManager { public readonly activatedWorkspaces = new Set(); + protected readonly isInterpreterSetForWorkspacePromises = new Map>(); + private readonly disposables: IDisposable[] = []; + private docOpenedHandler?: IDisposable; + constructor( @multiInject(IExtensionActivationService) private readonly activationServices: IExtensionActivationService[], @multiInject(IExtensionSingleActivationService) @@ -35,12 +39,11 @@ export class ExtensionActivationManager implements IExtensionActivationManager { @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IFileSystem) private readonly fileSystem: IFileSystem, @inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService, - @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, + @inject(IExperimentService) private readonly experiments: IExperimentService, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, - @inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService, ) {} - public dispose() { + public dispose(): void { while (this.disposables.length > 0) { const disposable = this.disposables.shift()!; disposable.dispose(); @@ -50,6 +53,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.docOpenedHandler = undefined; } } + public async activate(): Promise { await this.initialize(); // Activate all activation services together. @@ -59,18 +63,18 @@ export class ExtensionActivationManager implements IExtensionActivationManager { ]); await this.autoSelection.autoSelectInterpreter(undefined); } + @traceDecorators.error('Failed to activate a workspace') - public async activateWorkspace(resource: Resource) { + public async activateWorkspace(resource: Resource): Promise { const key = this.getWorkspaceKey(resource); if (this.activatedWorkspaces.has(key)) { return; } this.activatedWorkspaces.add(key); - if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) { + if (this.experiments.inExperimentSync(DeprecatePythonPath.experiment)) { await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource); } - this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); // Get latest interpreter list in the background. this.interpreterService.getInterpreters(resource).ignoreErrors(); @@ -78,15 +82,16 @@ export class ExtensionActivationManager implements IExtensionActivationManager { await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource); await this.autoSelection.autoSelectInterpreter(resource); - await this.evaluateAutoSelectedInterpreterSafety(resource); await Promise.all(this.activationServices.map((item) => item.activate(resource))); await this.appDiagnostics.performPreStartupHealthCheck(resource); } - public async initialize() { + + public async initialize(): Promise { this.addHandlers(); this.addRemoveDocOpenedHandlers(); } - public onDocOpened(doc: TextDocument) { + + public onDocOpened(doc: TextDocument): void { if (doc.languageId !== PYTHON_LANGUAGE) { return; } @@ -102,39 +107,11 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.activateWorkspace(folder ? folder.uri : undefined).ignoreErrors(); } - public async evaluateAutoSelectedInterpreterSafety(resource: Resource) { - if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) { - const workspaceKey = this.getWorkspaceKey(resource); - const interpreterSettingValue = this.interpreterPathService.get(resource); - if (interpreterSettingValue.length === 0 || interpreterSettingValue === DEFAULT_INTERPRETER_SETTING) { - // Setting is not set, extension will use the autoselected value. Make sure it's safe. - const interpreter = this.autoSelection.getAutoSelectedInterpreter(resource); - if (interpreter) { - const isInterpreterSetForWorkspace = createDeferred(); - this.isInterpreterSetForWorkspacePromises.set(workspaceKey, isInterpreterSetForWorkspace); - await Promise.race([ - isInterpreterSetForWorkspace.promise, - this.interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter, resource), - ]); - } - } else { - // Resolve any concurrent calls waiting on the promise - if (this.isInterpreterSetForWorkspacePromises.has(workspaceKey)) { - this.isInterpreterSetForWorkspacePromises.get(workspaceKey)!.resolve(); - this.isInterpreterSetForWorkspacePromises.delete(workspaceKey); - } - } - } - this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); - } - - protected addHandlers() { + protected addHandlers(): void { this.disposables.push(this.workspaceService.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this)); - this.disposables.push( - this.interpreterPathService.onDidChange((i) => this.evaluateAutoSelectedInterpreterSafety(i.uri)), - ); } - protected addRemoveDocOpenedHandlers() { + + protected addRemoveDocOpenedHandlers(): void { if (this.hasMultipleWorkspaces()) { if (!this.docOpenedHandler) { this.docOpenedHandler = this.documentManager.onDidOpenTextDocument(this.onDocOpened, this); @@ -146,8 +123,9 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.docOpenedHandler = undefined; } } - protected onWorkspaceFoldersChanged() { - //If an activated workspace folder was removed, delete its key + + protected onWorkspaceFoldersChanged(): void { + // If an activated workspace folder was removed, delete its key const workspaceKeys = this.workspaceService.workspaceFolders!.map((workspaceFolder) => this.getWorkspaceKey(workspaceFolder.uri), ); @@ -160,10 +138,12 @@ export class ExtensionActivationManager implements IExtensionActivationManager { } this.addRemoveDocOpenedHandlers(); } - protected hasMultipleWorkspaces() { + + protected hasMultipleWorkspaces(): boolean { return this.workspaceService.hasWorkspaceFolders && this.workspaceService.workspaceFolders!.length > 1; } - protected getWorkspaceKey(resource: Resource) { + + protected getWorkspaceKey(resource: Resource): string { return this.workspaceService.getWorkspaceFolderIdentifier(resource, ''); } } diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index 3e4f5e0e5334..2be54d3a0a49 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -13,7 +13,7 @@ import { import { ChildProcess } from 'child_process'; import { DeprecatePythonPath } from '../../common/experiments/groups'; import { traceDecorators, traceError } from '../../common/logger'; -import { IExperimentsManager, IInterpreterPathService, Resource } from '../../common/types'; +import { IExperimentService, IInterpreterPathService, Resource } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; import { LanguageServerSymbolProvider } from '../../providers/symbolProvider'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -41,7 +41,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { constructor( @inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory, @inject(ITestingService) private readonly testManager: ITestingService, - @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, + @inject(IExperimentService) private readonly experiments: IExperimentService, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, ) {} @@ -166,7 +166,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { const progressReporting = new ProgressReporting(this.languageClient!); this.disposables.push(progressReporting); - if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) { + if (this.experiments.inExperimentSync(DeprecatePythonPath.experiment)) { this.disposables.push( this.interpreterPathService.onDidChange(() => { // Manually send didChangeConfiguration in order to get the server to re-query diff --git a/src/client/activation/node/languageServerProxy.ts b/src/client/activation/node/languageServerProxy.ts index 2426b0a0f5a0..a309b6ec1d85 100644 --- a/src/client/activation/node/languageServerProxy.ts +++ b/src/client/activation/node/languageServerProxy.ts @@ -13,13 +13,7 @@ import { import { DeprecatePythonPath } from '../../common/experiments/groups'; import { traceDecorators, traceError } from '../../common/logger'; -import { - IConfigurationService, - IExperimentService, - IExperimentsManager, - IInterpreterPathService, - Resource, -} from '../../common/types'; +import { IConfigurationService, IExperimentService, IInterpreterPathService, Resource } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { swallowExceptions } from '../../common/utils/decorators'; import { noop } from '../../common/utils/misc'; @@ -71,7 +65,6 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { @inject(ITestingService) private readonly testManager: ITestingService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ILanguageServerFolderService) private readonly folderService: ILanguageServerFolderService, - @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, @inject(IExperimentService) private readonly experimentService: IExperimentService, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, @inject(IEnvironmentVariablesProvider) private readonly environmentService: IEnvironmentVariablesProvider, @@ -188,7 +181,7 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { const progressReporting = new ProgressReporting(this.languageClient!); this.disposables.push(progressReporting); - if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) { + if (this.experimentService.inExperimentSync(DeprecatePythonPath.experiment)) { this.disposables.push( this.interpreterPathService.onDidChange(() => { // Manually send didChangeConfiguration in order to get the server to requery diff --git a/src/client/application/diagnostics/checks/macPythonInterpreter.ts b/src/client/application/diagnostics/checks/macPythonInterpreter.ts index 6c165a09edc3..000600d34e2d 100644 --- a/src/client/application/diagnostics/checks/macPythonInterpreter.ts +++ b/src/client/application/diagnostics/checks/macPythonInterpreter.ts @@ -11,7 +11,7 @@ import { IPlatformService } from '../../../common/platform/types'; import { IConfigurationService, IDisposableRegistry, - IExperimentsManager, + IExperimentService, IInterpreterPathService, InterpreterConfigurationScope, Resource, @@ -148,11 +148,10 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService { const workspaceService = this.serviceContainer.get(IWorkspaceService); const disposables = this.serviceContainer.get(IDisposableRegistry); const interpreterPathService = this.serviceContainer.get(IInterpreterPathService); - const experiments = this.serviceContainer.get(IExperimentsManager); - if (experiments.inExperiment(DeprecatePythonPath.experiment)) { + const experiments = this.serviceContainer.get(IExperimentService); + if (experiments.inExperimentSync(DeprecatePythonPath.experiment)) { disposables.push(interpreterPathService.onDidChange((i) => this.onDidChangeConfiguration(undefined, i))); } - experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this))); } diff --git a/src/client/application/diagnostics/checks/pythonPathDeprecated.ts b/src/client/application/diagnostics/checks/pythonPathDeprecated.ts index 98e6142f5195..73c292ef3515 100644 --- a/src/client/application/diagnostics/checks/pythonPathDeprecated.ts +++ b/src/client/application/diagnostics/checks/pythonPathDeprecated.ts @@ -3,11 +3,10 @@ // eslint-disable-next-line max-classes-per-file import { inject, named } from 'inversify'; -import { ConfigurationTarget, DiagnosticSeverity } from 'vscode'; +import { DiagnosticSeverity } from 'vscode'; import { IWorkspaceService } from '../../../common/application/types'; -import { STANDARD_OUTPUT_CHANNEL } from '../../../common/constants'; import { DeprecatePythonPath } from '../../../common/experiments/groups'; -import { IDisposableRegistry, IExperimentsManager, IOutputChannel, Resource } from '../../../common/types'; +import { IDisposableRegistry, IExperimentService, Resource } from '../../../common/types'; import { Common, Diagnostics } from '../../../common/utils/localize'; import { IServiceContainer } from '../../../ioc/types'; import { BaseDiagnostic, BaseDiagnosticsService } from '../base'; @@ -33,8 +32,6 @@ export const PythonPathDeprecatedDiagnosticServiceId = 'PythonPathDeprecatedDiag export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsService { private workspaceService: IWorkspaceService; - private output: IOutputChannel; - constructor( @inject(IServiceContainer) serviceContainer: IServiceContainer, @inject(IDiagnosticHandlerService) @@ -44,13 +41,11 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic ) { super([DiagnosticCodes.PythonPathDeprecatedDiagnostic], serviceContainer, disposableRegistry, true); this.workspaceService = this.serviceContainer.get(IWorkspaceService); - this.output = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); } public async diagnose(resource: Resource): Promise { - const experiments = this.serviceContainer.get(IExperimentsManager); - experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); - if (!experiments.inExperiment(DeprecatePythonPath.experiment)) { + const experiments = this.serviceContainer.get(IExperimentService); + if (!experiments.inExperimentSync(DeprecatePythonPath.experiment)) { return []; } const setting = this.workspaceService.getConfiguration('python', resource).inspect('pythonPath'); @@ -65,14 +60,6 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic return []; } - public async _removePythonPathFromWorkspaceSettings(resource: Resource): Promise { - const workspaceConfig = this.workspaceService.getConfiguration('python', resource); - await Promise.all([ - workspaceConfig.update('pythonPath', undefined, ConfigurationTarget.Workspace), - workspaceConfig.update('pythonPath', undefined, ConfigurationTarget.WorkspaceFolder), - ]); - } - protected async onHandle(diagnostics: IDiagnostic[]): Promise { if (diagnostics.length === 0 || !(await this.canHandle(diagnostics[0]))) { return; @@ -81,22 +68,14 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic if (await this.filterService.shouldIgnoreDiagnostic(diagnostic.code)) { return; } - await this._removePythonPathFromWorkspaceSettings(diagnostic.resource); const commandFactory = this.serviceContainer.get(IDiagnosticsCommandFactory); const options = [ { - prompt: Common.openOutputPanel(), - command: { - diagnostic, - invoke: async (): Promise => this.output.show(true), - }, - }, - { - prompt: Common.doNotShowAgain(), - command: commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global }), + prompt: Common.ok(), }, ]; - + const command = commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global }); + await command.invoke(); await this.messageService.handle(diagnostic, { commandPrompts: options }); } } diff --git a/src/client/application/diagnostics/checks/upgradeCodeRunner.ts b/src/client/application/diagnostics/checks/upgradeCodeRunner.ts index d22f09c5b677..8a01dfedb4f3 100644 --- a/src/client/application/diagnostics/checks/upgradeCodeRunner.ts +++ b/src/client/application/diagnostics/checks/upgradeCodeRunner.ts @@ -7,7 +7,7 @@ import { DiagnosticSeverity } from 'vscode'; import { IWorkspaceService } from '../../../common/application/types'; import { CODE_RUNNER_EXTENSION_ID } from '../../../common/constants'; import { DeprecatePythonPath } from '../../../common/experiments/groups'; -import { IDisposableRegistry, IExperimentsManager, IExtensions, Resource } from '../../../common/types'; +import { IDisposableRegistry, IExperimentService, IExtensions, Resource } from '../../../common/types'; import { Common, Diagnostics } from '../../../common/utils/localize'; import { IServiceContainer } from '../../../ioc/types'; import { BaseDiagnostic, BaseDiagnosticsService } from '../base'; @@ -51,9 +51,8 @@ export class UpgradeCodeRunnerDiagnosticService extends BaseDiagnosticsService { if (this._diagnosticReturned) { return []; } - const experiments = this.serviceContainer.get(IExperimentsManager); - experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); - if (!experiments.inExperiment(DeprecatePythonPath.experiment)) { + const experiments = this.serviceContainer.get(IExperimentService); + if (!experiments.inExperimentSync(DeprecatePythonPath.experiment)) { return []; } const extension = this.extensions.getExtension(CODE_RUNNER_EXTENSION_ID); diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 15fb75857304..fa6735a6d0fa 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -20,7 +20,6 @@ interface ICommandNameWithoutArgumentTypeMapping { [Commands.SwitchToInsidersDaily]: []; [Commands.SwitchToInsidersWeekly]: []; [Commands.ClearWorkspaceInterpreter]: []; - [Commands.ResetInterpreterSecurityStorage]: []; [Commands.SwitchOffInsidersChannel]: []; [Commands.Set_Interpreter]: []; [Commands.Set_ShebangInterpreter]: []; @@ -75,7 +74,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu ['setContext']: [string, boolean] | ['python.vscode.channel', Channel]; ['python.reloadVSCode']: [string]; ['revealLine']: [{ lineNumber: number; at: 'top' | 'center' | 'bottom' }]; - ['python._loadLanguageServerExtension']: {}[]; + ['python._loadLanguageServerExtension']: Record[]; ['python.SelectAndInsertDebugConfiguration']: [TextDocument, Position, CancellationToken]; ['vscode.open']: [Uri]; ['notebook.execute']: []; diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index e8602be7ded8..da291555bed6 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -15,7 +15,7 @@ import { } from 'vscode'; import { LanguageServerType } from '../activation/types'; import './extensions'; -import { IInterpreterAutoSelectionProxyService, IInterpreterSecurityService } from '../interpreter/autoSelection/types'; +import { IInterpreterAutoSelectionProxyService } from '../interpreter/autoSelection/types'; import { LogLevel } from '../logging/levels'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; @@ -32,7 +32,7 @@ import { IAutoCompleteSettings, IDefaultLanguageServer, IExperiments, - IExperimentsManager, + IExperimentService, IFormattingSettings, IInterpreterPathService, ILintingSettings, @@ -148,8 +148,6 @@ export class PythonSettings implements IPythonSettings { public logging: ILoggingSettings = { level: LogLevel.Error }; - public useIsolation = true; - protected readonly changed = new EventEmitter(); private workspaceRoot: Resource; @@ -166,9 +164,8 @@ export class PythonSettings implements IPythonSettings { workspaceFolder: Resource, private readonly interpreterAutoSelectionService: IInterpreterAutoSelectionProxyService, workspace?: IWorkspaceService, - private readonly experimentsManager?: IExperimentsManager, + private readonly experimentsManager?: IExperimentService, private readonly interpreterPathService?: IInterpreterPathService, - private readonly interpreterSecurityService?: IInterpreterSecurityService, private readonly defaultLS?: IDefaultLanguageServer, ) { this.workspace = workspace || new WorkspaceService(); @@ -180,9 +177,8 @@ export class PythonSettings implements IPythonSettings { resource: Uri | undefined, interpreterAutoSelectionService: IInterpreterAutoSelectionProxyService, workspace?: IWorkspaceService, - experimentsManager?: IExperimentsManager, + experimentsManager?: IExperimentService, interpreterPathService?: IInterpreterPathService, - interpreterSecurityService?: IInterpreterSecurityService, defaultLS?: IDefaultLanguageServer, ): PythonSettings { workspace = workspace || new WorkspaceService(); @@ -196,7 +192,6 @@ export class PythonSettings implements IPythonSettings { workspace, experimentsManager, interpreterPathService, - interpreterSecurityService, defaultLS, ); PythonSettings.pythonSettings.set(workspaceFolderKey, settings); @@ -266,6 +261,12 @@ export class PythonSettings implements IPythonSettings { const defaultInterpreterPath = systemVariables.resolveAny(pythonSettings.get('defaultInterpreterPath')); this.defaultInterpreterPath = defaultInterpreterPath || DEFAULT_INTERPRETER_SETTING; + if (this.defaultInterpreterPath === DEFAULT_INTERPRETER_SETTING) { + const autoSelectedPythonInterpreter = this.interpreterAutoSelectionService.getAutoSelectedInterpreter( + this.workspaceRoot, + ); + this.defaultInterpreterPath = autoSelectedPythonInterpreter?.path ?? this.defaultInterpreterPath; + } this.defaultInterpreterPath = getAbsolutePath(this.defaultInterpreterPath, workspaceRoot); this.venvPath = systemVariables.resolveAny(pythonSettings.get('venvPath'))!; @@ -284,8 +285,6 @@ export class PythonSettings implements IPythonSettings { pythonSettings.get('autoUpdateLanguageServer', true), )!; - this.useIsolation = systemVariables.resolveAny(pythonSettings.get('useIsolation', true))!; - // Get as a string and verify; don't just accept. let userLS = pythonSettings.get('languageServer'); userLS = systemVariables.resolveAny(userLS); @@ -632,9 +631,6 @@ export class PythonSettings implements IPythonSettings { this.disposables.push( this.interpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this)), ); - if (this.interpreterSecurityService) { - this.disposables.push(this.interpreterSecurityService.onDidChangeSafeInterpreters(onDidChange.bind(this))); - } this.disposables.push( this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { if (event.affectsConfiguration('python')) { @@ -668,8 +664,7 @@ export class PythonSettings implements IPythonSettings { * But we can still use it here for this particular experiment. Reason being that this experiment only changes * `pythonPath` setting, and I've checked that `pythonPath` setting is not accessed anywhere in the constructor. */ - const inExperiment = this.experimentsManager?.inExperiment(DeprecatePythonPath.experiment); - this.experimentsManager?.sendTelemetryIfInExperiment(DeprecatePythonPath.control); + const inExperiment = this.experimentsManager?.inExperimentSync(DeprecatePythonPath.experiment); // Use the interpreter path service if in the experiment otherwise use the normal settings this.pythonPath = systemVariables.resolveAny( inExperiment && this.interpreterPathService @@ -684,12 +679,8 @@ export class PythonSettings implements IPythonSettings { const autoSelectedPythonInterpreter = this.interpreterAutoSelectionService.getAutoSelectedInterpreter( this.workspaceRoot, ); - if (inExperiment && this.interpreterSecurityService) { - if ( - autoSelectedPythonInterpreter && - this.interpreterSecurityService.isSafe(autoSelectedPythonInterpreter) && - this.workspaceRoot - ) { + if (inExperiment) { + if (autoSelectedPythonInterpreter && this.workspaceRoot) { this.pythonPath = autoSelectedPythonInterpreter.path; this.interpreterAutoSelectionService .setWorkspaceInterpreter(this.workspaceRoot, autoSelectedPythonInterpreter) diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index 4a0e0919bc32..5fb87a0280fb 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; -import { IInterpreterAutoSelectionService, IInterpreterSecurityService } from '../../interpreter/autoSelection/types'; +import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelection/types'; import { IServiceContainer } from '../../ioc/types'; import { IWorkspaceService } from '../application/types'; import { PythonSettings } from '../configSettings'; @@ -12,7 +12,7 @@ import { DeprecatePythonPath } from '../experiments/groups'; import { IConfigurationService, IDefaultLanguageServer, - IExperimentsManager, + IExperimentService, IInterpreterPathService, IPythonSettings, } from '../types'; @@ -30,10 +30,7 @@ export class ConfigurationService implements IConfigurationService { IInterpreterAutoSelectionService, ); const interpreterPathService = this.serviceContainer.get(IInterpreterPathService); - const experiments = this.serviceContainer.get(IExperimentsManager); - const interpreterSecurityService = this.serviceContainer.get( - IInterpreterSecurityService, - ); + const experiments = this.serviceContainer.get(IExperimentService); const defaultLS = this.serviceContainer.tryGet(IDefaultLanguageServer); return PythonSettings.getInstance( resource, @@ -41,7 +38,6 @@ export class ConfigurationService implements IConfigurationService { this.workspaceService, experiments, interpreterPathService, - interpreterSecurityService, defaultLS, ); } @@ -53,10 +49,9 @@ export class ConfigurationService implements IConfigurationService { resource?: Uri, configTarget?: ConfigurationTarget, ): Promise { - const experiments = this.serviceContainer.get(IExperimentsManager); + const experiments = this.serviceContainer.get(IExperimentService); const interpreterPathService = this.serviceContainer.get(IInterpreterPathService); - const inExperiment = experiments.inExperiment(DeprecatePythonPath.experiment); - experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); + const inExperiment = experiments.inExperimentSync(DeprecatePythonPath.experiment); const defaultSetting = { uri: resource, target: configTarget || ConfigurationTarget.WorkspaceFolder, diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 4f0b3051e800..75b3aac014c5 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -1,3 +1,5 @@ +/* eslint-disable camelcase */ +/* eslint-disable @typescript-eslint/no-namespace */ export const PYTHON_LANGUAGE = 'python'; export const PYTHON_WARNINGS = 'PYTHONWARNINGS'; @@ -75,12 +77,13 @@ export namespace Commands { export const GetSelectedInterpreterPath = 'python.interpreterPath'; export const ClearStorage = 'python.clearPersistentStorage'; export const ClearWorkspaceInterpreter = 'python.clearWorkspaceInterpreter'; - export const ResetInterpreterSecurityStorage = 'python.resetInterpreterSecurityStorage'; export const OpenStartPage = 'python.startPage.open'; export const LaunchTensorBoard = 'python.launchTensorBoard'; export const RefreshTensorBoard = 'python.refreshTensorBoard'; export const ReportIssue = 'python.reportIssue'; } + +// Look at https://microsoft.github.io/vscode-codicons/dist/codicon.html for other Octicon icon ids export namespace Octicons { export const Test_Pass = '$(check)'; export const Test_Fail = '$(alert)'; @@ -89,6 +92,7 @@ export namespace Octicons { export const Downloading = '$(cloud-download)'; export const Installing = '$(desktop-download)'; export const Search_Stop = '$(search-stop)'; + export const Star = '$(star)'; } export const Button_Text_Tests_View_Output = 'View Output'; diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 1bc44c34e174..602b2ccf0860 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -7,8 +7,7 @@ export enum ShowExtensionSurveyPrompt { * Experiment to check whether the extension should deprecate `python.pythonPath` setting */ export enum DeprecatePythonPath { - control = 'DeprecatePythonPath - control', - experiment = 'DeprecatePythonPath - experiment', + experiment = 'pythonDeprecatePythonPath', } // Experiment to switch Jedi to use an LSP instead of direct providers diff --git a/src/client/common/experiments/manager.ts b/src/client/common/experiments/manager.ts deleted file mode 100644 index 4ae0f4133214..000000000000 --- a/src/client/common/experiments/manager.ts +++ /dev/null @@ -1,256 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// Refer to A/B testing wiki for more details: https://en.wikipedia.org/wiki/A/B_testing - -'use strict'; - -import { inject, injectable, named } from 'inversify'; -import { parse } from 'jsonc-parser'; -import * as path from 'path'; -import { sendTelemetryEvent } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; -import { IApplicationEnvironment } from '../application/types'; -import { EXTENSION_ROOT_DIR, STANDARD_OUTPUT_CHANNEL } from '../constants'; -import { traceDecorators, traceError } from '../logger'; -import { IFileSystem } from '../platform/types'; -import { - ABExperiments, - IConfigurationService, - ICryptoUtils, - IExperimentsManager, - IOutputChannel, - IPersistentState, - IPersistentStateFactory, - IPythonSettings, -} from '../types'; -import { swallowExceptions } from '../utils/decorators'; -import { Experiments } from '../utils/localize'; - -export const experimentStorageKey = 'EXPERIMENT_STORAGE_KEY'; -/** - * Local experiments config file. We have this to ensure that experiments are used in the first session itself, - * as about 40% of the users never come back for the second session. - */ -const configFile = path.join(EXTENSION_ROOT_DIR, 'experiments.json'); -export const oldExperimentSalts = ['LS']; - -/** - * Manages and stores experiments, implements the AB testing functionality - * @deprecated - */ -@injectable() -export class ExperimentsManager implements IExperimentsManager { - /** - * Keeps track of the list of experiments user is in - */ - public userExperiments: ABExperiments = []; - /** - * Experiments user requested to opt into manually - */ - public _experimentsOptedInto: string[] = []; - /** - * Experiments user requested to opt out from manually - */ - public _experimentsOptedOutFrom: string[] = []; - /** - * Returns `true` if experiments are enabled, else `false`. - */ - public _enabled: boolean = true; - /** - * Keeps track of the experiments to be used in the current session - */ - private experimentStorage: IPersistentState; - - /** - * Keeps track if the storage needs updating or not. - * Note this has to be separate from the actual storage as - * download storages by itself should not have an Expiry (so that it can be used in the next session even when download fails in the current session) - */ - private activatedOnce: boolean = false; - private settings!: IPythonSettings; - - constructor( - @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, - @inject(ICryptoUtils) private readonly crypto: ICryptoUtils, - @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel, - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(IConfigurationService) private readonly configurationService: IConfigurationService, - ) { - this.experimentStorage = this.persistentStateFactory.createGlobalPersistentState( - experimentStorageKey, - undefined, - ); - } - - @swallowExceptions('Failed to activate experiments') - public async activate(): Promise { - if (this.activatedOnce) { - return; - } - this.activatedOnce = true; - this.settings = this.configurationService.getSettings(undefined); - this._experimentsOptedInto = this.settings.experiments.optInto; - this._experimentsOptedOutFrom = this.settings.experiments.optOutFrom; - this._enabled = this.settings.experiments.enabled; - if (!this._enabled) { - sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_DISABLED); - return; - } - await this.updateExperimentStorage(); - this.populateUserExperiments(); - } - - @traceDecorators.error('Failed to identify if user is in experiment') - public inExperiment(experimentName: string): boolean { - if (!this._enabled) { - return false; - } - this.sendTelemetryIfInExperiment(experimentName); - return this.userExperiments.find((exp) => exp.name === experimentName) ? true : false; - } - - /** - * Populates list of experiments user is in - */ - @traceDecorators.error('Failed to populate user experiments') - public populateUserExperiments(): void { - this.cleanUpExperimentsOptList(); - if (Array.isArray(this.experimentStorage.value)) { - const remainingExperiments: ABExperiments = []; - // First process experiments in order of user preference (if they have opted out or opted in). - for (const experiment of this.experimentStorage.value) { - try { - if ( - this._experimentsOptedOutFrom.includes('All') || - this._experimentsOptedOutFrom.includes(experiment.name) - ) { - continue; - } - - if ( - this._experimentsOptedInto.includes('All') || - this._experimentsOptedInto.includes(experiment.name) - ) { - this.userExperiments.push(experiment); - } else { - remainingExperiments.push(experiment); - } - } catch (ex) { - traceError(`Failed to populate experiment list for experiment '${experiment.name}'`, ex); - } - } - - // Add users (based on algorithm) to experiments they haven't already opted out of or opted into. - remainingExperiments - .filter((experiment) => this.isUserInRange(experiment.min, experiment.max, experiment.salt)) - .filter((experiment) => !this.userExperiments.some((existing) => existing.salt === experiment.salt)) - .forEach((experiment) => { - this.userExperiments.push(experiment); - // Only log experiments that are assigned by this framework. - // The opted-in/out ones are logged by the ExperimentService instance. - this.output.appendLine(Experiments.inGroup().format(experiment.name)); - }); - } - } - - @traceDecorators.error('Failed to send telemetry when user is in experiment') - public sendTelemetryIfInExperiment(experimentName: string): void { - if (this.userExperiments.find((exp) => exp.name === experimentName)) { - sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS, undefined, { expName: experimentName }); - } - } - - /** - * Checks if user falls between the range of the experiment - * @param min The lower limit - * @param max The upper limit - * @param salt The experiment salt value - */ - public isUserInRange(min: number, max: number, salt: string): boolean { - if (typeof this.appEnvironment.machineId !== 'string') { - throw new Error('Machine ID should be a string'); - } - let hash: number; - if (oldExperimentSalts.find((oldSalt) => oldSalt === salt)) { - hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'number', 'SHA512'); - } else { - hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'number', 'FNV'); - } - return hash % 100 >= min && hash % 100 < max; - } - - /** - * Do best effort to populate experiment storage. - * Attempt to update experiment storage by using appropriate local data if available. - * Local data could be a default experiments file shipped with the extension. - * - Note this file is only used when experiment storage is empty, which is usually the case the first time the extension loads. - * - We have this local file to ensure that experiments are used in the first session itself, - * as about 40% of the users never come back for the second session. - */ - @swallowExceptions('Failed to update experiment storage') - public async updateExperimentStorage(): Promise { - if (!process.env.VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE) { - if (Array.isArray(this.experimentStorage.value)) { - // Experiment storage already contains latest experiments, do not use the following techniques - return; - } - } - - // Update experiment storage using local experiments file if available. - if (await this.fs.fileExists(configFile)) { - const content = await this.fs.readFile(configFile); - try { - const experiments = parse(content, [], { allowTrailingComma: true, disallowComments: false }); - if (!this.areExperimentsValid(experiments)) { - throw new Error('Parsed experiments are not valid'); - } - await this.experimentStorage.updateValue(experiments); - } catch (ex) { - traceError('Failed to parse experiments configuration file to update storage', ex); - } - } - } - - /** - * Checks that experiments are not invalid or incomplete - * @param experiments Local or downloaded experiments - * @returns `true` if type of experiments equals `ABExperiments` type, `false` otherwise - */ - public areExperimentsValid(experiments: ABExperiments): boolean { - if (!Array.isArray(experiments)) { - traceError('Experiments are not of array type'); - return false; - } - for (const exp of experiments) { - if (exp.name === undefined || exp.salt === undefined || exp.min === undefined || exp.max === undefined) { - traceError('Experiments are missing fields from ABExperiments type'); - return false; - } - } - return true; - } - - public _activated(): boolean { - return this.activatedOnce; - } - - /** - * You can only opt in or out of experiment groups, not control groups. So remove requests for control groups. - */ - private cleanUpExperimentsOptList(): void { - for (let i = 0; i < this._experimentsOptedInto.length; i += 1) { - if (this._experimentsOptedInto[i].endsWith('control')) { - this._experimentsOptedInto[i] = ''; - } - } - for (let i = 0; i < this._experimentsOptedOutFrom.length; i += 1) { - if (this._experimentsOptedOutFrom[i].endsWith('control')) { - this._experimentsOptedOutFrom[i] = ''; - } - } - this._experimentsOptedInto = this._experimentsOptedInto.filter((exp) => exp !== ''); - this._experimentsOptedOutFrom = this._experimentsOptedOutFrom.filter((exp) => exp !== ''); - } -} diff --git a/src/client/common/interpreterPathService.ts b/src/client/common/interpreterPathService.ts index acc41161c4a2..e3607323702f 100644 --- a/src/client/common/interpreterPathService.ts +++ b/src/client/common/interpreterPathService.ts @@ -71,12 +71,13 @@ export class InterpreterPathService implements IInterpreterPathService { undefined, ); } - const globalValue = this.workspaceService.getConfiguration('python')!.inspect('defaultInterpreterPath')! - .globalValue; + const defaultInterpreterPath = this.workspaceService + .getConfiguration('python', resource)! + .inspect('defaultInterpreterPath')!; return { - globalValue, - workspaceFolderValue: workspaceFolderSetting?.value, - workspaceValue: workspaceSetting?.value, + globalValue: defaultInterpreterPath.globalValue, + workspaceFolderValue: workspaceFolderSetting?.value || defaultInterpreterPath.workspaceFolderValue, + workspaceValue: workspaceSetting?.value || defaultInterpreterPath.workspaceValue, }; } @@ -197,10 +198,6 @@ export class InterpreterPathService implements IInterpreterPathService { const shouldUpdateGlobalSetting = !isGlobalSettingCopiedStorage.value; if (shouldUpdateGlobalSetting) { await this.update(undefined, ConfigurationTarget.Global, value); - // Make sure to delete the original setting after copying it - await this.workspaceService - .getConfiguration('python') - .update('pythonPath', undefined, ConfigurationTarget.Global); await isGlobalSettingCopiedStorage.updateValue(true); } } diff --git a/src/client/common/process/internal/python.ts b/src/client/common/process/internal/python.ts index 8731ed4eec88..2cf083fe4f08 100644 --- a/src/client/common/process/internal/python.ts +++ b/src/client/common/process/internal/python.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { _ISOLATED as ISOLATED, getUseIsolationSetting, maybeIsolated } from './scripts'; - // "python" contains functions corresponding to the various ways that // the extension invokes a Python executable internally. Each function // takes arguments relevant to the specific use case. However, each @@ -15,28 +13,21 @@ import { _ISOLATED as ISOLATED, getUseIsolationSetting, maybeIsolated } from './ // into the corresponding object or objects. "parse()" takes a single // string as the stdout text and returns the relevant data. -export function execCode(code: string, isolated = true): string[] { +export function execCode(code: string): string[] { let args = ['-c', code]; - if (isolated) { - args = maybeIsolated(args); - } // "code" isn't specific enough to know how to parse it, // so we only return the args. return args; } -export function execModule(name: string, moduleArgs: string[], isolated = true): string[] { +export function execModule(name: string, moduleArgs: string[]): string[] { const args = ['-m', name, ...moduleArgs]; - if (isolated && getUseIsolationSetting()) { - args[0] = ISOLATED; // replace - } // "code" isn't specific enough to know how to parse it, // so we only return the args. return args; } export function getVersion(): [string[], (out: string) => string] { - // There is no need to isolate this. const args = ['--version']; function parse(out: string): string { @@ -47,7 +38,7 @@ export function getVersion(): [string[], (out: string) => string] { } export function getSysPrefix(): [string[], (out: string) => string] { - const args = maybeIsolated(['-c', 'import sys;print(sys.prefix)']); + const args = ['-c', 'import sys;print(sys.prefix)']; function parse(out: string): string { return out.trim(); @@ -57,7 +48,7 @@ export function getSysPrefix(): [string[], (out: string) => string] { } export function getExecutable(): [string[], (out: string) => string] { - const args = maybeIsolated(['-c', 'import sys;print(sys.executable)']); + const args = ['-c', 'import sys;print(sys.executable)']; function parse(out: string): string { return out.trim(); @@ -70,7 +61,7 @@ export function getSitePackages(): [string[], (out: string) => string] { // On windows we also need the libs path (second item will // return c:\xxx\lib\site-packages). This is returned by // the following: get_python_lib - const args = maybeIsolated(['-c', 'from distutils.sysconfig import get_python_lib; print(get_python_lib())']); + const args = ['-c', 'from distutils.sysconfig import get_python_lib; print(get_python_lib())']; function parse(out: string): string { return out.trim(); @@ -80,7 +71,7 @@ export function getSitePackages(): [string[], (out: string) => string] { } export function getUserSitePackages(): [string[], (out: string) => string] { - const args = maybeIsolated(['site', '--user-site']); + const args = ['site', '--user-site']; function parse(out: string): string { return out.trim(); @@ -90,7 +81,6 @@ export function getUserSitePackages(): [string[], (out: string) => string] { } export function isValid(): [string[], (out: string) => boolean] { - // There is no need to isolate this. const args = ['-c', 'print(1234)']; function parse(out: string): boolean { @@ -101,7 +91,7 @@ export function isValid(): [string[], (out: string) => boolean] { } export function isModuleInstalled(name: string): [string[], (out: string) => boolean] { - const args = maybeIsolated(['-c', `import ${name}`]); + const args = ['-c', `import ${name}`]; function parse(_out: string): boolean { // If the command did not fail then the module is installed. @@ -112,7 +102,7 @@ export function isModuleInstalled(name: string): [string[], (out: string) => boo } export function getModuleVersion(name: string): [string[], (out: string) => string] { - const args = maybeIsolated(['-c', `import ${name}; print(${name}.__version__)`]); + const args = ['-c', `import ${name}; print(${name}.__version__)`]; function parse(out: string): string { return out.trim(); diff --git a/src/client/common/process/internal/scripts/constants.ts b/src/client/common/process/internal/scripts/constants.ts index 53765275b8b3..4448f7e639ce 100644 --- a/src/client/common/process/internal/scripts/constants.ts +++ b/src/client/common/process/internal/scripts/constants.ts @@ -6,4 +6,3 @@ import { EXTENSION_ROOT_DIR } from '../../../constants'; // It is simpler to hard-code it instead of using vscode.ExtensionContext.extensionPath. export const _SCRIPTS_DIR = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); -export const _ISOLATED = path.join(_SCRIPTS_DIR, 'pyvsc-run-isolated.py'); diff --git a/src/client/common/process/internal/scripts/index.ts b/src/client/common/process/internal/scripts/index.ts index c8220c10934d..a647222a9641 100644 --- a/src/client/common/process/internal/scripts/index.ts +++ b/src/client/common/process/internal/scripts/index.ts @@ -2,31 +2,10 @@ // Licensed under the MIT License. import * as path from 'path'; -import { workspace } from 'vscode'; -import { _ISOLATED, _SCRIPTS_DIR } from './constants'; +import { _SCRIPTS_DIR } from './constants'; import { CompletionResponse, SymbolProviderSymbols } from './types'; const SCRIPTS_DIR = _SCRIPTS_DIR; -const ISOLATED = _ISOLATED; - -// Re-export it so external modules can use it too. -export { _ISOLATED } from './constants'; - -export function getUseIsolationSetting(): boolean { - try { - return workspace.getConfiguration('python').get('useIsolation', true); - } catch (ex) { - // If we can't get the setting for any reason we assume default - return true; - } -} - -export function maybeIsolated(args: string[]): string[] { - if (getUseIsolationSetting()) { - args.splice(0, 0, ISOLATED); - } - return args; -} // "scripts" contains everything relevant to the scripts found under // the top-level "pythonFiles" directory. Each of those scripts has @@ -65,7 +44,7 @@ export type InterpreterInfoJson = { export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJson] { const script = path.join(SCRIPTS_DIR, 'interpreterInfo.py'); - const args = maybeIsolated([script]); + const args = [script]; function parse(out: string): InterpreterInfoJson { let json: InterpreterInfoJson; @@ -84,7 +63,7 @@ export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJs export function completion(jediPath?: string): [string[], (out: string) => CompletionResponse[]] { const script = path.join(SCRIPTS_DIR, 'completion.py'); - const args = maybeIsolated([script]); + const args = [script]; if (jediPath) { args.push('custom'); args.push(jediPath); @@ -101,7 +80,7 @@ export function completion(jediPath?: string): [string[], (out: string) => Compl export function sortImports(filename: string, sortArgs?: string[]): [string[], (out: string) => string] { const script = path.join(SCRIPTS_DIR, 'sortImports.py'); - const args = maybeIsolated([script, filename, '--diff']); + const args = [script, filename, '--diff']; if (sortArgs) { args.push(...sortArgs); } @@ -118,7 +97,7 @@ export function sortImports(filename: string, sortArgs?: string[]): [string[], ( export function refactor(root: string): [string[], (out: string) => Record[]] { const script = path.join(SCRIPTS_DIR, 'refactor.py'); - const args = maybeIsolated([script, root]); + const args = [script, root]; // TODO: Make the return type more specific, like we did // with completion(). @@ -137,7 +116,7 @@ export function refactor(root: string): [string[], (out: string) => Record string] { const script = path.join(SCRIPTS_DIR, 'normalizeSelection.py'); - const args = maybeIsolated([script]); + const args = [script]; function parse(out: string) { // The text will be used as-is. @@ -155,7 +134,7 @@ export function symbolProvider( text?: string, ): [string[], (out: string) => SymbolProviderSymbols] { const script = path.join(SCRIPTS_DIR, 'symbolProvider.py'); - const args = maybeIsolated([script, filename]); + const args = [script, filename]; if (text) { args.push(text); } @@ -171,7 +150,7 @@ export function symbolProvider( export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessEnv] { const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgument(); - const args = maybeIsolated([script]); + const args = [script]; function parse(out: string): NodeJS.ProcessEnv { return JSON.parse(out); @@ -187,14 +166,14 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[ const script = path.join(SCRIPTS_DIR, 'shell_exec.py'); // We don't bother with a "parse" function since the output // could be anything. - return maybeIsolated([ + return [ script, command.fileToCommandArgument(), // The shell args must come after the command // but before the lockfile. ...shellArgs, lockfile.fileToCommandArgument(), - ]); + ]; } // testlauncher.py @@ -202,7 +181,7 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[ export function testlauncher(testArgs: string[]): string[] { const script = path.join(SCRIPTS_DIR, 'testlauncher.py'); // There is no output to parse, so we do not return a function. - return maybeIsolated([script, ...testArgs]); + return [script, ...testArgs]; } // visualstudio_py_testlauncher.py @@ -218,5 +197,5 @@ export function visualstudio_py_testlauncher(testArgs: string[]): string[] { export function tensorboardLauncher(args: string[]): string[] { const script = path.join(SCRIPTS_DIR, 'tensorboard_launcher.py'); - return maybeIsolated([script, ...args]); + return [script, ...args]; } diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 74dcb36f5350..def5ae48c3fd 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import { IExtensionSingleActivationService } from '../activation/types'; import { - IExperimentService, IFileDownloader, IHttpClient, IInterpreterPathService, @@ -47,7 +46,6 @@ import { ConfigurationService } from './configuration/service'; import { PipEnvExecutionPath } from './configuration/executionSettings/pipEnvExecution'; import { CryptoUtils } from './crypto'; import { EditorUtils } from './editor'; -import { ExperimentsManager } from './experiments/manager'; import { ExperimentService } from './experiments/service'; import { ExtensionInsidersDailyChannelRule, @@ -109,7 +107,7 @@ import { ICryptoUtils, ICurrentProcess, IEditorUtils, - IExperimentsManager, + IExperimentService, IExtensions, IInstaller, IPathUtils, @@ -166,7 +164,6 @@ export function registerTypes(serviceManager: IServiceManager) { PowershellTerminalActivationFailedHandler, ); serviceManager.addSingleton(ICryptoUtils, CryptoUtils); - serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); serviceManager.addSingleton(IExperimentService, ExperimentService); serviceManager.addSingleton(ITerminalHelper, TerminalHelper); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index d42c804c45f9..8b9e423dea73 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -203,7 +203,6 @@ export interface IPythonSettings { readonly languageServerIsDefault: boolean; readonly defaultInterpreterPath: string; readonly logging: ILoggingSettings; - readonly useIsolation: boolean; readonly tensorBoard: ITensorBoardSettings | undefined; initialize(): void; } @@ -528,33 +527,6 @@ export type ABExperiments = { max: number; // Upper limit for the experiment }[]; -/** - * Interface used to implement AB testing - */ -export const IExperimentsManager = Symbol('IExperimentsManager'); -/** - * @deprecated Use IExperimentService instead - */ -export interface IExperimentsManager { - /** - * Checks if experiments are enabled, sets required environment to be used for the experiments, logs experiment groups - */ - activate(): Promise; - - /** - * Checks if user is in experiment or not - * @param experimentName Name of the experiment - * @returns `true` if user is in experiment, `false` if user is not in experiment - */ - inExperiment(experimentName: string): boolean; - - /** - * Sends experiment telemetry if user is in experiment - * @param experimentName Name of the experiment - */ - sendTelemetryIfInExperiment(experimentName: string): void; -} - /** * Experiment service leveraging VS Code's experiment framework. */ diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 6408f1aeb3a7..0b1c8397b2fd 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -35,7 +35,7 @@ export namespace Diagnostics { ); export const removedPythonPathFromSettings = localize( 'diagnostics.removedPythonPathFromSettings', - 'We removed the "python.pythonPath" setting from your settings.json file as the setting is no longer used by the Python extension. You can get the path of your selected interpreter in the Python output channel. [Learn more](https://aka.ms/AA7jfor).', + 'The "python.pythonPath" setting in your settings.json is no longer used by the Python extension. If you want, you can use a new setting called "python.defaultInterpreterPath" instead. Keep in mind that you need to change the value of this setting manually as the Python extension doesn’t modify it when you change interpreters. [Learn more](https://aka.ms/AA7jfor).', ); export const invalidPythonPathInDebuggerSettings = localize( 'diagnostics.invalidPythonPathInDebuggerSettings', @@ -270,10 +270,6 @@ export namespace Interpreters { 'Interpreters.condaInheritEnvMessage', 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings.', ); - export const unsafeInterpreterMessage = localize( - 'Interpreters.unsafeInterpreterMessage', - 'We found a Python environment in this workspace. Do you want to select it to start up the features in the Python extension? Only accept if you trust this environment.', - ); export const environmentPromptMessage = localize( 'Interpreters.environmentPromptMessage', 'We noticed a new virtual environment has been created. Do you want to select it for the workspace folder?', @@ -296,6 +292,15 @@ export namespace InterpreterQuickPickList { label: localize('InterpreterQuickPickList.enterPath.label', 'Enter interpreter path...'), placeholder: localize('InterpreterQuickPickList.enterPath.placeholder', 'Enter path to a Python interpreter.'), }; + export const defaultInterpreterPath = { + label: (): string => { + const labelText = localize( + 'InterpreterQuickPickList.defaultInterpreterPath.label', + 'Use default Python interpreter path', + ); + return `${Octicons.Star} ${labelText()}`; + }, + }; export const findPath = { detail: localize( 'InterpreterQuickPickList.findPath.detail', diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 7a2c1fede396..56aa7b112d70 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -21,7 +21,6 @@ import { IConfigurationService, IDisposableRegistry, IExperimentService, - IExperimentsManager, IExtensions, IOutputChannel, } from './common/types'; @@ -137,9 +136,6 @@ async function activateLegacy(ext: ExtensionState): Promise { // XXX Move this *after* abExperiments is activated? setLoggingLevel(configuration.getSettings().logging.level); - const abExperiments = serviceContainer.get(IExperimentsManager); - await abExperiments.activate(); - const languageServerType = configuration.getSettings().languageServer; // Language feature registrations. diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index be5b57d21391..5bc13628e3b5 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -6,10 +6,9 @@ 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 { IExperimentsManager, IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; +import { 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'; @@ -21,7 +20,6 @@ import { IInterpreterAutoSelectionRule, IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService, - IInterpreterSecurityService, } from './types'; const preferredGlobalInterpreter = 'preferredGlobalPyInterpreter'; @@ -68,8 +66,6 @@ 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 // the best available interpreters based on other rules (cache for furture use). @@ -132,15 +128,6 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio // Do not execute anycode other than fetching fromm a property. // 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; - } - - public _getAutoSelectedInterpreter(resource: Resource): PythonEnvironment | undefined { const workspaceState = this.getWorkspaceState(resource); if (workspaceState && workspaceState.value) { return workspaceState.value; diff --git a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation.ts b/src/client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation.ts deleted file mode 100644 index 31e6073207fd..000000000000 --- a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation.ts +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; -import { IApplicationShell } from '../../../common/application/types'; -import { IBrowserService, Resource } from '../../../common/types'; -import { Common, Interpreters } from '../../../common/utils/localize'; -import { PythonEnvironment } from '../../../pythonEnvironments/info'; -import { sendTelemetryEvent } from '../../../telemetry'; -import { EventName } from '../../../telemetry/constants'; -import { IInterpreterHelper } from '../../contracts'; -import { isInterpreterLocatedInWorkspace } from '../../helpers'; -import { learnMoreOnInterpreterSecurityURI } from '../constants'; -import { IInterpreterEvaluation, IInterpreterSecurityStorage } from '../types'; - -const prompts = [Common.bannerLabelYes(), Common.bannerLabelNo(), Common.learnMore(), Common.doNotShowAgain()]; -const telemetrySelections: ['Yes', 'No', 'Learn more', 'Do not show again'] = [ - 'Yes', - 'No', - 'Learn more', - 'Do not show again', -]; - -@injectable() -export class InterpreterEvaluation implements IInterpreterEvaluation { - constructor( - @inject(IApplicationShell) private readonly appShell: IApplicationShell, - @inject(IBrowserService) private browserService: IBrowserService, - @inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper, - @inject(IInterpreterSecurityStorage) private readonly interpreterSecurityStorage: IInterpreterSecurityStorage, - ) {} - - public async evaluateIfInterpreterIsSafe(interpreter: PythonEnvironment, resource: Resource): Promise { - const activeWorkspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource)?.folderUri; - if (!activeWorkspaceUri) { - return true; - } - const isSafe = this.inferValueUsingCurrentState(interpreter, resource); - return isSafe !== undefined ? isSafe : this._inferValueUsingPrompt(activeWorkspaceUri); - } - - public inferValueUsingCurrentState(interpreter: PythonEnvironment, resource: Resource) { - const activeWorkspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource)?.folderUri; - if (!activeWorkspaceUri) { - return true; - } - if (!isInterpreterLocatedInWorkspace(interpreter, activeWorkspaceUri)) { - return true; - } - const isSafe = this.interpreterSecurityStorage.hasUserApprovedWorkspaceInterpreters(activeWorkspaceUri).value; - if (isSafe !== undefined) { - return isSafe; - } - if (!this.interpreterSecurityStorage.unsafeInterpreterPromptEnabled.value) { - // If the prompt is disabled, assume all environments are safe from now on. - return true; - } - } - - public async _inferValueUsingPrompt(activeWorkspaceUri: Uri): Promise { - const areInterpretersInWorkspaceSafe = this.interpreterSecurityStorage.hasUserApprovedWorkspaceInterpreters( - activeWorkspaceUri, - ); - await this.interpreterSecurityStorage.storeKeyForWorkspace(activeWorkspaceUri); - let selection = await this.showPromptAndGetSelection(); - while (selection === Common.learnMore()) { - this.browserService.launch(learnMoreOnInterpreterSecurityURI); - selection = await this.showPromptAndGetSelection(); - } - if (!selection || selection === Common.bannerLabelNo()) { - await areInterpretersInWorkspaceSafe.updateValue(false); - return false; - } else if (selection === Common.doNotShowAgain()) { - await this.interpreterSecurityStorage.unsafeInterpreterPromptEnabled.updateValue(false); - } - await areInterpretersInWorkspaceSafe.updateValue(true); - return true; - } - - private async showPromptAndGetSelection(): Promise { - const selection = await this.appShell.showInformationMessage( - Interpreters.unsafeInterpreterMessage(), - ...prompts, - ); - sendTelemetryEvent(EventName.UNSAFE_INTERPRETER_PROMPT, undefined, { - selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined, - }); - return selection; - } -} diff --git a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService.ts b/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService.ts deleted file mode 100644 index ed7d0321aa2f..000000000000 --- a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService.ts +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import { Event, EventEmitter } from 'vscode'; -import { Resource } from '../../../common/types'; -import { PythonEnvironment } from '../../../pythonEnvironments/info'; -import { IInterpreterEvaluation, IInterpreterSecurityService, IInterpreterSecurityStorage } from '../types'; - -@injectable() -export class InterpreterSecurityService implements IInterpreterSecurityService { - public _didSafeInterpretersChange = new EventEmitter(); - constructor( - @inject(IInterpreterSecurityStorage) private readonly interpreterSecurityStorage: IInterpreterSecurityStorage, - @inject(IInterpreterEvaluation) private readonly interpreterEvaluation: IInterpreterEvaluation, - ) {} - - public isSafe(interpreter: PythonEnvironment, resource?: Resource): boolean | undefined { - const unsafeInterpreters = this.interpreterSecurityStorage.unsafeInterpreters.value; - if (unsafeInterpreters.includes(interpreter.path)) { - return false; - } - const safeInterpreters = this.interpreterSecurityStorage.safeInterpreters.value; - if (safeInterpreters.includes(interpreter.path)) { - return true; - } - return this.interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - } - - public async evaluateAndRecordInterpreterSafety(interpreter: PythonEnvironment, resource: Resource): Promise { - const unsafeInterpreters = this.interpreterSecurityStorage.unsafeInterpreters.value; - const safeInterpreters = this.interpreterSecurityStorage.safeInterpreters.value; - if (unsafeInterpreters.includes(interpreter.path) || safeInterpreters.includes(interpreter.path)) { - return; - } - const isSafe = await this.interpreterEvaluation.evaluateIfInterpreterIsSafe(interpreter, resource); - if (isSafe) { - await this.interpreterSecurityStorage.safeInterpreters.updateValue([interpreter.path, ...safeInterpreters]); - } else { - await this.interpreterSecurityStorage.unsafeInterpreters.updateValue([ - interpreter.path, - ...unsafeInterpreters, - ]); - } - this._didSafeInterpretersChange.fire(); - } - - public get onDidChangeSafeInterpreters(): Event { - return this._didSafeInterpretersChange.event; - } -} diff --git a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage.ts b/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage.ts deleted file mode 100644 index fb0d9343cc6f..000000000000 --- a/src/client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage.ts +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; -import { ICommandManager, IWorkspaceService } from '../../../common/application/types'; -import { Commands } from '../../../common/constants'; -import { IDisposable, IDisposableRegistry, IPersistentState, IPersistentStateFactory } from '../../../common/types'; -import { - flaggedWorkspacesKeysStorageKey, - safeInterpretersKey, - unsafeInterpreterPromptKey, - unsafeInterpretersKey, -} from '../constants'; -import { IInterpreterSecurityStorage } from '../types'; - -@injectable() -export class InterpreterSecurityStorage implements IInterpreterSecurityStorage { - public get unsafeInterpreterPromptEnabled(): IPersistentState { - return this._unsafeInterpreterPromptEnabled; - } - public get unsafeInterpreters(): IPersistentState { - return this._unsafeInterpreters; - } - public get safeInterpreters(): IPersistentState { - return this._safeInterpreters; - } - private _unsafeInterpreterPromptEnabled: IPersistentState; - private _unsafeInterpreters: IPersistentState; - private _safeInterpreters: IPersistentState; - private flaggedWorkspacesKeysStorage: IPersistentState; - - constructor( - @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(ICommandManager) private readonly commandManager: ICommandManager, - @inject(IDisposableRegistry) private readonly disposables: IDisposable[], - ) { - this._unsafeInterpreterPromptEnabled = this.persistentStateFactory.createGlobalPersistentState( - unsafeInterpreterPromptKey, - true, - ); - this._unsafeInterpreters = this.persistentStateFactory.createGlobalPersistentState( - unsafeInterpretersKey, - [], - ); - this._safeInterpreters = this.persistentStateFactory.createGlobalPersistentState( - safeInterpretersKey, - [], - ); - this.flaggedWorkspacesKeysStorage = this.persistentStateFactory.createGlobalPersistentState( - flaggedWorkspacesKeysStorageKey, - [], - ); - } - - public hasUserApprovedWorkspaceInterpreters(resource: Uri): IPersistentState { - return this.persistentStateFactory.createGlobalPersistentState( - this._getKeyForWorkspace(resource), - undefined, - ); - } - - public async activate(): Promise { - this.disposables.push( - this.commandManager.registerCommand( - Commands.ResetInterpreterSecurityStorage, - this.resetInterpreterSecurityStorage.bind(this), - ), - ); - } - - public async resetInterpreterSecurityStorage(): Promise { - this.flaggedWorkspacesKeysStorage.value.forEach(async (key) => { - const areInterpretersInWorkspaceSafe = this.persistentStateFactory.createGlobalPersistentState< - boolean | undefined - >(key, undefined); - await areInterpretersInWorkspaceSafe.updateValue(undefined); - }); - await this.flaggedWorkspacesKeysStorage.updateValue([]); - await this._safeInterpreters.updateValue([]); - await this._unsafeInterpreters.updateValue([]); - await this._unsafeInterpreterPromptEnabled.updateValue(true); - } - - public _getKeyForWorkspace(resource: Uri): string { - return `ARE_INTERPRETERS_SAFE_FOR_WS_${this.workspaceService.getWorkspaceFolderIdentifier(resource)}`; - } - - public async storeKeyForWorkspace(resource: Uri): Promise { - const key = this._getKeyForWorkspace(resource); - const flaggedWorkspacesKeys = this.flaggedWorkspacesKeysStorage.value; - if (!flaggedWorkspacesKeys.includes(key)) { - await this.flaggedWorkspacesKeysStorage.updateValue([key, ...flaggedWorkspacesKeys]); - } - } -} diff --git a/src/client/interpreter/autoSelection/rules/settings.ts b/src/client/interpreter/autoSelection/rules/settings.ts index 8257fb7820c0..3a5803d730a7 100644 --- a/src/client/interpreter/autoSelection/rules/settings.ts +++ b/src/client/interpreter/autoSelection/rules/settings.ts @@ -7,7 +7,7 @@ import { inject, injectable } from 'inversify'; import { IWorkspaceService } from '../../../common/application/types'; import { DeprecatePythonPath } from '../../../common/experiments/groups'; import { IFileSystem } from '../../../common/platform/types'; -import { IExperimentsManager, IInterpreterPathService, IPersistentStateFactory, Resource } from '../../../common/types'; +import { IExperimentService, IInterpreterPathService, IPersistentStateFactory, Resource } from '../../../common/types'; import { AutoSelectionRule, IInterpreterAutoSelectionService } from '../types'; import { BaseRuleService, NextAction } from './baseRule'; @@ -17,7 +17,7 @@ export class SettingsInterpretersAutoSelectionRule extends BaseRuleService { @inject(IFileSystem) fs: IFileSystem, @inject(IPersistentStateFactory) stateFactory: IPersistentStateFactory, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, + @inject(IExperimentService) private readonly experiments: IExperimentService, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, ) { super(AutoSelectionRule.settings, fs, stateFactory); @@ -27,10 +27,9 @@ export class SettingsInterpretersAutoSelectionRule extends BaseRuleService { _manager?: IInterpreterAutoSelectionService, ): Promise { const pythonConfig = this.workspaceService.getConfiguration('python', null as any)!; - const pythonPathInConfig = this.experiments.inExperiment(DeprecatePythonPath.experiment) + const pythonPathInConfig = this.experiments.inExperimentSync(DeprecatePythonPath.experiment) ? this.interpreterPathService.inspect(undefined) : pythonConfig.inspect('pythonPath')!; - this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); // No need to store python paths defined in settings in our caches, they can be retrieved from the settings directly. return pythonPathInConfig.globalValue && pythonPathInConfig.globalValue !== 'python' ? NextAction.exit diff --git a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts index 4cc581958747..85a2280a6b57 100644 --- a/src/client/interpreter/autoSelection/rules/workspaceEnv.ts +++ b/src/client/interpreter/autoSelection/rules/workspaceEnv.ts @@ -10,13 +10,7 @@ import { DeprecatePythonPath } from '../../../common/experiments/groups'; import { inDiscoveryExperiment } from '../../../common/experiments/helpers'; import { traceVerbose } from '../../../common/logger'; import { IFileSystem, IPlatformService } from '../../../common/platform/types'; -import { - IExperimentService, - IExperimentsManager, - IInterpreterPathService, - IPersistentStateFactory, - Resource, -} from '../../../common/types'; +import { IExperimentService, IInterpreterPathService, IPersistentStateFactory, Resource } from '../../../common/types'; import { OSType } from '../../../common/utils/platform'; import { IServiceContainer } from '../../../ioc/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; @@ -38,7 +32,6 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, - @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, @inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter, @inject(IExperimentService) private readonly experimentService: IExperimentService, @@ -56,10 +49,9 @@ export class WorkspaceVirtualEnvInterpretersAutoSelectionRule extends BaseRuleSe } const pythonConfig = this.workspaceService.getConfiguration('python', workspacePath.folderUri)!; - const pythonPathInConfig = this.experiments.inExperiment(DeprecatePythonPath.experiment) + const pythonPathInConfig = this.experimentService.inExperimentSync(DeprecatePythonPath.experiment) ? this.interpreterPathService.inspect(workspacePath.folderUri) : pythonConfig.inspect('pythonPath')!; - this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); // If user has defined custom values in settings for this workspace folder, then use that. if (pythonPathInConfig.workspaceFolderValue || pythonPathInConfig.workspaceValue) { return NextAction.runNextRule; diff --git a/src/client/interpreter/autoSelection/types.ts b/src/client/interpreter/autoSelection/types.ts index 5ecd35e09ca0..9f125c002af6 100644 --- a/src/client/interpreter/autoSelection/types.ts +++ b/src/client/interpreter/autoSelection/types.ts @@ -4,8 +4,7 @@ 'use strict'; import { Event, Uri } from 'vscode'; -import { IExtensionSingleActivationService } from '../../activation/types'; -import { IPersistentState, Resource } from '../../common/types'; +import { Resource } from '../../common/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; export const IInterpreterAutoSelectionProxyService = Symbol('IInterpreterAutoSelectionProxyService'); @@ -50,25 +49,3 @@ export interface IInterpreterAutoSelectionRule { autoSelectInterpreter(resource: Resource, manager?: IInterpreterAutoSelectionService): Promise; getPreviouslyAutoSelectedInterpreter(resource: Resource): PythonEnvironment | undefined; } - -export const IInterpreterSecurityService = Symbol('IInterpreterSecurityService'); -export interface IInterpreterSecurityService { - readonly onDidChangeSafeInterpreters: Event; - evaluateAndRecordInterpreterSafety(interpreter: PythonEnvironment, resource: Resource): Promise; - isSafe(interpreter: PythonEnvironment, resource?: Resource): boolean | undefined; -} - -export const IInterpreterSecurityStorage = Symbol('IInterpreterSecurityStorage'); -export interface IInterpreterSecurityStorage extends IExtensionSingleActivationService { - readonly unsafeInterpreterPromptEnabled: IPersistentState; - readonly unsafeInterpreters: IPersistentState; - readonly safeInterpreters: IPersistentState; - hasUserApprovedWorkspaceInterpreters(resource: Uri): IPersistentState; - storeKeyForWorkspace(resource: Uri): Promise; -} - -export const IInterpreterEvaluation = Symbol('IInterpreterEvaluation'); -export interface IInterpreterEvaluation { - evaluateIfInterpreterIsSafe(interpreter: PythonEnvironment, resource: Resource): Promise; - inferValueUsingCurrentState(interpreter: PythonEnvironment, resource: Resource): boolean | undefined; -} diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index bdeb7606cf04..7dfee2ce1fa6 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -74,10 +74,21 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { : InterpreterQuickPickList.enterPath.detail(), alwaysShow: true, }; + const { defaultInterpreterPath } = this.configurationService.getSettings(state.workspace); + const defaultInterpreterPathSuggestion = { + label: InterpreterQuickPickList.defaultInterpreterPath.label(), + detail: this.pathUtils.getDisplayName( + defaultInterpreterPath, + state.workspace ? state.workspace.fsPath : undefined, + ), + path: defaultInterpreterPath, + alwaysShow: true, + }; const suggestions: (IInterpreterQuickPickItem | IFindInterpreterQuickPickItem)[] = [ manualEntrySuggestion, ...interpreterSuggestions, + defaultInterpreterPathSuggestion, ]; const currentPythonPath = this.pathUtils.getDisplayName( @@ -104,7 +115,11 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { callback: async (quickPick) => { quickPick.busy = true; interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace, true); - quickPick.items = [manualEntrySuggestion, ...interpreterSuggestions]; + quickPick.items = [ + manualEntrySuggestion, + ...interpreterSuggestions, + defaultInterpreterPathSuggestion, + ]; quickPick.busy = false; }, }, diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 50df3e685137..6bb63c3476b1 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -5,10 +5,8 @@ import { inject, injectable } from 'inversify'; import { Disposable, Uri } from 'vscode'; -import { DeprecatePythonPath } from '../../../common/experiments/groups'; -import { IExperimentsManager, IPathUtils, Resource } from '../../../common/types'; +import { IPathUtils, Resource } from '../../../common/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; -import { IInterpreterSecurityService } from '../../autoSelection/types'; import { IInterpreterService } from '../../contracts'; import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector } from '../types'; @@ -19,22 +17,18 @@ export class InterpreterSelector implements IInterpreterSelector { constructor( @inject(IInterpreterService) private readonly interpreterManager: IInterpreterService, @inject(IInterpreterComparer) private readonly interpreterComparer: IInterpreterComparer, - @inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager, - @inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService, @inject(IPathUtils) private readonly pathUtils: IPathUtils, ) {} - public dispose() { + + public dispose(): void { this.disposables.forEach((disposable) => disposable.dispose()); } - public async getSuggestions(resource: Resource, ignoreCache?: boolean) { - let interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true, ignoreCache }); - if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) { - interpreters = interpreters - ? interpreters.filter((item) => this.interpreterSecurityService.isSafe(item) !== false) - : []; - } - this.experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control); + public async getSuggestions(resource: Resource, ignoreCache?: boolean): Promise { + const interpreters = await this.interpreterManager.getInterpreters(resource, { + onSuggestion: true, + ignoreCache, + }); interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer)); return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } diff --git a/src/client/interpreter/configuration/pythonPathUpdaterServiceFactory.ts b/src/client/interpreter/configuration/pythonPathUpdaterServiceFactory.ts index e7014a4479d8..2bd97beaa39c 100644 --- a/src/client/interpreter/configuration/pythonPathUpdaterServiceFactory.ts +++ b/src/client/interpreter/configuration/pythonPathUpdaterServiceFactory.ts @@ -2,7 +2,7 @@ import { inject, injectable } from 'inversify'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; import { DeprecatePythonPath } from '../../common/experiments/groups'; -import { IExperimentsManager, IInterpreterPathService } from '../../common/types'; +import { IExperimentService, IInterpreterPathService } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { GlobalPythonPathUpdaterService } from './services/globalUpdaterService'; import { WorkspaceFolderPythonPathUpdaterService } from './services/workspaceFolderUpdaterService'; @@ -15,11 +15,10 @@ export class PythonPathUpdaterServiceFactory implements IPythonPathUpdaterServic private readonly workspaceService: IWorkspaceService; private readonly interpreterPathService: IInterpreterPathService; constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { - const experiments = serviceContainer.get(IExperimentsManager); + const experiments = serviceContainer.get(IExperimentService); this.workspaceService = serviceContainer.get(IWorkspaceService); this.interpreterPathService = serviceContainer.get(IInterpreterPathService); - this.inDeprecatePythonPathExperiment = experiments.inExperiment(DeprecatePythonPath.experiment); - experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); + this.inDeprecatePythonPathExperiment = experiments.inExperimentSync(DeprecatePythonPath.experiment); } public getGlobalPythonPathConfigurationService(): IPythonPathUpdaterService { return new GlobalPythonPathUpdaterService( diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index a59c18164e16..50d78f980189 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -12,7 +12,6 @@ import { IConfigurationService, IDisposableRegistry, IExperimentService, - IExperimentsManager, IInterpreterPathService, IPersistentState, IPersistentStateFactory, @@ -79,7 +78,7 @@ export class InterpreterService implements Disposable, IInterpreterService { private readonly interpreterPathService: IInterpreterPathService; - private readonly experimentsManager: IExperimentsManager; + private readonly experimentsManager: IExperimentService; private readonly didChangeInterpreterEmitter = new EventEmitter(); @@ -97,7 +96,7 @@ export class InterpreterService implements Disposable, IInterpreterService { this.persistentStateFactory = this.serviceContainer.get(IPersistentStateFactory); this.configService = this.serviceContainer.get(IConfigurationService); this.interpreterPathService = this.serviceContainer.get(IInterpreterPathService); - this.experimentsManager = this.serviceContainer.get(IExperimentsManager); + this.experimentsManager = this.serviceContainer.get(IExperimentService); } public async refresh(resource?: Uri): Promise { @@ -116,7 +115,7 @@ export class InterpreterService implements Disposable, IInterpreterService { const workspaceService = this.serviceContainer.get(IWorkspaceService); const pySettings = this.configService.getSettings(); this._pythonPathSetting = pySettings.pythonPath; - if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) { + if (this.experimentsManager.inExperimentSync(DeprecatePythonPath.experiment)) { disposables.push( this.interpreterPathService.onDidChange((i) => { this._onConfigChanged(i.uri); @@ -135,7 +134,6 @@ export class InterpreterService implements Disposable, IInterpreterService { }); disposables.push(disposable); } - this.experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control); } public async getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise { diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index df4187a2af1e..2b0fb02c06b1 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -8,9 +8,6 @@ import { IServiceManager } from '../ioc/types'; import { EnvironmentActivationService } from './activation/service'; import { IEnvironmentActivationService } from './activation/types'; import { InterpreterAutoSelectionService } from './autoSelection/index'; -import { InterpreterEvaluation } from './autoSelection/interpreterSecurity/interpreterEvaluation'; -import { InterpreterSecurityService } from './autoSelection/interpreterSecurity/interpreterSecurityService'; -import { InterpreterSecurityStorage } from './autoSelection/interpreterSecurity/interpreterSecurityStorage'; import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy'; import { CachedInterpretersAutoSelectionRule } from './autoSelection/rules/cached'; import { CurrentPathInterpretersAutoSelectionRule } from './autoSelection/rules/currentPath'; @@ -23,9 +20,6 @@ import { IInterpreterAutoSelectionRule, IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService, - IInterpreterEvaluation, - IInterpreterSecurityService, - IInterpreterSecurityStorage, } from './autoSelection/types'; import { InterpreterComparer } from './configuration/interpreterComparer'; import { ResetInterpreterCommand } from './configuration/interpreterSelector/commands/resetInterpreter'; @@ -64,10 +58,6 @@ import { VirtualEnvironmentPrompt } from './virtualEnvs/virtualEnvPrompt'; */ export function registerInterpreterTypes(serviceManager: IServiceManager): void { - serviceManager.addSingleton( - IExtensionSingleActivationService, - InterpreterSecurityStorage, - ); serviceManager.addSingleton( IExtensionSingleActivationService, SetInterpreterCommand, @@ -80,9 +70,6 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void IExtensionSingleActivationService, SetShebangInterpreterCommand, ); - serviceManager.addSingleton(IInterpreterEvaluation, InterpreterEvaluation); - serviceManager.addSingleton(IInterpreterSecurityStorage, InterpreterSecurityStorage); - serviceManager.addSingleton(IInterpreterSecurityService, InterpreterSecurityService); serviceManager.addSingleton(IExtensionActivationService, VirtualEnvironmentPrompt); diff --git a/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts index 7c1240057810..63fe1fdd60b7 100644 --- a/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/environmentsResolver.ts @@ -22,7 +22,6 @@ export class PythonEnvsResolver implements ILocator { constructor( private readonly parentLocator: ILocator, private readonly environmentInfoService: IEnvironmentInfoService, - private readonly isEnvSafe: (env: PythonEnvInfo) => boolean, ) {} public async resolveEnv(env: string | PythonEnvInfo): Promise { @@ -95,9 +94,6 @@ export class PythonEnvsResolver implements ILocator { didUpdate: EventEmitter, seen: PythonEnvInfo[], ) { - if (!this.isEnvSafe(seen[envIndex])) { - return; - } state.pending += 1; // It's essential we increment the pending call count before any asynchronus calls in this method. // We want this to be run even when `resolveInBackground` is called in background. diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts index d526a79a7471..4a7b58487318 100644 --- a/src/client/pythonEnvironments/index.ts +++ b/src/client/pythonEnvironments/index.ts @@ -27,16 +27,14 @@ import { WindowsRegistryLocator } from './discovery/locators/services/windowsReg import { WindowsStoreLocator } from './discovery/locators/services/windowsStoreLocator'; import { getEnvironmentInfoService } from './info/environmentInfoService'; import { isComponentEnabled, registerLegacyDiscoveryForIOC, registerNewDiscoveryForIOC } from './legacyIOC'; -import { EnvironmentsSecurity, IEnvironmentsSecurity } from './security'; import { PoetryLocator } from './discovery/locators/services/poetryLocator'; /** * Set up the Python environments component (during extension activation).' */ export async function initialize(ext: ExtensionState): Promise { - const environmentsSecurity = new EnvironmentsSecurity(); const api = new PythonEnvironments( - () => createLocators(ext, environmentsSecurity), + () => createLocators(ext), // Other sub-components (e.g. config, "current" env) will go here. ); ext.disposables.push(api); @@ -48,7 +46,6 @@ export async function initialize(ext: ExtensionState): Promise { // Create the low-level locators. let locators: ILocator = new ExtensionLocators( @@ -102,8 +98,6 @@ async function createLocators( locators, // These are shared. envInfoService, - // Class methods may depend on other properties which belong to the class, so bind the correct context. - environmentsSecurity.isEnvSafe.bind(environmentsSecurity), ); const caching = await createCachingLocator( ext, diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index 2e5c9f9536bd..fc939cf5c617 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -65,7 +65,6 @@ import { } from './discovery/locators/services/workspaceVirtualEnvService'; import { WorkspaceVirtualEnvWatcherService } from './discovery/locators/services/workspaceVirtualEnvWatcherService'; import { EnvironmentType, PythonEnvironment } from './info'; -import { EnvironmentsSecurity, IEnvironmentsSecurity } from './security'; import { toSemverLikeVersion } from './base/info/pythonVersion'; import { PythonVersion } from './info/pythonVersion'; import { IExtensionSingleActivationService } from '../activation/types'; @@ -142,12 +141,9 @@ class ComponentAdapter implements IComponentAdapter { private readonly refreshed = new vscode.EventEmitter(); - private allowOnSuggestionRefresh = true; - constructor( // The adapter only wraps one thing: the component API. private readonly api: IPythonEnvironments, - private readonly environmentsSecurity: IEnvironmentsSecurity, ) {} // For use in VirtualEnvironmentPrompt.activate() @@ -281,15 +277,6 @@ class ComponentAdapter implements IComponentAdapter { options?: GetInterpreterOptions, source?: PythonEnvSource[], ): Promise { - if (options?.onSuggestion && this.allowOnSuggestionRefresh) { - // For now, until we have the concept of trusted workspaces, we assume all interpreters as safe - // to run once user has triggered discovery, i.e interacted with the extension. - this.environmentsSecurity.markAllEnvsAsSafe(); - // We can now run certain executables to collect more information than what is currently in - // cache, so trigger discovery for fresh envs in background. - getEnvs(this.api.iterEnvs({ ignoreCache: true })).ignoreErrors(); - this.allowOnSuggestionRefresh = false; // We only need to refresh on suggestion once every session. - } const query: PythonLocatorQuery = { ignoreCache: options?.ignoreCache }; if (resource !== undefined) { const wsFolder = vscode.workspace.getWorkspaceFolder(resource); @@ -428,13 +415,6 @@ export async function registerLegacyDiscoveryForIOC(serviceManager: IServiceMana serviceManager.addSingleton(ICondaService, CondaService); } -export function registerNewDiscoveryForIOC( - serviceManager: IServiceManager, - api: IPythonEnvironments, - environmentsSecurity: EnvironmentsSecurity, -): void { - serviceManager.addSingletonInstance( - IComponentAdapter, - new ComponentAdapter(api, environmentsSecurity), - ); +export function registerNewDiscoveryForIOC(serviceManager: IServiceManager, api: IPythonEnvironments): void { + serviceManager.addSingletonInstance(IComponentAdapter, new ComponentAdapter(api)); } diff --git a/src/client/pythonEnvironments/security.ts b/src/client/pythonEnvironments/security.ts deleted file mode 100644 index ca1d86ed8b33..000000000000 --- a/src/client/pythonEnvironments/security.ts +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import * as vscode from 'vscode'; -import { PythonEnvInfo } from './base/info'; -import { isParentPath } from './common/externalDependencies'; - -/** - * Keeps track of which environments are safe to execute. - */ -export interface IEnvironmentsSecurity { - /** - * Returns `true` the environment is safe to execute, `false` otherwise. - */ - isEnvSafe(env: PythonEnvInfo): boolean; - /** - * Mark all environments to be safe to execute. - */ - markAllEnvsAsSafe(): void; -} - -/** - * Keeps track of which environments are safe to execute. - */ -export class EnvironmentsSecurity implements IEnvironmentsSecurity { - /** - * Carries `true` if it's secure to run all environment executables, `false` otherwise. - */ - private areAllEnvsSafe = false; - - public isEnvSafe(env: PythonEnvInfo): boolean { - if (this.areAllEnvsSafe) { - return true; - } - const folders = vscode.workspace.workspaceFolders; - if (!folders) { - return true; - } - for (const root of folders.map((f) => f.uri.fsPath)) { - // Note `env.searchLocation` carries the root where the env was discovered which may - // not be related this workspace root. Hence use `env.executable.filename` directly. - if (isParentPath(env.executable.filename, root)) { - // For now we consider all "workspace environments" to be unsafe by default. - return false; - } - } - return true; - } - - public markAllEnvsAsSafe(): void { - this.areAllEnvsSafe = true; - } -} diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts index c41632622e48..80cabecb87a3 100644 --- a/src/client/startupTelemetry.ts +++ b/src/client/startupTelemetry.ts @@ -8,7 +8,7 @@ import { traceError } from './common/logger'; import { ITerminalHelper } from './common/terminal/types'; import { IConfigurationService, - IExperimentsManager, + IExperimentService, IInterpreterPathService, InspectInterpreterSettingType, Resource, @@ -77,16 +77,15 @@ function isUsingGlobalInterpreterInWorkspace(currentPythonPath: string, serviceC } export function hasUserDefinedPythonPath(resource: Resource, serviceContainer: IServiceContainer) { - const abExperiments = serviceContainer.get(IExperimentsManager); + const abExperiments = serviceContainer.get(IExperimentService); const workspaceService = serviceContainer.get(IWorkspaceService); const interpreterPathService = serviceContainer.get(IInterpreterPathService); let settings: InspectInterpreterSettingType; - if (abExperiments.inExperiment(DeprecatePythonPath.experiment)) { + if (abExperiments.inExperimentSync(DeprecatePythonPath.experiment)) { settings = interpreterPathService.inspect(resource); } else { settings = workspaceService.getConfiguration('python', resource)!.inspect('pythonPath')!; } - abExperiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control); return (settings.workspaceFolderValue && settings.workspaceFolderValue !== 'python') || (settings.workspaceValue && settings.workspaceValue !== 'python') || (settings.globalValue && settings.globalValue !== 'python') diff --git a/src/client/testing/unittest/services/discoveryService.ts b/src/client/testing/unittest/services/discoveryService.ts index d9ba0609a6bc..0f42d36d1fcc 100644 --- a/src/client/testing/unittest/services/discoveryService.ts +++ b/src/client/testing/unittest/services/discoveryService.ts @@ -37,7 +37,7 @@ export class TestDiscoveryService implements ITestDiscoveryService { const runOptions: Options = { // unittest needs to load modules in the workspace // isolating it breaks unittest discovery - args: internalPython.execCode(pythonScript, false), + args: internalPython.execCode(pythonScript), cwd: options.cwd, workspaceFolder: options.workspaceFolder, token: options.token, diff --git a/src/test/activation/activationManager.unit.test.ts b/src/test/activation/activationManager.unit.test.ts index 3e389485942b..4cd2b10ce188 100644 --- a/src/test/activation/activationManager.unit.test.ts +++ b/src/test/activation/activationManager.unit.test.ts @@ -5,9 +5,9 @@ import { assert, expect } from 'chai'; import * as sinon from 'sinon'; -import { anything, instance, mock, reset, verify, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; -import { TextDocument, Uri } from 'vscode'; +import { TextDocument, Uri, WorkspaceFolder } from 'vscode'; import { ExtensionActivationManager } from '../../client/activation/activationManager'; import { LanguageServerExtensionActivationService } from '../../client/activation/activationService'; import { IExtensionActivationService, IExtensionSingleActivationService } from '../../client/activation/types'; @@ -17,17 +17,11 @@ import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../ import { WorkspaceService } from '../../client/common/application/workspace'; import { PYTHON_LANGUAGE } from '../../client/common/constants'; import { DeprecatePythonPath } from '../../client/common/experiments/groups'; -import { ExperimentsManager } from '../../client/common/experiments/manager'; -import { InterpreterPathService } from '../../client/common/interpreterPathService'; +import { ExperimentService } from '../../client/common/experiments/service'; import { FileSystem } from '../../client/common/platform/fileSystem'; import { IFileSystem } from '../../client/common/platform/types'; -import { IDisposable, IExperimentsManager, IInterpreterPathService } from '../../client/common/types'; -import { createDeferred, createDeferredFromPromise } from '../../client/common/utils/async'; -import { InterpreterSecurityService } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService'; -import { - IInterpreterAutoSelectionService, - IInterpreterSecurityService, -} from '../../client/interpreter/autoSelection/types'; +import { IDisposable, IExperimentService, IInterpreterPathService } from '../../client/common/types'; +import { IInterpreterAutoSelectionService } from '../../client/interpreter/autoSelection/types'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { InterpreterService } from '../../client/interpreter/interpreterService'; import * as EnvFileTelemetry from '../../client/telemetry/envFileTelemetry'; @@ -55,15 +49,13 @@ suite('Activation Manager', () => { let interpreterService: IInterpreterService; let activeResourceService: IActiveResourceService; let documentManager: typemoq.IMock; - let interpreterSecurityService: IInterpreterSecurityService; let interpreterPathService: typemoq.IMock; - let experiments: IExperimentsManager; + let experiments: IExperimentService; let activationService1: IExtensionActivationService; let activationService2: IExtensionActivationService; let fileSystem: IFileSystem; setup(() => { - interpreterSecurityService = mock(InterpreterSecurityService); - experiments = mock(ExperimentsManager); + experiments = mock(ExperimentService); interpreterPathService = typemoq.Mock.ofType(); workspaceService = mock(WorkspaceService); activeResourceService = mock(ActiveResourceService); @@ -89,11 +81,9 @@ suite('Activation Manager', () => { instance(activeResourceService), instance(experiments), interpreterPathService.object, - instance(interpreterSecurityService), ); sinon.stub(EnvFileTelemetry, 'sendActivationTelemetry').resolves(); - managerTest.evaluateAutoSelectedInterpreterSafety = () => Promise.resolve(); }); teardown(() => { @@ -104,7 +94,10 @@ suite('Activation Manager', () => { const disposable = typemoq.Mock.ofType(); const disposable2 = typemoq.Mock.ofType(); when(workspaceService.onDidChangeWorkspaceFolders).thenReturn(() => disposable.object); - when(workspaceService.workspaceFolders).thenReturn([1 as any, 2 as any]); + when(workspaceService.workspaceFolders).thenReturn([ + (1 as unknown) as WorkspaceFolder, + (2 as unknown) as WorkspaceFolder, + ]); when(workspaceService.hasWorkspaceFolders).thenReturn(true); const eventDef = () => disposable2.object; documentManager @@ -132,7 +125,10 @@ suite('Activation Manager', () => { const disposable = typemoq.Mock.ofType(); const disposable2 = typemoq.Mock.ofType(); when(workspaceService.onDidChangeWorkspaceFolders).thenReturn(() => disposable.object); - when(workspaceService.workspaceFolders).thenReturn([1 as any, 2 as any]); + when(workspaceService.workspaceFolders).thenReturn([ + (1 as unknown) as WorkspaceFolder, + (2 as unknown) as WorkspaceFolder, + ]); when(workspaceService.hasWorkspaceFolders).thenReturn(true); const eventDef = () => disposable2.object; documentManager @@ -169,6 +165,7 @@ suite('Activation Manager', () => { const disposable1 = typemoq.Mock.ofType(); const disposable2 = typemoq.Mock.ofType(); let fileOpenedHandler!: (e: TextDocument) => Promise; + // eslint-disable-next-line @typescript-eslint/ban-types let workspaceFoldersChangedHandler!: Function; const documentUri = Uri.file('a'); const document = typemoq.Mock.ofType(); @@ -181,7 +178,9 @@ suite('Activation Manager', () => { }); documentManager .setup((w) => w.onDidOpenTextDocument(typemoq.It.isAny(), typemoq.It.isAny())) - .callback((cb) => (fileOpenedHandler = cb)) + .callback(function (cb) { + fileOpenedHandler = cb; + }) .returns(() => disposable2.object) .verifiable(typemoq.Times.once()); @@ -251,7 +250,7 @@ suite('Activation Manager', () => { when(activationService1.activate(resource)).thenResolve(); when(activationService2.activate(resource)).thenResolve(); when(interpreterService.getInterpreters(anything())).thenResolve(); - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); + when(experiments.inExperimentSync(DeprecatePythonPath.experiment)).thenReturn(true); interpreterPathService .setup((i) => i.copyOldInterpreterStorageValuesToNew(resource)) .returns(() => Promise.resolve()) @@ -301,7 +300,7 @@ suite('Activation Manager', () => { languageId: 'NOT PYTHON', }; - managerTest.onDocOpened(doc as any); + managerTest.onDocOpened((doc as unknown) as TextDocument); verify(workspaceService.getWorkspaceFolderIdentifier(doc.uri, anything())).never(); }); @@ -313,7 +312,7 @@ suite('Activation Manager', () => { when(workspaceService.getWorkspaceFolderIdentifier(doc.uri, anything())).thenReturn(''); when(workspaceService.hasWorkspaceFolders).thenReturn(true); - managerTest.onDocOpened(doc as any); + managerTest.onDocOpened((doc as unknown) as TextDocument); verify(workspaceService.getWorkspaceFolderIdentifier(doc.uri, anything())).once(); verify(workspaceService.getWorkspaceFolder(doc.uri)).never(); @@ -327,7 +326,7 @@ suite('Activation Manager', () => { when(workspaceService.getWorkspaceFolderIdentifier(doc.uri, anything())).thenReturn('key'); managerTest.activatedWorkspaces.add('key'); - managerTest.onDocOpened(doc as any); + managerTest.onDocOpened((doc as unknown) as TextDocument); verify(workspaceService.getWorkspaceFolderIdentifier(doc.uri, anything())).once(); verify(workspaceService.getWorkspaceFolder(doc.uri)).never(); @@ -337,6 +336,7 @@ suite('Activation Manager', () => { const disposable1 = typemoq.Mock.ofType(); const disposable2 = typemoq.Mock.ofType(); let docOpenedHandler!: (e: TextDocument) => Promise; + // eslint-disable-next-line @typescript-eslint/ban-types let workspaceFoldersChangedHandler!: Function; const documentUri = Uri.file('a'); const document = typemoq.Mock.ofType(); @@ -348,7 +348,9 @@ suite('Activation Manager', () => { }); documentManager .setup((w) => w.onDidOpenTextDocument(typemoq.It.isAny(), typemoq.It.isAny())) - .callback((cb) => (docOpenedHandler = cb)) + .callback(function (cb) { + docOpenedHandler = cb; + }) .returns(() => disposable2.object) .verifiable(typemoq.Times.once()); @@ -377,7 +379,7 @@ suite('Activation Manager', () => { verify(workspaceService.workspaceFolders).atLeast(1); verify(workspaceService.hasWorkspaceFolders).once(); - //Removed no. of folders to one + // Removed no. of folders to one when(workspaceService.workspaceFolders).thenReturn([folder1]); when(workspaceService.hasWorkspaceFolders).thenReturn(true); disposable2.setup((d) => d.dispose()).verifiable(typemoq.Times.once()); @@ -399,21 +401,19 @@ suite('Activation Manager', () => { let interpreterService: IInterpreterService; let activeResourceService: IActiveResourceService; let documentManager: typemoq.IMock; - let interpreterSecurityService: IInterpreterSecurityService; let activationService1: IExtensionActivationService; let activationService2: IExtensionActivationService; let fileSystem: IFileSystem; let singleActivationService: typemoq.IMock; - let initialize: sinon.SinonStub; - let activateWorkspace: sinon.SinonStub; + let initialize: sinon.SinonStub; + let activateWorkspace: sinon.SinonStub; let managerTest: ExtensionActivationManager; const resource = Uri.parse('a'); let interpreterPathService: typemoq.IMock; - let experiments: IExperimentsManager; + let experiments: IExperimentService; setup(() => { - interpreterSecurityService = mock(InterpreterSecurityService); - experiments = mock(ExperimentsManager); + experiments = mock(ExperimentService); workspaceService = mock(WorkspaceService); activeResourceService = mock(ActiveResourceService); appDiagnostics = typemoq.Mock.ofType(); @@ -444,9 +444,7 @@ suite('Activation Manager', () => { instance(activeResourceService), instance(experiments), interpreterPathService.object, - instance(interpreterSecurityService), ); - managerTest.evaluateAutoSelectedInterpreterSafety = () => Promise.resolve(); }); teardown(() => { @@ -484,135 +482,4 @@ suite('Activation Manager', () => { await expect(promise).to.eventually.be.rejectedWith('Kaboom'); }); }); - - suite('Selected Python Activation - evaluateIfAutoSelectedInterpreterIsSafe()', () => { - let workspaceService: IWorkspaceService; - let appDiagnostics: typemoq.IMock; - let autoSelection: typemoq.IMock; - let interpreterService: IInterpreterService; - let activeResourceService: IActiveResourceService; - let documentManager: typemoq.IMock; - let activationService1: IExtensionActivationService; - let activationService2: IExtensionActivationService; - let fileSystem: IFileSystem; - let managerTest: ExtensionActivationManager; - const resource = Uri.parse('a'); - let interpreterSecurityService: IInterpreterSecurityService; - let interpreterPathService: IInterpreterPathService; - let experiments: IExperimentsManager; - setup(() => { - interpreterSecurityService = mock(InterpreterSecurityService); - experiments = mock(ExperimentsManager); - fileSystem = mock(FileSystem); - interpreterPathService = mock(InterpreterPathService); - workspaceService = mock(WorkspaceService); - activeResourceService = mock(ActiveResourceService); - appDiagnostics = typemoq.Mock.ofType(); - autoSelection = typemoq.Mock.ofType(); - interpreterService = mock(InterpreterService); - documentManager = typemoq.Mock.ofType(); - activationService1 = mock(LanguageServerExtensionActivationService); - activationService2 = mock(LanguageServerExtensionActivationService); - when(experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control)).thenReturn(undefined); - managerTest = new ExtensionActivationManager( - [instance(activationService1), instance(activationService2)], - [], - documentManager.object, - instance(interpreterService), - autoSelection.object, - appDiagnostics.object, - instance(workspaceService), - instance(fileSystem), - instance(activeResourceService), - instance(experiments), - instance(interpreterPathService), - instance(interpreterSecurityService), - ); - }); - - test(`If in Deprecate PythonPath experiment, and setting is not set, fetch autoselected interpreter but don't evaluate it if it equals 'undefined'`, async () => { - const interpreter = undefined; - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(workspaceService.getWorkspaceFolderIdentifier(resource)).thenReturn('1'); - autoSelection - .setup((a) => a.getAutoSelectedInterpreter(resource)) - .returns(() => interpreter as any) - .verifiable(typemoq.Times.once()); - when(interpreterPathService.get(resource)).thenReturn('python'); - when( - interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource), - ).thenResolve(); - await managerTest.evaluateAutoSelectedInterpreterSafety(resource); - autoSelection.verifyAll(); - verify(interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource)).never(); - }); - - ['', 'python'].forEach((setting) => { - test(`If in Deprecate PythonPath experiment, and setting equals '${setting}', fetch autoselected interpreter and evaluate it`, async () => { - const interpreter = { path: 'pythonPath' }; - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(workspaceService.getWorkspaceFolderIdentifier(resource)).thenReturn('1'); - autoSelection - .setup((a) => a.getAutoSelectedInterpreter(resource)) - .returns(() => interpreter as any) - .verifiable(typemoq.Times.once()); - when(interpreterPathService.get(resource)).thenReturn(setting); - when( - interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource), - ).thenResolve(); - await managerTest.evaluateAutoSelectedInterpreterSafety(resource); - autoSelection.verifyAll(); - verify( - interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource), - ).once(); - }); - }); - - test(`If in Deprecate PythonPath experiment, and setting is not set, fetch autoselected interpreter but don't evaluate it if it equals 'undefined'`, async () => { - const interpreter = undefined; - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(workspaceService.getWorkspaceFolderIdentifier(resource)).thenReturn('1'); - autoSelection - .setup((a) => a.getAutoSelectedInterpreter(resource)) - .returns(() => interpreter as any) - .verifiable(typemoq.Times.once()); - when(interpreterPathService.get(resource)).thenReturn('python'); - when( - interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource), - ).thenResolve(); - await managerTest.evaluateAutoSelectedInterpreterSafety(resource); - autoSelection.verifyAll(); - verify(interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource)).never(); - }); - - test(`If in Deprecate PythonPath experiment, and setting is set, simply return`, async () => { - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(workspaceService.getWorkspaceFolderIdentifier(resource)).thenReturn('1'); - autoSelection.setup((a) => a.getAutoSelectedInterpreter(resource)).verifiable(typemoq.Times.never()); - when(interpreterPathService.get(resource)).thenReturn('settingSetToSomePath'); - await managerTest.evaluateAutoSelectedInterpreterSafety(resource); - autoSelection.verifyAll(); - }); - - test(`If in Deprecate PythonPath experiment, if setting is set during evaluation, don't wait for the evaluation to finish to resolve method promise`, async () => { - const interpreter = { path: 'pythonPath' }; - const evaluateIfInterpreterIsSafeDeferred = createDeferred(); - when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(workspaceService.getWorkspaceFolderIdentifier(resource)).thenReturn('1'); - autoSelection.setup((a) => a.getAutoSelectedInterpreter(resource)).returns(() => interpreter as any); - when(interpreterPathService.get(resource)).thenReturn('python'); - when( - interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource), - ).thenReturn(evaluateIfInterpreterIsSafeDeferred.promise); - const deferredPromise = createDeferredFromPromise( - managerTest.evaluateAutoSelectedInterpreterSafety(resource), - ); - expect(deferredPromise.completed).to.equal(false, 'Promise should not be resolved yet'); - reset(interpreterPathService); - when(interpreterPathService.get(resource)).thenReturn('settingSetToSomePath'); - await managerTest.evaluateAutoSelectedInterpreterSafety(resource); - await sleep(1); - expect(deferredPromise.completed).to.equal(true, 'Promise should be resolved'); - }); - }); }); diff --git a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts index ca8bf7ba2038..98d51eb027f2 100644 --- a/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts +++ b/src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts @@ -32,7 +32,7 @@ import { IPlatformService } from '../../../../client/common/platform/types'; import { IConfigurationService, IDisposableRegistry, - IExperimentsManager, + IExperimentService, IInterpreterPathService, InterpreterConfigurationScope, IPythonSettings, @@ -476,14 +476,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { const serviceContainerObject = createContainer(); serviceContainer.setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))).returns(() => workspaceService); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); diagnosticService = new (class extends InvalidMacPythonInterpreterService { protected async onDidChangeConfiguration(_event: ConfigurationChangeEvent) { invoked = true; @@ -498,14 +495,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { const workspaceService = { onDidChangeConfiguration: noop } as any; const serviceContainerObject = createContainer(); serviceContainer.setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))).returns(() => workspaceService); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); diagnosticService = new (class extends InvalidMacPythonInterpreterService { protected async onDidChangeConfiguration(_event: ConfigurationChangeEvent) { invoked = true; @@ -530,14 +524,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { serviceContainer .setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))) .returns(() => workspaceService.object); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const diagnosticSvc = new (class extends InvalidMacPythonInterpreterService { constructor( arg1: IServiceContainer, @@ -585,14 +576,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { serviceContainer .setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))) .returns(() => workspaceService.object); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const diagnosticSvc = new (class extends InvalidMacPythonInterpreterService { constructor( arg1: IServiceContainer, @@ -632,14 +620,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { serviceContainer .setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))) .returns(() => workspaceService.object); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const diagnosticSvc = new (class extends InvalidMacPythonInterpreterService { constructor( arg1: IServiceContainer, @@ -680,14 +665,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { serviceContainer .setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))) .returns(() => workspaceService.object); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const diagnosticSvc = new (class extends InvalidMacPythonInterpreterService { constructor( arg1: IServiceContainer, @@ -742,14 +724,11 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => { .returns(() => interpreterPathService.object); serviceContainer.setup((s) => s.get(typemoq.It.isValue(IWorkspaceService))).returns(() => workspaceService); - const experiments = typemoq.Mock.ofType(); + const experiments = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experiments.object); - experiments.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experiments - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); diagnosticService = new (class extends InvalidMacPythonInterpreterService {})( serviceContainerObject, undefined as any, diff --git a/src/test/application/diagnostics/checks/pythonPathDeprecated.unit.test.ts b/src/test/application/diagnostics/checks/pythonPathDeprecated.unit.test.ts index 9bfc5117c057..efe0a406eeac 100644 --- a/src/test/application/diagnostics/checks/pythonPathDeprecated.unit.test.ts +++ b/src/test/application/diagnostics/checks/pythonPathDeprecated.unit.test.ts @@ -6,13 +6,13 @@ import { assert, expect } from 'chai'; import * as sinon from 'sinon'; import * as typemoq from 'typemoq'; -import { ConfigurationTarget, DiagnosticSeverity, Uri, WorkspaceConfiguration } from 'vscode'; +import { DiagnosticSeverity, Uri, WorkspaceConfiguration } from 'vscode'; import { BaseDiagnostic, BaseDiagnosticsService } from '../../../../client/application/diagnostics/base'; import { PythonPathDeprecatedDiagnostic, PythonPathDeprecatedDiagnosticService, } from '../../../../client/application/diagnostics/checks/pythonPathDeprecated'; -import { CommandOption, IDiagnosticsCommandFactory } from '../../../../client/application/diagnostics/commands/types'; +import { IDiagnosticsCommandFactory } from '../../../../client/application/diagnostics/commands/types'; import { DiagnosticCodes } from '../../../../client/application/diagnostics/constants'; import { DiagnosticCommandPromptHandlerServiceId, @@ -26,9 +26,8 @@ import { IDiagnosticHandlerService, } from '../../../../client/application/diagnostics/types'; import { IWorkspaceService } from '../../../../client/common/application/types'; -import { STANDARD_OUTPUT_CHANNEL } from '../../../../client/common/constants'; import { DeprecatePythonPath } from '../../../../client/common/experiments/groups'; -import { IDisposableRegistry, IExperimentsManager, IOutputChannel, Resource } from '../../../../client/common/types'; +import { IDisposableRegistry, IExperimentService, Resource } from '../../../../client/common/types'; import { Common, Diagnostics } from '../../../../client/common/utils/localize'; import { IServiceContainer } from '../../../../client/ioc/types'; @@ -39,17 +38,12 @@ suite('Application Diagnostics - Python Path Deprecated', () => { let commandFactory: typemoq.IMock; let workspaceService: typemoq.IMock; let filterService: typemoq.IMock; - let experimentsManager: typemoq.IMock; - let output: typemoq.IMock; + let experimentsManager: typemoq.IMock; let serviceContainer: typemoq.IMock; function createContainer() { serviceContainer = typemoq.Mock.ofType(); filterService = typemoq.Mock.ofType(); - output = typemoq.Mock.ofType(); - serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IOutputChannel), STANDARD_OUTPUT_CHANNEL)) - .returns(() => output.object); - experimentsManager = typemoq.Mock.ofType(); + experimentsManager = typemoq.Mock.ofType(); messageHandler = typemoq.Mock.ofType>(); serviceContainer .setup((s) => @@ -64,7 +58,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { .setup((s) => s.get(typemoq.It.isValue(IDiagnosticFilterService))) .returns(() => filterService.object); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experimentsManager.object); serviceContainer .setup((s) => s.get(typemoq.It.isValue(IDiagnosticsCommandFactory))) @@ -145,8 +139,13 @@ suite('Application Diagnostics - Python Path Deprecated', () => { messageHandler.verifyAll(); }); test('Python Path Deprecated Diagnostic is handled as expected', async () => { + let invoked = false; const diagnostic = new PythonPathDeprecatedDiagnostic('message', resource); - const ignoreCmd = ({ cmd: 'ignoreCmd' } as any) as IDiagnosticCommand; + const ignoreCmd = ({ + invoke: () => { + invoked = true; + }, + } as any) as IDiagnosticCommand; filterService .setup((f) => f.shouldIgnoreDiagnostic(typemoq.It.isValue(DiagnosticCodes.PythonPathDeprecatedDiagnostic)), @@ -160,41 +159,29 @@ suite('Application Diagnostics - Python Path Deprecated', () => { .verifiable(typemoq.Times.once()); commandFactory - .setup((f) => - f.createCommand( - typemoq.It.isAny(), - typemoq.It.isObjectWith>({ type: 'ignore' }), - ), - ) + .setup((f) => f.createCommand(typemoq.It.isAny(), typemoq.It.isAny())) + .callback((a, b) => { + expect(a).to.be.deep.equal(diagnostic); + expect(b).to.be.deep.equal({ + type: 'ignore', + options: DiagnosticScope.Global, + }); + }) .returns(() => ignoreCmd) .verifiable(typemoq.Times.once()); - const _removePythonPathFromWorkspaceSettings = sinon.stub( - PythonPathDeprecatedDiagnosticService.prototype, - '_removePythonPathFromWorkspaceSettings', - ); - _removePythonPathFromWorkspaceSettings.resolves(); await diagnosticService.handle([diagnostic]); - assert(_removePythonPathFromWorkspaceSettings.calledOnceWith(resource)); + expect(invoked).to.equal(true, 'Command should be invoked'); messageHandler.verifyAll(); commandFactory.verifyAll(); - expect(messagePrompt).not.be.equal(undefined, 'Message prompt not set'); - expect(messagePrompt!.commandPrompts.length).to.equal(2, 'Incorrect length'); - expect(messagePrompt!.commandPrompts[0].command).not.be.equal(undefined, 'Command not set'); - expect(messagePrompt!.commandPrompts[0].command!.diagnostic).to.be.deep.equal(diagnostic); - expect(messagePrompt!.commandPrompts[0].prompt).to.be.deep.equal(Common.openOutputPanel()); - expect(messagePrompt!.commandPrompts[1]).to.be.deep.equal({ - prompt: Common.doNotShowAgain(), - command: ignoreCmd, + expect(messagePrompt).to.be.deep.equal({ + commandPrompts: [ + { + prompt: Common.ok(), + }, + ], }); - - output - .setup((o) => o.show(true)) - .returns(() => undefined) - .verifiable(typemoq.Times.once()); - await messagePrompt!.commandPrompts[0].command!.invoke(); - output.verifyAll(); }); test('Handling an empty diagnostic should not show a message nor return a command', async () => { const diagnostics: IDiagnostic[] = []; @@ -241,10 +228,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { }); test('If not in DeprecatePythonPath experiment, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = typemoq.Mock.ofType(); workspaceService .setup((w) => w.getConfiguration('python', resource)) @@ -267,10 +251,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { }); test('If a workspace is opened and only workspace value is set, diagnostic with appropriate message is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); workspaceService.setup((w) => w.workspaceFile).returns(() => Uri.parse('path/to/workspaceFile')); workspaceService @@ -295,10 +276,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { }); test('If folder is directly opened and workspace folder value is set, diagnostic with appropriate message is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); workspaceService.setup((w) => w.workspaceFile).returns(() => undefined); workspaceService @@ -324,10 +302,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { }); test('If a workspace is opened and both workspace folder value & workspace value is set, diagnostic with appropriate message is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); workspaceService.setup((w) => w.workspaceFile).returns(() => Uri.parse('path/to/workspaceFile')); workspaceService @@ -353,10 +328,7 @@ suite('Application Diagnostics - Python Path Deprecated', () => { }); test('Otherwise an empty diagnostic is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); workspaceService.setup((w) => w.workspaceFile).returns(() => Uri.parse('path/to/workspaceFile')); workspaceService @@ -370,23 +342,5 @@ suite('Application Diagnostics - Python Path Deprecated', () => { workspaceService.verifyAll(); }); - - test('Method _removePythonPathFromWorkspaceSettings() removes `python.pythonPath` setting from Workspace & Workspace Folder', async () => { - const workspaceConfig = typemoq.Mock.ofType(); - workspaceService.setup((w) => w.workspaceFile).returns(() => Uri.parse('path/to/workspaceFile')); - workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object); - workspaceConfig - .setup((w) => w.update('pythonPath', undefined, ConfigurationTarget.Workspace)) - .returns(() => Promise.resolve()) - .verifiable(typemoq.Times.once()); - workspaceConfig - .setup((w) => w.update('pythonPath', undefined, ConfigurationTarget.WorkspaceFolder)) - .returns(() => Promise.resolve()) - .verifiable(typemoq.Times.once()); - - await diagnosticService._removePythonPathFromWorkspaceSettings(resource); - - workspaceConfig.verifyAll(); - }); }); }); diff --git a/src/test/application/diagnostics/checks/upgradeCodeRunner.unit.test.ts b/src/test/application/diagnostics/checks/upgradeCodeRunner.unit.test.ts index e84506f5731e..4240186bd04a 100644 --- a/src/test/application/diagnostics/checks/upgradeCodeRunner.unit.test.ts +++ b/src/test/application/diagnostics/checks/upgradeCodeRunner.unit.test.ts @@ -27,7 +27,7 @@ import { import { IWorkspaceService } from '../../../../client/common/application/types'; import { CODE_RUNNER_EXTENSION_ID } from '../../../../client/common/constants'; import { DeprecatePythonPath } from '../../../../client/common/experiments/groups'; -import { IDisposableRegistry, IExperimentsManager, IExtensions, Resource } from '../../../../client/common/types'; +import { IDisposableRegistry, IExperimentService, IExtensions, Resource } from '../../../../client/common/types'; import { Common, Diagnostics } from '../../../../client/common/utils/localize'; import { IServiceContainer } from '../../../../client/ioc/types'; @@ -39,12 +39,12 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { let workspaceService: typemoq.IMock; let filterService: typemoq.IMock; let serviceContainer: typemoq.IMock; - let experimentsManager: typemoq.IMock; + let experimentsManager: typemoq.IMock; let extensions: typemoq.IMock; function createContainer() { extensions = typemoq.Mock.ofType(); serviceContainer = typemoq.Mock.ofType(); - experimentsManager = typemoq.Mock.ofType(); + experimentsManager = typemoq.Mock.ofType(); filterService = typemoq.Mock.ofType(); messageHandler = typemoq.Mock.ofType>(); serviceContainer @@ -57,7 +57,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { .returns(() => messageHandler.object); commandFactory = typemoq.Mock.ofType(); serviceContainer - .setup((s) => s.get(typemoq.It.isValue(IExperimentsManager))) + .setup((s) => s.get(typemoq.It.isValue(IExperimentService))) .returns(() => experimentsManager.object); serviceContainer .setup((s) => s.get(typemoq.It.isValue(IDiagnosticFilterService))) @@ -226,10 +226,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If not in DeprecatePythonPath experiment, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const diagnostics = await diagnosticService.diagnose(resource); @@ -237,10 +234,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If Code Runner extension is not installed, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); extensions.setup((e) => e.getExtension(CODE_RUNNER_EXTENSION_ID)).returns(() => undefined); const diagnostics = await diagnosticService.diagnose(resource); @@ -249,10 +243,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If Code Runner extension is installed but the appropriate feature flag is set in package.json, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const extension = typemoq.Mock.ofType>(); extensions.setup((e) => e.getExtension(CODE_RUNNER_EXTENSION_ID)).returns(() => extension.object); extension @@ -273,10 +264,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If old version of Code Runner extension is installed but setting `code-runner.executorMap.python` is not set, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); const extension = typemoq.Mock.ofType>(); extensions.setup((e) => e.getExtension(CODE_RUNNER_EXTENSION_ID)).returns(() => extension.object); @@ -294,10 +282,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If old version of Code Runner extension is installed but $pythonPath is not being used, empty diagnostics is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); const extension = typemoq.Mock.ofType>(); extensions.setup((e) => e.getExtension(CODE_RUNNER_EXTENSION_ID)).returns(() => extension.object); @@ -315,10 +300,7 @@ suite('Application Diagnostics - Upgrade Code Runner', () => { }); test('If old version of Code Runner extension is installed and $pythonPath is being used, diagnostic with appropriate message is returned', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); const workspaceConfig = typemoq.Mock.ofType(); const extension = typemoq.Mock.ofType>(); extensions.setup((e) => e.getExtension(CODE_RUNNER_EXTENSION_ID)).returns(() => extension.object); diff --git a/src/test/common/configSettings/configSettings.pythonPath.unit.test.ts b/src/test/common/configSettings/configSettings.pythonPath.unit.test.ts index cf54ff32f9ae..bced9e2f8c6d 100644 --- a/src/test/common/configSettings/configSettings.pythonPath.unit.test.ts +++ b/src/test/common/configSettings/configSettings.pythonPath.unit.test.ts @@ -6,17 +6,18 @@ import { expect } from 'chai'; import * as path from 'path'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; import { Uri, WorkspaceConfiguration } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { PythonSettings } from '../../../client/common/configSettings'; import { DeprecatePythonPath } from '../../../client/common/experiments/groups'; -import { IExperimentsManager, IInterpreterPathService } from '../../../client/common/types'; +import { IExperimentService, IInterpreterPathService } from '../../../client/common/types'; import { noop } from '../../../client/common/utils/misc'; -import { IInterpreterSecurityService } from '../../../client/interpreter/autoSelection/types'; +import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; import * as EnvFileTelemetry from '../../../client/telemetry/envFileTelemetry'; import { MockAutoSelectionService } from '../../mocks/autoSelector'; + const untildify = require('untildify'); suite('Python Settings - pythonPath', () => { @@ -24,23 +25,27 @@ suite('Python Settings - pythonPath', () => { public update(settings: WorkspaceConfiguration) { return super.update(settings); } + + // eslint-disable-next-line class-methods-use-this protected getPythonExecutable(pythonPath: string) { return pythonPath; } + + // eslint-disable-next-line class-methods-use-this public initialize() { noop(); } } let configSettings: CustomPythonSettings; let workspaceService: typemoq.IMock; - let experimentsManager: typemoq.IMock; + let experimentsManager: typemoq.IMock; let interpreterPathService: typemoq.IMock; let pythonSettings: typemoq.IMock; setup(() => { pythonSettings = typemoq.Mock.ofType(); sinon.stub(EnvFileTelemetry, 'sendSettingTelemetry').returns(); interpreterPathService = typemoq.Mock.ofType(); - experimentsManager = typemoq.Mock.ofType(); + experimentsManager = typemoq.Mock.ofType(); workspaceService = typemoq.Mock.ofType(); pythonSettings.setup((p) => p.get(typemoq.It.isValue('defaultInterpreterPath'))).returns(() => 'python'); pythonSettings.setup((p) => p.get('logging')).returns(() => ({ level: 'error' })); @@ -115,7 +120,7 @@ suite('Python Settings - pythonPath', () => { }); test("If we don't have a custom python path and we do have an auto selected interpreter, then use it", () => { const pythonPath = path.join(__dirname, 'this is a python path that was auto selected'); - const interpreter: any = { path: pythonPath }; + const interpreter = { path: pythonPath } as PythonEnvironment; const workspaceFolderUri = Uri.file(__dirname); const selectionService = mock(MockAutoSelectionService); when(selectionService.getAutoSelectedInterpreter(workspaceFolderUri)).thenReturn(interpreter); @@ -128,49 +133,26 @@ suite('Python Settings - pythonPath', () => { configSettings.update(pythonSettings.object); expect(configSettings.pythonPath).to.be.equal(pythonPath); - verify(selectionService.getAutoSelectedInterpreter(workspaceFolderUri)).once(); }); - test("If user is in Deprecate Python Path experiment and we don't have a custom python path, get the autoselected interpreter and use it if it's safe", () => { - const resource = Uri.parse('a'); + test("If we don't have a custom default python path and we do have an auto selected interpreter, then use it", () => { const pythonPath = path.join(__dirname, 'this is a python path that was auto selected'); - const interpreter: any = { path: pythonPath }; + const interpreter = { path: pythonPath } as PythonEnvironment; + const workspaceFolderUri = Uri.file(__dirname); const selectionService = mock(MockAutoSelectionService); - const interpreterSecurityService = typemoq.Mock.ofType(); - when(selectionService.getAutoSelectedInterpreter(resource)).thenReturn(interpreter); - interpreterSecurityService.setup((i) => i.isSafe(interpreter)).returns(() => true); - when(selectionService.setWorkspaceInterpreter(resource, anything())).thenResolve(); - configSettings = new CustomPythonSettings( - resource, - instance(selectionService), - workspaceService.object, - experimentsManager.object, - interpreterPathService.object, - interpreterSecurityService.object, - ); - experimentsManager - .setup((e) => e.inExperiment(DeprecatePythonPath.experiment)) - .returns(() => true) - .verifiable(typemoq.Times.once()); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); - interpreterPathService.setup((i) => i.get(resource)).returns(() => 'python'); + when(selectionService.getAutoSelectedInterpreter(workspaceFolderUri)).thenReturn(interpreter); + configSettings = new CustomPythonSettings(workspaceFolderUri, instance(selectionService)); + pythonSettings.setup((p) => p.get(typemoq.It.isValue('pythonPath'))).returns(() => 'custom'); + pythonSettings.setup((p) => p.get(typemoq.It.isValue('defaultInterpreterPath'))).returns(() => 'python'); configSettings.update(pythonSettings.object); - expect(configSettings.pythonPath).to.be.equal(pythonPath); - experimentsManager.verifyAll(); - interpreterPathService.verifyAll(); - pythonSettings.verifyAll(); - verify(selectionService.getAutoSelectedInterpreter(resource)).once(); + expect(configSettings.defaultInterpreterPath).to.be.equal(pythonPath); }); - test("If user is in Deprecate Python Path experiment and we don't have a custom python path, get the autoselected interpreter and but don't use it if it's not safe", () => { + test("If user is in Deprecate Python Path experiment and we don't have a custom python path, get the autoselected interpreter and use it if it's safe", () => { const resource = Uri.parse('a'); const pythonPath = path.join(__dirname, 'this is a python path that was auto selected'); - const interpreter: any = { path: pythonPath }; + const interpreter = { path: pythonPath } as PythonEnvironment; const selectionService = mock(MockAutoSelectionService); - const interpreterSecurityService = typemoq.Mock.ofType(); when(selectionService.getAutoSelectedInterpreter(resource)).thenReturn(interpreter); - interpreterSecurityService.setup((i) => i.isSafe(interpreter)).returns(() => false); when(selectionService.setWorkspaceInterpreter(resource, anything())).thenResolve(); configSettings = new CustomPythonSettings( resource, @@ -178,23 +160,18 @@ suite('Python Settings - pythonPath', () => { workspaceService.object, experimentsManager.object, interpreterPathService.object, - interpreterSecurityService.object, ); experimentsManager - .setup((e) => e.inExperiment(DeprecatePythonPath.experiment)) + .setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)) .returns(() => true) .verifiable(typemoq.Times.once()); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); interpreterPathService.setup((i) => i.get(resource)).returns(() => 'python'); configSettings.update(pythonSettings.object); - expect(configSettings.pythonPath).to.be.equal('a'); + expect(configSettings.pythonPath).to.be.equal(pythonPath); experimentsManager.verifyAll(); interpreterPathService.verifyAll(); pythonSettings.verifyAll(); - verify(selectionService.getAutoSelectedInterpreter(resource)).once(); }); test('If user is in Deprecate Python Path experiment, use the new API to fetch Python Path', () => { const resource = Uri.parse('a'); @@ -208,13 +185,9 @@ suite('Python Settings - pythonPath', () => { const pythonPath = 'This is the new API python Path'; pythonSettings.setup((p) => p.get(typemoq.It.isValue('pythonPath'))).verifiable(typemoq.Times.never()); experimentsManager - .setup((e) => e.inExperiment(DeprecatePythonPath.experiment)) + .setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)) .returns(() => true) .verifiable(typemoq.Times.once()); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined) - .verifiable(typemoq.Times.once()); interpreterPathService .setup((i) => i.get(resource)) .returns(() => pythonPath) @@ -241,13 +214,9 @@ suite('Python Settings - pythonPath', () => { .returns(() => pythonPath) .verifiable(typemoq.Times.atLeastOnce()); experimentsManager - .setup((e) => e.inExperiment(DeprecatePythonPath.experiment)) + .setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)) .returns(() => false) .verifiable(typemoq.Times.once()); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined) - .verifiable(typemoq.Times.once()); interpreterPathService.setup((i) => i.get(resource)).verifiable(typemoq.Times.never()); configSettings.update(pythonSettings.object); diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index c6eee0771ac8..20fe922fc90b 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -78,7 +78,7 @@ suite('Python Settings', async () => { } // boolean settings - for (const name of ['downloadLanguageServer', 'autoUpdateLanguageServer', 'useIsolation']) { + for (const name of ['downloadLanguageServer', 'autoUpdateLanguageServer']) { config .setup((c) => c.get(name, true)) @@ -144,7 +144,7 @@ suite('Python Settings', async () => { }); suite('Boolean settings', async () => { - ['downloadLanguageServer', 'autoUpdateLanguageServer', 'globalModuleInstallation', 'useIsolation'].forEach( + ['downloadLanguageServer', 'autoUpdateLanguageServer', 'globalModuleInstallation'].forEach( async (settingName) => { testIfValueIsUpdated(settingName, true); }, diff --git a/src/test/common/configuration/service.unit.test.ts b/src/test/common/configuration/service.unit.test.ts index 241c3212862e..db997d3f0c51 100644 --- a/src/test/common/configuration/service.unit.test.ts +++ b/src/test/common/configuration/service.unit.test.ts @@ -10,24 +10,19 @@ import { IWorkspaceService } from '../../../client/common/application/types'; import { PythonSettings } from '../../../client/common/configSettings'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { DeprecatePythonPath } from '../../../client/common/experiments/groups'; -import { IExperimentsManager, IInterpreterPathService } from '../../../client/common/types'; -import { - IInterpreterAutoSelectionService, - IInterpreterSecurityService, -} from '../../../client/interpreter/autoSelection/types'; +import { IExperimentService, IInterpreterPathService } from '../../../client/common/types'; +import { IInterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection/types'; import { IServiceContainer } from '../../../client/ioc/types'; suite('Configuration Service', () => { const resource = Uri.parse('a'); let workspaceService: TypeMoq.IMock; let interpreterPathService: TypeMoq.IMock; - let experimentsManager: TypeMoq.IMock; + let experimentsManager: TypeMoq.IMock; let serviceContainer: TypeMoq.IMock; - let interpreterSecurityService: TypeMoq.IMock; let configService: ConfigurationService; setup(() => { workspaceService = TypeMoq.Mock.ofType(); - interpreterSecurityService = TypeMoq.Mock.ofType(); workspaceService .setup((w) => w.getWorkspaceFolder(resource)) .returns(() => ({ @@ -37,13 +32,10 @@ suite('Configuration Service', () => { })); interpreterPathService = TypeMoq.Mock.ofType(); serviceContainer = TypeMoq.Mock.ofType(); - experimentsManager = TypeMoq.Mock.ofType(); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager = TypeMoq.Mock.ofType(); serviceContainer.setup((s) => s.get(IWorkspaceService)).returns(() => workspaceService.object); serviceContainer.setup((s) => s.get(IInterpreterPathService)).returns(() => interpreterPathService.object); - serviceContainer.setup((s) => s.get(IExperimentsManager)).returns(() => experimentsManager.object); + serviceContainer.setup((s) => s.get(IExperimentService)).returns(() => experimentsManager.object); configService = new ConfigurationService(serviceContainer.object); }); @@ -57,10 +49,6 @@ suite('Configuration Service', () => { test('Fetching settings goes as expected', () => { const interpreterAutoSelectionProxyService = TypeMoq.Mock.ofType(); - serviceContainer - .setup((s) => s.get(IInterpreterSecurityService)) - .returns(() => interpreterSecurityService.object) - .verifiable(TypeMoq.Times.once()); serviceContainer .setup((s) => s.get(IInterpreterAutoSelectionService)) .returns(() => interpreterAutoSelectionProxyService.object) @@ -70,7 +58,7 @@ suite('Configuration Service', () => { }); test('Do not update global settings if global value is already equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig @@ -87,7 +75,7 @@ suite('Configuration Service', () => { }); test('Update global settings if global value is not equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig @@ -104,7 +92,7 @@ suite('Configuration Service', () => { }); test('Do not update workspace settings if workspace value is already equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig @@ -121,7 +109,7 @@ suite('Configuration Service', () => { }); test('Update workspace settings if workspace value is not equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig @@ -138,7 +126,7 @@ suite('Configuration Service', () => { }); test('Do not update workspace folder settings if workspace folder value is already equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig .setup((w) => w.inspect('setting')) @@ -160,7 +148,7 @@ suite('Configuration Service', () => { }); test('Update workspace folder settings if workspace folder value is not equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig .setup((w) => w.inspect('setting')) @@ -182,7 +170,7 @@ suite('Configuration Service', () => { }); test('If in Deprecate PythonPath experiment & setting to update is `python.pythonPath`, update settings using new API if stored value is not equal to the new value', async () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); interpreterPathService .setup((w) => w.inspect(resource)) diff --git a/src/test/common/experiments/manager.unit.test.ts b/src/test/common/experiments/manager.unit.test.ts deleted file mode 100644 index 26dfc562e1b7..000000000000 --- a/src/test/common/experiments/manager.unit.test.ts +++ /dev/null @@ -1,447 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { assert, expect } from 'chai'; -import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import * as TypeMoq from 'typemoq'; -import { ApplicationEnvironment } from '../../../client/common/application/applicationEnvironment'; -import { IApplicationEnvironment } from '../../../client/common/application/types'; -import { PythonSettings } from '../../../client/common/configSettings'; -import { ConfigurationService } from '../../../client/common/configuration/service'; -import { CryptoUtils } from '../../../client/common/crypto'; -import { - ExperimentsManager, - experimentStorageKey, - oldExperimentSalts, -} from '../../../client/common/experiments/manager'; -import { PersistentStateFactory } from '../../../client/common/persistentState'; -import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { IFileSystem } from '../../../client/common/platform/types'; -import { - ICryptoUtils, - IExperiments, - IOutputChannel, - IPersistentState, - IPersistentStateFactory, -} from '../../../client/common/types'; -import { noop } from '../../core'; - -suite('A/B experiments', () => { - let crypto: ICryptoUtils; - let appEnvironment: IApplicationEnvironment; - let persistentStateFactory: IPersistentStateFactory; - let experimentStorage: TypeMoq.IMock>; - let output: TypeMoq.IMock; - let fs: IFileSystem; - let expManager: ExperimentsManager; - let configurationService: ConfigurationService; - let experiments: TypeMoq.IMock; - setup(() => { - crypto = mock(CryptoUtils); - appEnvironment = mock(ApplicationEnvironment); - persistentStateFactory = mock(PersistentStateFactory); - experimentStorage = TypeMoq.Mock.ofType>(); - output = TypeMoq.Mock.ofType(); - configurationService = mock(ConfigurationService); - experiments = TypeMoq.Mock.ofType(); - const settings = mock(PythonSettings); - when(settings.experiments).thenReturn(experiments.object); - experiments.setup((e) => e.optInto).returns(() => []); - experiments.setup((e) => e.optOutFrom).returns(() => []); - when(configurationService.getSettings(undefined)).thenReturn(instance(settings)); - fs = mock(FileSystem); - when(persistentStateFactory.createGlobalPersistentState(experimentStorageKey, undefined as any)).thenReturn( - experimentStorage.object, - ); - expManager = new ExperimentsManager( - instance(persistentStateFactory), - instance(crypto), - instance(appEnvironment), - output.object, - instance(fs), - instance(configurationService), - ); - }); - - teardown(() => { - sinon.restore(); - }); - - async function testEnablingExperiments(enabled: boolean) { - const updateExperimentStorage = sinon.stub(ExperimentsManager.prototype, 'updateExperimentStorage'); - updateExperimentStorage.callsFake(() => Promise.resolve()); - const populateUserExperiments = sinon.stub(ExperimentsManager.prototype, 'populateUserExperiments'); - populateUserExperiments.callsFake(() => Promise.resolve()); - experiments - .setup((e) => e.enabled) - .returns(() => enabled) - .verifiable(TypeMoq.Times.atLeastOnce()); - - expManager = new ExperimentsManager( - instance(persistentStateFactory), - instance(crypto), - instance(appEnvironment), - output.object, - instance(fs), - instance(configurationService), - ); - await expManager.activate(); - - // If experiments are disabled, then none of these methods will be invoked & vice versa. - assert.equal(updateExperimentStorage.callCount, enabled ? 1 : 0); - assert.equal(populateUserExperiments.callCount, enabled ? 1 : 0); - - experiments.verifyAll(); - } - test('Ensure experiments are not initialized when it is disabled', async () => testEnablingExperiments(false)); - - test('Ensure experiments are initialized when it is enabled', async () => testEnablingExperiments(true)); - - async function testEnablingExperimentsToCheckIfInExperiment(enabled: boolean) { - const sendTelemetry = sinon.stub(ExperimentsManager.prototype, 'sendTelemetryIfInExperiment'); - sendTelemetry.callsFake((_: string) => noop()); - - expManager = new ExperimentsManager( - instance(persistentStateFactory), - instance(crypto), - instance(appEnvironment), - output.object, - instance(fs), - instance(configurationService), - ); - - expManager._enabled = enabled; - expManager.userExperiments.push({ name: 'this should be in experiment', max: 0, min: 0, salt: '' }); - - // If experiments are disabled, then `inExperiment` will return false & vice versa. - assert.equal(expManager.inExperiment('this should be in experiment'), enabled); - // This experiment does not exist, hence `inExperiment` will always be `false` for this. - assert.equal(expManager.inExperiment('this should never be in experiment'), false); - - experiments.verifyAll(); - } - test('Ensure inExperiment is true when experiments are enabled', async () => - testEnablingExperimentsToCheckIfInExperiment(true)); - - test('Ensure inExperiment is false when experiments are disabled', async () => - testEnablingExperimentsToCheckIfInExperiment(false)); - - test('Ensure experiments can only be activated once', async () => { - const updateExperimentStorage = sinon.stub(ExperimentsManager.prototype, 'updateExperimentStorage'); - updateExperimentStorage.callsFake(() => Promise.resolve()); - const populateUserExperiments = sinon.stub(ExperimentsManager.prototype, 'populateUserExperiments'); - populateUserExperiments.callsFake(() => Promise.resolve()); - expManager = new ExperimentsManager( - instance(persistentStateFactory), - // instance(httpClient), - instance(crypto), - instance(appEnvironment), - output.object, - instance(fs), - instance(configurationService), - ); - - assert.isFalse(expManager._activated()); - await expManager.activate(); - - // Ensure activated flag is set - assert.isTrue(expManager._activated()); - }); - - const testsForInExperiment = [ - { - testName: "If experiment's name is not in user experiment list, user is not in experiment", - experimentName: 'imaginary experiment', - userExperiments: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: false, - }, - { - testName: - "If experiment's name is in user experiment list and hash modulo output is in range, user is in experiment", - experimentName: 'experiment1', - userExperiments: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: true, - }, - ]; - - testsForInExperiment.forEach((testParams) => { - test(testParams.testName, async () => { - expManager.userExperiments = testParams.userExperiments; - expect(expManager.inExperiment(testParams.experimentName)).to.equal( - testParams.expectedResult, - 'Incorrectly identified', - ); - }); - }); - - const testsForIsUserInRange = [ - // Note min equals 79 and max equals 94 - { - testName: 'Returns true if hash modulo output is in range', - hash: 1181, - expectedResult: true, - }, - { - testName: 'Returns false if hash modulo is less than min', - hash: 967, - expectedResult: false, - }, - { - testName: 'Returns false if hash modulo is more than max', - hash: 3297, - expectedResult: false, - }, - { - testName: 'If checking if user is in range fails with error, throw error', - hash: 3297, - error: true, - expectedResult: false, - }, - { - testName: 'If machine ID is bogus, throw error', - hash: 3297, - machineIdError: true, - expectedResult: false, - }, - ]; - - suite('Function IsUserInRange()', () => { - testsForIsUserInRange.forEach((testParams) => { - test(testParams.testName, async () => { - when(appEnvironment.machineId).thenReturn('101'); - if (testParams.machineIdError) { - when(appEnvironment.machineId).thenReturn(undefined as any); - expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw(); - } else if (testParams.error) { - const error = new Error('Kaboom'); - when(crypto.createHash(anything(), 'number', anything())).thenThrow(error); - expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw(error); - } else { - when(crypto.createHash(anything(), 'number', anything())).thenReturn(testParams.hash); - expect(expManager.isUserInRange(79, 94, 'salt')).to.equal( - testParams.expectedResult, - 'Incorrectly identified', - ); - } - }); - }); - test('If experiment salt belongs to an old experiment, keep using `SHA512` algorithm', async () => { - when(appEnvironment.machineId).thenReturn('101'); - when(crypto.createHash(anything(), 'number', 'SHA512')).thenReturn(644); - when(crypto.createHash(anything(), anything(), 'FNV')).thenReturn(1293); - expManager.isUserInRange(79, 94, 'LS'); - verify(crypto.createHash(anything(), 'number', 'SHA512')).once(); - verify(crypto.createHash(anything(), anything(), 'FNV')).never(); - }); - test('If experiment salt does not belong to an old experiment, use `FNV` algorithm', async () => { - when(appEnvironment.machineId).thenReturn('101'); - when(crypto.createHash(anything(), anything(), 'SHA512')).thenReturn(644); - when(crypto.createHash(anything(), 'number', 'FNV')).thenReturn(1293); - expManager.isUserInRange(79, 94, 'NewExperimentSalt'); - verify(crypto.createHash(anything(), anything(), 'SHA512')).never(); - verify(crypto.createHash(anything(), 'number', 'FNV')).once(); - }); - test('Use the expected list of old experiments', async () => { - const expectedOldExperimentSalts = ['LS']; - assert.deepEqual(expectedOldExperimentSalts, oldExperimentSalts); - }); - }); - - const testsForPopulateUserExperiments = [ - { - testName: 'User experiments list is empty if experiment storage value is not an array', - experimentStorageValue: undefined, - expectedResult: [], - }, - { - testName: 'User experiments list is empty if experiment storage value is an empty array', - experimentStorageValue: [], - expectedResult: [], - }, - { - testName: - 'User experiments list does not contain any experiments if user has requested to opt out of all experiments', - experimentStorageValue: [ - { name: 'experiment1 - control', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2 - control', salt: 'salt', min: 80, max: 90 }, - ], - hash: 8187, - experimentsOptedOutFrom: ['All'], - expectedResult: [], - }, - { - testName: - 'User experiments list contains all experiments if user has requested to opt into all experiments', - experimentStorageValue: [ - { name: 'experiment1 - control', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2 - control', salt: 'salt', min: 80, max: 90 }, - ], - hash: 8187, - experimentsOptedInto: ['All'], - expectedResult: [ - { name: 'experiment1 - control', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2 - control', salt: 'salt', min: 80, max: 90 }, - ], - }, - { - testName: - 'User experiments list contains the experiment if user has requested to opt in a control group but is not in experiment range', - experimentStorageValue: [{ name: 'experiment2 - control', salt: 'salt', min: 19, max: 30 }], - hash: 8187, - experimentsOptedInto: ['experiment2 - control'], - expectedResult: [], - }, - { - testName: - 'User experiments list contains the experiment if user has requested to opt out of a control group but user is in experiment range', - experimentStorageValue: [ - { name: 'experiment1 - control', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2 - control', salt: 'salt', min: 19, max: 30 }, - ], - hash: 8187, - experimentsOptedOutFrom: ['experiment1 - control'], - expectedResult: [{ name: 'experiment1 - control', salt: 'salt', min: 79, max: 94 }], - }, - { - testName: - 'User experiments list does not contains the experiment if user has opted out of experiment even though user is in experiment range', - experimentStorageValue: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - hash: 8187, - experimentsOptedOutFrom: ['experiment1'], - expectedResult: [], - }, - { - testName: - 'User experiments list contains the experiment if user has opted into the experiment even though user is not in experiment range', - experimentStorageValue: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - hash: 8187, - experimentsOptedInto: ['experiment1'], - expectedResult: [{ name: 'experiment1', salt: 'salt', min: 79, max: 94 }], - }, - { - testName: - 'User experiments list contains the experiment user has opened into and not the control experiment even if user is in the control experiment range', - experimentStorageValue: [ - { name: 'control', salt: 'salt', min: 0, max: 100 }, - { name: 'experiment', salt: 'salt', min: 0, max: 0 }, - ], - hash: 8187, - experimentsOptedInto: ['experiment'], - expectedResult: [{ name: 'experiment', salt: 'salt', min: 0, max: 0 }], - }, - { - testName: - 'User experiments list does not contain the experiment if user has both opted in and out of an experiment', - experimentStorageValue: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - hash: 8187, - experimentsOptedInto: ['experiment1'], - experimentsOptedOutFrom: ['experiment1'], - expectedResult: [], - }, - { - testName: 'Otherwise user experiments list contains the experiment if user is in experiment range', - experimentStorageValue: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - hash: 8187, - expectedResult: [{ name: 'experiment1', salt: 'salt', min: 79, max: 94 }], - }, - ]; - - suite('Function populateUserExperiments', async () => { - testsForPopulateUserExperiments.forEach((testParams) => { - test(testParams.testName, async () => { - experimentStorage.setup((n) => n.value).returns(() => testParams.experimentStorageValue); - when(appEnvironment.machineId).thenReturn('101'); - if (testParams.hash) { - when(crypto.createHash(anything(), 'number', anything())).thenReturn(testParams.hash); - } - if (testParams.experimentsOptedInto) { - expManager._experimentsOptedInto = testParams.experimentsOptedInto; - } - if (testParams.experimentsOptedOutFrom) { - expManager._experimentsOptedOutFrom = testParams.experimentsOptedOutFrom; - } - expManager.populateUserExperiments(); - assert.deepEqual(expManager.userExperiments, testParams.expectedResult); - }); - }); - }); - - const testsForAreExperimentsValid = [ - { - testName: 'If experiments are not an array, return false', - experiments: undefined, - expectedResult: false, - }, - { - testName: 'If any experiment have `min` field missing, return false', - experiments: [ - { name: 'experiment1', salt: 'salt', max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: false, - }, - { - testName: 'If any experiment have `max` field missing, return false', - experiments: [ - { name: 'experiment1', salt: 'salt', min: 79 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: false, - }, - { - testName: 'If any experiment have `salt` field missing, return false', - experiments: [ - { name: 'experiment1', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: false, - }, - { - testName: 'If any experiment have `name` field missing, return false', - experiments: [ - { salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: false, - }, - { - testName: 'If all experiments contain all the fields in type `ABExperiment`, return true', - experiments: [ - { name: 'experiment1', salt: 'salt', min: 79, max: 94 }, - { name: 'experiment2', salt: 'salt', min: 19, max: 30 }, - ], - expectedResult: true, - }, - ]; - - suite('Function areExperimentsValid()', () => { - testsForAreExperimentsValid.forEach((testParams) => { - test(testParams.testName, () => { - expect(expManager.areExperimentsValid(testParams.experiments as any)).to.equal( - testParams.expectedResult, - ); - }); - }); - }); -}); diff --git a/src/test/common/experiments/service.unit.test.ts b/src/test/common/experiments/service.unit.test.ts index 4146f1c85186..7050194fd2c2 100644 --- a/src/test/common/experiments/service.unit.test.ts +++ b/src/test/common/experiments/service.unit.test.ts @@ -170,9 +170,9 @@ suite('Experimentation service', () => { }); isCachedFlightEnabledStub = sinon.stub().returns(Promise.resolve(true)); - sinon.stub(tasClient, 'getExperimentationService').returns({ + sinon.stub(tasClient, 'getExperimentationService').returns(({ isCachedFlightEnabled: isCachedFlightEnabledStub, - } as never); + } as unknown) as tasClient.IExperimentationService); configureApplicationEnvironment('stable', extensionVersion); }); @@ -340,9 +340,9 @@ suite('Experimentation service', () => { }); getTreatmentVariable = sinon.stub().returns(Promise.resolve(true)); - sinon.stub(tasClient, 'getExperimentationService').returns({ + sinon.stub(tasClient, 'getExperimentationService').returns(({ getTreatmentVariable, - } as never); + } as unknown) as tasClient.IExperimentationService); configureApplicationEnvironment('stable', extensionVersion); }); @@ -501,9 +501,9 @@ suite('Experimentation service', () => { setup(() => { getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve('value')); - sinon.stub(tasClient, 'getExperimentationService').returns({ + sinon.stub(tasClient, 'getExperimentationService').returns(({ getTreatmentVariableAsync: getTreatmentVariableAsyncStub, - } as never); + } as unknown) as tasClient.IExperimentationService); configureApplicationEnvironment('stable', extensionVersion); }); diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index a868a542fcfe..6735ec5d31ce 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -27,7 +27,6 @@ import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegi import { ConfigurationService } from '../../client/common/configuration/service'; import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; -import { ExperimentsManager } from '../../client/common/experiments/manager'; import { ExperimentService } from '../../client/common/experiments/service'; import { ExtensionInsidersDailyChannelRule, @@ -100,7 +99,6 @@ import { ICurrentProcess, IEditorUtils, IExperimentService, - IExperimentsManager, IExtensions, IFileDownloader, IHttpClient, @@ -224,7 +222,6 @@ suite('Installer', () => { PowershellTerminalActivationFailedHandler, ); ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); - ioc.serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton(ITerminalHelper, TerminalHelper); @@ -307,8 +304,7 @@ suite('Installer', () => { const checkInstalledDef = createDeferred(); processService.onExec((_file, args, _options, callback) => { const moduleName = installer.translateProductToModuleName(product, ModuleNamePurpose.run); - // args[0] is pyvsc-run-isolated.py. - if (args.length > 1 && args[1] === '-c' && args[2] === `import ${moduleName}`) { + if (args.length > 1 && args[0] === '-c' && args[1] === `import ${moduleName}`) { checkInstalledDef.resolve(true); } callback({ stdout: '' }); diff --git a/src/test/common/installer/moduleInstaller.unit.test.ts b/src/test/common/installer/moduleInstaller.unit.test.ts index 53ec6c219a8f..d093e41240a1 100644 --- a/src/test/common/installer/moduleInstaller.unit.test.ts +++ b/src/test/common/installer/moduleInstaller.unit.test.ts @@ -55,9 +55,6 @@ import { } from '../../../client/interpreter/contracts'; import { IServiceContainer } from '../../../client/ioc/types'; import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; - -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); /* Complex test to ensure we cover all combinations: We could have written separate tests for each installer, but we'd be replicate code. @@ -374,7 +371,7 @@ suite('Module Installer', () => { const proxyArgs = proxyServer.length === 0 ? [] : ['--proxy', proxyServer]; const expectedArgs = [ - isolated, + '-m', 'pip', ...proxyArgs, 'install', @@ -417,7 +414,7 @@ suite('Module Installer', () => { const proxyArgs = proxyServer.length === 0 ? [] : ['--proxy', proxyServer]; const expectedArgs = [ - isolated, + '-m', 'pip', ...proxyArgs, 'install', @@ -478,7 +475,7 @@ suite('Module Installer', () => { } catch (ex) { noop(); } - const args = [isolated, 'executionInfo']; + const args = ['-m', 'executionInfo']; assert.ok(elevatedInstall.calledOnceWith(pythonPath, args)); interpreterService.verifyAll(); }); @@ -493,7 +490,7 @@ suite('Module Installer', () => { fs.setup((f) => f.isDirReadonly(path.dirname(pythonPath))).returns(() => Promise.resolve(false), ); - const args = [isolated, 'executionInfo']; + const args = ['-m', 'executionInfo']; terminalService .setup((t) => t.sendCommand(pythonPath, args, undefined)) .returns(() => Promise.resolve()) @@ -514,7 +511,7 @@ suite('Module Installer', () => { info.setup((t) => t.version).returns(() => new SemVer('3.5.0-final')); setActiveInterpreter(info.object); pythonSettings.setup((p) => p.globalModuleInstallation).returns(() => false); - const args = [isolated, 'executionInfo', '--user']; + const args = ['-m', 'executionInfo', '--user']; terminalService .setup((t) => t.sendCommand(pythonPath, args, undefined)) .returns(() => Promise.resolve()) @@ -550,7 +547,7 @@ suite('Module Installer', () => { } catch (ex) { noop(); } - const args = [isolated, 'executionInfo']; + const args = ['-m', 'executionInfo']; assert.ok(elevatedInstall.calledOnceWith(pythonPath, args)); interpreterService.verifyAll(); }); @@ -595,7 +592,7 @@ suite('Module Installer', () => { test(`Test Args (${product.name})`, async () => { setActiveInterpreter(); const proxyArgs = proxyServer.length === 0 ? [] : ['--proxy', proxyServer]; - const expectedArgs = [isolated, 'pip', ...proxyArgs, 'install', '-U', moduleName]; + const expectedArgs = ['-m', 'pip', ...proxyArgs, 'install', '-U', moduleName]; await installModuleAndVerifyCommand(pythonPath, expectedArgs); interpreterService.verifyAll(); }); diff --git a/src/test/common/interpreterPathService.unit.test.ts b/src/test/common/interpreterPathService.unit.test.ts index bb32159ee663..6a898b7b4028 100644 --- a/src/test/common/interpreterPathService.unit.test.ts +++ b/src/test/common/interpreterPathService.unit.test.ts @@ -200,7 +200,7 @@ suite('Interpreter Path Service', async () => { persistentState.verifyAll(); }); - test('If the one-off transfer to new storage has not happened yet for the user setting, do it, record the transfer and remove the original user setting', async () => { + test('If the one-off transfer to new storage has not happened yet for the user setting, do it, record the transfer', async () => { const update = sinon.stub(InterpreterPathService.prototype, 'update'); const persistentState = TypeMoq.Mock.ofType>(); persistentStateFactory @@ -208,19 +208,12 @@ suite('Interpreter Path Service', async () => { .returns(() => persistentState.object); persistentState.setup((p) => p.value).returns(() => false); persistentState.setup((p) => p.updateValue(true)).verifiable(TypeMoq.Times.once()); - const workspaceConfig = TypeMoq.Mock.ofType(); - workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); - workspaceConfig - .setup((w) => w.update('pythonPath', undefined, ConfigurationTarget.Global)) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []); await interpreterPathService._moveGlobalSettingValueToNewStorage('globalPythonPath'); assert(update.calledWith(undefined, ConfigurationTarget.Global, 'globalPythonPath')); persistentState.verifyAll(); - workspaceConfig.verifyAll(); }); test('If the one-off transfer to new storage has already happened for the user setting, do not update and simply return', async () => { @@ -449,7 +442,9 @@ suite('Interpreter Path Service', async () => { test('Inspecting settings returns as expected if no workspace is opened', async () => { const workspaceConfig = TypeMoq.Mock.ofType(); - workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); + workspaceService + .setup((w) => w.getConfiguration('python', TypeMoq.It.isAny())) + .returns(() => workspaceConfig.object); workspaceConfig .setup((w) => w.inspect('defaultInterpreterPath')) .returns( @@ -481,7 +476,7 @@ suite('Interpreter Path Service', async () => { // No workspace file is present if a folder is directly opened workspaceService.setup((w) => w.workspaceFile).returns(() => undefined); workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); - workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); + workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object); workspaceConfig .setup((w) => w.inspect('defaultInterpreterPath')) .returns( @@ -517,7 +512,7 @@ suite('Interpreter Path Service', async () => { // A workspace file is present in case of multiroot workspace folders workspaceService.setup((w) => w.workspaceFile).returns(() => workspaceFileUri); workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); - workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); + workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object); workspaceConfig .setup((w) => w.inspect('defaultInterpreterPath')) .returns( @@ -549,24 +544,56 @@ suite('Interpreter Path Service', async () => { }); }); - test(`Getting setting value returns workspace folder value if it's defined`, async () => { - interpreterPathService.inspect = () => ({ + test('Inspecting settings falls back to default interpreter setting if no interpreter is set', async () => { + const workspaceFileUri = Uri.parse('path/to/workspaceFile'); + const expectedWorkspaceSettingKey = `WORKSPACE_INTERPRETER_PATH_${fs.normCase(workspaceFileUri.fsPath)}`; + const expectedWorkspaceFolderSettingKey = `WORKSPACE_FOLDER_INTERPRETER_PATH_${resource.fsPath}`; + const workspaceConfig = TypeMoq.Mock.ofType(); + // A workspace file is present in case of multiroot workspace folders + workspaceService.setup((w) => w.workspaceFile).returns(() => workspaceFileUri); + workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); + workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object); + workspaceConfig + .setup((w) => w.inspect('defaultInterpreterPath')) + .returns( + () => + ({ + globalValue: 'default/path/to/interpreter', + workspaceValue: 'defaultWorkspaceValue', + workspaceFolderValue: 'defaultWorkspaceFolderValue', + } as any), + ); + const workspaceFolderPersistentState = TypeMoq.Mock.ofType>(); + const workspacePersistentState = TypeMoq.Mock.ofType>(); + workspaceService.setup((w) => w.workspaceFolders).returns(() => undefined); + persistentStateFactory + .setup((p) => + p.createGlobalPersistentState(expectedWorkspaceFolderSettingKey, undefined), + ) + .returns(() => workspaceFolderPersistentState.object); + persistentStateFactory + .setup((p) => p.createGlobalPersistentState(expectedWorkspaceSettingKey, undefined)) + .returns(() => workspacePersistentState.object); + workspaceFolderPersistentState.setup((p) => p.value).returns(() => undefined); + workspacePersistentState.setup((p) => p.value).returns(() => undefined); + + const settings = interpreterPathService.inspect(resource); + + assert.deepEqual(settings, { globalValue: 'default/path/to/interpreter', - workspaceFolderValue: 'workspaceFolderValue', - workspaceValue: 'workspaceValue', + workspaceFolderValue: 'defaultWorkspaceFolderValue', + workspaceValue: 'defaultWorkspaceValue', }); - const settingValue = interpreterPathService.get(resource); - expect(settingValue).to.equal('workspaceFolderValue'); }); - test(`Getting setting value returns workspace value if workspace folder value is 'undefined'`, async () => { + test(`Getting setting value returns workspace folder value if it's defined`, async () => { interpreterPathService.inspect = () => ({ globalValue: 'default/path/to/interpreter', - workspaceFolderValue: undefined, + workspaceFolderValue: 'workspaceFolderValue', workspaceValue: 'workspaceValue', }); const settingValue = interpreterPathService.get(resource); - expect(settingValue).to.equal('workspaceValue'); + expect(settingValue).to.equal('workspaceFolderValue'); }); test(`Getting setting value returns workspace value if workspace folder value is 'undefined'`, async () => { diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 6d1b4b504b11..38967b97b226 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -34,7 +34,6 @@ import { ConfigurationService } from '../../client/common/configuration/service' import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { DiscoveryVariants } from '../../client/common/experiments/groups'; -import { ExperimentsManager } from '../../client/common/experiments/manager'; import { ExperimentService } from '../../client/common/experiments/service'; import { ExtensionInsidersDailyChannelRule, @@ -99,7 +98,6 @@ import { ICurrentProcess, IEditorUtils, IExperimentService, - IExperimentsManager, IExtensions, IFileDownloader, IHttpClient, @@ -129,7 +127,6 @@ import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironme import { ImportTracker } from '../../client/telemetry/importTracker'; import { IImportTracker } from '../../client/telemetry/types'; import { getExtensionSettings, PYTHON_PATH, rootWorkspaceUri } from '../common'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants'; import { MockModuleInstaller } from '../mocks/moduleInstaller'; import { MockProcessService } from '../mocks/proc'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; @@ -139,8 +136,6 @@ import { IJupyterNotInstalledNotificationHelper } from '../../client/jupyter/typ chaiUse(chaiAsPromised); -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); - const info: PythonEnvironment = { architecture: Architecture.Unknown, companyDisplayName: '', @@ -262,7 +257,6 @@ suite('Module Installer', () => { PowershellTerminalActivationFailedHandler, ); ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); - ioc.serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton( @@ -569,7 +563,7 @@ suite('Module Installer', () => { mockTerminalFactory.verifyAll(); expect(argsSent.join(' ')).equal( - `${isolated} pip install -U ${moduleName} --user`, + `-m pip install -U ${moduleName} --user`, 'Invalid command sent to terminal for installation.', ); }); @@ -610,7 +604,7 @@ suite('Module Installer', () => { mockTerminalFactory.verifyAll(); expect(argsSent.join(' ')).equal( - `${isolated} pip install -U ${moduleName}`, + `-m pip install -U ${moduleName}`, 'Invalid command sent to terminal for installation.', ); }); diff --git a/src/test/common/process/pythonEnvironment.unit.test.ts b/src/test/common/process/pythonEnvironment.unit.test.ts index 3abc2a87c646..cd28544b576d 100644 --- a/src/test/common/process/pythonEnvironment.unit.test.ts +++ b/src/test/common/process/pythonEnvironment.unit.test.ts @@ -3,7 +3,6 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import * as path from 'path'; import { SemVer } from 'semver'; import * as TypeMoq from 'typemoq'; import { IFileSystem } from '../../../client/common/platform/types'; @@ -14,9 +13,6 @@ import { } from '../../../client/common/process/pythonEnvironment'; import { IProcessService, StdErrError } from '../../../client/common/process/types'; import { Architecture } from '../../../client/common/utils/platform'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; - -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); use(chaiAsPromised); @@ -177,7 +173,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.pathExists(pythonPath)).returns(() => Promise.resolve(false)); - const argv = [isolated, '-c', 'import sys;print(sys.executable)']; + const argv = ['-c', 'import sys;print(sys.executable)']; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) .returns(() => Promise.resolve({ stdout: executablePath })); @@ -191,7 +187,7 @@ suite('PythonEnvironment', () => { test('getExecutablePath should throw if the result of exec() writes to stderr', async () => { const stderr = 'bar'; fileSystem.setup((f) => f.pathExists(pythonPath)).returns(() => Promise.resolve(false)); - const argv = [isolated, '-c', 'import sys;print(sys.executable)']; + const argv = ['-c', 'import sys;print(sys.executable)']; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) .returns(() => Promise.reject(new StdErrError(stderr))); @@ -204,7 +200,7 @@ suite('PythonEnvironment', () => { test('isModuleInstalled should call processService.exec()', async () => { const moduleName = 'foo'; - const argv = [isolated, '-c', `import ${moduleName}`]; + const argv = ['-c', `import ${moduleName}`]; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) .returns(() => Promise.resolve({ stdout: '' })) @@ -218,7 +214,7 @@ suite('PythonEnvironment', () => { test('isModuleInstalled should return true when processService.exec() succeeds', async () => { const moduleName = 'foo'; - const argv = [isolated, '-c', `import ${moduleName}`]; + const argv = ['-c', `import ${moduleName}`]; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) .returns(() => Promise.resolve({ stdout: '' })); @@ -231,7 +227,7 @@ suite('PythonEnvironment', () => { test('isModuleInstalled should return false when processService.exec() throws', async () => { const moduleName = 'foo'; - const argv = [isolated, '-c', `import ${moduleName}`]; + const argv = ['-c', `import ${moduleName}`]; processService .setup((p) => p.exec(pythonPath, argv, { throwOnStdErr: true })) .returns(() => Promise.reject(new StdErrError('bar'))); diff --git a/src/test/common/process/pythonProcess.unit.test.ts b/src/test/common/process/pythonProcess.unit.test.ts index f80af3940895..d799e08b08b5 100644 --- a/src/test/common/process/pythonProcess.unit.test.ts +++ b/src/test/common/process/pythonProcess.unit.test.ts @@ -3,17 +3,13 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { IFileSystem } from '../../../client/common/platform/types'; import { createPythonEnv } from '../../../client/common/process/pythonEnvironment'; import { createPythonProcessService } from '../../../client/common/process/pythonProcess'; import { IProcessService, StdErrError } from '../../../client/common/process/types'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; import { noop } from '../../core'; -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); - use(chaiAsPromised); suite('PythonProcessService', () => { @@ -50,7 +46,7 @@ suite('PythonProcessService', () => { test('execModuleObservable should call processService.execObservable with the -m argument', () => { const args = ['-a', 'b', '-c']; const moduleName = 'foo'; - const expectedArgs = [isolated, moduleName, ...args]; + const expectedArgs = ['-m', moduleName, ...args]; const options = {}; const observable = { proc: undefined, @@ -87,7 +83,7 @@ suite('PythonProcessService', () => { test('execModule should call processService.exec with the -m argument', async () => { const args = ['-a', 'b', '-c']; const moduleName = 'foo'; - const expectedArgs = [isolated, moduleName, ...args]; + const expectedArgs = ['-m', moduleName, ...args]; const options = {}; const stdout = 'bar'; processService @@ -105,13 +101,13 @@ suite('PythonProcessService', () => { test('execModule should throw an error if the module is not installed', async () => { const args = ['-a', 'b', '-c']; const moduleName = 'foo'; - const expectedArgs = [isolated, moduleName, ...args]; + const expectedArgs = ['-m', moduleName, ...args]; const options = {}; processService .setup((p) => p.exec(pythonPath, expectedArgs, options)) .returns(() => Promise.resolve({ stdout: 'bar', stderr: `Error: No module named ${moduleName}` })); processService - .setup((p) => p.exec(pythonPath, [isolated, '-c', `import ${moduleName}`], { throwOnStdErr: true })) + .setup((p) => p.exec(pythonPath, ['-c', `import ${moduleName}`], { throwOnStdErr: true })) .returns(() => Promise.reject(new StdErrError('not installed'))); const env = createPythonEnv(pythonPath, processService.object, fileSystem.object); const procs = createPythonProcessService(processService.object, env); diff --git a/src/test/common/serviceRegistry.unit.test.ts b/src/test/common/serviceRegistry.unit.test.ts index f397f25f71c5..72ba200bddcb 100644 --- a/src/test/common/serviceRegistry.unit.test.ts +++ b/src/test/common/serviceRegistry.unit.test.ts @@ -32,7 +32,6 @@ import { ConfigurationService } from '../../client/common/configuration/service' import { PipEnvExecutionPath } from '../../client/common/configuration/executionSettings/pipEnvExecution'; import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; -import { ExperimentsManager } from '../../client/common/experiments/manager'; import { ExtensionInsidersDailyChannelRule, ExtensionInsidersOffChannelRule, @@ -86,7 +85,6 @@ import { ICryptoUtils, ICurrentProcess, IEditorUtils, - IExperimentsManager, IExtensions, IHttpClient, IInstaller, @@ -133,7 +131,6 @@ suite('Common - Service Registry', () => { [ITerminalActivator, TerminalActivator], [ITerminalActivationHandler, PowershellTerminalActivationFailedHandler], [ICryptoUtils, CryptoUtils], - [IExperimentsManager, ExperimentsManager], [ITerminalHelper, TerminalHelper], [ITerminalActivationCommandProvider, PyEnvActivationCommandProvider, TerminalActivationProviders.pyenv], [ITerminalActivationCommandProvider, Bash, TerminalActivationProviders.bashCShellFish], diff --git a/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts b/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts index e33c1782afc3..275d628bbfd4 100644 --- a/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts +++ b/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts @@ -10,7 +10,7 @@ import * as sinon from 'sinon'; import * as vscode from 'vscode'; import { DeprecatePythonPath } from '../../../../client/common/experiments/groups'; import { FileSystem } from '../../../../client/common/platform/fileSystem'; -import { IExperimentsManager } from '../../../../client/common/types'; +import { IExperimentService } from '../../../../client/common/types'; import { PYTHON_VIRTUAL_ENVS_LOCATION } from '../../../ciConstants'; import { PYTHON_PATH, @@ -58,7 +58,7 @@ suite('Activation of Environments in Terminal', () => { }; let terminalSettings: any; let pythonSettings: any; - let experiments: IExperimentsManager; + let experiments: IExperimentService; const sandbox = sinon.createSandbox(); suiteSetup(async () => { sandbox.stub(ExperimentHelpers, 'inDiscoveryExperiment').resolves(false); @@ -68,7 +68,7 @@ suite('Activation of Environments in Terminal', () => { defaultShell.Windows = terminalSettings.inspect('integrated.shell.windows').globalValue; defaultShell.Linux = terminalSettings.inspect('integrated.shell.linux').globalValue; await terminalSettings.update('integrated.shell.linux', '/bin/bash', vscode.ConfigurationTarget.Global); - experiments = (await initialize()).serviceContainer.get(IExperimentsManager); + experiments = (await initialize()).serviceContainer.get(IExperimentService); }); setup(async () => { @@ -111,7 +111,7 @@ suite('Activation of Environments in Terminal', () => { ); await terminalSettings.update('integrated.shell.linux', defaultShell.Linux, vscode.ConfigurationTarget.Global); await pythonSettings.update('condaPath', undefined, vscode.ConfigurationTarget.Workspace); - if (experiments.inExperiment(DeprecatePythonPath.experiment)) { + if (experiments.inExperimentSync(DeprecatePythonPath.experiment)) { await resetGlobalInterpreterPathSetting(); } else { await restorePythonPathInWorkspaceRoot(); @@ -156,7 +156,7 @@ suite('Activation of Environments in Terminal', () => { vscode.workspace.workspaceFolders![0].uri, vscode.ConfigurationTarget.WorkspaceFolder, ); - if (experiments.inExperiment(DeprecatePythonPath.experiment)) { + if (experiments.inExperimentSync(DeprecatePythonPath.experiment)) { await setGlobalInterpreterPath(envPath); } else { await setPythonPathInWorkspaceRoot(envPath); diff --git a/src/test/common/terminals/synchronousTerminalService.unit.test.ts b/src/test/common/terminals/synchronousTerminalService.unit.test.ts index e302b6974826..ec1a67a4b302 100644 --- a/src/test/common/terminals/synchronousTerminalService.unit.test.ts +++ b/src/test/common/terminals/synchronousTerminalService.unit.test.ts @@ -66,7 +66,6 @@ suite('Terminal Service (synchronous)', () => { }); }); suite('sendCommand', () => { - const isolated = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'); const shellExecFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'shell_exec.py'); test('run sendCommand in terminalService if there is no cancellation token', async () => { @@ -106,7 +105,7 @@ suite('Terminal Service (synchronous)', () => { verify( terminalService.sendCommand( 'python', - deepEqual([isolated, shellExecFile, 'cmd', '1', '2', tmpFile.filePath.fileToCommandArgument()]), + deepEqual([shellExecFile, 'cmd', '1', '2', tmpFile.filePath.fileToCommandArgument()]), ), ).once(); }).timeout(1_000); @@ -142,7 +141,7 @@ suite('Terminal Service (synchronous)', () => { verify( terminalService.sendCommand( 'python', - deepEqual([isolated, shellExecFile, 'cmd', '1', '2', tmpFile.filePath.fileToCommandArgument()]), + deepEqual([shellExecFile, 'cmd', '1', '2', tmpFile.filePath.fileToCommandArgument()]), ), ).once(); }).timeout(2_000); diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 1c13e73a0e5f..5cbe2dab1ff4 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -102,6 +102,13 @@ suite('Set Interpreter Command', () => { path: 'This is the selected Python path', interpreter: {} as PythonEnvironment, }; + const defaultInterpreterPath = 'defaultInterpreterPath'; + const defaultInterpreterPathSuggestion = { + label: InterpreterQuickPickList.defaultInterpreterPath.label(), + detail: defaultInterpreterPath, + path: defaultInterpreterPath, + alwaysShow: true, + }; const refreshedItem: IInterpreterQuickPickItem = { description: '', @@ -143,6 +150,7 @@ suite('Set Interpreter Command', () => { .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([refreshedItem])); pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); + pythonSettings.setup((p) => p.defaultInterpreterPath).returns(() => defaultInterpreterPath); setInterpreterCommand = new SetInterpreterCommand( appShell.object, new PathUtils(false), @@ -177,7 +185,7 @@ suite('Set Interpreter Command', () => { test('Picker should be displayed with expected items: Not in find path experiment', async () => { const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; const multiStepInput = TypeMoq.Mock.ofType>(); - const suggestions = [expectedEnterInterpreterPathSuggestion, item]; + const suggestions = [expectedEnterInterpreterPathSuggestion, item, defaultInterpreterPathSuggestion]; const expectedParameters = { placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, @@ -209,7 +217,7 @@ suite('Set Interpreter Command', () => { await refreshButtonCallback!(quickPick as any); // Invoke callback, meaning that the refresh button is clicked. assert.deepStrictEqual( quickPick.items, - [expectedEnterInterpreterPathSuggestion, refreshedItem], + [expectedEnterInterpreterPathSuggestion, refreshedItem, defaultInterpreterPathSuggestion], 'Quickpick not updated correctly', ); }); @@ -234,7 +242,7 @@ suite('Set Interpreter Command', () => { ); const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; const multiStepInput = TypeMoq.Mock.ofType>(); - const suggestions = [expectedFindInterpreterPathSuggestion, item]; + const suggestions = [expectedFindInterpreterPathSuggestion, item, defaultInterpreterPathSuggestion]; const expectedParameters = { placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, @@ -266,7 +274,7 @@ suite('Set Interpreter Command', () => { await refreshButtonCallback!(quickPick as any); // Invoke callback, meaning that the refresh button is clicked. assert.deepStrictEqual( quickPick.items, - [expectedFindInterpreterPathSuggestion, refreshedItem], + [expectedFindInterpreterPathSuggestion, refreshedItem, defaultInterpreterPathSuggestion], 'Quickpick not updated correctly', ); }); diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index ad90e6f98949..a211cd0fc091 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// eslint-disable-next-line max-classes-per-file import * as assert from 'assert'; import { SemVer } from 'semver'; import * as TypeMoq from 'typemoq'; @@ -8,9 +9,8 @@ import { Uri } from 'vscode'; import { DeprecatePythonPath } from '../../../client/common/experiments/groups'; import { PathUtils } from '../../../client/common/platform/pathUtils'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IExperimentsManager } from '../../../client/common/types'; +import { IExperimentService } from '../../../client/common/types'; import { Architecture } from '../../../client/common/utils/platform'; -import { IInterpreterSecurityService } from '../../../client/interpreter/autoSelection/types'; import { InterpreterSelector } from '../../../client/interpreter/configuration/interpreterSelector/interpreterSelector'; import { IInterpreterComparer, IInterpreterQuickPickItem } from '../../../client/interpreter/configuration/types'; import { IInterpreterService } from '../../../client/interpreter/contracts'; @@ -30,11 +30,15 @@ const info: PythonEnvironment = { class InterpreterQuickPickItem implements IInterpreterQuickPickItem { public path: string; + public label: string; + public description!: string; + public detail?: string; - public interpreter = {} as any; + public interpreter = ({} as unknown) as PythonEnvironment; + constructor(l: string, p: string) { this.path = p; this.label = l; @@ -45,9 +49,7 @@ suite('Interpreters - selector', () => { let interpreterService: TypeMoq.IMock; let fileSystem: TypeMoq.IMock; let comparer: TypeMoq.IMock; - let experimentsManager: TypeMoq.IMock; - let interpreterSecurityService: TypeMoq.IMock; - const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; + let experimentsManager: TypeMoq.IMock; const ignoreCache = false; class TestInterpreterSelector extends InterpreterSelector { public async suggestionToQuickPickItem( @@ -61,12 +63,8 @@ suite('Interpreters - selector', () => { let selector: TestInterpreterSelector; setup(() => { - experimentsManager = TypeMoq.Mock.ofType(); - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); - interpreterSecurityService = TypeMoq.Mock.ofType(); + experimentsManager = TypeMoq.Mock.ofType(); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); comparer = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); fileSystem = TypeMoq.Mock.ofType(); @@ -75,13 +73,7 @@ suite('Interpreters - selector', () => { .returns((a: string, b: string) => a === b); comparer.setup((c) => c.compare(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => 0); - selector = new TestInterpreterSelector( - interpreterService.object, - comparer.object, - experimentsManager.object, - interpreterSecurityService.object, - new PathUtils(false), - ); + selector = new TestInterpreterSelector(interpreterService.object, comparer.object, new PathUtils(false)); }); [true, false].forEach((isWindows) => { @@ -89,8 +81,6 @@ suite('Interpreters - selector', () => { selector = new TestInterpreterSelector( interpreterService.object, comparer.object, - experimentsManager.object, - interpreterSecurityService.object, new PathUtils(isWindows), ); @@ -134,26 +124,4 @@ suite('Interpreters - selector', () => { } }); }); - - test('When in Deprecate PythonPath experiment, remove unsafe interpreters from the suggested interpreters list', async () => { - const interpreterList = ['interpreter1', 'interpreter2', 'interpreter3'] as any; - interpreterService - .setup((i) => i.getInterpreters(folder1.uri, { onSuggestion: true, ignoreCache })) - .returns(() => interpreterList); - - interpreterSecurityService.setup((i) => i.isSafe('interpreter1' as any)).returns(() => true); - - interpreterSecurityService.setup((i) => i.isSafe('interpreter2' as any)).returns(() => false); - - interpreterSecurityService.setup((i) => i.isSafe('interpreter3' as any)).returns(() => undefined); - experimentsManager.reset(); - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); - - selector.suggestionToQuickPickItem = (item, _) => Promise.resolve(item as any); - const suggestion = await selector.getSuggestions(folder1.uri, ignoreCache); - assert.deepEqual(suggestion, ['interpreter1', 'interpreter3']); - }); }); diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts index 69a4869a10fa..ba8461971806 100644 --- a/src/test/interpreters/activation/service.unit.test.ts +++ b/src/test/interpreters/activation/service.unit.test.ts @@ -140,12 +140,11 @@ suite('Interpreters Activation - Python Environment Variables', () => { const shellCmd = capture(processService.shellExec).first()[0]; - const isolated = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'); const printEnvPyFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printEnvVariables.py'); const expectedCommand = [ ...cmd, `echo '${getEnvironmentPrefix}'`, - `python ${isolated.toCommandArgument()} ${printEnvPyFile.fileToCommandArgument()}`, + `python ${printEnvPyFile.fileToCommandArgument()}`, ].join(' && '); expect(shellCmd).to.equal(expectedCommand); diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index 7debe6b74ef2..4c05d91afd5e 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -5,18 +5,17 @@ import { expect } from 'chai'; import { SemVer } from 'semver'; -import { anything, instance, mock, reset, verify, when } from 'ts-mockito'; +import { anything, instance, mock, 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 { ExperimentService } from '../../../client/common/experiments/service'; import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IExperimentsManager, IPersistentStateFactory, Resource } from '../../../client/common/types'; +import { IExperimentService, 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'; import { InterpreterAutoSelectionProxyService } from '../../../client/interpreter/autoSelection/proxy'; import { CachedInterpretersAutoSelectionRule } from '../../../client/interpreter/autoSelection/rules/cached'; import { CurrentPathInterpretersAutoSelectionRule } from '../../../client/interpreter/autoSelection/rules/currentPath'; @@ -27,7 +26,6 @@ import { WorkspaceVirtualEnvInterpretersAutoSelectionRule } from '../../../clien import { IInterpreterAutoSelectionRule, IInterpreterAutoSelectionProxyService, - IInterpreterSecurityService, } from '../../../client/interpreter/autoSelection/types'; import { IInterpreterHelper } from '../../../client/interpreter/contracts'; import { InterpreterHelper } from '../../../client/interpreter/helpers'; @@ -51,8 +49,7 @@ suite('Interpreters - Auto Selection', () => { let state: PersistentState; let helper: IInterpreterHelper; let proxy: IInterpreterAutoSelectionProxyService; - let interpreterSecurityService: IInterpreterSecurityService; - let experiments: IExperimentsManager; + let experiments: IExperimentService; class InterpreterAutoSelectionServiceTest extends InterpreterAutoSelectionService { public initializeStore(resource: Resource): Promise { return super.initializeStore(resource); @@ -67,7 +64,6 @@ suite('Interpreters - Auto Selection', () => { } } setup(() => { - interpreterSecurityService = mock(InterpreterSecurityService); workspaceService = mock(WorkspaceService); stateFactory = mock(PersistentStateFactory); state = mock(PersistentState) as PersistentState; @@ -80,8 +76,8 @@ suite('Interpreters - Auto Selection', () => { workspaceInterpreter = mock(WorkspaceVirtualEnvInterpretersAutoSelectionRule); helper = mock(InterpreterHelper); proxy = mock(InterpreterAutoSelectionProxyService); - experiments = mock(ExperimentsManager); - when(experiments.inExperiment(anything())).thenReturn(false); + experiments = mock(ExperimentService); + when(experiments.inExperimentSync(anything())).thenReturn(false); autoSelectionService = new InterpreterAutoSelectionServiceTest( instance(workspaceService), @@ -95,8 +91,6 @@ suite('Interpreters - Auto Selection', () => { instance(workspaceInterpreter), instance(proxy), instance(helper), - instance(experiments), - instance(interpreterSecurityService), ); }); @@ -239,15 +233,6 @@ suite('Interpreters - Auto Selection', () => { expect(selectedInterpreter).to.deep.equal(interpreterInfo); expect(eventFired).to.deep.equal(false, 'event fired'); }); - 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); - when(interpreterSecurityService.isSafe(interpreterInfo)).thenReturn(false); - const selectedInterpreter = autoSelectionService.getAutoSelectedInterpreter(undefined); - expect(selectedInterpreter).to.equal(undefined, 'Should be undefined'); - }); test('Do not store global interpreter info in state store when resource is undefined and version is lower than one already in state', async () => { let eventFired = false; const pythonPath = 'Hello World'; diff --git a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterEvaluation.unit.test.ts b/src/test/interpreters/autoSelection/interpreterSecurity/interpreterEvaluation.unit.test.ts deleted file mode 100644 index 3598d22af55a..000000000000 --- a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterEvaluation.unit.test.ts +++ /dev/null @@ -1,247 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { expect } from 'chai'; -import * as Typemoq from 'typemoq'; -import { Uri } from 'vscode'; -import { IApplicationShell } from '../../../../client/common/application/types'; -import { IBrowserService, IPersistentState } from '../../../../client/common/types'; -import { Common, Interpreters } from '../../../../client/common/utils/localize'; -import { learnMoreOnInterpreterSecurityURI } from '../../../../client/interpreter/autoSelection/constants'; -import { InterpreterEvaluation } from '../../../../client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation'; -import { IInterpreterSecurityStorage } from '../../../../client/interpreter/autoSelection/types'; -import { IInterpreterHelper } from '../../../../client/interpreter/contracts'; - -const prompts = [Common.bannerLabelYes(), Common.bannerLabelNo(), Common.learnMore(), Common.doNotShowAgain()]; - -suite('Interpreter Evaluation', () => { - const resource = Uri.parse('a'); - let applicationShell: Typemoq.IMock; - let browserService: Typemoq.IMock; - let interpreterHelper: Typemoq.IMock; - let interpreterSecurityStorage: Typemoq.IMock; - let unsafeInterpreterPromptEnabled: Typemoq.IMock>; - let areInterpretersInWorkspaceSafe: Typemoq.IMock>; - let interpreterEvaluation: InterpreterEvaluation; - setup(() => { - applicationShell = Typemoq.Mock.ofType(); - browserService = Typemoq.Mock.ofType(); - interpreterHelper = Typemoq.Mock.ofType(); - interpreterSecurityStorage = Typemoq.Mock.ofType(); - unsafeInterpreterPromptEnabled = Typemoq.Mock.ofType>(); - areInterpretersInWorkspaceSafe = Typemoq.Mock.ofType>(); - interpreterSecurityStorage - .setup((i) => i.hasUserApprovedWorkspaceInterpreters(resource)) - .returns(() => areInterpretersInWorkspaceSafe.object); - interpreterSecurityStorage - .setup((i) => i.unsafeInterpreterPromptEnabled) - .returns(() => unsafeInterpreterPromptEnabled.object); - interpreterSecurityStorage.setup((i) => i.storeKeyForWorkspace(resource)).returns(() => Promise.resolve()); - interpreterEvaluation = new InterpreterEvaluation( - applicationShell.object, - browserService.object, - interpreterHelper.object, - interpreterSecurityStorage.object, - ); - }); - - suite('Method evaluateIfInterpreterIsSafe()', () => { - test('If no workspaces are opened, return true', async () => { - const interpreter = { path: 'interpreterPath' } as any; - interpreterHelper.setup((i) => i.getActiveWorkspaceUri(resource)).returns(() => undefined); - const isSafe = await interpreterEvaluation.evaluateIfInterpreterIsSafe(interpreter, resource); - expect(isSafe).to.equal(true, 'Should be true'); - }); - - test('If method inferValueFromStorage() returns a defined value, return the value', async () => { - const interpreter = { path: 'interpreterPath' } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - - interpreterEvaluation.inferValueUsingCurrentState = () => 'storageValue' as any; - const isSafe = await interpreterEvaluation.evaluateIfInterpreterIsSafe(interpreter, resource); - expect(isSafe).to.equal('storageValue'); - }); - - test('If method inferValueFromStorage() returns a undefined value, infer the value using the prompt and return it', async () => { - const interpreter = { path: 'interpreterPath' } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - interpreterEvaluation.inferValueUsingCurrentState = () => undefined; - - interpreterEvaluation._inferValueUsingPrompt = () => 'promptValue' as any; - const isSafe = await interpreterEvaluation.evaluateIfInterpreterIsSafe(interpreter, resource); - expect(isSafe).to.equal('promptValue'); - }); - }); - - suite('Method inferValueUsingStorage()', () => { - test('If no workspaces are opened, return true', async () => { - const interpreter = { path: 'interpreterPath' } as any; - interpreterHelper.setup((i) => i.getActiveWorkspaceUri(resource)).returns(() => undefined); - const isSafe = interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - expect(isSafe).to.equal(true, 'Should be true'); - }); - - test('If interpreter is stored outside the workspace, return true', async () => { - const interpreter = { path: 'interpreterPath' } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - const isSafe = interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - expect(isSafe).to.equal(true, 'Should be true'); - }); - - test('If interpreter is stored in the workspace but method _areInterpretersInWorkspaceSafe() returns a defined value, return the value', async () => { - const interpreter = { path: `${resource.fsPath}/interpreterPath` } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - areInterpretersInWorkspaceSafe - .setup((i) => i.value) - - .returns(() => 'areInterpretersInWorkspaceSafeValue' as any); - const isSafe = interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - expect(isSafe).to.equal('areInterpretersInWorkspaceSafeValue'); - }); - - test('If prompt has been disabled, return true', async () => { - const interpreter = { path: `${resource.fsPath}/interpreterPath` } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - areInterpretersInWorkspaceSafe.setup((i) => i.value).returns(() => undefined); - unsafeInterpreterPromptEnabled.setup((s) => s.value).returns(() => false); - const isSafe = interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - expect(isSafe).to.equal(true, 'Should be true'); - }); - - test('Otherwise return `undefined`', async () => { - const interpreter = { path: `${resource.fsPath}/interpreterPath` } as any; - interpreterHelper - .setup((i) => i.getActiveWorkspaceUri(resource)) - .returns( - () => - ({ - folderUri: resource, - } as any), - ); - areInterpretersInWorkspaceSafe.setup((i) => i.value).returns(() => undefined); - unsafeInterpreterPromptEnabled.setup((s) => s.value).returns(() => true); - const isSafe = interpreterEvaluation.inferValueUsingCurrentState(interpreter, resource); - expect(isSafe).to.equal(undefined, 'Should be undefined'); - }); - }); - - suite('Method _inferValueUsingPrompt()', () => { - test('Active workspace key is stored in security storage', async () => { - interpreterSecurityStorage - .setup((i) => i.storeKeyForWorkspace(resource)) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - await interpreterEvaluation._inferValueUsingPrompt(resource); - interpreterSecurityStorage.verifyAll(); - }); - test('If `Learn more` is selected, launch URL & keep showing the prompt again until user clicks some other option', async () => { - let promptDisplayCount = 0; - // Select `Learn more` 2 times, then select something else the 3rd time. - const showInformationMessage = () => { - promptDisplayCount += 1; - return Promise.resolve(promptDisplayCount < 3 ? Common.learnMore() : 'Some other option'); - }; - applicationShell - .setup((a) => a.showInformationMessage(Interpreters.unsafeInterpreterMessage(), ...prompts)) - .returns(showInformationMessage) - .verifiable(Typemoq.Times.exactly(3)); - browserService - .setup((b) => b.launch(learnMoreOnInterpreterSecurityURI)) - .returns(() => undefined) - .verifiable(Typemoq.Times.exactly(2)); - - await interpreterEvaluation._inferValueUsingPrompt(resource); - - applicationShell.verifyAll(); - browserService.verifyAll(); - }); - - test('If `No` is selected, update the areInterpretersInWorkspaceSafe storage to unsafe and return false', async () => { - applicationShell - .setup((a) => a.showInformationMessage(Interpreters.unsafeInterpreterMessage(), ...prompts)) - .returns(() => Promise.resolve(Common.bannerLabelNo())) - .verifiable(Typemoq.Times.once()); - areInterpretersInWorkspaceSafe - .setup((i) => i.updateValue(false)) - .returns(() => Promise.resolve(undefined)) - .verifiable(Typemoq.Times.once()); - - const result = await interpreterEvaluation._inferValueUsingPrompt(resource); - expect(result).to.equal(false, 'Should be false'); - - applicationShell.verifyAll(); - areInterpretersInWorkspaceSafe.verifyAll(); - }); - - test('If `Yes` is selected, update the areInterpretersInWorkspaceSafe storage to safe and return true', async () => { - applicationShell - .setup((a) => a.showInformationMessage(Interpreters.unsafeInterpreterMessage(), ...prompts)) - .returns(() => Promise.resolve(Common.bannerLabelYes())) - .verifiable(Typemoq.Times.once()); - areInterpretersInWorkspaceSafe - .setup((i) => i.updateValue(true)) - .returns(() => Promise.resolve(undefined)) - .verifiable(Typemoq.Times.once()); - - const result = await interpreterEvaluation._inferValueUsingPrompt(resource); - expect(result).to.equal(true, 'Should be true'); - - applicationShell.verifyAll(); - areInterpretersInWorkspaceSafe.verifyAll(); - }); - - test('If no selection is made, update the areInterpretersInWorkspaceSafe storage to unsafe and return false', async () => { - applicationShell - .setup((a) => a.showInformationMessage(Interpreters.unsafeInterpreterMessage(), ...prompts)) - .returns(() => Promise.resolve(undefined)) - .verifiable(Typemoq.Times.once()); - areInterpretersInWorkspaceSafe - .setup((i) => i.updateValue(false)) - .returns(() => Promise.resolve(undefined)) - .verifiable(Typemoq.Times.once()); - - const result = await interpreterEvaluation._inferValueUsingPrompt(resource); - expect(result).to.equal(false, 'Should be false'); - - applicationShell.verifyAll(); - areInterpretersInWorkspaceSafe.verifyAll(); - }); - }); -}); diff --git a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityService.unit.test.ts b/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityService.unit.test.ts deleted file mode 100644 index 9358228875d3..000000000000 --- a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityService.unit.test.ts +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { expect } from 'chai'; -import * as Typemoq from 'typemoq'; -import { EventEmitter, Uri } from 'vscode'; -import { IPersistentState } from '../../../../client/common/types'; -import { createDeferred, sleep } from '../../../../client/common/utils/async'; -import { InterpreterSecurityService } from '../../../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService'; -import { - IInterpreterEvaluation, - IInterpreterSecurityStorage, -} from '../../../../client/interpreter/autoSelection/types'; - -suite('Interpreter Security service', () => { - const safeInterpretersList = ['safe1', 'safe2']; - const unsafeInterpretersList = ['unsafe1', 'unsafe2']; - const resource = Uri.parse('a'); - let interpreterSecurityStorage: Typemoq.IMock; - let interpreterEvaluation: Typemoq.IMock; - let unsafeInterpreters: Typemoq.IMock>; - let safeInterpreters: Typemoq.IMock>; - let interpreterSecurityService: InterpreterSecurityService; - setup(() => { - interpreterEvaluation = Typemoq.Mock.ofType(); - unsafeInterpreters = Typemoq.Mock.ofType>(); - safeInterpreters = Typemoq.Mock.ofType>(); - interpreterSecurityStorage = Typemoq.Mock.ofType(); - safeInterpreters.setup((s) => s.value).returns(() => safeInterpretersList); - unsafeInterpreters.setup((s) => s.value).returns(() => unsafeInterpretersList); - interpreterSecurityStorage.setup((p) => p.unsafeInterpreters).returns(() => unsafeInterpreters.object); - interpreterSecurityStorage.setup((p) => p.safeInterpreters).returns(() => safeInterpreters.object); - interpreterSecurityService = new InterpreterSecurityService( - interpreterSecurityStorage.object, - interpreterEvaluation.object, - ); - }); - - suite('Method isSafe()', () => { - test('Returns `true` if interpreter is in the safe interpreters list', () => { - let isSafe = interpreterSecurityService.isSafe({ path: 'safe1' } as any); - expect(isSafe).to.equal(true, ''); - - isSafe = interpreterSecurityService.isSafe({ path: 'safe2' } as any); - expect(isSafe).to.equal(true, ''); - }); - - test('Returns `false` if interpreter is in the unsafe intepreters list', () => { - let isSafe = interpreterSecurityService.isSafe({ path: 'unsafe1' } as any); - expect(isSafe).to.equal(false, ''); - - isSafe = interpreterSecurityService.isSafe({ path: 'unsafe2' } as any); - expect(isSafe).to.equal(false, ''); - }); - - test('Returns `undefined` if interpreter is not in either of these lists', () => { - const interpreter = { path: 'random' } as any; - interpreterEvaluation - .setup((i) => i.inferValueUsingCurrentState(interpreter, resource)) - - .returns(() => 'value' as any) - .verifiable(Typemoq.Times.once()); - const isSafe = interpreterSecurityService.isSafe(interpreter, resource); - expect(isSafe).to.equal('value', ''); - interpreterEvaluation.verifyAll(); - }); - }); - - suite('Method evaluateInterpreterSafety()', () => { - test("If interpreter to be evaluated already exists in the safe intepreters list, simply return and don't evaluate", async () => { - const interpreter = { path: 'safe2' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .verifiable(Typemoq.Times.never()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - }); - - test("If interpreter to be evaluated already exists in the unsafe intepreters list, simply return and don't evaluate", async () => { - const interpreter = { path: 'unsafe1' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .verifiable(Typemoq.Times.never()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - }); - - test('If interpreter to be evaluated does not exists in the either of the intepreters list, evaluate the interpreters', async () => { - const interpreter = { path: 'notInEitherLists' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - }); - - test('If interpreter is evaluated to be safe, add it in the safe interpreters list', async () => { - const interpreter = { path: 'notInEitherLists' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .returns(() => Promise.resolve(true)) - .verifiable(Typemoq.Times.once()); - safeInterpreters - .setup((s) => s.updateValue(['notInEitherLists', ...safeInterpretersList])) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - safeInterpreters.verifyAll(); - }); - - test('If interpreter is evaluated to be unsafe, add it in the unsafe interpreters list', async () => { - const interpreter = { path: 'notInEitherLists' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .returns(() => Promise.resolve(false)) - .verifiable(Typemoq.Times.once()); - unsafeInterpreters - .setup((s) => s.updateValue(['notInEitherLists', ...unsafeInterpretersList])) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - unsafeInterpreters.verifyAll(); - }); - - test('Ensure an event is fired at the end of the method execution', async () => { - const _didSafeInterpretersChange = Typemoq.Mock.ofType>(); - const interpreter = { path: 'notInEitherLists' }; - interpreterEvaluation - .setup((i) => i.evaluateIfInterpreterIsSafe(Typemoq.It.isAny(), Typemoq.It.isAny())) - .returns(() => Promise.resolve(false)); - unsafeInterpreters - .setup((s) => s.updateValue(['notInEitherLists', ...unsafeInterpretersList])) - .returns(() => Promise.resolve()); - interpreterSecurityService._didSafeInterpretersChange = _didSafeInterpretersChange.object; - _didSafeInterpretersChange - .setup((d) => d.fire()) - .returns(() => undefined) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter as any, resource); - interpreterEvaluation.verifyAll(); - unsafeInterpreters.verifyAll(); - _didSafeInterpretersChange.verifyAll(); - }); - }); - - test('Ensure onDidChangeSafeInterpreters() method captures the fired event', async () => { - const deferred = createDeferred(); - interpreterSecurityService.onDidChangeSafeInterpreters(() => { - deferred.resolve(true); - }); - interpreterSecurityService._didSafeInterpretersChange.fire(); - const eventCaptured = await Promise.race([deferred.promise, sleep(1000).then(() => false)]); - expect(eventCaptured).to.equal(true, 'Event should be captured'); - }); -}); diff --git a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityStorage.unit.test.ts b/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityStorage.unit.test.ts deleted file mode 100644 index 91525e66ca88..000000000000 --- a/src/test/interpreters/autoSelection/interpreterSecurity/interpreterSecurityStorage.unit.test.ts +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { assert, expect } from 'chai'; -import * as Typemoq from 'typemoq'; -import { Uri } from 'vscode'; -import { ICommandManager, IWorkspaceService } from '../../../../client/common/application/types'; -import { Commands } from '../../../../client/common/constants'; -import { IDisposable, IPersistentState, IPersistentStateFactory } from '../../../../client/common/types'; -import { - flaggedWorkspacesKeysStorageKey, - safeInterpretersKey, - unsafeInterpreterPromptKey, - unsafeInterpretersKey, -} from '../../../../client/interpreter/autoSelection/constants'; -import { InterpreterSecurityStorage } from '../../../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage'; - -suite('Interpreter Security Storage', () => { - const resource = Uri.parse('a'); - let persistentStateFactory: Typemoq.IMock; - let interpreterSecurityStorage: InterpreterSecurityStorage; - let unsafeInterpreters: Typemoq.IMock>; - let safeInterpreters: Typemoq.IMock>; - let flaggedWorkspacesKeysStorage: Typemoq.IMock>; - let commandManager: Typemoq.IMock; - let workspaceService: Typemoq.IMock; - let areInterpretersInWorkspaceSafe: Typemoq.IMock>; - let unsafeInterpreterPromptEnabled: Typemoq.IMock>; - setup(() => { - persistentStateFactory = Typemoq.Mock.ofType(); - unsafeInterpreters = Typemoq.Mock.ofType>(); - safeInterpreters = Typemoq.Mock.ofType>(); - flaggedWorkspacesKeysStorage = Typemoq.Mock.ofType>(); - unsafeInterpreterPromptEnabled = Typemoq.Mock.ofType>(); - commandManager = Typemoq.Mock.ofType(); - workspaceService = Typemoq.Mock.ofType(); - areInterpretersInWorkspaceSafe = Typemoq.Mock.ofType>(); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(unsafeInterpretersKey, [])) - .returns(() => unsafeInterpreters.object); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(safeInterpretersKey, [])) - .returns(() => safeInterpreters.object); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(unsafeInterpreterPromptKey, true)) - .returns(() => unsafeInterpreterPromptEnabled.object); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(flaggedWorkspacesKeysStorageKey, [])) - .returns(() => flaggedWorkspacesKeysStorage.object); - interpreterSecurityStorage = new InterpreterSecurityStorage( - persistentStateFactory.object, - workspaceService.object, - commandManager.object, - [], - ); - }); - - test('Command is registered in the activate() method', async () => { - commandManager - .setup((c) => c.registerCommand(Commands.ResetInterpreterSecurityStorage, Typemoq.It.isAny())) - .returns(() => Typemoq.Mock.ofType().object) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityStorage.activate(); - - commandManager.verifyAll(); - }); - - test('Flagged workspace keys are stored correctly', async () => { - flaggedWorkspacesKeysStorage - .setup((f) => f.value) - .returns(() => ['workspace1Key']) - .verifiable(Typemoq.Times.once()); - const workspace2 = Uri.parse('2'); - workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(workspace2)).returns(() => workspace2.fsPath); - - const workspace2Key = interpreterSecurityStorage._getKeyForWorkspace(workspace2); - - flaggedWorkspacesKeysStorage - .setup((f) => f.updateValue(['workspace1Key', workspace2Key])) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityStorage.storeKeyForWorkspace(workspace2); - - expect(workspace2Key).to.equal(`ARE_INTERPRETERS_SAFE_FOR_WS_${workspace2.fsPath}`); - }); - - test('All kinds of storages are cleared upon invoking the command', async () => { - const areInterpretersInWorkspace1Safe = Typemoq.Mock.ofType>(); - const areInterpretersInWorkspace2Safe = Typemoq.Mock.ofType>(); - - flaggedWorkspacesKeysStorage.setup((f) => f.value).returns(() => ['workspace1Key', 'workspace2Key']); - safeInterpreters - .setup((s) => s.updateValue([])) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - unsafeInterpreters - .setup((s) => s.updateValue([])) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - unsafeInterpreterPromptEnabled - .setup((s) => s.updateValue(true)) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState('workspace1Key', undefined)) - .returns(() => areInterpretersInWorkspace1Safe.object); - areInterpretersInWorkspace1Safe - .setup((s) => s.updateValue(undefined)) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState('workspace2Key', undefined)) - .returns(() => areInterpretersInWorkspace2Safe.object); - areInterpretersInWorkspace2Safe - .setup((s) => s.updateValue(undefined)) - .returns(() => Promise.resolve()) - .verifiable(Typemoq.Times.once()); - - await interpreterSecurityStorage.resetInterpreterSecurityStorage(); - - areInterpretersInWorkspace1Safe.verifyAll(); - areInterpretersInWorkspace2Safe.verifyAll(); - safeInterpreters.verifyAll(); - unsafeInterpreterPromptEnabled.verifyAll(); - unsafeInterpreters.verifyAll(); - }); - - test('Method areInterpretersInWorkspaceSafe() returns the areInterpretersInWorkspaceSafe storage', () => { - workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); - persistentStateFactory - .setup((p) => - p.createGlobalPersistentState( - `ARE_INTERPRETERS_SAFE_FOR_WS_${resource.fsPath}`, - undefined, - ), - ) - .returns(() => areInterpretersInWorkspaceSafe.object); - const result = interpreterSecurityStorage.hasUserApprovedWorkspaceInterpreters(resource); - assert(areInterpretersInWorkspaceSafe.object === result); - }); - - test('Get unsafeInterpreterPromptEnabled() returns the unsafeInterpreterPromptEnabled storage', () => { - const result = interpreterSecurityStorage.unsafeInterpreterPromptEnabled; - assert(unsafeInterpreterPromptEnabled.object === result); - }); - - test('Get unsafeInterpreters() returns the unsafeInterpreters storage', () => { - const result = interpreterSecurityStorage.unsafeInterpreters; - assert(unsafeInterpreters.object === result); - }); - - test('Get safeInterpreters() returns the safeInterpreters storage', () => { - const result = interpreterSecurityStorage.safeInterpreters; - assert(safeInterpreters.object === result); - }); -}); diff --git a/src/test/interpreters/autoSelection/rules/settings.unit.test.ts b/src/test/interpreters/autoSelection/rules/settings.unit.test.ts index 7fd9ff00081b..2d32383df38c 100644 --- a/src/test/interpreters/autoSelection/rules/settings.unit.test.ts +++ b/src/test/interpreters/autoSelection/rules/settings.unit.test.ts @@ -8,13 +8,13 @@ import { anything, instance, mock, when } from 'ts-mockito'; import { IWorkspaceService } from '../../../../client/common/application/types'; import { WorkspaceService } from '../../../../client/common/application/workspace'; import { DeprecatePythonPath } from '../../../../client/common/experiments/groups'; -import { ExperimentsManager } from '../../../../client/common/experiments/manager'; +import { ExperimentService } from '../../../../client/common/experiments/service'; import { InterpreterPathService } from '../../../../client/common/interpreterPathService'; import { PersistentState, PersistentStateFactory } from '../../../../client/common/persistentState'; import { FileSystem } from '../../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../../client/common/platform/types'; import { - IExperimentsManager, + IExperimentService, IInterpreterPathService, IPersistentStateFactory, Resource, @@ -31,7 +31,7 @@ suite('Interpreters - Auto Selection - Settings Rule', () => { let fs: IFileSystem; let state: PersistentState; let workspaceService: IWorkspaceService; - let experimentsManager: IExperimentsManager; + let experimentsManager: IExperimentService; let interpreterPathService: IInterpreterPathService; class SettingsInterpretersAutoSelectionRuleTest extends SettingsInterpretersAutoSelectionRule { public async onAutoSelectInterpreter( @@ -46,9 +46,8 @@ suite('Interpreters - Auto Selection - Settings Rule', () => { state = mock(PersistentState) as PersistentState; fs = mock(FileSystem); workspaceService = mock(WorkspaceService); - experimentsManager = mock(ExperimentsManager); + experimentsManager = mock(ExperimentService); interpreterPathService = mock(InterpreterPathService); - when(experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control)).thenReturn(undefined); when(stateFactory.createGlobalPersistentState(anything(), undefined)).thenReturn( instance(state), @@ -66,7 +65,7 @@ suite('Interpreters - Auto Selection - Settings Rule', () => { const manager = mock(InterpreterAutoSelectionService); const pythonPathInConfig = {}; - when(experimentsManager.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); + when(experimentsManager.inExperimentSync(DeprecatePythonPath.experiment)).thenReturn(true); when(interpreterPathService.inspect(undefined)).thenReturn(pythonPathInConfig as any); const nextAction = await rule.onAutoSelectInterpreter(undefined, manager); @@ -77,7 +76,7 @@ suite('Interpreters - Auto Selection - Settings Rule', () => { const manager = mock(InterpreterAutoSelectionService); const pythonPathInConfig = { globalValue: 'python' }; - when(experimentsManager.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); + when(experimentsManager.inExperimentSync(DeprecatePythonPath.experiment)).thenReturn(true); when(interpreterPathService.inspect(undefined)).thenReturn(pythonPathInConfig as any); const nextAction = await rule.onAutoSelectInterpreter(undefined, manager); @@ -88,7 +87,7 @@ suite('Interpreters - Auto Selection - Settings Rule', () => { const manager = mock(InterpreterAutoSelectionService); const pythonPathInConfig = { globalValue: 'something else' }; - when(experimentsManager.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); + when(experimentsManager.inExperimentSync(DeprecatePythonPath.experiment)).thenReturn(true); when(interpreterPathService.inspect(undefined)).thenReturn(pythonPathInConfig as any); const nextAction = await rule.onAutoSelectInterpreter(undefined, manager); diff --git a/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts b/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts index 427dff6a8670..2f7324d6dfe8 100644 --- a/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts +++ b/src/test/interpreters/autoSelection/rules/workspaceEnv.unit.test.ts @@ -13,7 +13,6 @@ import { Uri, WorkspaceFolder } from 'vscode'; import { IWorkspaceService } from '../../../../client/common/application/types'; import { WorkspaceService } from '../../../../client/common/application/workspace'; import { DeprecatePythonPath, DiscoveryVariants } from '../../../../client/common/experiments/groups'; -import { ExperimentsManager } from '../../../../client/common/experiments/manager'; import { ExperimentService } from '../../../../client/common/experiments/service'; import { InterpreterPathService } from '../../../../client/common/interpreterPathService'; import { PersistentState, PersistentStateFactory } from '../../../../client/common/persistentState'; @@ -22,7 +21,6 @@ import { PlatformService } from '../../../../client/common/platform/platformServ import { IFileSystem, IPlatformService } from '../../../../client/common/platform/types'; import { IExperimentService, - IExperimentsManager, IInterpreterPathService, IPersistentStateFactory, Resource, @@ -58,7 +56,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { let workspaceService: IWorkspaceService; let experimentService: IExperimentService; let componentAdapter: IComponentAdapter; - let experimentsManager: IExperimentsManager; let interpreterPathService: IInterpreterPathService; class WorkspaceVirtualEnvInterpretersAutoSelectionRuleTest extends WorkspaceVirtualEnvInterpretersAutoSelectionRule { public async setGlobalInterpreter( @@ -94,7 +91,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { serviceContainer.get(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE), ).thenReturn(instance(virtualEnvLocator)); when(experimentService.inExperiment(DiscoveryVariants.discoverWithFileWatching)).thenResolve(false); - experimentsManager = mock(ExperimentsManager); interpreterPathService = mock(InterpreterPathService); componentAdapter = mock(); @@ -108,7 +104,6 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { instance(platform), instance(workspaceService), instance(serviceContainer), - instance(experimentsManager), instance(interpreterPathService), instance(componentAdapter), instance(experimentService), @@ -191,8 +186,7 @@ suite('Interpreters - Auto Selection - Workspace Virtual Envs Rule', () => { when(helper.getActiveWorkspaceUri(anything())).thenReturn({ folderUri } as any); when(nextRule.autoSelectInterpreter(someUri, manager)).thenResolve(); when(workspaceService.getConfiguration('python', folderUri)).thenReturn(pythonPath as any); - when(experimentsManager.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true); - when(experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control)).thenReturn(undefined); + when(experimentService.inExperimentSync(DeprecatePythonPath.experiment)).thenReturn(true); when(interpreterPathService.inspect(folderUri)).thenReturn(pythonPathInConfig.object); rule.setNextRule(instance(nextRule)); diff --git a/src/test/interpreters/currentPathService.unit.test.ts b/src/test/interpreters/currentPathService.unit.test.ts index 755a810e3980..6985f3cc36e4 100644 --- a/src/test/interpreters/currentPathService.unit.test.ts +++ b/src/test/interpreters/currentPathService.unit.test.ts @@ -4,7 +4,6 @@ 'use strict'; import { expect } from 'chai'; -import * as path from 'path'; import { SemVer } from 'semver'; import * as TypeMoq from 'typemoq'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; @@ -26,9 +25,6 @@ import { PythonInPathCommandProvider, } from '../../client/pythonEnvironments/discovery/locators/services/currentPathService'; import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironments/info'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants'; - -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); suite('Interpreters CurrentPath Service', () => { let processService: TypeMoq.IMock; @@ -100,7 +96,7 @@ suite('Interpreters CurrentPath Service', () => { .setup((v) => v.getInterpreterInformation(TypeMoq.It.isAny())) .returns(() => Promise.resolve({ version })); - const execArgs = [isolated, '-c', 'import sys;print(sys.executable)']; + const execArgs = ['-c', 'import sys;print(sys.executable)']; pythonSettings.setup((p) => p.pythonPath).returns(() => 'root:Python'); processService .setup((p) => diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts index 421e287e5e06..2ef5788f3f38 100644 --- a/src/test/interpreters/interpreterService.unit.test.ts +++ b/src/test/interpreters/interpreterService.unit.test.ts @@ -20,7 +20,6 @@ import { IConfigurationService, IDisposableRegistry, IExperimentService, - IExperimentsManager, IInterpreterPathService, InterpreterConfigurationScope, IPersistentState, @@ -73,7 +72,6 @@ suite('Interpreters service', () => { let pythonExecutionService: TypeMoq.IMock; let configService: TypeMoq.IMock; let interpreterPathService: TypeMoq.IMock; - let experimentsManager: TypeMoq.IMock; let experimentService: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; @@ -82,7 +80,6 @@ suite('Interpreters service', () => { serviceManager = new ServiceManager(cont); serviceContainer = new ServiceContainer(cont); - experimentsManager = TypeMoq.Mock.ofType(); experimentService = TypeMoq.Mock.ofType(); interpreterPathService = TypeMoq.Mock.ofType(); updater = TypeMoq.Mock.ofType(); @@ -131,7 +128,6 @@ suite('Interpreters service', () => { INTERPRETER_LOCATOR_SERVICE, ); serviceManager.addSingletonInstance(IFileSystem, fileSystem.object); - serviceManager.addSingletonInstance(IExperimentsManager, experimentsManager.object); serviceManager.addSingletonInstance(IExperimentService, experimentService.object); serviceManager.addSingletonInstance( IInterpreterPathService, @@ -198,10 +194,7 @@ suite('Interpreters service', () => { const service = new InterpreterService(serviceContainer, pyenvs.object, experimentService.object); const documentManager = TypeMoq.Mock.ofType(); - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentService.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); workspace.setup((w) => w.hasWorkspaceFolders).returns(() => true); workspace.setup((w) => w.workspaceFolders).returns(() => [{ uri: '' }] as any); let activeTextEditorChangeHandler: (e: TextEditor | undefined) => any | undefined; @@ -228,10 +221,7 @@ suite('Interpreters service', () => { const service = new InterpreterService(serviceContainer, pyenvs.object, experimentService.object); const documentManager = TypeMoq.Mock.ofType(); - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentService.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); workspace.setup((w) => w.hasWorkspaceFolders).returns(() => true); workspace.setup((w) => w.workspaceFolders).returns(() => [{ uri: '' }] as any); let activeTextEditorChangeHandler: (e?: TextEditor | undefined) => any | undefined; @@ -253,10 +243,7 @@ suite('Interpreters service', () => { const service = new InterpreterService(serviceContainer, pyenvs.object, experimentService.object); const documentManager = TypeMoq.Mock.ofType(); - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentService.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); workspace.setup((w) => w.hasWorkspaceFolders).returns(() => true); workspace.setup((w) => w.workspaceFolders).returns(() => [{ uri: '' }] as any); let interpreterPathServiceHandler: (e: InterpreterConfigurationScope) => any | undefined = () => { diff --git a/src/test/interpreters/interpreterVersion.unit.test.ts b/src/test/interpreters/interpreterVersion.unit.test.ts index b13963b367d3..39b56aced700 100644 --- a/src/test/interpreters/interpreterVersion.unit.test.ts +++ b/src/test/interpreters/interpreterVersion.unit.test.ts @@ -8,9 +8,6 @@ import '../../client/common/extensions'; import { IProcessService, IProcessServiceFactory } from '../../client/common/process/types'; import { IInterpreterVersionService } from '../../client/interpreter/contracts'; import { InterpreterVersionService } from '../../client/interpreter/interpreterVersion'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants'; - -const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); suite('InterpreterVersionService', () => { let processService: typeMoq.IMock; @@ -34,7 +31,7 @@ suite('InterpreterVersionService', () => { .setup((p) => p.exec( typeMoq.It.isValue(pythonPath), - typeMoq.It.isValue([isolated, '-c', 'import pip; print(pip.__version__)']), + typeMoq.It.isValue(['-c', 'import pip; print(pip.__version__)']), typeMoq.It.isAny(), ), ) @@ -51,7 +48,7 @@ suite('InterpreterVersionService', () => { .setup((p) => p.exec( typeMoq.It.isValue(pythonPath), - typeMoq.It.isValue([isolated, '-c', 'import pip; print(pip.__version__)']), + typeMoq.It.isValue(['-c', 'import pip; print(pip.__version__)']), typeMoq.It.isAny(), ), ) diff --git a/src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts b/src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts index d02c9fc0a92a..a7e782d1b7d5 100644 --- a/src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts +++ b/src/test/interpreters/pythonPathUpdaterFactory.unit.test.ts @@ -2,8 +2,7 @@ import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; -import { DeprecatePythonPath } from '../../client/common/experiments/groups'; -import { IExperimentsManager, IInterpreterPathService } from '../../client/common/types'; +import { IExperimentService, IInterpreterPathService } from '../../client/common/types'; import { PythonPathUpdaterServiceFactory } from '../../client/interpreter/configuration/pythonPathUpdaterServiceFactory'; import { IPythonPathUpdaterServiceFactory } from '../../client/interpreter/configuration/types'; import { IServiceContainer } from '../../client/ioc/types'; @@ -11,23 +10,20 @@ import { IServiceContainer } from '../../client/ioc/types'; suite('Python Path Settings Updater', () => { let serviceContainer: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; - let experimentsManager: TypeMoq.IMock; + let experimentsManager: TypeMoq.IMock; let interpreterPathService: TypeMoq.IMock; let updaterServiceFactory: IPythonPathUpdaterServiceFactory; function setupMocks(inExperiment: boolean = false) { serviceContainer = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); - experimentsManager = TypeMoq.Mock.ofType(); - experimentsManager.setup((e) => e.inExperiment(TypeMoq.It.isAny())).returns(() => inExperiment); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); + experimentsManager = TypeMoq.Mock.ofType(); + experimentsManager.setup((e) => e.inExperimentSync(TypeMoq.It.isAny())).returns(() => inExperiment); interpreterPathService = TypeMoq.Mock.ofType(); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) .returns(() => workspaceService.object); serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IExperimentsManager))) + .setup((c) => c.get(TypeMoq.It.isValue(IExperimentService))) .returns(() => experimentsManager.object); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterPathService))) diff --git a/src/test/interpreters/serviceRegistry.unit.test.ts b/src/test/interpreters/serviceRegistry.unit.test.ts index 8465e286767a..1ffa57f914a7 100644 --- a/src/test/interpreters/serviceRegistry.unit.test.ts +++ b/src/test/interpreters/serviceRegistry.unit.test.ts @@ -8,9 +8,6 @@ import { IExtensionActivationService, IExtensionSingleActivationService } from ' import { EnvironmentActivationService } from '../../client/interpreter/activation/service'; import { IEnvironmentActivationService } from '../../client/interpreter/activation/types'; import { InterpreterAutoSelectionService } from '../../client/interpreter/autoSelection'; -import { InterpreterEvaluation } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation'; -import { InterpreterSecurityService } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService'; -import { InterpreterSecurityStorage } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage'; import { InterpreterAutoSelectionProxyService } from '../../client/interpreter/autoSelection/proxy'; import { CachedInterpretersAutoSelectionRule } from '../../client/interpreter/autoSelection/rules/cached'; import { CurrentPathInterpretersAutoSelectionRule } from '../../client/interpreter/autoSelection/rules/currentPath'; @@ -23,9 +20,6 @@ import { IInterpreterAutoSelectionRule, IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService, - IInterpreterEvaluation, - IInterpreterSecurityService, - IInterpreterSecurityStorage, } from '../../client/interpreter/autoSelection/types'; import { InterpreterComparer } from '../../client/interpreter/configuration/interpreterComparer'; import { ResetInterpreterCommand } from '../../client/interpreter/configuration/interpreterSelector/commands/resetInterpreter'; @@ -67,10 +61,6 @@ suite('Interpreters - Service Registry', () => { [IExtensionSingleActivationService, SetInterpreterCommand], [IExtensionSingleActivationService, ResetInterpreterCommand], [IExtensionSingleActivationService, SetShebangInterpreterCommand], - [IExtensionSingleActivationService, InterpreterSecurityStorage], - [IInterpreterEvaluation, InterpreterEvaluation], - [IInterpreterSecurityStorage, InterpreterSecurityStorage], - [IInterpreterSecurityService, InterpreterSecurityService], [IExtensionActivationService, VirtualEnvironmentPrompt], diff --git a/src/test/providers/importSortProvider.unit.test.ts b/src/test/providers/importSortProvider.unit.test.ts index a675085a0ef2..9823354c691c 100644 --- a/src/test/providers/importSortProvider.unit.test.ts +++ b/src/test/providers/importSortProvider.unit.test.ts @@ -39,8 +39,6 @@ import { IServiceContainer } from '../../client/ioc/types'; import { SortImportsEditingProvider } from '../../client/providers/importSortProvider'; import { sleep } from '../core'; -const ISOLATED = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'); - suite('Import Sort Provider', () => { let serviceContainer: TypeMoq.IMock; let shell: TypeMoq.IMock; @@ -436,7 +434,7 @@ suite('Import Sort Provider', () => { dispose: noop, }; const importScript = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'sortImports.py'); - const expectedArgs = [ISOLATED, importScript, '-', '--diff', '1', '2']; + const expectedArgs = [importScript, '-', '--diff', '1', '2']; processExeService .setup((p) => p.execObservable( diff --git a/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts index 24a3dea5ceb9..2526ad27edc6 100644 --- a/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/environmentsResolver.unit.test.ts @@ -9,7 +9,6 @@ import { ImportMock } from 'ts-mock-imports'; import { EventEmitter } from 'vscode'; import { ExecutionResult } from '../../../../../client/common/process/types'; import { IDisposableRegistry } from '../../../../../client/common/types'; -import { createDeferred } from '../../../../../client/common/utils/async'; import { Architecture } from '../../../../../client/common/utils/platform'; import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info'; import { parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion'; @@ -78,7 +77,7 @@ suite('Python envs locator - Environments Resolver', () => { const env4 = createNamedEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); const environmentsToBeIterated = [env1, env2, env3, env4]; const parentLocator = new SimpleLocator(environmentsToBeIterated); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const iterator = resolver.iterEnvs(); const envs = await getEnvs(iterator); @@ -93,7 +92,7 @@ suite('Python envs locator - Environments Resolver', () => { const environmentsToBeIterated = [env1, env2]; const parentLocator = new SimpleLocator(environmentsToBeIterated); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const iterator = resolver.iterEnvs(); // Act @@ -134,7 +133,7 @@ suite('Python envs locator - Environments Resolver', () => { const env2 = createNamedEnv('env2', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); const environmentsToBeIterated = [env1, env2]; const parentLocator = new SimpleLocator(environmentsToBeIterated); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); // Act const iterator = resolver.iterEnvs(); @@ -152,7 +151,7 @@ suite('Python envs locator - Environments Resolver', () => { const didUpdate = new EventEmitter(); const parentLocator = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event }); const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const iterator = resolver.iterEnvs(); // Act @@ -184,47 +183,6 @@ suite('Python envs locator - Environments Resolver', () => { assert.equal(onUpdatedEvents[length - 1], null, 'Last update should be null'); didUpdate.dispose(); }); - - test('No updates events are sent for environment which are not safe to execute', async () => { - // Arrange - const env1 = createNamedEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec1')); - const env2 = createNamedEnv('env2', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2')); - const environmentsToBeIterated = [env1, env2]; - const parentLocator = new SimpleLocator(environmentsToBeIterated); - const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = []; - const resolver = new PythonEnvsResolver( - parentLocator, - envInfoService, - (env: PythonEnvInfo) => env.executable.filename === env1.executable.filename, - ); - - const iterator = resolver.iterEnvs(); // Act - - // Assert - let { onUpdated } = iterator; - expect(onUpdated).to.not.equal(undefined, ''); - - // Arrange - onUpdated = onUpdated!; - const ready = createDeferred(); - onUpdated((e) => { - onUpdatedEvents.push(e); - if (e === null) { - ready.resolve(); - } - }); - // Act - await getEnvs(iterator); - await ready.promise; // Resolve pending calls in the background - - // Assert - const expectedUpdates = [ - // Only update event for env1 is sent as env2 is unsafe. - { index: 0, old: env1, update: createExpectedEnvInfo(env1) }, - null, - ]; - assert.deepEqual(onUpdatedEvents, expectedUpdates); - }); }); test('onChanged fires iff onChanged from resolver fires', () => { @@ -232,7 +190,7 @@ suite('Python envs locator - Environments Resolver', () => { const event1: PythonEnvsChangedEvent = {}; const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; const expected = [event1, event2]; - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const events: PythonEnvsChangedEvent[] = []; resolver.onChanged((e) => events.push(e)); @@ -278,7 +236,7 @@ suite('Python envs locator - Environments Resolver', () => { throw new Error('Incorrect environment sent to the resolver'); }, }); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const expected = await resolver.resolveEnv(env); @@ -306,7 +264,7 @@ suite('Python envs locator - Environments Resolver', () => { throw new Error('Incorrect environment sent to the resolver'); }, }); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const expected = await resolver.resolveEnv(env); @@ -337,7 +295,7 @@ suite('Python envs locator - Environments Resolver', () => { throw new Error('Incorrect environment sent to the resolver'); }, }); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const expected = await resolver.resolveEnv(env); @@ -349,7 +307,7 @@ suite('Python envs locator - Environments Resolver', () => { const parentLocator = new SimpleLocator([], { resolve: async () => undefined, }); - const resolver = new PythonEnvsResolver(parentLocator, envInfoService, () => true); + const resolver = new PythonEnvsResolver(parentLocator, envInfoService); const expected = await resolver.resolveEnv(env); diff --git a/src/test/pythonEnvironments/info/executable.unit.test.ts b/src/test/pythonEnvironments/info/executable.unit.test.ts index 56e81fca8ad5..6f585cda6005 100644 --- a/src/test/pythonEnvironments/info/executable.unit.test.ts +++ b/src/test/pythonEnvironments/info/executable.unit.test.ts @@ -2,14 +2,10 @@ // Licensed under the MIT License. import { expect } from 'chai'; -import { join as pathJoin } from 'path'; import { IMock, Mock, MockBehavior } from 'typemoq'; import { StdErrError } from '../../../client/common/process/types'; import { buildPythonExecInfo } from '../../../client/pythonEnvironments/exec'; import { getExecutablePath } from '../../../client/pythonEnvironments/info/executable'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; - -const isolated = pathJoin(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); type ExecResult = { stdout: string; @@ -28,7 +24,7 @@ suite('getExecutablePath()', () => { test('should get the value by running python', async () => { const expected = 'path/to/dummy/executable'; - const argv = [isolated, '-c', 'import sys;print(sys.executable)']; + const argv = ['-c', 'import sys;print(sys.executable)']; deps.setup((d) => d.exec(python.command, argv)) // Return the expected value. .returns(() => Promise.resolve({ stdout: expected })); @@ -42,7 +38,7 @@ suite('getExecutablePath()', () => { test('should throw if exec() fails', async () => { const stderr = 'oops'; - const argv = [isolated, '-c', 'import sys;print(sys.executable)']; + const argv = ['-c', 'import sys;print(sys.executable)']; deps.setup((d) => d.exec(python.command, argv)) // Throw an error. .returns(() => Promise.reject(new StdErrError(stderr))); diff --git a/src/test/pythonEnvironments/info/interpreter.unit.test.ts b/src/test/pythonEnvironments/info/interpreter.unit.test.ts index 02c9e47615e8..a113ece62843 100644 --- a/src/test/pythonEnvironments/info/interpreter.unit.test.ts +++ b/src/test/pythonEnvironments/info/interpreter.unit.test.ts @@ -11,7 +11,6 @@ import { buildPythonExecInfo } from '../../../client/pythonEnvironments/exec'; import { getInterpreterInfo } from '../../../client/pythonEnvironments/info/interpreter'; import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; -const isolated = pathJoin(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py'); const script = pathJoin(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'interpreterInfo.py'); suite('extractInterpreterInfo()', () => { @@ -41,7 +40,7 @@ suite('getInterpreterInfo()', () => { version: '3.7.5rc1 (default, Oct 18 2019, 14:48:48) \n[Clang 11.0.0 (clang-1100.0.33.8)]', is64Bit: true, }; - const cmd = `"${python.command}" "${isolated}" "${script}"`; + const cmd = `"${python.command}" "${script}"`; deps // Checking the args is the key point of this test. .setup((d) => d.shellExec(cmd, 15000)) @@ -61,7 +60,7 @@ suite('getInterpreterInfo()', () => { is64Bit: true, }; const _python = buildPythonExecInfo(' path to /my python '); - const cmd = `" path to /my python " "${isolated}" "${script}"`; + const cmd = `" path to /my python " "${script}"`; deps // Checking the args is the key point of this test. .setup((d) => d.shellExec(cmd, 15000)) @@ -81,7 +80,7 @@ suite('getInterpreterInfo()', () => { is64Bit: true, }; const _python = buildPythonExecInfo(['path/to/conda', 'run', '-n', 'my-env', 'python']); - const cmd = `"path/to/conda" "run" "-n" "my-env" "python" "${isolated}" "${script}"`; + const cmd = `"path/to/conda" "run" "-n" "my-env" "python" "${script}"`; deps // Checking the args is the key point of this test. .setup((d) => d.shellExec(cmd, 15000)) diff --git a/src/test/pythonEnvironments/legacyIOC.ts b/src/test/pythonEnvironments/legacyIOC.ts index 7ba5f4c8167c..b76aca7bcf85 100644 --- a/src/test/pythonEnvironments/legacyIOC.ts +++ b/src/test/pythonEnvironments/legacyIOC.ts @@ -6,7 +6,6 @@ import { IServiceContainer, IServiceManager } from '../../client/ioc/types'; import { PythonEnvironments } from '../../client/pythonEnvironments/api'; import { initializeExternalDependencies } from '../../client/pythonEnvironments/common/externalDependencies'; import { registerLegacyDiscoveryForIOC, registerNewDiscoveryForIOC } from '../../client/pythonEnvironments/legacyIOC'; -import { EnvironmentsSecurity } from '../../client/pythonEnvironments/security'; /** * This is here to support old tests. @@ -18,10 +17,6 @@ export async function registerForIOC( ): Promise { initializeExternalDependencies(serviceContainer); // The old tests do not need real instances, directly pass in mocks. - registerNewDiscoveryForIOC( - serviceManager, - instance(mock(PythonEnvironments)), - instance(mock(EnvironmentsSecurity)), - ); + registerNewDiscoveryForIOC(serviceManager, instance(mock(PythonEnvironments))); await registerLegacyDiscoveryForIOC(serviceManager); } diff --git a/src/test/startPage/startPageIocContainer.ts b/src/test/startPage/startPageIocContainer.ts index 090a17021c57..220e11f38344 100644 --- a/src/test/startPage/startPageIocContainer.ts +++ b/src/test/startPage/startPageIocContainer.ts @@ -43,7 +43,6 @@ import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { PythonSettings } from '../../client/common/configSettings'; import { EXTENSION_ROOT_DIR } from '../../client/common/constants'; -import { ExperimentsManager } from '../../client/common/experiments/manager'; import { ExperimentService } from '../../client/common/experiments/service'; import { InstallationChannelManager } from '../../client/common/installer/channelManager'; import { IInstallationChannelManager } from '../../client/common/installer/types'; @@ -280,8 +279,8 @@ export class StartPageIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(IExtensionContext, mockExtensionContext.object); // Turn off experiments. - const experimentManager = mock(ExperimentsManager); - when(experimentManager.inExperiment(anything())).thenCall((exp) => { + const experimentManager = mock(ExperimentService); + when(experimentManager.inExperimentSync(anything())).thenCall((exp) => { const setState = this.experimentState.get(exp); if (setState === undefined) { // All experiments to true by default if not mocking jupyter diff --git a/src/test/startupTelemetry.unit.test.ts b/src/test/startupTelemetry.unit.test.ts index f5377d03ba3e..ef79039c75ba 100644 --- a/src/test/startupTelemetry.unit.test.ts +++ b/src/test/startupTelemetry.unit.test.ts @@ -8,25 +8,22 @@ import * as TypeMoq from 'typemoq'; import { Uri, WorkspaceConfiguration } from 'vscode'; import { IWorkspaceService } from '../client/common/application/types'; import { DeprecatePythonPath } from '../client/common/experiments/groups'; -import { IExperimentsManager, IInterpreterPathService } from '../client/common/types'; +import { IExperimentService, IInterpreterPathService } from '../client/common/types'; import { IServiceContainer } from '../client/ioc/types'; import { hasUserDefinedPythonPath } from '../client/startupTelemetry'; suite('Startup Telemetry - hasUserDefinedPythonPath()', async () => { const resource = Uri.parse('a'); let serviceContainer: TypeMoq.IMock; - let experimentsManager: TypeMoq.IMock; + let experimentsManager: TypeMoq.IMock; let interpreterPathService: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; setup(() => { serviceContainer = TypeMoq.Mock.ofType(); - experimentsManager = TypeMoq.Mock.ofType(); + experimentsManager = TypeMoq.Mock.ofType(); interpreterPathService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); - experimentsManager - .setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control)) - .returns(() => undefined); - serviceContainer.setup((s) => s.get(IExperimentsManager)).returns(() => experimentsManager.object); + serviceContainer.setup((s) => s.get(IExperimentService)).returns(() => experimentsManager.object); serviceContainer.setup((s) => s.get(IWorkspaceService)).returns(() => workspaceService.object); serviceContainer.setup((s) => s.get(IInterpreterPathService)).returns(() => interpreterPathService.object); }); @@ -44,7 +41,7 @@ suite('Startup Telemetry - hasUserDefinedPythonPath()', async () => { [undefined, 'python'].forEach((workspaceFolderValue) => { test(`Return false if using settings equals {globalValue: ${globalValue}, workspaceValue: ${workspaceValue}, workspaceFolderValue: ${workspaceFolderValue}}`, () => { experimentsManager - .setup((e) => e.inExperiment(DeprecatePythonPath.experiment)) + .setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)) .returns(() => false); const workspaceConfig = setupConfigProvider(); @@ -60,7 +57,7 @@ suite('Startup Telemetry - hasUserDefinedPythonPath()', async () => { }); test('Return true if using setting value equals something else', () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false); const workspaceConfig = setupConfigProvider(); workspaceConfig.setup((w) => w.inspect('pythonPath')).returns(() => ({ globalValue: 'something else' } as any)); @@ -69,7 +66,7 @@ suite('Startup Telemetry - hasUserDefinedPythonPath()', async () => { }); test('If in Deprecate PythonPath experiment, use the new API to inspect settings', () => { - experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true); + experimentsManager.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true); interpreterPathService .setup((i) => i.inspect(resource)) .returns(() => ({})) diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index e94b1e2feb40..5d77f4f291c3 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -200,13 +200,8 @@ suite('Unit Tests - Debug Launcher', () => { expected = getDefaultDebugConfig(); } expected.rules = [{ path: path.join(EXTENSION_ROOT_DIR, 'pythonFiles'), include: false }]; - if (testProvider === 'unittest') { - expected.program = testLaunchScript; - expected.args = options.args; - } else { - expected.program = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'); - expected.args = [testLaunchScript, ...options.args]; - } + expected.program = testLaunchScript; + expected.args = options.args; if (!expected.cwd) { expected.cwd = workspaceFolders[0].uri.fsPath; diff --git a/src/test/testing/navigation/symbolNavigator.unit.test.ts b/src/test/testing/navigation/symbolNavigator.unit.test.ts index c391560bce60..6658848bbeb9 100644 --- a/src/test/testing/navigation/symbolNavigator.unit.test.ts +++ b/src/test/testing/navigation/symbolNavigator.unit.test.ts @@ -85,11 +85,7 @@ suite('Unit Tests - Navigation Command Handler', () => { }); test('Ensure no symbols are returned when there are no symbols to be returned', async () => { const docUri = Uri.file(__filename); - const args = [ - path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'), - path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'symbolProvider.py'), - docUri.fsPath, - ]; + const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'symbolProvider.py'), docUri.fsPath]; const proc: ExecutionResult = { stdout: JSON.stringify({ classes: [], methods: [], functions: [] }), }; @@ -117,11 +113,7 @@ suite('Unit Tests - Navigation Command Handler', () => { }); test('Ensure symbols are returned', async () => { const docUri = Uri.file(__filename); - const args = [ - path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'pyvsc-run-isolated.py'), - path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'symbolProvider.py'), - docUri.fsPath, - ]; + const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'symbolProvider.py'), docUri.fsPath]; const proc: ExecutionResult = { stdout: JSON.stringify({ classes: [ diff --git a/src/test/testing/serviceRegistry.ts b/src/test/testing/serviceRegistry.ts index 8785c1844ccd..51fe4d0f30ba 100644 --- a/src/test/testing/serviceRegistry.ts +++ b/src/test/testing/serviceRegistry.ts @@ -1,17 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. + 'use strict'; + import { Uri } from 'vscode'; import { IProcessServiceFactory } from '../../client/common/process/types'; -import { InterpreterEvaluation } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation'; -import { InterpreterSecurityService } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService'; -import { InterpreterSecurityStorage } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage'; -import { - IInterpreterEvaluation, - IInterpreterSecurityService, - IInterpreterSecurityStorage, -} from '../../client/interpreter/autoSelection/types'; import { IInterpreterHelper } from '../../client/interpreter/contracts'; import { InterpreterHelper } from '../../client/interpreter/helpers'; import { IServiceContainer } from '../../client/ioc/types'; @@ -58,21 +52,17 @@ import { IocContainer } from '../serviceRegistry'; import { MockUnitTestSocketServer } from './mocks'; export class UnitTestIocContainer extends IocContainer { - constructor() { - super(); - } public async getPythonMajorVersion(resource: Uri): Promise { const procServiceFactory = this.serviceContainer.get(IProcessServiceFactory); const procService = await procServiceFactory.create(resource); const pythonVersion = await getPythonSemVer(procService); if (pythonVersion) { return pythonVersion.major; - } else { - return -1; // log warning already issued by underlying functions... } + return -1; // log warning already issued by underlying functions... } - public registerTestVisitors() { + public registerTestVisitors(): void { this.serviceManager.add(ITestVisitor, TestFlatteningVisitor, 'TestFlatteningVisitor'); this.serviceManager.add(ITestVisitor, TestResultResetVisitor, 'TestResultResetVisitor'); this.serviceManager.addSingleton( @@ -82,27 +72,27 @@ export class UnitTestIocContainer extends IocContainer { this.serviceManager.addSingleton(ITestContextService, TestContextService); } - public registerTestStorage() { + public registerTestStorage(): void { this.serviceManager.addSingleton( ITestCollectionStorageService, TestCollectionStorageService, ); } - public registerTestsHelper() { + public registerTestsHelper(): void { this.serviceManager.addSingleton(ITestsHelper, TestsHelper); } - public registerTestResultsHelper() { + public registerTestResultsHelper(): void { this.serviceManager.add(ITestResultsService, TestResultsService); } - public registerTestParsers() { + public registerTestParsers(): void { this.serviceManager.add(ITestsParser, UnitTestTestsParser, UNITTEST_PROVIDER); this.serviceManager.add(ITestsParser, NoseTestTestsParser, NOSETEST_PROVIDER); } - public registerTestDiscoveryServices() { + public registerTestDiscoveryServices(): void { this.serviceManager.add( ITestDiscoveryService, UnitTestTestDiscoveryService, @@ -122,11 +112,11 @@ export class UnitTestIocContainer extends IocContainer { this.serviceManager.add(ITestDiscoveredTestParser, TestDiscoveredTestParser); } - public registerTestDiagnosticServices() { + public registerTestDiagnosticServices(): void { this.serviceManager.addSingleton(ITestDiagnosticService, UnitTestDiagnosticService); } - public registerTestManagers() { + public registerTestManagers(): void { this.serviceManager.addFactory(ITestManagerFactory, (context) => { return (testProvider: TestProvider, workspaceFolder: Uri, rootDirectory: string) => { const serviceContainer = context.container.get(IServiceContainer); @@ -149,14 +139,11 @@ export class UnitTestIocContainer extends IocContainer { }); } - public registerInterpreterStorageTypes() { - this.serviceManager.add(IInterpreterSecurityStorage, InterpreterSecurityStorage); - this.serviceManager.add(IInterpreterSecurityService, InterpreterSecurityService); - this.serviceManager.add(IInterpreterEvaluation, InterpreterEvaluation); + public registerInterpreterStorageTypes(): void { this.serviceManager.add(IInterpreterHelper, InterpreterHelper); } - public registerTestManagerService() { + public registerTestManagerService(): void { this.serviceManager.addFactory(ITestManagerServiceFactory, (context) => { return (workspaceFolder: Uri) => { const serviceContainer = context.container.get(IServiceContainer); @@ -166,7 +153,7 @@ export class UnitTestIocContainer extends IocContainer { }); } - public registerMockUnitTestSocketServer() { + public registerMockUnitTestSocketServer(): void { this.serviceManager.addSingleton(IUnitTestSocketServer, MockUnitTestSocketServer); } }