diff --git a/extensions/positron-python/.github/actions/build-vsix/action.yml b/extensions/positron-python/.github/actions/build-vsix/action.yml index b632eb0a25c..52d6d1cdbdd 100644 --- a/extensions/positron-python/.github/actions/build-vsix/action.yml +++ b/extensions/positron-python/.github/actions/build-vsix/action.yml @@ -23,7 +23,7 @@ runs: # Jedi LS depends on dataclasses which is not in the stdlib in Python 3.7. - name: Use Python 3.8 for JediLSP - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: 3.8 cache: 'pip' diff --git a/extensions/positron-python/.github/workflows/pr-check.yml b/extensions/positron-python/.github/workflows/pr-check.yml index 2f00cd7a761..9b2aaa09974 100644 --- a/extensions/positron-python/.github/workflows/pr-check.yml +++ b/extensions/positron-python/.github/workflows/pr-check.yml @@ -91,7 +91,8 @@ jobs: # macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case. os: [ubuntu-latest, windows-latest] # Run the tests on the oldest and most recent versions of Python. - python: ['3.8', '3.x', '3.12-dev'] + python: ['3.8', '3.x'] # run for 3 pytest versions, most recent stable, oldest version supported and pre-release + pytest-version: ['pytest', 'pytest@pre-release', 'pytest==6.2.0'] steps: - name: Checkout @@ -104,6 +105,18 @@ jobs: with: python-version: ${{ matrix.python }} + - name: Install specific pytest version + if: matrix.pytest-version == 'pytest@pre-release' + run: | + python -m pip install --pre pytest + + - name: Install specific pytest version + if: matrix.pytest-version != 'pytest@pre-release' + run: | + python -m pip install "${{ matrix.pytest-version }}" + + - name: Install specific pytest version + run: python -m pytest --version - name: Install base Python requirements uses: brettcannon/pip-secure-install@v1 with: diff --git a/extensions/positron-python/ThirdPartyNotices-Repository.txt b/extensions/positron-python/ThirdPartyNotices-Repository.txt index 9e7e822af1b..bbb00d523f9 100644 --- a/extensions/positron-python/ThirdPartyNotices-Repository.txt +++ b/extensions/positron-python/ThirdPartyNotices-Repository.txt @@ -17,6 +17,7 @@ Microsoft Python extension for Visual Studio Code incorporates third party mater 11. vscode-cpptools (https://github.com/microsoft/vscode-cpptools) 12. mocha (https://github.com/mochajs/mocha) 13. get-pip (https://github.com/pypa/get-pip) +14. vscode-js-debug (https://github.com/microsoft/vscode-js-debug) %% Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE @@ -1032,3 +1033,31 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ========================================= END OF get-pip NOTICES, INFORMATION, AND LICENSE + + +%% vscode-js-debug NOTICES, INFORMATION, AND LICENSE BEGIN HERE +========================================= + +MIT License + +Copyright (c) Microsoft Corporation. All rights reserved. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE +========================================= +END OF vscode-js-debug NOTICES, INFORMATION, AND LICENSE diff --git a/extensions/positron-python/build/license-header.txt b/extensions/positron-python/build/license-header.txt index 2a8122642cb..2970b03d7a1 100644 --- a/extensions/positron-python/build/license-header.txt +++ b/extensions/positron-python/build/license-header.txt @@ -1,7 +1,7 @@ PLEASE NOTE: This is the license for the Python extension for Visual Studio Code. The Python extension automatically installs other extensions as optional dependencies, which can be uninstalled at any time. These extensions have separate licenses: - - The Jupyter extension is released under an MIT License: - https://marketplace.visualstudio.com/items/ms-toolsai.jupyter/license + - The Python Debugger extension is released under an MIT License: + https://marketplace.visualstudio.com/items/ms-python.debugpy/license - The Pylance extension is only available in binary form and is released under a Microsoft proprietary license, the terms of which are available here: https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license diff --git a/extensions/positron-python/build/pinned-test-requirements.txt b/extensions/positron-python/build/pinned-test-requirements.txt index 3029fdb17df..5458496ae99 100644 --- a/extensions/positron-python/build/pinned-test-requirements.txt +++ b/extensions/positron-python/build/pinned-test-requirements.txt @@ -12,7 +12,7 @@ prospector==0.12.2 # avoid breaking changes in pytest 8 https://github.com/microsoft/vscode-python/issues/22798 pytest==7.4.4 # --- End Positron --- -Flask==3.0.1 +Flask==3.0.2 fastapi==0.109.2 uvicorn==0.27.0.post1 Django==5.0.1; python_version >= '3.10' @@ -25,8 +25,8 @@ torch-tb-profiler==0.4.3 freezegun==1.4.0 # iPython Kernel tests fastcore==1.5.29 -ipykernel==6.29.0 -ipywidgets==8.1.1 +ipykernel==6.29.3 +ipywidgets==8.1.2 matplotlib==3.8.2; python_version >= '3.9' matplotlib==3.7.4; python_version < '3.9' # Require fonttools >= 4.43.0 to avoid CVE-2023-45139 @@ -37,10 +37,10 @@ pandas==2.2.0; python_version >= '3.9' pandas==2.0.3; python_version < '3.9' # Require pillow >= 10.0.1 to avoid CVE-2023-4863 pillow==10.2.0 -pyright==1.1.349 +pyright==1.1.351 pytest-asyncio==0.23.4 pytest-mock==3.12.0 -polars==0.20.6 +polars==0.20.11 torch==2.1.2; python_version < '3.12' # Require wheel >= 0.38.0 to avoid CVE-2022-40898 wheel==0.38.0 diff --git a/extensions/positron-python/package.json b/extensions/positron-python/package.json index 6fd37b70f3d..ad12b3c101b 100644 --- a/extensions/positron-python/package.json +++ b/extensions/positron-python/package.json @@ -492,9 +492,6 @@ "enum": [ "off", "prompt" - ], - "tags": [ - "experimental" ] }, "python.condaPath": { @@ -1646,7 +1643,7 @@ "vscode-jsonrpc": "^8.2.0", "vscode-languageclient": "^9.0.1", "vscode-languageserver-protocol": "^3.17.5", - "vscode-tas-client": "^0.1.75", + "vscode-tas-client": "^0.1.84", "which": "^2.0.2", "winreg": "^1.2.4", "xml2js": "^0.5.0" diff --git a/extensions/positron-python/pythonFiles/install_debugpy.py b/extensions/positron-python/pythonFiles/install_debugpy.py index 9377d00237d..c217e81fd21 100644 --- a/extensions/positron-python/pythonFiles/install_debugpy.py +++ b/extensions/positron-python/pythonFiles/install_debugpy.py @@ -13,7 +13,7 @@ DEBUGGER_DEST = os.path.join(EXTENSION_ROOT, "pythonFiles", "lib", "python") DEBUGGER_PACKAGE = "debugpy" DEBUGGER_PYTHON_ABI_VERSIONS = ("cp310",) -DEBUGGER_VERSION = "1.8.0" # can also be "latest" +DEBUGGER_VERSION = "1.8.1" # can also be "latest" def _contains(s, parts=()): diff --git a/extensions/positron-python/pythonFiles/pyproject.toml b/extensions/positron-python/pythonFiles/pyproject.toml index 0167f82d92e..151bee5fc45 100644 --- a/extensions/positron-python/pythonFiles/pyproject.toml +++ b/extensions/positron-python/pythonFiles/pyproject.toml @@ -48,7 +48,8 @@ ignore = [ 'tests/testing_tools/adapter/test_util.py', 'tests/testing_tools/adapter/pytest/test_cli.py', 'tests/testing_tools/adapter/pytest/test_discovery.py', - 'tests/unittestadapter/.data/unittest_skip/unittest_skip_function.py' + 'tests/unittestadapter/.data/unittest_skip/unittest_skip_function.py', + 'tests/pytestadapter/helpers.py' ] [tool.ruff] diff --git a/extensions/positron-python/pythonFiles/pythonrc.py b/extensions/positron-python/pythonFiles/pythonrc.py index 1cb72b0ec34..374888ddada 100644 --- a/extensions/positron-python/pythonFiles/pythonrc.py +++ b/extensions/positron-python/pythonFiles/pythonrc.py @@ -49,7 +49,7 @@ def __str__(self): exit_code = 1 else: exit_code = 0 - + self.hooks.failure_flag = False # Guide following official VS Code doc for shell integration sequence: result = "" # For non-windows allow recent_command history. diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/.data/dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/.data/dual_level_nested_folder/nested_folder_one/test_bottom_folder.py similarity index 100% rename from extensions/positron-python/pythonFiles/tests/pytestadapter/.data/dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py rename to extensions/positron-python/pythonFiles/tests/pytestadapter/.data/dual_level_nested_folder/nested_folder_one/test_bottom_folder.py diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index d4e91f56b5f..7fbb0c5c43e 100644 --- a/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -317,7 +317,7 @@ # └── test_top_folder.py # └── test_top_function_t # └── test_top_function_f -# └── z_nested_folder_one +# └── nested_folder_one # └── test_bottom_folder.py # └── test_bottom_function_t # └── test_bottom_function_f @@ -326,14 +326,14 @@ TEST_DATA_PATH / "dual_level_nested_folder" / "test_top_folder.py" ) -test_z_nested_folder_one_path = ( - TEST_DATA_PATH / "dual_level_nested_folder" / "z_nested_folder_one" +test_nested_folder_one_path = ( + TEST_DATA_PATH / "dual_level_nested_folder" / "nested_folder_one" ) test_bottom_folder_path = ( TEST_DATA_PATH / "dual_level_nested_folder" - / "z_nested_folder_one" + / "nested_folder_one" / "test_bottom_folder.py" ) @@ -392,10 +392,10 @@ ], }, { - "name": "z_nested_folder_one", - "path": os.fspath(test_z_nested_folder_one_path), + "name": "nested_folder_one", + "path": os.fspath(test_nested_folder_one_path), "type_": "folder", - "id_": os.fspath(test_z_nested_folder_one_path), + "id_": os.fspath(test_nested_folder_one_path), "children": [ { "name": "test_bottom_folder.py", @@ -412,11 +412,11 @@ ), "type_": "test", "id_": get_absolute_test_id( - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_t", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_t", test_bottom_folder_path, ), "runID": get_absolute_test_id( - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_t", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_t", test_bottom_folder_path, ), }, @@ -429,11 +429,11 @@ ), "type_": "test", "id_": get_absolute_test_id( - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_f", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_f", test_bottom_folder_path, ), "runID": get_absolute_test_id( - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_f", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_f", test_bottom_folder_path, ), }, @@ -994,3 +994,78 @@ ], "id_": str(TEST_DATA_PATH), } +SYMLINK_FOLDER_PATH = TEST_DATA_PATH / "symlink_folder" +SYMLINK_FOLDER_PATH_TESTS = TEST_DATA_PATH / "symlink_folder" / "tests" +SYMLINK_FOLDER_PATH_TESTS_TEST_A = ( + TEST_DATA_PATH / "symlink_folder" / "tests" / "test_a.py" +) +SYMLINK_FOLDER_PATH_TESTS_TEST_B = ( + TEST_DATA_PATH / "symlink_folder" / "tests" / "test_b.py" +) + +symlink_expected_discovery_output = { + "name": "symlink_folder", + "path": str(SYMLINK_FOLDER_PATH), + "type_": "folder", + "children": [ + { + "name": "tests", + "path": str(SYMLINK_FOLDER_PATH_TESTS), + "type_": "folder", + "id_": str(SYMLINK_FOLDER_PATH_TESTS), + "children": [ + { + "name": "test_a.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), + "type_": "file", + "id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), + "children": [ + { + "name": "test_a_function", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), + "lineno": find_test_line_number( + "test_a_function", + os.path.join(tests_path, "test_a.py"), + ), + "type_": "test", + "id_": get_absolute_test_id( + "tests/test_a.py::test_a_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_A, + ), + "runID": get_absolute_test_id( + "tests/test_a.py::test_a_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_A, + ), + } + ], + }, + { + "name": "test_b.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), + "type_": "file", + "id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), + "children": [ + { + "name": "test_b_function", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), + "lineno": find_test_line_number( + "test_b_function", + os.path.join(tests_path, "test_b.py"), + ), + "type_": "test", + "id_": get_absolute_test_id( + "tests/test_b.py::test_b_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_B, + ), + "runID": get_absolute_test_id( + "tests/test_b.py::test_b_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_B, + ), + } + ], + }, + ], + } + ], + "id_": str(SYMLINK_FOLDER_PATH), +} diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_execution_test_output.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_execution_test_output.py index cf8997a252d..db4e493c3da 100644 --- a/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_execution_test_output.py +++ b/extensions/positron-python/pythonFiles/tests/pytestadapter/expected_execution_test_output.py @@ -308,7 +308,7 @@ # └── test_top_folder.py # └── test_top_function_t: success # └── test_top_function_f: failure -# └── z_nested_folder_one +# └── nested_folder_one # └── test_bottom_folder.py # └── test_bottom_function_t: success # └── test_bottom_function_f: failure @@ -318,7 +318,7 @@ dual_level_nested_folder_bottom_path = ( TEST_DATA_PATH / "dual_level_nested_folder" - / "z_nested_folder_one" + / "nested_folder_one" / "test_bottom_folder.py" ) dual_level_nested_folder_execution_expected_output = { @@ -345,11 +345,11 @@ "subtest": None, }, get_absolute_test_id( - "z_nested_folder_one/test_bottom_folder.py::test_bottom_function_t", + "nested_folder_one/test_bottom_folder.py::test_bottom_function_t", dual_level_nested_folder_bottom_path, ): { "test": get_absolute_test_id( - "z_nested_folder_one/test_bottom_folder.py::test_bottom_function_t", + "nested_folder_one/test_bottom_folder.py::test_bottom_function_t", dual_level_nested_folder_bottom_path, ), "outcome": "success", @@ -358,11 +358,11 @@ "subtest": None, }, get_absolute_test_id( - "z_nested_folder_one/test_bottom_folder.py::test_bottom_function_f", + "nested_folder_one/test_bottom_folder.py::test_bottom_function_f", dual_level_nested_folder_bottom_path, ): { "test": get_absolute_test_id( - "z_nested_folder_one/test_bottom_folder.py::test_bottom_function_f", + "nested_folder_one/test_bottom_folder.py::test_bottom_function_f", dual_level_nested_folder_bottom_path, ), "outcome": "failure", @@ -479,7 +479,7 @@ dual_level_nested_folder_bottom_path = ( TEST_DATA_PATH / "dual_level_nested_folder" - / "z_nested_folder_one" + / "nested_folder_one" / "test_bottom_folder.py" ) unittest_folder_add_path = TEST_DATA_PATH / "unittest_folder" / "test_add.py" @@ -684,3 +684,15 @@ "subtest": None, }, } + +# Constant for the symlink execution test where TEST_DATA_PATH / "root" the target and TEST_DATA_PATH / "symlink_folder" the symlink +test_a_symlink_path = TEST_DATA_PATH / "symlink_folder" / "tests" / "test_a.py" +symlink_run_expected_execution_output = { + get_absolute_test_id("test_a.py::test_a_function", test_a_symlink_path): { + "test": get_absolute_test_id("test_a.py::test_a_function", test_a_symlink_path), + "outcome": "success", + "message": None, + "traceback": None, + "subtest": None, + } +} diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/helpers.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/helpers.py index 2d36da59956..a3ed21cc553 100644 --- a/extensions/positron-python/pythonFiles/tests/pytestadapter/helpers.py +++ b/extensions/positron-python/pythonFiles/tests/pytestadapter/helpers.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import contextlib import io import json import os @@ -27,6 +28,23 @@ def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str: return absolute_test_id +@contextlib.contextmanager +def create_symlink(root: pathlib.Path, target_ext: str, destination_ext: str): + try: + destination = root / destination_ext + target = root / target_ext + if destination.exists(): + print("destination already exists", destination) + try: + destination.symlink_to(target) + except Exception as e: + print("error occurred when attempting to create a symlink", e) + yield target, destination + finally: + destination.unlink() + print("destination unlinked", destination) + + def create_server( host: str = "127.0.0.1", port: int = 0, diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/test_discovery.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/test_discovery.py index 2630ddef68b..a1f4e4f266a 100644 --- a/extensions/positron-python/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/extensions/positron-python/pythonFiles/tests/pytestadapter/test_discovery.py @@ -1,15 +1,27 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import json import os +import pathlib import shutil +import sys from typing import Any, Dict, List, Optional import pytest +script_dir = pathlib.Path(__file__).parent.parent +sys.path.append(os.fspath(script_dir)) + +from tests.tree_comparison_helper import is_same_tree + from . import expected_discovery_test_output -from .helpers import TEST_DATA_PATH, runner, runner_with_cwd +from .helpers import TEST_DATA_PATH, runner, runner_with_cwd, create_symlink +@pytest.mark.skipif( + sys.platform == "win32", + reason="See https://github.com/microsoft/vscode-python/issues/22965", +) def test_import_error(tmp_path): """Test pytest discovery on a file that has a pytest marker but does not import pytest. @@ -51,6 +63,10 @@ def test_import_error(tmp_path): assert False +@pytest.mark.skipif( + sys.platform == "win32", + reason="See https://github.com/microsoft/vscode-python/issues/22965", +) def test_syntax_error(tmp_path): """Test pytest discovery on a file that has a syntax error. @@ -195,7 +211,46 @@ def test_pytest_collect(file, expected_const): assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) assert actual_item.get("status") == "success" assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH) - assert actual_item.get("tests") == expected_const + assert is_same_tree(actual_item.get("tests"), expected_const) + + +def test_symlink_root_dir(): + """ + Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. + Discovery should succeed and testids should be relative to the symlinked root directory. + """ + with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as ( + source, + destination, + ): + assert destination.is_symlink() + + # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). + actual = runner_with_cwd( + ["--collect-only", f"--rootdir={os.fspath(destination)}"], source + ) + expected = expected_discovery_test_output.symlink_expected_discovery_output + assert actual + actual_list: List[Dict[str, Any]] = actual + if actual_list is not None: + assert actual_list.pop(-1).get("eot") + actual_item = actual_list.pop(0) + try: + # Check if all requirements + assert all( + item in actual_item.keys() for item in ("status", "cwd", "error") + ), "Required keys are missing" + assert actual_item.get("status") == "success", "Status is not 'success'" + assert actual_item.get("cwd") == os.fspath( + destination + ), f"CWD does not match: {os.fspath(destination)}" + assert ( + actual_item.get("tests") == expected + ), "Tests do not match expected value" + except AssertionError as e: + # Print the actual_item in JSON format if an assertion fails + print(json.dumps(actual_item, indent=4)) + pytest.fail(str(e)) def test_pytest_root_dir(): @@ -219,9 +274,9 @@ def test_pytest_root_dir(): assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) assert actual_item.get("status") == "success" assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH / "root") - assert ( - actual_item.get("tests") - == expected_discovery_test_output.root_with_config_expected_output + assert is_same_tree( + actual_item.get("tests"), + expected_discovery_test_output.root_with_config_expected_output, ) @@ -245,7 +300,7 @@ def test_pytest_config_file(): assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) assert actual_item.get("status") == "success" assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH / "root") - assert ( - actual_item.get("tests") - == expected_discovery_test_output.root_with_config_expected_output + assert is_same_tree( + actual_item.get("tests"), + expected_discovery_test_output.root_with_config_expected_output, ) diff --git a/extensions/positron-python/pythonFiles/tests/pytestadapter/test_execution.py b/extensions/positron-python/pythonFiles/tests/pytestadapter/test_execution.py index 767d54a6cab..a8336089d0a 100644 --- a/extensions/positron-python/pythonFiles/tests/pytestadapter/test_execution.py +++ b/extensions/positron-python/pythonFiles/tests/pytestadapter/test_execution.py @@ -1,14 +1,22 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import json import os import shutil from typing import Any, Dict, List import pytest +import sys from tests.pytestadapter import expected_execution_test_output -from .helpers import TEST_DATA_PATH, runner, runner_with_cwd +from .helpers import ( + TEST_DATA_PATH, + create_symlink, + get_absolute_test_id, + runner, + runner_with_cwd, +) def test_config_file(): @@ -64,6 +72,10 @@ def test_rootdir_specified(): assert actual_result_dict == expected_const +@pytest.mark.skipif( + sys.platform == "win32", + reason="See https://github.com/microsoft/vscode-python/issues/22965", +) def test_syntax_error_execution(tmp_path): """Test pytest execution on a file that has a syntax error. @@ -193,8 +205,8 @@ def test_bad_id_error_execution(): [ "dual_level_nested_folder/test_top_folder.py::test_top_function_t", "dual_level_nested_folder/test_top_folder.py::test_top_function_f", - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_t", - "dual_level_nested_folder/z_nested_folder_one/test_bottom_folder.py::test_bottom_function_f", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_t", + "dual_level_nested_folder/nested_folder_one/test_bottom_folder.py::test_bottom_function_f", ], expected_execution_test_output.dual_level_nested_folder_execution_expected_output, ), @@ -276,3 +288,50 @@ def test_pytest_execution(test_ids, expected_const): if actual_result_dict[key]["traceback"] is not None: actual_result_dict[key]["traceback"] = "TRACEBACK" assert actual_result_dict == expected_const + + +def test_symlink_run(): + """ + Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. + Discovery should succeed and testids should be relative to the symlinked root directory. + """ + with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as ( + source, + destination, + ): + assert destination.is_symlink() + test_a_path = TEST_DATA_PATH / "symlink_folder" / "tests" / "test_a.py" + test_a_id = get_absolute_test_id( + "tests/test_a.py::test_a_function", + test_a_path, + ) + + # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). + actual = runner_with_cwd( + [f"--rootdir={os.fspath(destination)}", test_a_id], source + ) + + expected_const = ( + expected_execution_test_output.symlink_run_expected_execution_output + ) + assert actual + actual_list: List[Dict[str, Any]] = actual + if actual_list is not None: + assert actual_list.pop(-1).get("eot") + actual_item = actual_list.pop(0) + try: + # Check if all requirements + assert all( + item in actual_item.keys() for item in ("status", "cwd", "result") + ), "Required keys are missing" + assert actual_item.get("status") == "success", "Status is not 'success'" + assert actual_item.get("cwd") == os.fspath( + destination + ), f"CWD does not match: {os.fspath(destination)}" + actual_result_dict = dict() + actual_result_dict.update(actual_item["result"]) + assert actual_result_dict == expected_const + except AssertionError as e: + # Print the actual_item in JSON format if an assertion fails + print(json.dumps(actual_item, indent=4)) + pytest.fail(str(e)) diff --git a/extensions/positron-python/pythonFiles/tests/unittestadapter/helpers.py b/extensions/positron-python/pythonFiles/tests/tree_comparison_helper.py similarity index 80% rename from extensions/positron-python/pythonFiles/tests/unittestadapter/helpers.py rename to extensions/positron-python/pythonFiles/tests/tree_comparison_helper.py index 303d021368f..edf6aa8ff86 100644 --- a/extensions/positron-python/pythonFiles/tests/unittestadapter/helpers.py +++ b/extensions/positron-python/pythonFiles/tests/tree_comparison_helper.py @@ -1,10 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -import pathlib - -TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data" - def is_same_tree(tree1, tree2) -> bool: """Helper function to test if two test trees are the same. @@ -17,8 +13,9 @@ def is_same_tree(tree1, tree2) -> bool: # Compare child test nodes if they exist, otherwise compare test items. if "children" in tree1 and "children" in tree2: - children1 = tree1["children"] - children2 = tree2["children"] + # sort children by path before comparing since order doesn't matter of children + children1 = sorted(tree1["children"], key=lambda x: x["path"]) + children2 = sorted(tree2["children"], key=lambda x: x["path"]) # Compare test nodes. if len(children1) != len(children2): diff --git a/extensions/positron-python/pythonFiles/tests/unittestadapter/expected_discovery_test_output.py b/extensions/positron-python/pythonFiles/tests/unittestadapter/expected_discovery_test_output.py index db509ebeca3..1007a8f42df 100644 --- a/extensions/positron-python/pythonFiles/tests/unittestadapter/expected_discovery_test_output.py +++ b/extensions/positron-python/pythonFiles/tests/unittestadapter/expected_discovery_test_output.py @@ -3,9 +3,11 @@ import os from unittestadapter.pvsc_utils import TestNodeTypeEnum -from .helpers import TEST_DATA_PATH import pathlib +TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data" + + skip_unittest_folder_discovery_output = { "path": os.fspath(TEST_DATA_PATH / "unittest_skip"), "name": "unittest_skip", diff --git a/extensions/positron-python/pythonFiles/tests/unittestadapter/test_discovery.py b/extensions/positron-python/pythonFiles/tests/unittestadapter/test_discovery.py index 4249ca4faef..a68774d3f2d 100644 --- a/extensions/positron-python/pythonFiles/tests/unittestadapter/test_discovery.py +++ b/extensions/positron-python/pythonFiles/tests/unittestadapter/test_discovery.py @@ -3,14 +3,21 @@ import os import pathlib +import sys from typing import List import pytest from unittestadapter.discovery import discover_tests from unittestadapter.pvsc_utils import TestNodeTypeEnum, parse_unittest_args +script_dir = pathlib.Path(__file__).parent.parent +sys.path.append(os.fspath(script_dir)) + + from . import expected_discovery_test_output -from .helpers import TEST_DATA_PATH, is_same_tree +from tests.tree_comparison_helper import is_same_tree + +TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data" @pytest.mark.parametrize( diff --git a/extensions/positron-python/pythonFiles/tests/unittestadapter/test_utils.py b/extensions/positron-python/pythonFiles/tests/unittestadapter/test_utils.py index 9fe2af8256c..d5f6fbbe9f1 100644 --- a/extensions/positron-python/pythonFiles/tests/unittestadapter/test_utils.py +++ b/extensions/positron-python/pythonFiles/tests/unittestadapter/test_utils.py @@ -3,6 +3,7 @@ import os import pathlib +import sys import unittest import pytest @@ -15,7 +16,13 @@ get_test_case, ) -from .helpers import TEST_DATA_PATH, is_same_tree +script_dir = pathlib.Path(__file__).parent.parent +sys.path.append(os.fspath(script_dir)) + +from tests.tree_comparison_helper import is_same_tree + + +TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data" @pytest.mark.parametrize( diff --git a/extensions/positron-python/pythonFiles/vscode_pytest/__init__.py b/extensions/positron-python/pythonFiles/vscode_pytest/__init__.py index 3b61156a59c..05e83e5aaf2 100644 --- a/extensions/positron-python/pythonFiles/vscode_pytest/__init__.py +++ b/extensions/positron-python/pythonFiles/vscode_pytest/__init__.py @@ -57,6 +57,7 @@ def __init__(self, message): collected_tests_so_far = list() TEST_PORT = os.getenv("TEST_PORT") TEST_UUID = os.getenv("TEST_UUID") +SYMLINK_PATH = None def pytest_load_initial_conftests(early_config, parser, args): @@ -75,6 +76,25 @@ def pytest_load_initial_conftests(early_config, parser, args): global IS_DISCOVERY IS_DISCOVERY = True + # check if --rootdir is in the args + for arg in args: + if "--rootdir=" in arg: + rootdir = arg.split("--rootdir=")[1] + if not os.path.exists(rootdir): + raise VSCodePytestError( + f"The path set in the argument --rootdir={rootdir} does not exist." + ) + if ( + os.path.islink(rootdir) + and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd() + ): + print( + f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.", + "Therefore setting symlink path to rootdir argument.", + ) + global SYMLINK_PATH + SYMLINK_PATH = pathlib.Path(rootdir) + def pytest_internalerror(excrepr, excinfo): """A pytest hook that is called when an internal error occurs. @@ -205,6 +225,8 @@ def pytest_report_teststatus(report, config): config -- configuration object. """ cwd = pathlib.Path.cwd() + if SYMLINK_PATH: + cwd = SYMLINK_PATH if report.when == "call": traceback = None @@ -326,6 +348,10 @@ def pytest_sessionfinish(session, exitstatus): Exit code 5: No tests were collected """ cwd = pathlib.Path.cwd() + if SYMLINK_PATH: + print("Plugin warning[vscode-pytest]: SYMLINK set, adjusting cwd.") + cwd = pathlib.Path(SYMLINK_PATH) + if IS_DISCOVERY: if not (exitstatus == 0 or exitstatus == 1 or exitstatus == 5): errorNode: TestNode = { @@ -388,6 +414,11 @@ def build_test_tree(session: pytest.Session) -> TestNode: class_nodes_dict: Dict[str, TestNode] = {} function_nodes_dict: Dict[str, TestNode] = {} + # Check to see if the global variable for symlink path is set + if SYMLINK_PATH: + session_node["path"] = SYMLINK_PATH + session_node["id_"] = os.fspath(SYMLINK_PATH) + for test_case in session.items: test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): @@ -645,14 +676,39 @@ class EOTPayloadDict(TypedDict): def get_node_path(node: Any) -> pathlib.Path: - """A function that returns the path of a node given the switch to pathlib.Path.""" - path = getattr(node, "path", None) or pathlib.Path(node.fspath) + """ + A function that returns the path of a node given the switch to pathlib.Path. + It also evaluates if the node is a symlink and returns the equivalent path. + """ + node_path = getattr(node, "path", None) or pathlib.Path(node.fspath) - if not path: + if not node_path: raise VSCodePytestError( f"Unable to find path for node: {node}, node.path: {node.path}, node.fspath: {node.fspath}" ) - return path + + # Check for the session node since it has the symlink already. + if SYMLINK_PATH and not isinstance(node, pytest.Session): + # Get relative between the cwd (resolved path) and the node path. + try: + # check to see if the node path contains the symlink root already + common_path = os.path.commonpath([SYMLINK_PATH, node_path]) + if common_path == os.fsdecode(SYMLINK_PATH): + # node path is already relative to the SYMLINK_PATH root therefore return + return node_path + else: + # if the node path is not a symlink, then we need to calculate the equivalent symlink path + # get the relative path between the cwd and the node path (as the node path is not a symlink) + rel_path = node_path.relative_to(pathlib.Path.cwd()) + # combine the difference between the cwd and the node path with the symlink path + sym_path = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path)) + return sym_path + except Exception as e: + raise VSCodePytestError( + f"Error occurred while calculating symlink equivalent from node path: {e}" + f"\n SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {node_path}, \n cwd: {pathlib.Path.cwd()}" + ) + return node_path __socket = None @@ -687,7 +743,6 @@ def post_response(cwd: str, session_node: TestNode) -> None: cwd (str): Current working directory. session_node (TestNode): Node information of the test session. """ - payload: DiscoveryPayloadDict = { "cwd": cwd, "status": "success" if not ERRORS else "error", diff --git a/extensions/positron-python/pythonFiles/vscode_pytest/run_pytest_script.py b/extensions/positron-python/pythonFiles/vscode_pytest/run_pytest_script.py index 8005f3a32c0..d6381e9b520 100644 --- a/extensions/positron-python/pythonFiles/vscode_pytest/run_pytest_script.py +++ b/extensions/positron-python/pythonFiles/vscode_pytest/run_pytest_script.py @@ -58,6 +58,8 @@ except json.JSONDecodeError: # JSON decoding error, the complete JSON object is not yet received continue + except UnicodeDecodeError: + continue except socket.error as e: print(f"Error: Could not connect to runTestIdsPort: {e}") print("Error: Could not connect to runTestIdsPort") diff --git a/extensions/positron-python/src/client/activation/activationManager.ts b/extensions/positron-python/src/client/activation/activationManager.ts index 763ce1ae881..9e97c5c4885 100644 --- a/extensions/positron-python/src/client/activation/activationManager.ts +++ b/extensions/positron-python/src/client/activation/activationManager.ts @@ -11,6 +11,7 @@ import { PYTHON_LANGUAGE } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IDisposable, IInterpreterPathService, Resource } from '../common/types'; import { Deferred } from '../common/utils/async'; +import { StopWatch } from '../common/utils/stopWatch'; import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types'; import { traceDecoratorError } from '../logging'; import { sendActivationTelemetry } from '../telemetry/envFileTelemetry'; @@ -69,7 +70,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { } } - public async activate(): Promise { + public async activate(startupStopWatch: StopWatch): Promise { this.filterServices(); await this.initialize(); @@ -77,12 +78,12 @@ export class ExtensionActivationManager implements IExtensionActivationManager { await Promise.all([ ...this.singleActivationServices.map((item) => item.activate()), - this.activateWorkspace(this.activeResourceService.getActiveResource()), + this.activateWorkspace(this.activeResourceService.getActiveResource(), startupStopWatch), ]); } @traceDecoratorError('Failed to activate a workspace') - public async activateWorkspace(resource: Resource): Promise { + public async activateWorkspace(resource: Resource, startupStopWatch?: StopWatch): Promise { const folder = this.workspaceService.getWorkspaceFolder(resource); resource = folder ? folder.uri : undefined; const key = this.getWorkspaceKey(resource); @@ -97,7 +98,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource); } await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource); - await Promise.all(this.activationServices.map((item) => item.activate(resource))); + await Promise.all(this.activationServices.map((item) => item.activate(resource, startupStopWatch))); await this.appDiagnostics.performPreStartupHealthCheck(resource); } diff --git a/extensions/positron-python/src/client/activation/types.ts b/extensions/positron-python/src/client/activation/types.ts index 2a177bb570b..3a949e480d4 100644 --- a/extensions/positron-python/src/client/activation/types.ts +++ b/extensions/positron-python/src/client/activation/types.ts @@ -6,6 +6,7 @@ import { Event } from 'vscode'; import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node'; import type { IDisposable, ILogOutputChannel, Resource } from '../common/types'; +import { StopWatch } from '../common/utils/stopWatch'; import { PythonEnvironment } from '../pythonEnvironments/info'; export const IExtensionActivationManager = Symbol('IExtensionActivationManager'); @@ -23,7 +24,7 @@ export interface IExtensionActivationManager extends IDisposable { * @returns {Promise} * @memberof IExtensionActivationManager */ - activate(): Promise; + activate(startupStopWatch: StopWatch): Promise; /** * Method invoked when a workspace is loaded. * This is where we place initialization scripts for each workspace. @@ -47,7 +48,7 @@ export const IExtensionActivationService = Symbol('IExtensionActivationService') */ export interface IExtensionActivationService { supportedWorkspaceTypes: { untrustedWorkspace: boolean; virtualWorkspace: boolean }; - activate(resource: Resource): Promise; + activate(resource: Resource, startupStopWatch?: StopWatch): Promise; } export enum LanguageServerType { diff --git a/extensions/positron-python/src/client/common/persistentState.ts b/extensions/positron-python/src/client/common/persistentState.ts index 2959a2dc821..ea86b2e145b 100644 --- a/extensions/positron-python/src/client/common/persistentState.ts +++ b/extensions/positron-python/src/client/common/persistentState.ts @@ -108,7 +108,7 @@ export class PersistentState implements IPersistentState { export const GLOBAL_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_GLOBAL_STORAGE_KEYS'; export const WORKSPACE_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_WORKSPACE_STORAGE_KEYS'; -const GLOBAL_PERSISTENT_KEYS = 'PYTHON_GLOBAL_STORAGE_KEYS'; +export const GLOBAL_PERSISTENT_KEYS = 'PYTHON_GLOBAL_STORAGE_KEYS'; const WORKSPACE_PERSISTENT_KEYS = 'PYTHON_WORKSPACE_STORAGE_KEYS'; type KeysStorageType = 'global' | 'workspace'; export type KeysStorage = { key: string; defaultValue: unknown }; diff --git a/extensions/positron-python/src/client/debugger/constants.ts b/extensions/positron-python/src/client/debugger/constants.ts index 08b8619ce03..a2ac198a597 100644 --- a/extensions/positron-python/src/client/debugger/constants.ts +++ b/extensions/positron-python/src/client/debugger/constants.ts @@ -4,3 +4,4 @@ 'use strict'; export const DebuggerTypeName = 'python'; +export const PythonDebuggerTypeName = 'debugpy'; diff --git a/extensions/positron-python/src/client/debugger/types.ts b/extensions/positron-python/src/client/debugger/types.ts index 3e884cf8f64..80585ba7d87 100644 --- a/extensions/positron-python/src/client/debugger/types.ts +++ b/extensions/positron-python/src/client/debugger/types.ts @@ -5,7 +5,7 @@ import { DebugConfiguration } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol/lib/debugProtocol'; -import { DebuggerTypeName } from './constants'; +import { DebuggerTypeName, PythonDebuggerTypeName } from './constants'; export enum DebugOptions { RedirectOutput = 'RedirectOutput', @@ -123,14 +123,14 @@ export interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArguments, IKnownLaunchRequestArguments, DebugConfiguration { - type: typeof DebuggerTypeName; + type: typeof DebuggerTypeName | typeof PythonDebuggerTypeName; } export interface AttachRequestArguments extends DebugProtocol.AttachRequestArguments, IKnownAttachDebugArguments, DebugConfiguration { - type: typeof DebuggerTypeName; + type: typeof DebuggerTypeName | typeof PythonDebuggerTypeName; } export interface DebugConfigurationArguments extends LaunchRequestArguments, AttachRequestArguments {} diff --git a/extensions/positron-python/src/client/extension.ts b/extensions/positron-python/src/client/extension.ts index d1f5f25be2d..08ee1006aab 100644 --- a/extensions/positron-python/src/client/extension.ts +++ b/extensions/positron-python/src/client/extension.ts @@ -46,6 +46,7 @@ import { WorkspaceService } from './common/application/workspace'; import { disposeAll } from './common/utils/resourceLifecycle'; import { ProposedExtensionAPI } from './proposedApiTypes'; import { buildProposedApi } from './proposedApi'; +import { GLOBAL_PERSISTENT_KEYS } from './common/persistentState'; // --- Start Positron --- @@ -68,7 +69,9 @@ export async function activate(context: IExtensionContext): Promise; let serviceContainer: IServiceContainer; + let isFirstSession: boolean | undefined; try { + isFirstSession = context.globalState.get(GLOBAL_PERSISTENT_KEYS, []).length === 0; const workspaceService = new WorkspaceService(); context.subscriptions.push( workspaceService.onDidGrantWorkspaceTrust(async () => { @@ -94,7 +97,7 @@ export async function activate(context: IExtensionContext): Promise(); displayProgress(activationDeferred.promise); startupDurations.startActivateTime = startupStopWatch.elapsedTime; + const activationStopWatch = new StopWatch(); //=============================================== // activation starts here @@ -141,7 +145,7 @@ async function activateUnsafe( const components = await initializeComponents(ext); // Then we finish activating. - const componentsActivated = await activateComponents(ext, components); + const componentsActivated = await activateComponents(ext, components, activationStopWatch); activateFeatures(ext, components); const nonBlocking = componentsActivated.map((r) => r.fullyReady); diff --git a/extensions/positron-python/src/client/extensionActivation.ts b/extensions/positron-python/src/client/extensionActivation.ts index cd9f99d400d..91497ea3ccd 100644 --- a/extensions/positron-python/src/client/extensionActivation.ts +++ b/extensions/positron-python/src/client/extensionActivation.ts @@ -51,11 +51,13 @@ import { registerCreateEnvironmentTriggers } from './pythonEnvironments/creation import { initializePersistentStateForTriggers } from './common/persistentState'; import { logAndNotifyOnLegacySettings } from './logging/settingLogs'; import { DebuggerTypeName } from './debugger/constants'; +import { StopWatch } from './common/utils/stopWatch'; export async function activateComponents( // `ext` is passed to any extra activation funcs. ext: ExtensionState, components: Components, + startupStopWatch: StopWatch, ): Promise { // Note that each activation returns a promise that resolves // when that activation completes. However, it might have started @@ -73,7 +75,7 @@ export async function activateComponents( // activate them in parallel with the other components. // https://github.com/microsoft/vscode-python/issues/15380 // These will go away eventually once everything is refactored into components. - const legacyActivationResult = await activateLegacy(ext); + const legacyActivationResult = await activateLegacy(ext, startupStopWatch); const workspaceService = new WorkspaceService(); if (!workspaceService.isTrusted) { return [legacyActivationResult]; @@ -105,7 +107,7 @@ export function activateFeatures(ext: ExtensionState, _components: Components): // init and activation: move them to activateComponents(). // See https://github.com/microsoft/vscode-python/issues/10454. -async function activateLegacy(ext: ExtensionState): Promise { +async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): Promise { const { legacyIOC } = ext; const { serviceManager, serviceContainer } = legacyIOC; @@ -183,7 +185,7 @@ async function activateLegacy(ext: ExtensionState): Promise { const manager = serviceContainer.get(IExtensionActivationManager); disposables.push(manager); - const activationPromise = manager.activate(); + const activationPromise = manager.activate(startupStopWatch); return { fullyReady: activationPromise }; } diff --git a/extensions/positron-python/src/client/interpreter/autoSelection/index.ts b/extensions/positron-python/src/client/interpreter/autoSelection/index.ts index 7714c487ed3..4310374fc00 100644 --- a/extensions/positron-python/src/client/interpreter/autoSelection/index.ts +++ b/extensions/positron-python/src/client/interpreter/autoSelection/index.ts @@ -6,11 +6,13 @@ import { inject, injectable } from 'inversify'; import { Event, EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; +import { DiscoveryUsingWorkers } from '../../common/experiments/groups'; import '../../common/extensions'; import { IFileSystem } from '../../common/platform/types'; -import { IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; +import { IExperimentService, IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/pythonVersion'; +import { ProgressReportStage } from '../../pythonEnvironments/base/locator'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; @@ -44,6 +46,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio @inject(IInterpreterComparer) private readonly envTypeComparer: IInterpreterComparer, @inject(IInterpreterAutoSelectionProxyService) proxy: IInterpreterAutoSelectionProxyService, @inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper, + @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { proxy.registerInstance!(this); } @@ -183,7 +186,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio private getAutoSelectionQueriedOnceState(): IPersistentState { const key = `autoSelectionInterpretersQueriedOnce`; - return this.stateFactory.createWorkspacePersistentState(key, undefined); + return this.stateFactory.createGlobalPersistentState(key, undefined); } /** @@ -199,22 +202,44 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio private async autoselectInterpreterWithLocators(resource: Resource): Promise { // Do not perform a full interpreter search if we already have cached interpreters for this workspace. const queriedState = this.getAutoSelectionInterpretersQueryState(resource); - if (queriedState.value !== true && resource) { + const globalQueriedState = this.getAutoSelectionQueriedOnceState(); + if (globalQueriedState.value && queriedState.value !== true && resource) { await this.interpreterService.triggerRefresh({ searchLocations: { roots: [resource], doNotIncludeNonRooted: true }, }); } - const globalQueriedState = this.getAutoSelectionQueriedOnceState(); - if (!globalQueriedState.value) { - // Global interpreters are loaded the first time an extension loads, after which we don't need to - // wait on global interpreter promise refresh. - await this.interpreterService.refreshPromise; - } - const interpreters = this.interpreterService.getInterpreters(resource); + const inExperiment = this.experimentService.inExperimentSync(DiscoveryUsingWorkers.experiment); const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); + let recommendedInterpreter: PythonEnvironment | undefined; + if (inExperiment) { + if (!globalQueriedState.value) { + // Global interpreters are loaded the first time an extension loads, after which we don't need to + // wait on global interpreter promise refresh. + // Do not wait for validation of all interpreters to finish, we only need to validate the recommended interpreter. + await this.interpreterService.getRefreshPromise({ stage: ProgressReportStage.allPathsDiscovered }); + } + let interpreters = this.interpreterService.getInterpreters(resource); + + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + const details = recommendedInterpreter + ? await this.interpreterService.getInterpreterDetails(recommendedInterpreter.path) + : undefined; + if (!details || !recommendedInterpreter) { + await this.interpreterService.refreshPromise; // Interpreter is invalid, wait for all of validation to finish. + interpreters = this.interpreterService.getInterpreters(resource); + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + } + } else { + if (!globalQueriedState.value) { + // Global interpreters are loaded the first time an extension loads, after which we don't need to + // wait on global interpreter promise refresh. + await this.interpreterService.refreshPromise; + } + const interpreters = this.interpreterService.getInterpreters(resource); - const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + } if (!recommendedInterpreter) { return; } diff --git a/extensions/positron-python/src/client/interpreter/contracts.ts b/extensions/positron-python/src/client/interpreter/contracts.ts index bfaebd235f1..30a05c14024 100644 --- a/extensions/positron-python/src/client/interpreter/contracts.ts +++ b/extensions/positron-python/src/client/interpreter/contracts.ts @@ -4,6 +4,7 @@ import { FileChangeType } from '../common/platform/fileSystemWatcher'; import { Resource } from '../common/types'; import { PythonEnvSource } from '../pythonEnvironments/base/info'; import { + GetRefreshEnvironmentsOptions, ProgressNotificationEvent, PythonLocatorQuery, TriggerRefreshOptions, @@ -22,7 +23,7 @@ export const IComponentAdapter = Symbol('IComponentAdapter'); export interface IComponentAdapter { readonly onProgress: Event; triggerRefresh(query?: PythonLocatorQuery, options?: TriggerRefreshOptions): Promise; - getRefreshPromise(): Promise | undefined; + getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined; readonly onChanged: Event; // VirtualEnvPrompt onDidCreate(resource: Resource, callback: () => void): Disposable; @@ -74,6 +75,7 @@ export const IInterpreterService = Symbol('IInterpreterService'); export interface IInterpreterService { triggerRefresh(query?: PythonLocatorQuery, options?: TriggerRefreshOptions): Promise; readonly refreshPromise: Promise | undefined; + getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined; readonly onDidChangeInterpreters: Event; onDidChangeInterpreterConfiguration: Event; onDidChangeInterpreter: Event; diff --git a/extensions/positron-python/src/client/interpreter/interpreterService.ts b/extensions/positron-python/src/client/interpreter/interpreterService.ts index c97a35c4a97..e9829d978fb 100644 --- a/extensions/positron-python/src/client/interpreter/interpreterService.ts +++ b/extensions/positron-python/src/client/interpreter/interpreterService.ts @@ -38,7 +38,11 @@ import { Interpreters } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { cache } from '../common/utils/decorators'; -import { PythonLocatorQuery, TriggerRefreshOptions } from '../pythonEnvironments/base/locator'; +import { + GetRefreshEnvironmentsOptions, + PythonLocatorQuery, + TriggerRefreshOptions, +} from '../pythonEnvironments/base/locator'; import { sleep } from '../common/utils/async'; type StoredPythonEnvironment = PythonEnvironment & { store?: boolean }; @@ -59,6 +63,10 @@ export class InterpreterService implements Disposable, IInterpreterService { return this.pyenvs.getRefreshPromise(); } + public getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined { + return this.pyenvs.getRefreshPromise(options); + } + public get onDidChangeInterpreter(): Event { return this.didChangeInterpreterEmitter.event; } diff --git a/extensions/positron-python/src/client/languageServer/watcher.ts b/extensions/positron-python/src/client/languageServer/watcher.ts index d3eccb71144..39e6e0bb1ec 100644 --- a/extensions/positron-python/src/client/languageServer/watcher.ts +++ b/extensions/positron-python/src/client/languageServer/watcher.ts @@ -29,6 +29,7 @@ import { PylanceLSExtensionManager } from './pylanceLSExtensionManager'; import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; +import { StopWatch } from '../common/utils/stopWatch'; @injectable() /** @@ -73,14 +74,18 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang // IExtensionActivationService - public async activate(resource?: Resource): Promise { + public async activate(resource?: Resource, startupStopWatch?: StopWatch): Promise { this.register(); - await this.startLanguageServer(this.languageServerType, resource); + await this.startLanguageServer(this.languageServerType, resource, startupStopWatch); } // ILanguageServerWatcher - public async startLanguageServer(languageServerType: LanguageServerType, resource?: Resource): Promise { - await this.startAndGetLanguageServer(languageServerType, resource); + public async startLanguageServer( + languageServerType: LanguageServerType, + resource?: Resource, + startupStopWatch?: StopWatch, + ): Promise { + await this.startAndGetLanguageServer(languageServerType, resource, startupStopWatch); } public register(): void { @@ -124,6 +129,7 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang private async startAndGetLanguageServer( languageServerType: LanguageServerType, resource?: Resource, + startupStopWatch?: StopWatch, ): Promise { const lsResource = this.getWorkspaceUri(resource); const currentInterpreter = this.workspaceInterpreters.get(lsResource.fsPath); @@ -170,6 +176,12 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang if (languageServerExtensionManager.canStartLanguageServer(interpreter)) { // Start the language server. + if (startupStopWatch) { + // It means that startup is triggering this code, track time it takes since startup to activate this code. + sendTelemetryEvent(EventName.LANGUAGE_SERVER_TRIGGER_TIME, startupStopWatch.elapsedTime, { + triggerTime: startupStopWatch.elapsedTime, + }); + } await languageServerExtensionManager.startLanguageServer(lsResource, interpreter); logStartup(languageServerType, lsResource); diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locator.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locator.ts index ab3b17629bc..3fd5194c37d 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locator.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locator.ts @@ -20,7 +20,7 @@ export type PythonEnvUpdatedEvent = { /** * The iteration index of The env info that was previously provided. */ - index: number; + index?: number; /** * The env info that was previously provided. */ diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locatorUtils.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locatorUtils.ts index 97cb6298416..faeaa84bedf 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locatorUtils.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locatorUtils.ts @@ -85,7 +85,7 @@ export async function getEnvs(iterator: IPythonEnvsIterator(iterator: IPythonEnvsIterator imp await this.disposables.dispose(); } - public async *iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { - await this.activate(); + public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { const iterator = this.doIterEnvs(query); + const it = this._iterEnvs(iterator, query); + it.onUpdated = iterator.onUpdated; + return it; + } + + private async *_iterEnvs( + iterator: IPythonEnvsIterator, + query?: PythonLocatorQuery, + ): IPythonEnvsIterator { + await this.activate(); if (query?.envPath) { let result = await iterator.next(); while (!result.done) { diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index cc37b1f82cf..bcc9877ad14 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -141,12 +141,12 @@ export class EnvsCollectionService extends PythonEnvsWatcher(); if (iterator.onUpdated !== undefined) { - const listener = iterator.onUpdated(async (event) => { + iterator.onUpdated(async (event) => { if (isProgressEvent(event)) { switch (event.stage) { case ProgressReportStage.discoveryFinished: state.done = true; - listener.dispose(); + // listener.dispose(); break; case ProgressReportStage.allPathsDiscovered: if (!query) { @@ -157,13 +157,17 @@ export class EnvsCollectionService extends PythonEnvsWatcher { - state.pending += 1; + iterator.onUpdated((event) => { if (isProgressEvent(event)) { if (event.stage === ProgressReportStage.discoveryFinished) { state.done = true; - listener.dispose(); + // For super slow locators such as Windows registry, we expect updates even after discovery + // is "officially" finished, hence do not dispose listeners. + // listener.dispose(); } else { didUpdate.fire(event); } @@ -63,19 +65,15 @@ async function* iterEnvsIterator( throw new Error( 'Unsupported behavior: `undefined` environment updates are not supported from downstream locators in reducer', ); - } else if (seen[event.index] !== undefined) { + } else if (event.index !== undefined && seen[event.index] !== undefined) { const oldEnv = seen[event.index]; seen[event.index] = event.update; didUpdate.fire({ index: event.index, old: oldEnv, update: event.update }); - } else { - // This implies a problem in a downstream locator - traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`); + } else if (event.update) { + didUpdate.fire({ update: event.update }); } - state.pending -= 1; checkIfFinishedAndNotify(state, didUpdate); }); - } else { - didUpdate.fire({ stage: ProgressReportStage.discoveryStarted }); } let result = await iterator.next(); @@ -91,10 +89,8 @@ async function* iterEnvsIterator( } result = await iterator.next(); } - if (iterator.onUpdated === undefined) { - state.done = true; - checkIfFinishedAndNotify(state, didUpdate); - } + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); } async function resolveDifferencesInBackground( @@ -128,8 +124,8 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - didUpdate.dispose(); traceVerbose(`Finished with environment reducer`); + state.done = false; // No need to notify again. } } diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts index 2ba54e07ed9..752f5778c73 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts @@ -81,13 +81,15 @@ export class PythonEnvsResolver implements IResolvingLocator { const seen: PythonEnvInfo[] = []; if (iterator.onUpdated !== undefined) { - const listener = iterator.onUpdated(async (event) => { + iterator.onUpdated(async (event) => { state.pending += 1; if (isProgressEvent(event)) { if (event.stage === ProgressReportStage.discoveryFinished) { didUpdate.fire({ stage: ProgressReportStage.allPathsDiscovered }); state.done = true; - listener.dispose(); + // For super slow locators such as Windows registry, we expect updates even after discovery + // is "officially" finished, hence do not dispose listeners. + // listener.dispose(); } else { didUpdate.fire(event); } @@ -95,15 +97,14 @@ export class PythonEnvsResolver implements IResolvingLocator { throw new Error( 'Unsupported behavior: `undefined` environment updates are not supported from downstream locators in resolver', ); - } else if (seen[event.index] !== undefined) { + } else if (event.index && seen[event.index] !== undefined) { const old = seen[event.index]; await setKind(event.update, environmentKinds); seen[event.index] = await resolveBasicEnv(event.update); didUpdate.fire({ old, index: event.index, update: seen[event.index] }); this.resolveInBackground(event.index, state, didUpdate, seen).ignoreErrors(); } else { - // This implies a problem in a downstream locator - traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`); + didUpdate.fire({ update: await this.resolveEnv(event.update.executablePath) }); } state.pending -= 1; checkIfFinishedAndNotify(state, didUpdate); @@ -173,7 +174,6 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - didUpdate.dispose(); traceVerbose(`Finished with environment resolver`); } } diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index b2f5123069b..d158d1da156 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -4,10 +4,10 @@ /* eslint-disable max-classes-per-file */ import { Event } from 'vscode'; +import * as path from 'path'; import { IDisposable } from '../../../../common/types'; import { getSearchPathEntries } from '../../../../common/utils/exec'; import { Disposables } from '../../../../common/utils/resourceLifecycle'; -import { iterPythonExecutablesInDir, looksLikeBasicGlobalPython } from '../../../common/commonUtils'; import { isPyenvShimDir } from '../../../common/environmentManagers/pyenv'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; import { PythonEnvKind, PythonEnvSource } from '../../info'; @@ -17,6 +17,9 @@ import { getEnvs } from '../../locatorUtils'; import { PythonEnvsChangedEvent } from '../../watcher'; import { DirFilesLocator } from './filesLocator'; import { traceVerbose } from '../../../../logging'; +import { inExperiment, pathExists } from '../../../common/externalDependencies'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; +import { iterPythonExecutablesInDir, looksLikeBasicGlobalPython } from '../../../common/commonUtils'; /** * A locator for Windows locators found under the $PATH env var. @@ -34,6 +37,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos private readonly disposables = new Disposables(); constructor() { + const inExp = inExperiment(DiscoveryUsingWorkers.experiment); const dirLocators: (ILocator & IDisposable)[] = getSearchPathEntries() .filter( (dirname) => @@ -48,7 +52,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos !isMicrosoftStoreDir(dirname) && !isPyenvShimDir(dirname), ) // Build a locator for each directory. - .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.System, [PythonEnvSource.PathEnvVar])); + .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.System, [PythonEnvSource.PathEnvVar], inExp)); this.disposables.push(...dirLocators); this.locators = new Locators(dirLocators); this.onChanged = this.locators.onChanged; @@ -74,7 +78,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos } } -async function* getExecutables(dirname: string): AsyncIterableIterator { +async function* oldGetExecutables(dirname: string): AsyncIterableIterator { for await (const entry of iterPythonExecutablesInDir(dirname)) { if (await looksLikeBasicGlobalPython(entry)) { yield entry.filename; @@ -82,17 +86,26 @@ async function* getExecutables(dirname: string): AsyncIterableIterator { } } +async function* getExecutables(dirname: string): AsyncIterableIterator { + const executable = path.join(dirname, 'python.exe'); + if (await pathExists(executable)) { + yield executable; + } +} + function getDirFilesLocator( // These are passed through to DirFilesLocator. dirname: string, kind: PythonEnvKind, source?: PythonEnvSource[], + inExp?: boolean, ): ILocator & IDisposable { // For now we do not bother using a locator that watches for changes // in the directory. If we did then we would use // `DirFilesWatchingLocator`, but only if not \\windows\system32 and // the `isDirWatchable()` (from fsWatchingLocator.ts) returns true. - const locator = new DirFilesLocator(dirname, kind, getExecutables, source); + const executableFunc = inExp ? getExecutables : oldGetExecutables; + const locator = new DirFilesLocator(dirname, kind, executableFunc, source); const dispose = async () => undefined; // Really we should be checking for symlinks or something more diff --git a/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index 16b2167021d..c6ba64cdb46 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -1,9 +1,18 @@ +/* eslint-disable require-yield */ /* eslint-disable no-continue */ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { EventEmitter } from 'vscode'; import { PythonEnvKind, PythonEnvSource } from '../../info'; -import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; +import { + BasicEnvInfo, + IPythonEnvsIterator, + Locator, + ProgressNotificationEvent, + ProgressReportStage, + PythonEnvUpdatedEvent, +} from '../../locator'; import { getRegistryInterpreters } from '../../../common/windowsUtils'; import { traceError, traceVerbose } from '../../../../logging'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; @@ -18,29 +27,69 @@ export class WindowsRegistryLocator extends Locator { _?: unknown, useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), ): IPythonEnvsIterator { - const iterator = async function* () { - traceVerbose('Searching for windows registry interpreters'); - const interpreters = await getRegistryInterpreters(useWorkerThreads); - for (const interpreter of interpreters) { - try { - // Filter out Microsoft Store app directories. We have a store app locator that handles this. - // The python.exe available in these directories might not be python. It can be a store install - // shortcut that takes you to microsoft store. - if (isMicrosoftStoreDir(interpreter.interpreterPath)) { - continue; - } - const env: BasicEnvInfo = { - kind: PythonEnvKind.OtherGlobal, - executablePath: interpreter.interpreterPath, - source: [PythonEnvSource.WindowsRegistry], - }; - yield env; - } catch (ex) { - traceError(`Failed to process environment: ${interpreter}`, ex); - } + const didUpdate = new EventEmitter | ProgressNotificationEvent>(); + const iterator = useWorkerThreads ? iterEnvsIterator(didUpdate) : oldIterEnvsIterator(); + if (useWorkerThreads) { + iterator.onUpdated = didUpdate.event; + } + return iterator; + } +} + +/** + * Windows registry is slow and often not necessary, so notify completion immediately, while still updating lazily as we find stuff. + * To accomplish this, use an empty iterator while lazily firing environments using updates. + */ +async function* iterEnvsIterator( + didUpdate: EventEmitter | ProgressNotificationEvent>, +): IPythonEnvsIterator { + updateLazily(didUpdate).ignoreErrors(); +} + +async function updateLazily(didUpdate: EventEmitter | ProgressNotificationEvent>) { + traceVerbose('Searching for windows registry interpreters'); + const interpreters = await getRegistryInterpreters(true); + for (const interpreter of interpreters) { + try { + // Filter out Microsoft Store app directories. We have a store app locator that handles this. + // The python.exe available in these directories might not be python. It can be a store install + // shortcut that takes you to microsoft store. + if (isMicrosoftStoreDir(interpreter.interpreterPath)) { + continue; + } + const env: BasicEnvInfo = { + kind: PythonEnvKind.OtherGlobal, + executablePath: interpreter.interpreterPath, + source: [PythonEnvSource.WindowsRegistry], + }; + didUpdate.fire({ update: env }); + } catch (ex) { + traceError(`Failed to process environment: ${interpreter}`, ex); + } + } + didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); + traceVerbose('Finished searching for windows registry interpreters'); +} + +async function* oldIterEnvsIterator(): IPythonEnvsIterator { + const interpreters = await getRegistryInterpreters(false); + for (const interpreter of interpreters) { + try { + // Filter out Microsoft Store app directories. We have a store app locator that handles this. + // The python.exe available in these directories might not be python. It can be a store install + // shortcut that takes you to microsoft store. + if (isMicrosoftStoreDir(interpreter.interpreterPath)) { + continue; } - traceVerbose('Finished searching for windows registry interpreters'); - }; - return iterator(); + const env: BasicEnvInfo = { + kind: PythonEnvKind.OtherGlobal, + executablePath: interpreter.interpreterPath, + source: [PythonEnvSource.WindowsRegistry], + }; + yield env; + } catch (ex) { + traceError(`Failed to process environment: ${interpreter}`, ex); + } } + traceVerbose('Finished searching for windows registry interpreters'); } diff --git a/extensions/positron-python/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/extensions/positron-python/src/client/pythonEnvironments/common/environmentManagers/conda.ts index bc98e57829e..1cb2e490aef 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -10,6 +10,7 @@ import { readFile, onDidChangePythonSetting, exec, + inExperiment, } from '../externalDependencies'; import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info'; @@ -24,6 +25,7 @@ import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts'; import { splitLines } from '../../../common/stringUtils'; import { SpawnOptions } from '../../../common/process/types'; import { sleep } from '../../../common/utils/async'; +import { DiscoveryUsingWorkers } from '../../../common/experiments/groups'; export const AnacondaCompanyName = 'Anaconda, Inc.'; export const CONDAPATH_SETTING_KEY = 'condaPath'; @@ -269,8 +271,15 @@ export class Conda { readonly command: string, shellCommand?: string, private readonly shellPath?: string, - private readonly useWorkerThreads = true, + private readonly useWorkerThreads?: boolean, ) { + if (this.useWorkerThreads === undefined) { + try { + this.useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); + } catch { + this.useWorkerThreads = false; // Temporarily support for legacy tests + } + } this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { Conda.condaPromise = new Map>(); @@ -290,7 +299,15 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(shellPath?: string, useWorkerThreads = true): Promise { + private static async locate(shellPath?: string, useWorkerThread?: boolean): Promise { + let useWorkerThreads: boolean; + if (useWorkerThread === undefined) { + try { + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); + } catch { + useWorkerThreads = false; // Temporarily support for legacy tests + } + } traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); let customCondaPath: string | undefined = 'conda'; @@ -395,7 +412,7 @@ export class Conda { // Probe the candidates, and pick the first one that exists and does what we need. for await (const condaPath of getCandidates()) { traceVerbose(`Probing conda binary: ${condaPath}`); - let conda = new Conda(condaPath, undefined, shellPath, useWorkerThreads); + let conda = new Conda(condaPath, undefined, shellPath); try { await conda.getInfo(); if (getOSType() === OSType.Windows && (isTestExecution() || condaPath !== customCondaPath)) { diff --git a/extensions/positron-python/src/client/pythonEnvironments/legacyIOC.ts b/extensions/positron-python/src/client/pythonEnvironments/legacyIOC.ts index f116bdb63a8..31de503a0f3 100644 --- a/extensions/positron-python/src/client/pythonEnvironments/legacyIOC.ts +++ b/extensions/positron-python/src/client/pythonEnvironments/legacyIOC.ts @@ -9,7 +9,12 @@ import { Resource } from '../common/types'; import { IComponentAdapter, ICondaService, PythonEnvironmentsChangedEvent } from '../interpreter/contracts'; import { IServiceManager } from '../ioc/types'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource } from './base/info'; -import { IDiscoveryAPI, PythonLocatorQuery, TriggerRefreshOptions } from './base/locator'; +import { + GetRefreshEnvironmentsOptions, + IDiscoveryAPI, + PythonLocatorQuery, + TriggerRefreshOptions, +} from './base/locator'; import { isMacDefaultPythonPath } from './common/environmentManagers/macDefault'; import { isParentPath } from './common/externalDependencies'; import { EnvironmentType, PythonEnvironment } from './info'; @@ -102,8 +107,8 @@ class ComponentAdapter implements IComponentAdapter { return this.api.triggerRefresh(query, options); } - public getRefreshPromise() { - return this.api.getRefreshPromise(); + public getRefreshPromise(options?: GetRefreshEnvironmentsOptions) { + return this.api.getRefreshPromise(options); } public get onProgress() { diff --git a/extensions/positron-python/src/client/startupTelemetry.ts b/extensions/positron-python/src/client/startupTelemetry.ts index 43cf09c23c3..544fb24251e 100644 --- a/extensions/positron-python/src/client/startupTelemetry.ts +++ b/extensions/positron-python/src/client/startupTelemetry.ts @@ -22,6 +22,7 @@ export async function sendStartupTelemetry( durations: IStartupDurations, stopWatch: IStopWatch, serviceContainer: IServiceContainer, + isFirstSession: boolean, ) { if (isTestExecution()) { return; @@ -30,7 +31,7 @@ export async function sendStartupTelemetry( try { await activatedPromise; durations.totalNonBlockingActivateTime = stopWatch.elapsedTime - durations.startActivateTime; - const props = await getActivationTelemetryProps(serviceContainer); + const props = await getActivationTelemetryProps(serviceContainer, isFirstSession); sendTelemetryEvent(EventName.EDITOR_LOAD, durations, props); } catch (ex) { traceError('sendStartupTelemetry() failed.', ex); @@ -76,7 +77,10 @@ export function hasUserDefinedPythonPath(resource: Resource, serviceContainer: I : false; } -async function getActivationTelemetryProps(serviceContainer: IServiceContainer): Promise { +async function getActivationTelemetryProps( + serviceContainer: IServiceContainer, + isFirstSession?: boolean, +): Promise { // TODO: Not all of this data is showing up in the database... // TODO: If any one of these parts fails we send no info. We should @@ -88,7 +92,7 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer): const terminalHelper = serviceContainer.get(ITerminalHelper); const terminalShellType = terminalHelper.identifyTerminalShell(); if (!workspaceService.isTrusted) { - return { workspaceFolderCount, terminal: terminalShellType }; + return { workspaceFolderCount, terminal: terminalShellType, isFirstSession }; } const interpreterService = serviceContainer.get(IInterpreterService); const mainWorkspaceUri = workspaceService.workspaceFolders?.length @@ -132,5 +136,6 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer): usingUserDefinedInterpreter, usingGlobalInterpreter, appName, + isFirstSession, }; } diff --git a/extensions/positron-python/src/client/telemetry/constants.ts b/extensions/positron-python/src/client/telemetry/constants.ts index 07253761922..b405143c0ba 100644 --- a/extensions/positron-python/src/client/telemetry/constants.ts +++ b/extensions/positron-python/src/client/telemetry/constants.ts @@ -64,6 +64,7 @@ export enum EventName { EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT', LANGUAGE_SERVER_ENABLED = 'LANGUAGE_SERVER.ENABLED', + LANGUAGE_SERVER_TRIGGER_TIME = 'LANGUAGE_SERVER_TRIGGER_TIME', LANGUAGE_SERVER_STARTUP = 'LANGUAGE_SERVER.STARTUP', LANGUAGE_SERVER_READY = 'LANGUAGE_SERVER.READY', LANGUAGE_SERVER_TELEMETRY = 'LANGUAGE_SERVER.EVENT', diff --git a/extensions/positron-python/src/client/telemetry/index.ts b/extensions/positron-python/src/client/telemetry/index.ts index 42a73fb06e0..bdb098be520 100644 --- a/extensions/positron-python/src/client/telemetry/index.ts +++ b/extensions/positron-python/src/client/telemetry/index.ts @@ -681,7 +681,8 @@ export interface IEventNamePropertyMapping { "totalactivatetime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" }, "totalnonblockingactivatetime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" }, "usinguserdefinedinterpreter" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" }, - "usingglobalinterpreter" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" } + "usingglobalinterpreter" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" }, + "isfirstsession" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "luabud" } } */ [EventName.EDITOR_LOAD]: { @@ -723,6 +724,11 @@ export interface IEventNamePropertyMapping { * If global interpreter is being used */ usingGlobalInterpreter?: boolean; + /** + * Carries `true` if it is the very first session of the user. We check whether persistent cache is empty + * to approximately guess if it's the first session. + */ + isFirstSession?: boolean; }; /** * Telemetry event sent when substituting Environment variables to calculate value of variables @@ -1325,6 +1331,21 @@ export interface IEventNamePropertyMapping { [EventName.LANGUAGE_SERVER_READY]: { lsVersion?: string; }; + /** + * Track how long it takes to trigger language server activation code, after Python extension starts activating. + */ + /* __GDPR__ + "language_server_trigger_time" : { + "duration" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "karrtikr" }, + "triggerTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "karrtikr" } + } + */ + [EventName.LANGUAGE_SERVER_TRIGGER_TIME]: { + /** + * Time it took to trigger language server startup. + */ + triggerTime: number; + }; /** * Telemetry event sent when starting Node.js server */ diff --git a/extensions/positron-python/src/client/telemetry/pylance.ts b/extensions/positron-python/src/client/telemetry/pylance.ts index afa6b529808..67be7428c7b 100644 --- a/extensions/positron-python/src/client/telemetry/pylance.ts +++ b/extensions/positron-python/src/client/telemetry/pylance.ts @@ -118,6 +118,8 @@ "custom_completionitemtelemetrybuildtimeinms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, "custom_extensiontotaltimeinms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, "custom_selecteditemtelemetrybuildtimeinms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "custom_completiontype" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "custom_filetype" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, "lsversion" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, "parsecallcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, "parsetime" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, diff --git a/extensions/positron-python/src/client/terminals/codeExecution/codeExecutionManager.ts b/extensions/positron-python/src/client/terminals/codeExecution/codeExecutionManager.ts index bc559906143..d25bad05238 100644 --- a/extensions/positron-python/src/client/terminals/codeExecution/codeExecutionManager.ts +++ b/extensions/positron-python/src/client/terminals/codeExecution/codeExecutionManager.ts @@ -18,7 +18,7 @@ import { IDisposableRegistry, IConfigurationService, Resource } from '../../comm import { noop } from '../../common/utils/misc'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; -import { traceError } from '../../logging'; +import { traceError, traceVerbose } from '../../logging'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService } from '../../terminals/types'; @@ -48,6 +48,7 @@ export class CodeExecutionManager implements ICodeExecutionManager { (cmd) => { this.disposableRegistry.push( this.commandManager.registerCommand(cmd as any, async (file: Resource) => { + traceVerbose(`Attempting to run Python file`, file?.fsPath); const interpreterService = this.serviceContainer.get(IInterpreterService); const interpreter = await interpreterService.getActiveInterpreter(file); if (!interpreter) { diff --git a/extensions/positron-python/src/client/terminals/codeExecution/djangoShellCodeExecution.ts b/extensions/positron-python/src/client/terminals/codeExecution/djangoShellCodeExecution.ts index 67b877429a2..05a1470b572 100644 --- a/extensions/positron-python/src/client/terminals/codeExecution/djangoShellCodeExecution.ts +++ b/extensions/positron-python/src/client/terminals/codeExecution/djangoShellCodeExecution.ts @@ -6,7 +6,12 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Disposable, Uri } from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types'; +import { + IApplicationShell, + ICommandManager, + IDocumentManager, + IWorkspaceService, +} from '../../common/application/types'; import '../../common/extensions'; import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { ITerminalServiceFactory } from '../../common/terminal/types'; @@ -28,6 +33,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi @inject(IFileSystem) fileSystem: IFileSystem, @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IInterpreterService) interpreterService: IInterpreterService, + @inject(IApplicationShell) applicationShell: IApplicationShell, ) { super( terminalServiceFactory, @@ -37,6 +43,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi platformService, interpreterService, commandManager, + applicationShell, ); this.terminalTitle = 'Django Shell'; disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager)); diff --git a/extensions/positron-python/src/client/terminals/codeExecution/repl.ts b/extensions/positron-python/src/client/terminals/codeExecution/repl.ts index 45f19798c3d..bc9a30af1fa 100644 --- a/extensions/positron-python/src/client/terminals/codeExecution/repl.ts +++ b/extensions/positron-python/src/client/terminals/codeExecution/repl.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { Disposable } from 'vscode'; -import { ICommandManager, IWorkspaceService } from '../../common/application/types'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; import { IPlatformService } from '../../common/platform/types'; import { ITerminalServiceFactory } from '../../common/terminal/types'; import { IConfigurationService, IDisposableRegistry } from '../../common/types'; @@ -22,6 +22,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider { @inject(IPlatformService) platformService: IPlatformService, @inject(IInterpreterService) interpreterService: IInterpreterService, @inject(ICommandManager) commandManager: ICommandManager, + @inject(IApplicationShell) applicationShell: IApplicationShell, ) { super( terminalServiceFactory, @@ -31,6 +32,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider { platformService, interpreterService, commandManager, + applicationShell, ); this.terminalTitle = 'REPL'; } diff --git a/extensions/positron-python/src/client/terminals/codeExecution/terminalCodeExecution.ts b/extensions/positron-python/src/client/terminals/codeExecution/terminalCodeExecution.ts index a257fff20db..4d775dbf6f9 100644 --- a/extensions/positron-python/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/extensions/positron-python/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -6,11 +6,11 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Disposable, Uri } from 'vscode'; -import { ICommandManager, IWorkspaceService } from '../../common/application/types'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; import '../../common/extensions'; import { IPlatformService } from '../../common/platform/types'; import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types'; -import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; +import { IConfigurationService, IDisposable, IDisposableRegistry, Resource } from '../../common/types'; import { Diagnostics, Repl } from '../../common/utils/localize'; import { showWarningMessage } from '../../common/vscodeApis/windowApis'; import { IInterpreterService } from '../../interpreter/contracts'; @@ -30,6 +30,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { @inject(IPlatformService) protected readonly platformService: IPlatformService, @inject(IInterpreterService) protected readonly interpreterService: IInterpreterService, @inject(ICommandManager) protected readonly commandManager: ICommandManager, + @inject(IApplicationShell) protected readonly applicationShell: IApplicationShell, ) {} public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) { @@ -65,10 +66,34 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { } this.replActive = new Promise(async (resolve) => { const replCommandArgs = await this.getExecutableInfo(resource); + let listener: IDisposable; + Promise.race([ + new Promise((resolve) => setTimeout(() => resolve(true), 3000)), + new Promise((resolve) => { + let count = 0; + const terminalDataTimeout = setTimeout(() => { + resolve(true); // Fall back for test case scenarios. + }, 3000); + // Watch TerminalData to see if REPL launched. + listener = this.applicationShell.onDidWriteTerminalData((e) => { + for (let i = 0; i < e.data.length; i++) { + if (e.data[i] === '>') { + count++; + if (count === 3) { + clearTimeout(terminalDataTimeout); + resolve(true); + } + } + } + }); + }), + ]).then(() => { + if (listener) { + listener.dispose(); + } + resolve(true); + }); terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); - - // Give python repl time to start before we start sending text. - setTimeout(() => resolve(true), 1000); }); this.disposables.push( terminalService.onDidCloseTerminal(() => { diff --git a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts index b9a9fc17b3d..aaa4ccf23be 100644 --- a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts +++ b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts @@ -26,7 +26,7 @@ import { IPathUtils, } from '../../common/types'; import { Interpreters } from '../../common/utils/localize'; -import { traceError, traceVerbose, traceWarn } from '../../logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../../logging'; import { IInterpreterService } from '../../interpreter/contracts'; import { defaultShells } from '../../interpreter/activation/service'; import { IEnvironmentActivationService } from '../../interpreter/activation/types'; @@ -111,6 +111,14 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this, this.disposables, ); + this.shellIntegrationService.onDidChangeStatus( + async () => { + traceInfo("Shell integration status changed, can confirm it's working."); + await this._applyCollection(undefined).ignoreErrors(); + }, + this, + this.disposables, + ); this.applicationEnvironment.onDidChangeShell( async (shell: string) => { this.processEnvVars = undefined; @@ -122,10 +130,12 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.disposables, ); const { shell } = this.applicationEnvironment; - const isActive = await this.shellIntegrationService.isWorking(shell); + const isActive = await this.shellIntegrationService.isWorking(); const shellType = identifyShellFromShellPath(shell); if (!isActive && shellType !== TerminalShellType.commandPrompt) { - traceWarn(`Shell integration is not active, environment activated maybe overriden by the shell.`); + traceWarn( + `Shell integration may not be active, environment activated may be overridden by the shell.`, + ); } this.registeredOnce = true; } @@ -188,7 +198,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 in some cases is a shell variable (not an env variable) so "env" might not contain it, calculate it in that case. env.PS1 = await this.getPS1(shell, resource, env); - const prependOptions = await this.getPrependOptions(shell); + const defaultPrependOptions = await this.getPrependOptions(); // Clear any previously set env vars from collection envVarCollection.clear(); @@ -203,11 +213,19 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (value !== undefined) { if (key === 'PS1') { // We cannot have the full PS1 without executing in terminal, which we do not. Hence prepend it. - traceVerbose(`Prepending environment variable ${key} in collection with ${value}`); - envVarCollection.prepend(key, value, prependOptions); + traceVerbose( + `Prepending environment variable ${key} in collection with ${value} ${JSON.stringify( + defaultPrependOptions, + )}`, + ); + envVarCollection.prepend(key, value, defaultPrependOptions); return; } if (key === 'PATH') { + const options = { + applyAtShellIntegration: true, + applyAtProcessCreation: true, + }; if (processEnv.PATH && env.PATH?.endsWith(processEnv.PATH)) { // Prefer prepending to PATH instead of replacing it, as we do not want to replace any // changes to PATH users might have made it in their init scripts (~/.bashrc etc.) @@ -215,8 +233,12 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (deactivate) { value = `${deactivate}${this.separator}${value}`; } - traceVerbose(`Prepending environment variable ${key} in collection with ${value}`); - envVarCollection.prepend(key, value, prependOptions); + traceVerbose( + `Prepending environment variable ${key} in collection with ${value} ${JSON.stringify( + options, + )}`, + ); + envVarCollection.prepend(key, value, options); } else { if (!value.endsWith(this.separator)) { value = value.concat(this.separator); @@ -224,16 +246,23 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (deactivate) { value = `${deactivate}${this.separator}${value}`; } - traceVerbose(`Prepending environment variable ${key} in collection to ${value}`); - envVarCollection.prepend(key, value, prependOptions); + traceVerbose( + `Prepending environment variable ${key} in collection to ${value} ${JSON.stringify( + options, + )}`, + ); + envVarCollection.prepend(key, value, options); } return; } - traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - envVarCollection.replace(key, value, { + const options = { applyAtShellIntegration: true, applyAtProcessCreation: true, - }); + }; + traceVerbose( + `Setting environment variable ${key} in collection to ${value} ${JSON.stringify(options)}`, + ); + envVarCollection.replace(key, value, options); } } }); @@ -290,7 +319,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 should be set but no PS1 was set. return; } - const config = await this.shellIntegrationService.isWorking(shell); + const config = await this.shellIntegrationService.isWorking(); if (!config) { traceVerbose('PS1 is not set when shell integration is disabled.'); return; @@ -324,7 +353,16 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private async handleMicroVenv(resource: Resource) { try { + const settings = this.configurationService.getSettings(resource); const workspaceFolder = this.getWorkspaceFolder(resource); + if (!settings.terminal.activateEnvironment) { + this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); + traceVerbose( + 'Do not activate microvenv as activating environments in terminal is disabled for', + resource?.fsPath, + ); + return; + } const interpreter = await this.interpreterService.getActiveInterpreter(resource); if (interpreter?.envType === EnvironmentType.Venv) { const activatePath = path.join(path.dirname(interpreter.path), 'activate'); @@ -345,8 +383,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } } - private async getPrependOptions(shell: string): Promise { - const isActive = await this.shellIntegrationService.isWorking(shell); + private async getPrependOptions(): Promise { + const isActive = await this.shellIntegrationService.isWorking(); // Ideally we would want to prepend exactly once, either at shell integration or process creation. // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available. return isActive diff --git a/extensions/positron-python/src/client/terminals/envCollectionActivation/shellIntegrationService.ts b/extensions/positron-python/src/client/terminals/envCollectionActivation/shellIntegrationService.ts index 485af6f3cc9..cba2ccbc686 100644 --- a/extensions/positron-python/src/client/terminals/envCollectionActivation/shellIntegrationService.ts +++ b/extensions/positron-python/src/client/terminals/envCollectionActivation/shellIntegrationService.ts @@ -2,12 +2,18 @@ // Licensed under the MIT License. import { injectable, inject } from 'inversify'; -import { IApplicationShell, ITerminalManager, IWorkspaceService } from '../../common/application/types'; +import { EventEmitter } from 'vscode'; +import { + IApplicationEnvironment, + IApplicationShell, + ITerminalManager, + IWorkspaceService, +} from '../../common/application/types'; import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector'; import { TerminalShellType } from '../../common/terminal/types'; -import { createDeferred, sleep } from '../../common/utils/async'; -import { cache } from '../../common/utils/decorators'; -import { traceError, traceInfo, traceVerbose } from '../../logging'; +import { IDisposableRegistry, IPersistentStateFactory } from '../../common/types'; +import { sleep } from '../../common/utils/async'; +import { traceError, traceVerbose } from '../../logging'; import { IShellIntegrationService } from '../types'; /** @@ -22,64 +28,137 @@ const ShellIntegrationShells = [ TerminalShellType.fish, ]; +export enum isShellIntegrationWorking { + key = 'SHELL_INTEGRATION_WORKING_KEY', +} + @injectable() export class ShellIntegrationService implements IShellIntegrationService { + private isWorkingForShell = new Set(); + + private readonly didChange = new EventEmitter(); + + private isDataWriteEventWorking = true; + constructor( @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - ) {} + @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, + @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + ) { + try { + const activeShellType = identifyShellFromShellPath(this.appEnvironment.shell); + const key = getKeyForShell(activeShellType); + const persistedResult = this.persistentStateFactory.createGlobalPersistentState(key); + if (persistedResult.value) { + this.isWorkingForShell.add(activeShellType); + } + this.appShell.onDidWriteTerminalData( + (e) => { + traceVerbose(e.data); // Log this temporarily for analysis + if (e.data.includes('\x1b]633;A\x07') || e.data.includes('\x1b]133;A\x07')) { + let { shell } = this.appEnvironment; + if ('shellPath' in e.terminal.creationOptions && e.terminal.creationOptions.shellPath) { + shell = e.terminal.creationOptions.shellPath; + } + const shellType = identifyShellFromShellPath(shell); + traceVerbose('Received shell integration sequence for', shellType); + const wasWorking = this.isWorkingForShell.has(shellType); + this.isWorkingForShell.add(shellType); + if (!wasWorking) { + // If it wasn't working previously, status has changed. + this.didChange.fire(); + } + } + }, + this, + this.disposables, + ); + this.appEnvironment.onDidChangeShell( + async (shell: string) => { + this.createDummyHiddenTerminal(shell); + }, + this, + this.disposables, + ); + this.createDummyHiddenTerminal(this.appEnvironment.shell); + } catch (ex) { + this.isDataWriteEventWorking = false; + traceError('Unable to check if shell integration is active', ex); + } + const isEnabled = !!this.workspaceService + .getConfiguration('terminal') + .get('integrated.shellIntegration.enabled'); + if (!isEnabled) { + traceVerbose('Shell integration is disabled in user settings.'); + } + } + + public readonly onDidChangeStatus = this.didChange.event; - public async isWorking(shell: string): Promise { + public async isWorking(): Promise { + const { shell } = this.appEnvironment; return this._isWorking(shell).catch((ex) => { traceError(`Failed to determine if shell supports shell integration`, shell, ex); return false; }); } - @cache(-1, true) public async _isWorking(shell: string): Promise { - const isEnabled = this.workspaceService - .getConfiguration('terminal') - .get('integrated.shellIntegration.enabled')!; - if (!isEnabled) { - traceVerbose('Shell integrated is disabled in user settings.'); - } const shellType = identifyShellFromShellPath(shell); - const isSupposedToWork = isEnabled && ShellIntegrationShells.includes(shellType); + const isSupposedToWork = ShellIntegrationShells.includes(shellType); if (!isSupposedToWork) { return false; } - const deferred = createDeferred(); - const timestamp = new Date().getTime(); - const name = `Python ${timestamp}`; - const onDidExecuteTerminalCommand = this.appShell.onDidExecuteTerminalCommand?.bind(this.appShell); - if (!onDidExecuteTerminalCommand) { - // Proposed API is not available, assume shell integration is working at this point. + const key = getKeyForShell(shellType); + const persistedResult = this.persistentStateFactory.createGlobalPersistentState(key); + if (persistedResult.value !== undefined) { + return persistedResult.value; + } + const result = await this.useDataWriteApproach(shellType); + if (result) { + // Once we know that shell integration is working for a shell, persist it so we need not do this check every session. + await persistedResult.updateValue(result); + } + return result; + } + + private async useDataWriteApproach(shellType: TerminalShellType) { + // For now, based on problems with using the command approach, use terminal data write event. + if (!this.isDataWriteEventWorking) { + // Assume shell integration is working, if data write event isn't working. return true; } - try { - const disposable = onDidExecuteTerminalCommand((e) => { - if (e.terminal.name === name) { - deferred.resolve(); - } - }); - const terminal = this.terminalManager.createTerminal({ - name, - shellPath: shell, - hideFromUser: true, - }); - terminal.sendText(`echo ${shell}`); - const success = await Promise.race([sleep(3000).then(() => false), deferred.promise.then(() => true)]); - disposable.dispose(); - if (!success) { - traceInfo(`Shell integration is not working for ${shellType}`); - } - return success; - } catch (ex) { - traceVerbose(`Proposed API is not available, failed to subscribe to onDidExecuteTerminalCommand`, ex); - // Proposed API is not available, assume shell integration is working at this point. + if (shellType === TerminalShellType.powershell || shellType === TerminalShellType.powershellCore) { + // Due to upstream bug: https://github.com/microsoft/vscode/issues/204616, assume shell integration is working for now. return true; } + if (!this.isWorkingForShell.has(shellType)) { + // Maybe data write event has not been processed yet, wait a bit. + await sleep(1000); + } + traceVerbose( + 'Did we determine shell integration to be working for', + shellType, + '?', + this.isWorkingForShell.has(shellType), + ); + return this.isWorkingForShell.has(shellType); } + + /** + * Creates a dummy terminal so that we are guaranteed a data write event for this shell type. + */ + private createDummyHiddenTerminal(shell: string) { + this.terminalManager.createTerminal({ + shellPath: shell, + hideFromUser: true, + }); + } +} + +function getKeyForShell(shellType: TerminalShellType) { + return `${isShellIntegrationWorking.key}_${shellType}`; } diff --git a/extensions/positron-python/src/client/terminals/types.ts b/extensions/positron-python/src/client/terminals/types.ts index 0fb268fe192..4c73da63dd1 100644 --- a/extensions/positron-python/src/client/terminals/types.ts +++ b/extensions/positron-python/src/client/terminals/types.ts @@ -44,7 +44,8 @@ export interface ITerminalEnvVarCollectionService { export const IShellIntegrationService = Symbol('IShellIntegrationService'); export interface IShellIntegrationService { - isWorking(shell: string): Promise; + onDidChangeStatus: Event; + isWorking(): Promise; } export const ITerminalDeactivateService = Symbol('ITerminalDeactivateService'); diff --git a/extensions/positron-python/src/client/testing/common/debugLauncher.ts b/extensions/positron-python/src/client/testing/common/debugLauncher.ts index c76557699ff..85076461f22 100644 --- a/extensions/positron-python/src/client/testing/common/debugLauncher.ts +++ b/extensions/positron-python/src/client/testing/common/debugLauncher.ts @@ -5,7 +5,7 @@ import { IApplicationShell, IDebugService } from '../../common/application/types import { EXTENSION_ROOT_DIR } from '../../common/constants'; import * as internalScripts from '../../common/process/internal/scripts'; import { IConfigurationService, IPythonSettings } from '../../common/types'; -import { DebuggerTypeName } from '../../debugger/constants'; +import { DebuggerTypeName, PythonDebuggerTypeName } from '../../debugger/constants'; import { IDebugConfigurationResolver } from '../../debugger/extension/configuration/types'; import { DebugPurpose, LaunchRequestArguments } from '../../debugger/types'; import { IServiceContainer } from '../../ioc/types'; @@ -78,7 +78,7 @@ export class DebugLauncher implements ITestDebugLauncher { if (!debugConfig) { debugConfig = { name: 'Debug Unit Test', - type: 'python', + type: 'debugpy', request: 'test', subProcess: true, }; @@ -118,7 +118,7 @@ export class DebugLauncher implements ITestDebugLauncher { for (const cfg of configs) { if ( cfg.name && - cfg.type === DebuggerTypeName && + (cfg.type === DebuggerTypeName || cfg.type === PythonDebuggerTypeName) && (cfg.request === 'test' || (cfg as LaunchRequestArguments).purpose?.includes(DebugPurpose.DebugTest)) ) { diff --git a/extensions/positron-python/src/client/testing/testController/common/utils.ts b/extensions/positron-python/src/client/testing/testController/common/utils.ts index 23ee881a405..0e81154a899 100644 --- a/extensions/positron-python/src/client/testing/testController/common/utils.ts +++ b/extensions/positron-python/src/client/testing/testController/common/utils.ts @@ -349,3 +349,83 @@ export function splitTestNameWithRegex(testName: string): [string, string] { } return [testName, testName]; } + +/** + * Converts an array of strings (with or without '=') into a map. + * If a string contains '=', it is split into a key-value pair, with the portion + * before the '=' as the key and the portion after the '=' as the value. + * If no '=' is found in the string, the entire string becomes a key with a value of null. + * + * @param args - Readonly array of strings to be converted to a map. + * @returns A map representation of the input strings. + */ +export const argsToMap = (args: ReadonlyArray): { [key: string]: string | null | undefined } => { + const map: { [key: string]: string | null } = {}; + for (const arg of args) { + const delimiter = arg.indexOf('='); + if (delimiter === -1) { + map[arg] = null; + } else { + map[arg.slice(0, delimiter)] = arg.slice(delimiter + 1); + } + } + + return map; +}; + +/** + * Converts a map into an array of strings. + * Each key-value pair in the map is transformed into a string. + * If the value is null, only the key is represented in the string. + * If the value is defined (and not null), the string is in the format "key=value". + * If a value is undefined, the key-value pair is skipped. + * + * @param map - The map to be converted to an array of strings. + * @returns An array of strings representation of the input map. + */ +export const mapToArgs = (map: { [key: string]: string | null | undefined }): string[] => { + const out: string[] = []; + for (const key of Object.keys(map)) { + const value = map[key]; + if (value === undefined) { + // eslint-disable-next-line no-continue + continue; + } + + out.push(value === null ? key : `${key}=${value}`); + } + + return out; +}; + +/** + * Adds an argument to the map only if it doesn't already exist. + * + * @param map - The map of arguments. + * @param argKey - The argument key to be checked and added. + * @param argValue - The value to set for the argument if it's not already in the map. + * @returns The updated map. + */ +export function addArgIfNotExist( + map: { [key: string]: string | null | undefined }, + argKey: string, + argValue: string | null, +): { [key: string]: string | null | undefined } { + // Only add the argument if it doesn't exist in the map. + if (map[argKey] === undefined) { + map[argKey] = argValue; + } + + return map; +} + +/** + * Checks if an argument key exists in the map. + * + * @param map - The map of arguments. + * @param argKey - The argument key to be checked. + * @returns True if the argument key exists in the map, false otherwise. + */ +export function argKeyExists(map: { [key: string]: string | null | undefined }, argKey: string): boolean { + return map[argKey] !== undefined; +} diff --git a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 39dc87ed12c..ab44c96821e 100644 --- a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as path from 'path'; import { Uri } from 'vscode'; +import * as fs from 'fs'; import { ExecutionFactoryCreateWithEnvironmentOptions, IPythonExecutionFactory, @@ -10,7 +11,7 @@ import { import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; import { Deferred, createDeferred } from '../../../common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../constants'; -import { traceError, traceInfo, traceVerbose } from '../../../logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { DataReceivedEvent, DiscoveredTestPayload, @@ -24,6 +25,9 @@ import { createEOTPayload, createTestingDeferred, fixLogLinesNoTrailing, + argsToMap, + addArgIfNotExist, + mapToArgs, } from '../common/utils'; import { IEnvironmentVariablesProvider } from '../../../common/variables/types'; @@ -66,9 +70,18 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const relativePathToPytest = 'pythonFiles'; const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); const settings = this.configSettings.getSettings(uri); - const { pytestArgs } = settings.testing; + let pytestArgsMap = argsToMap(settings.testing.pytestArgs); const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; + // check for symbolic path + const stats = fs.lstatSync(cwd); + if (stats.isSymbolicLink()) { + traceWarn( + "The cwd is a symbolic link, adding '--rootdir' to pytestArgsMap only if it doesn't already exist.", + ); + pytestArgsMap = addArgIfNotExist(pytestArgsMap, '--rootdir', cwd); + } + // get and edit env vars const mutableEnv = { ...(await this.envVarsService?.getEnvironmentVariables(uri)), @@ -98,7 +111,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { }; const execService = await executionFactory?.createActivatedEnvironment(creationOptions); // delete UUID following entire discovery finishing. - const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs); + const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(mapToArgs(pytestArgsMap)); traceVerbose(`Running pytest discovery with command: ${execArgs.join(' ')} for workspace ${uri.fsPath}.`); const deferredTillExecClose: Deferred = createTestingDeferred(); diff --git a/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 8995a182a77..d366bdfc971 100644 --- a/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -129,15 +129,16 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { try { // Remove positional test folders and files, we will add as needed per node const testArgs = removePositionalFoldersAndFiles(pytestArgs); + let testArgsMap = utils.argsToMap(testArgs); // if user has provided `--rootdir` then use that, otherwise add `cwd` - if (testArgs.filter((a) => a.startsWith('--rootdir')).length === 0) { - // Make sure root dir is set so pytest can find the relative paths - testArgs.splice(0, 0, '--rootdir', uri.fsPath); - } + // root dir is required so pytest can find the relative paths and for symlinks + utils.addArgIfNotExist(testArgsMap, '--rootdir', cwd); - if (debugBool && !testArgs.some((a) => a.startsWith('--capture') || a === '-s')) { - testArgs.push('--capture', 'no'); + // -s and --capture are both command line options that control how pytest captures output. + // if neither are set, then set --capture=no to prevent pytest from capturing output. + if (debugBool && !utils.argKeyExists(testArgsMap, '-s')) { + testArgsMap = utils.addArgIfNotExist(testArgsMap, '--capture', 'no'); } // add port with run test ids to env vars @@ -162,7 +163,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const pytestUUID = uuid.toString(); const launchOptions: LaunchOptions = { cwd, - args: testArgs, + args: utils.mapToArgs(testArgsMap), token: spawnOptions.token, testProvider: PYTEST_PROVIDER, pytestPort, @@ -170,7 +171,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { runTestIdsPort: pytestRunTestIdsPort.toString(), }; traceInfo( - `Running DEBUG pytest with arguments: ${testArgs.join(' ')} for workspace ${uri.fsPath} \r\n`, + `Running DEBUG pytest with arguments: ${utils.mapToArgs(testArgsMap).join(' ')} for workspace ${ + uri.fsPath + } \r\n`, ); await debugLauncher!.launchDebugger(launchOptions, () => { deferredTillEOT?.resolve(); @@ -180,7 +183,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const deferredTillExecClose: Deferred = utils.createTestingDeferred(); // combine path to run script with run args const scriptPath = path.join(fullPluginPath, 'vscode_pytest', 'run_pytest_script.py'); - const runArgs = [scriptPath, ...testArgs]; + const runArgs = [scriptPath, ...utils.mapToArgs(testArgsMap)]; traceInfo(`Running pytest with arguments: ${runArgs.join(' ')} for workspace ${uri.fsPath} \r\n`); let resultProc: ChildProcess | undefined; diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index c7b412a1d95..bc5cea1f1e6 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -75,7 +75,7 @@ suite('Terminal Environment Variable Collection Service', () => { context = mock(); shell = mock(); shellIntegrationService = mock(); - when(shellIntegrationService.isWorking(anything())).thenResolve(true); + when(shellIntegrationService.isWorking()).thenResolve(true); globalCollection = mock(); collection = mock(); when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); @@ -345,7 +345,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', prependedPart, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Also prepend deactivate script location if available', async () => { @@ -381,7 +381,7 @@ suite('Terminal Environment Variable Collection Service', () => { const separator = getOSType() === OSType.Windows ? ';' : ':'; verify(collection.prepend('PATH', `scriptLocation${separator}${prependedPart}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Prepend full PATH with separator otherwise', async () => { @@ -414,7 +414,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', `${finalPath}${separator}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Prepend full PATH with separator otherwise', async () => { @@ -450,7 +450,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.clear()).once(); verify(collection.prepend('PATH', `scriptLocation${separator}${finalPath}${separator}`, anything())).once(); verify(collection.replace('PATH', anything(), anything())).never(); - assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true }); }); test('Verify envs are not applied if env activation is disabled', async () => { @@ -532,7 +532,7 @@ suite('Terminal Environment Variable Collection Service', () => { test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => { reset(shellIntegrationService); - when(shellIntegrationService.isWorking(anything())).thenResolve(false); + when(shellIntegrationService.isWorking()).thenResolve(false); when(platform.osType).thenReturn(OSType.Linux); const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env }; const ps1Shell = 'bash'; diff --git a/extensions/positron-python/src/test/interpreters/autoSelection/index.unit.test.ts b/extensions/positron-python/src/test/interpreters/autoSelection/index.unit.test.ts index d96d590628e..6c547354661 100644 --- a/extensions/positron-python/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/autoSelection/index.unit.test.ts @@ -14,7 +14,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace'; import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IPersistentStateFactory, Resource } from '../../../client/common/types'; +import { IExperimentService, IPersistentStateFactory, Resource } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; import { InterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection'; import { InterpreterAutoSelectionProxyService } from '../../../client/interpreter/autoSelection/proxy'; @@ -41,6 +41,7 @@ suite('Interpreters - Auto Selection', () => { let helper: IInterpreterHelper; let proxy: IInterpreterAutoSelectionProxyService; let interpreterService: IInterpreterService; + let experimentService: IExperimentService; let sendTelemetryEventStub: sinon.SinonStub; let telemetryEvents: { eventName: string; properties: Record }[] = []; class InterpreterAutoSelectionServiceTest extends InterpreterAutoSelectionService { @@ -64,6 +65,8 @@ suite('Interpreters - Auto Selection', () => { helper = mock(InterpreterHelper); proxy = mock(InterpreterAutoSelectionProxyService); interpreterService = mock(InterpreterService); + experimentService = mock(); + when(experimentService.inExperimentSync(anything())).thenReturn(false); const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); @@ -75,6 +78,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); when(interpreterService.refreshPromise).thenReturn(undefined); @@ -140,6 +144,12 @@ suite('Interpreters - Auto Selection', () => { undefined, ), ).thenReturn(instance(state)); + when( + stateFactory.createGlobalPersistentState( + 'autoSelectionInterpretersQueriedOnce', + undefined, + ), + ).thenReturn(instance(state)); when(workspaceService.getWorkspaceFolderIdentifier(anything(), '')).thenReturn('workspaceIdentifier'); autoSelectionService.onDidChangeAutoSelectedInterpreter(() => { @@ -208,6 +218,13 @@ suite('Interpreters - Auto Selection', () => { test('getInterpreters is called with ignoreCache at true if there is no value set in the workspace persistent state', async () => { const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); + + const globalQueriedState = mock(PersistentState) as PersistentState; + when(globalQueriedState.value).thenReturn(true); + when(stateFactory.createGlobalPersistentState(anyString(), undefined)).thenReturn( + instance(globalQueriedState), + ); + const queryState = mock(PersistentState) as PersistentState; when(queryState.value).thenReturn(undefined); @@ -236,6 +253,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -275,6 +293,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -309,6 +328,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -352,6 +372,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -385,6 +406,10 @@ suite('Interpreters - Auto Selection', () => { when(stateFactory.createWorkspacePersistentState(anyString(), undefined)).thenReturn( instance(queryState), ); + when(queryState.value).thenReturn(undefined); + when(stateFactory.createGlobalPersistentState(anyString(), undefined)).thenReturn( + instance(queryState), + ); let initialize = false; let eventFired = false; diff --git a/extensions/positron-python/src/test/pythonEnvironments/base/common.ts b/extensions/positron-python/src/test/pythonEnvironments/base/common.ts index 847d6e75227..9577e7ada49 100644 --- a/extensions/positron-python/src/test/pythonEnvironments/base/common.ts +++ b/extensions/positron-python/src/test/pythonEnvironments/base/common.ts @@ -203,7 +203,7 @@ export async function getEnvsWithUpdates( } updatesDone.resolve(); listener.dispose(); - } else { + } else if (event.index !== undefined) { const { index, update } = event; // We don't worry about if envs[index] is set already. envs[index] = update; diff --git a/extensions/positron-python/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts b/extensions/positron-python/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts index 592586118d1..a7f44abbbf9 100644 --- a/extensions/positron-python/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts +++ b/extensions/positron-python/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts @@ -3,16 +3,13 @@ import { assert, expect } from 'chai'; import * as path from 'path'; -import { EventEmitter } from 'vscode'; import { PythonEnvKind, PythonEnvSource } from '../../../../../client/pythonEnvironments/base/info'; import { PythonEnvsReducer } from '../../../../../client/pythonEnvironments/base/locators/composite/envsReducer'; import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher'; import { assertBasicEnvsEqual } from '../envTestUtils'; import { createBasicEnv, getEnvs, getEnvsWithUpdates, SimpleLocator } from '../../common'; import { - PythonEnvUpdatedEvent, BasicEnvInfo, - ProgressNotificationEvent, ProgressReportStage, isProgressEvent, } from '../../../../../client/pythonEnvironments/base/locator'; @@ -65,31 +62,6 @@ suite('Python envs locator - Environments Reducer', () => { assertBasicEnvsEqual(envs, expected); }); - test('Updates to environments from the incoming iterator replaces the previous info', async () => { - // Arrange - const env = createBasicEnv(PythonEnvKind.Poetry, path.join('path', 'to', 'exec1')); - const updatedEnv = createBasicEnv(PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); - const envsReturnedByParentLocator = [env]; - const didUpdate = new EventEmitter | ProgressNotificationEvent>(); - const parentLocator = new SimpleLocator(envsReturnedByParentLocator, { - onUpdated: didUpdate.event, - }); - const reducer = new PythonEnvsReducer(parentLocator); - - // Act - const iterator = reducer.iterEnvs(); - - const iteratorUpdateCallback = () => { - didUpdate.fire({ index: 0, old: env, update: updatedEnv }); - didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); // It is essential for the incoming iterator to fire "null" event signifying it's done - }; - const envs = await getEnvsWithUpdates(iterator, iteratorUpdateCallback); - - // Assert - assertBasicEnvsEqual(envs, [updatedEnv]); - didUpdate.dispose(); - }); - test('Ensure progress updates are emitted correctly', async () => { // Arrange const env1 = createBasicEnv(PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); diff --git a/extensions/positron-python/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts b/extensions/positron-python/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts index a69a643f52d..4197c36fa9f 100644 --- a/extensions/positron-python/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts +++ b/extensions/positron-python/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts @@ -222,7 +222,7 @@ suite('Windows Registry', () => { createBasicEnv(PythonEnvKind.OtherGlobal, path.join(regTestRoot, 'python38', 'python.exe')), ].map((e) => ({ ...e, source: [PythonEnvSource.WindowsRegistry] })); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assertBasicEnvsEqual(actualEnvs, expectedEnvs); @@ -233,7 +233,7 @@ suite('Windows Registry', () => { throw Error(); }); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assert.deepStrictEqual(actualEnvs, []); @@ -252,7 +252,7 @@ suite('Windows Registry', () => { createBasicEnv(PythonEnvKind.OtherGlobal, path.join(regTestRoot, 'python38', 'python.exe')), ].map((e) => ({ ...e, source: [PythonEnvSource.WindowsRegistry] })); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assertBasicEnvsEqual(actualEnvs, expectedEnvs); diff --git a/extensions/positron-python/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts b/extensions/positron-python/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts index cd7715a1bf7..ebebf2a8220 100644 --- a/extensions/positron-python/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts +++ b/extensions/positron-python/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts @@ -3,6 +3,7 @@ import { assert } from 'chai'; import * as path from 'path'; +import * as sinon from 'sinon'; import { getOSType, OSType } from '../../../../client/common/utils/platform'; import { PythonEnvKind, PythonEnvSource } from '../../../../client/pythonEnvironments/base/info'; import { BasicEnvInfo, PythonLocatorQuery } from '../../../../client/pythonEnvironments/base/locator'; @@ -10,6 +11,7 @@ import { WindowsPathEnvVarLocator } from '../../../../client/pythonEnvironments/ import { ensureFSTree } from '../../../utils/fs'; import { assertBasicEnvsEqual } from '../../base/locators/envTestUtils'; import { createBasicEnv, getEnvs } from '../../base/common'; +import * as externalDependencies from '../../../../client/pythonEnvironments/common/externalDependencies'; const IS_WINDOWS = getOSType() === OSType.Windows; @@ -71,17 +73,17 @@ suite('Python envs locator - WindowsPathEnvVarLocator', async () => { if (!process.env.PVSC_TEST_FORCE) { this.skip(); } - // eslint-disable-next-line global-require - const sinon = require('sinon'); + } + await ensureFSTree(dataTree, __dirname); + }); + setup(async () => { + if (!IS_WINDOWS) { // eslint-disable-next-line global-require const platformAPI = require('../../../../../client/common/utils/platform'); const stub = sinon.stub(platformAPI, 'getOSType'); stub.returns(OSType.Windows); } - - await ensureFSTree(dataTree, __dirname); - }); - setup(() => { + sinon.stub(externalDependencies, 'inExperiment').returns(true); cleanUps = []; const oldSearchPath = process.env[ENV_VAR]; @@ -97,6 +99,7 @@ suite('Python envs locator - WindowsPathEnvVarLocator', async () => { console.log(err); } }); + sinon.restore(); }); function getActiveLocator(...roots: string[]): WindowsPathEnvVarLocator { diff --git a/extensions/positron-python/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts b/extensions/positron-python/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts index 6c6cf5baec7..749d9467276 100644 --- a/extensions/positron-python/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts +++ b/extensions/positron-python/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts @@ -6,7 +6,12 @@ import * as path from 'path'; import * as TypeMoq from 'typemoq'; import * as sinon from 'sinon'; import { Disposable, Uri, WorkspaceFolder } from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types'; +import { + IApplicationShell, + ICommandManager, + IDocumentManager, + IWorkspaceService, +} from '../../../client/common/application/types'; import { IFileSystem, IPlatformService } from '../../../client/common/platform/types'; import { createCondaEnv } from '../../../client/common/process/pythonEnvironment'; import { createPythonProcessService } from '../../../client/common/process/pythonProcess'; @@ -32,12 +37,14 @@ suite('Terminal - Django Shell Code Execution', () => { let settings: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let pythonExecutionFactory: TypeMoq.IMock; + let applicationShell: TypeMoq.IMock; let disposables: Disposable[] = []; setup(() => { const terminalFactory = TypeMoq.Mock.ofType(); terminalSettings = TypeMoq.Mock.ofType(); terminalService = TypeMoq.Mock.ofType(); const configService = TypeMoq.Mock.ofType(); + applicationShell = TypeMoq.Mock.ofType(); workspace = TypeMoq.Mock.ofType(); workspace .setup((c) => c.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) @@ -62,6 +69,7 @@ suite('Terminal - Django Shell Code Execution', () => { fileSystem.object, disposables, interpreterService.object, + applicationShell.object, ); terminalFactory.setup((f) => f.getTerminalService(TypeMoq.It.isAny())).returns(() => terminalService.object); diff --git a/extensions/positron-python/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts b/extensions/positron-python/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts index 93845e6189e..4f60adb3b93 100644 --- a/extensions/positron-python/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts +++ b/extensions/positron-python/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts @@ -6,7 +6,12 @@ import * as path from 'path'; import { SemVer } from 'semver'; import * as TypeMoq from 'typemoq'; import { Disposable, Uri, WorkspaceFolder } from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types'; +import { + IApplicationShell, + ICommandManager, + IDocumentManager, + IWorkspaceService, +} from '../../../client/common/application/types'; import { IFileSystem, IPlatformService } from '../../../client/common/platform/types'; import { createCondaEnv } from '../../../client/common/process/pythonEnvironment'; import { createPythonProcessService } from '../../../client/common/process/pythonProcess'; @@ -47,6 +52,7 @@ suite('Terminal - Code Execution', () => { let pythonExecutionFactory: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let isDjangoRepl: boolean; + let applicationShell: TypeMoq.IMock; teardown(() => { disposables.forEach((disposable) => { @@ -71,6 +77,7 @@ suite('Terminal - Code Execution', () => { fileSystem = TypeMoq.Mock.ofType(); pythonExecutionFactory = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); + applicationShell = TypeMoq.Mock.ofType(); settings = TypeMoq.Mock.ofType(); settings.setup((s) => s.terminal).returns(() => terminalSettings.object); configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); @@ -85,6 +92,7 @@ suite('Terminal - Code Execution', () => { platform.object, interpreterService.object, commandManager.object, + applicationShell.object, ); break; } @@ -97,6 +105,7 @@ suite('Terminal - Code Execution', () => { platform.object, interpreterService.object, commandManager.object, + applicationShell.object, ); expectedTerminalTitle = 'REPL'; break; @@ -120,6 +129,7 @@ suite('Terminal - Code Execution', () => { fileSystem.object, disposables, interpreterService.object, + applicationShell.object, ); expectedTerminalTitle = 'Django Shell'; break; @@ -590,7 +600,7 @@ suite('Terminal - Code Execution', () => { ); }); - test('Ensure repl is re-initialized when terminal is closed', async () => { + test('Ensure REPL launches after reducing risk of command being ignored or duplicated', async () => { const pythonPath = 'usr/bin/python1234'; const terminalArgs = ['-a', 'b', 'c']; platform.setup((p) => p.isWindows).returns(() => false); @@ -599,43 +609,27 @@ suite('Terminal - Code Execution', () => { .returns(() => Promise.resolve(({ path: pythonPath } as unknown) as PythonEnvironment)); terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs); - let closeTerminalCallback: undefined | (() => void); - terminalService - .setup((t) => t.onDidCloseTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns((callback) => { - closeTerminalCallback = callback; - return { - dispose: noop, - }; - }); - await executor.execute('cmd1'); await executor.execute('cmd2'); await executor.execute('cmd3'); - const expectedTerminalArgs = isDjangoRepl ? terminalArgs.concat(['manage.py', 'shell']) : terminalArgs; - - expect(closeTerminalCallback).not.to.be.an('undefined', 'Callback not initialized'); - terminalService.verify( - async (t) => - t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)), - TypeMoq.Times.once(), + // Now check if sendCommand from the initializeRepl is called atLeastOnce. + // This is due to newly added Promise race and fallback to lower risk of swollen first command. + applicationShell.verify( + async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.atLeastOnce(), ); - closeTerminalCallback!.call(terminalService.object); await executor.execute('cmd4'); - terminalService.verify( - async (t) => - t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)), - TypeMoq.Times.exactly(2), + applicationShell.verify( + async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.atLeastOnce(), ); - closeTerminalCallback!.call(terminalService.object); await executor.execute('cmd5'); - terminalService.verify( - async (t) => - t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)), - TypeMoq.Times.exactly(3), + applicationShell.verify( + async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.atLeastOnce(), ); }); diff --git a/extensions/positron-python/src/test/testing/common/debugLauncher.unit.test.ts b/extensions/positron-python/src/test/testing/common/debugLauncher.unit.test.ts index bbb65f0b2e2..1c69bac2059 100644 --- a/extensions/positron-python/src/test/testing/common/debugLauncher.unit.test.ts +++ b/extensions/positron-python/src/test/testing/common/debugLauncher.unit.test.ts @@ -16,7 +16,7 @@ import { IApplicationShell, IDebugService } from '../../../client/common/applica import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import '../../../client/common/extensions'; import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; -import { DebuggerTypeName } from '../../../client/debugger/constants'; +import { PythonDebuggerTypeName } from '../../../client/debugger/constants'; import { IDebugEnvironmentVariablesService } from '../../../client/debugger/extension/configuration/resolvers/helper'; import { LaunchConfigurationResolver } from '../../../client/debugger/extension/configuration/resolvers/launch'; import { DebugOptions } from '../../../client/debugger/types'; @@ -166,7 +166,7 @@ suite('Unit Tests - Debug Launcher', () => { function getDefaultDebugConfig(): DebugConfiguration { return { name: 'Debug Unit Test', - type: DebuggerTypeName, + type: PythonDebuggerTypeName, request: 'launch', console: 'internalConsole', env: {}, @@ -329,7 +329,7 @@ suite('Unit Tests - Debug Launcher', () => { }; const expected = getDefaultDebugConfig(); expected.name = 'spam'; - setupSuccess(options, 'unittest', expected, [{ name: 'spam', type: DebuggerTypeName, request: 'test' }]); + setupSuccess(options, 'unittest', expected, [{ name: 'spam', type: PythonDebuggerTypeName, request: 'test' }]); await debugLauncher.launchDebugger(options); @@ -363,7 +363,7 @@ suite('Unit Tests - Debug Launcher', () => { }; const expected = { name: 'my tests', - type: DebuggerTypeName, + type: PythonDebuggerTypeName, request: 'launch', python: 'some/dir/bin/py3', debugAdapterPython: 'some/dir/bin/py3', @@ -388,7 +388,7 @@ suite('Unit Tests - Debug Launcher', () => { setupSuccess(options, 'unittest', expected, [ { name: 'my tests', - type: DebuggerTypeName, + type: PythonDebuggerTypeName, request: 'test', pythonPath: expected.python, stopOnEntry: expected.stopOnEntry, @@ -417,9 +417,9 @@ suite('Unit Tests - Debug Launcher', () => { const expected = getDefaultDebugConfig(); expected.name = 'spam1'; setupSuccess(options, 'unittest', expected, [ - { name: 'spam1', type: DebuggerTypeName, request: 'test' }, - { name: 'spam2', type: DebuggerTypeName, request: 'test' }, - { name: 'spam3', type: DebuggerTypeName, request: 'test' }, + { name: 'spam1', type: PythonDebuggerTypeName, request: 'test' }, + { name: 'spam2', type: PythonDebuggerTypeName, request: 'test' }, + { name: 'spam3', type: PythonDebuggerTypeName, request: 'test' }, ]); await debugLauncher.launchDebugger(options); @@ -446,7 +446,7 @@ suite('Unit Tests - Debug Launcher', () => { '// test 2 \n\ { \n\ "name": "spam", \n\ - "type": "python", \n\ + "type": "debugpy", \n\ "request": "test" \n\ } \n\ ', @@ -454,7 +454,7 @@ suite('Unit Tests - Debug Launcher', () => { [ \n\ { \n\ "name": "spam", \n\ - "type": "python", \n\ + "type": "debugpy", \n\ "request": "test" \n\ } \n\ ] \n\ @@ -464,7 +464,7 @@ suite('Unit Tests - Debug Launcher', () => { "configurations": [ \n\ { \n\ "name": "spam", \n\ - "type": "python", \n\ + "type": "debugpy", \n\ "request": "test" \n\ } \n\ ] \n\ @@ -499,10 +499,10 @@ suite('Unit Tests - Debug Launcher', () => { setupSuccess(options, 'unittest', expected, [ {} as DebugConfiguration, { name: 'spam1' } as DebugConfiguration, - { name: 'spam2', type: DebuggerTypeName } as DebugConfiguration, + { name: 'spam2', type: PythonDebuggerTypeName } as DebugConfiguration, { name: 'spam3', request: 'test' } as DebugConfiguration, - { type: DebuggerTypeName } as DebugConfiguration, - { type: DebuggerTypeName, request: 'test' } as DebugConfiguration, + { type: PythonDebuggerTypeName } as DebugConfiguration, + { type: PythonDebuggerTypeName, request: 'test' } as DebugConfiguration, { request: 'test' } as DebugConfiguration, ]); @@ -532,7 +532,7 @@ suite('Unit Tests - Debug Launcher', () => { testProvider: 'unittest', }; const expected = getDefaultDebugConfig(); - setupSuccess(options, 'unittest', expected, [{ name: 'spam', type: DebuggerTypeName, request: 'bogus' }]); + setupSuccess(options, 'unittest', expected, [{ name: 'spam', type: PythonDebuggerTypeName, request: 'bogus' }]); await debugLauncher.launchDebugger(options); @@ -547,8 +547,8 @@ suite('Unit Tests - Debug Launcher', () => { }; const expected = getDefaultDebugConfig(); setupSuccess(options, 'unittest', expected, [ - { name: 'spam', type: DebuggerTypeName, request: 'launch' }, - { name: 'spam', type: DebuggerTypeName, request: 'attach' }, + { name: 'spam', type: PythonDebuggerTypeName, request: 'launch' }, + { name: 'spam', type: PythonDebuggerTypeName, request: 'attach' }, ]); await debugLauncher.launchDebugger(options); @@ -567,9 +567,9 @@ suite('Unit Tests - Debug Launcher', () => { setupSuccess(options, 'unittest', expected, [ { name: 'foo1', type: 'other', request: 'bar' }, { name: 'foo2', type: 'other', request: 'bar' }, - { name: 'spam1', type: DebuggerTypeName, request: 'launch' }, - { name: 'spam2', type: DebuggerTypeName, request: 'test' }, - { name: 'spam3', type: DebuggerTypeName, request: 'attach' }, + { name: 'spam1', type: PythonDebuggerTypeName, request: 'launch' }, + { name: 'spam2', type: PythonDebuggerTypeName, request: 'test' }, + { name: 'spam3', type: PythonDebuggerTypeName, request: 'attach' }, { name: 'xyz', type: 'another', request: 'abc' }, ]); @@ -599,7 +599,7 @@ suite('Unit Tests - Debug Launcher', () => { { \n\ // "test" debug config \n\ "name": "spam", /* non-empty */ \n\ - "type": "python", /* must be "python" */ \n\ + "type": "debugpy", /* must be "python" */ \n\ "request": "test", /* must be "test" */ \n\ // extra stuff here: \n\ "stopOnEntry": true \n\ diff --git a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts index a9ed25194fa..8e2f6200003 100644 --- a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts +++ b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts @@ -5,6 +5,7 @@ import { TestController, TestRun, Uri } from 'vscode'; import * as typeMoq from 'typemoq'; import * as path from 'path'; import * as assert from 'assert'; +import * as fs from 'fs'; import { PytestTestDiscoveryAdapter } from '../../../client/testing/testController/pytest/pytestDiscoveryAdapter'; import { ITestController, ITestResultResolver } from '../../../client/testing/testController/common/types'; import { PythonTestServer } from '../../../client/testing/testController/common/server'; @@ -59,8 +60,25 @@ suite('End to End Tests: test adapters', () => { 'testTestingRootWkspc', 'discoveryErrorWorkspace', ); + const rootPathDiscoverySymlink = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testTestingRootWkspc', + 'symlinkWorkspace', + ); suiteSetup(async () => { serviceContainer = (await initialize()).serviceContainer; + + // create symlink for specific symlink test + const target = rootPathSmallWorkspace; + const dest = rootPathDiscoverySymlink; + fs.symlink(target, dest, 'dir', (err) => { + if (err) { + console.error(err); + } else { + console.log('Symlink created successfully for end to end tests.'); + } + }); }); setup(async () => { @@ -96,6 +114,17 @@ suite('End to End Tests: test adapters', () => { teardown(async () => { pythonTestServer.dispose(); }); + suiteTeardown(async () => { + // remove symlink + const dest = rootPathDiscoverySymlink; + fs.unlink(dest, (err) => { + if (err) { + console.error(err); + } else { + console.log('Symlink removed successfully after tests.'); + } + }); + }); test('unittest discovery adapter small workspace', async () => { // result resolver and saved data for assertions let actualData: { @@ -232,6 +261,90 @@ suite('End to End Tests: test adapters', () => { assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); }); }); + test('pytest discovery adapter small workspace with symlink', async () => { + // result resolver and saved data for assertions + let actualData: { + cwd: string; + tests?: unknown; + status: 'success' | 'error'; + error?: string[]; + }; + // set workspace to test workspace folder + const testSimpleSymlinkPath = path.join(rootPathDiscoverySymlink, 'test_simple.py'); + workspaceUri = Uri.parse(rootPathDiscoverySymlink); + const stats = fs.lstatSync(rootPathDiscoverySymlink); + + // confirm that the path is a symbolic link + assert.ok(stats.isSymbolicLink(), 'The path is not a symbolic link but must be for this test.'); + + resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri); + let callCount = 0; + resultResolver._resolveDiscovery = async (payload, _token?) => { + traceLog(`resolveDiscovery ${payload}`); + callCount = callCount + 1; + actualData = payload; + return Promise.resolve(); + }; + // run pytest discovery + const discoveryAdapter = new PytestTestDiscoveryAdapter( + pythonTestServer, + configService, + testOutputChannel.object, + resultResolver, + envVarsService, + ); + + await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { + // verification after discovery is complete + + // 1. Check the status is "success" + assert.strictEqual( + actualData.status, + 'success', + `Expected status to be 'success' instead status is ${actualData.status}`, + ); // 2. Confirm no errors + assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + // 4. Confirm that the cwd returned is the symlink path and the test's path is also using the symlink as the root + if (process.platform === 'win32') { + // covert string to lowercase for windows as the path is case insensitive + traceLog('windows machine detected, converting path to lowercase for comparison'); + const a = actualData.cwd.toLowerCase(); + const b = rootPathDiscoverySymlink.toLowerCase(); + const testSimpleActual = (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path.toLowerCase(); + const testSimpleExpected = testSimpleSymlinkPath.toLowerCase(); + assert.strictEqual(a, b, `Expected cwd to be the symlink path actual: ${a} expected: ${b}`); + assert.strictEqual( + testSimpleActual, + testSimpleExpected, + `Expected test path to be the symlink path actual: ${testSimpleActual} expected: ${testSimpleExpected}`, + ); + } else { + assert.strictEqual( + path.join(actualData.cwd), + path.join(rootPathDiscoverySymlink), + 'Expected cwd to be the symlink path, check for non-windows machines', + ); + assert.strictEqual( + (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path, + testSimpleSymlinkPath, + 'Expected test path to be the symlink path, check for non windows machines', + ); + } + + // 5. Confirm that resolveDiscovery was called once + assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + }); + }); test('pytest discovery adapter large workspace', async () => { // result resolver and saved data for assertions let actualData: { diff --git a/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts b/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts index 7badb5a0350..45f6d4ffa8e 100644 --- a/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts +++ b/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts @@ -6,6 +6,8 @@ import { Uri } from 'vscode'; import * as typeMoq from 'typemoq'; import * as path from 'path'; import { Observable } from 'rxjs/Observable'; +import * as fs from 'fs'; +import * as sinon from 'sinon'; import { IConfigurationService, ITestOutputChannel } from '../../../../client/common/types'; import { PytestTestDiscoveryAdapter } from '../../../../client/testing/testController/pytest/pytestDiscoveryAdapter'; import { ITestServer } from '../../../../client/testing/testController/common/types'; @@ -94,6 +96,9 @@ suite('pytest test discovery adapter', () => { }; }); }); + teardown(() => { + sinon.restore(); + }); test('Discovery should call exec with correct basic args', async () => { // set up exec mock deferred = createDeferred(); @@ -104,6 +109,7 @@ suite('pytest test discovery adapter', () => { deferred.resolve(); return Promise.resolve(execService.object); }); + sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); adapter = new PytestTestDiscoveryAdapter(testServer.object, configService, outputChannel.object); adapter.discoverTests(uri, execFactory.object); // add in await and trigger @@ -143,6 +149,8 @@ suite('pytest test discovery adapter', () => { }), } as unknown) as IConfigurationService; + sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); + // set up exec mock deferred = createDeferred(); execFactory = typeMoq.Mock.ofType(); @@ -161,6 +169,7 @@ suite('pytest test discovery adapter', () => { mockProc.trigger('close'); // verification + const expectedArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only', '.', 'abc', 'xyz']; execService.verify( (x) => @@ -176,4 +185,63 @@ suite('pytest test discovery adapter', () => { typeMoq.Times.once(), ); }); + test('Test discovery adds cwd to pytest args when path is symlink', async () => { + sinon.stub(fs, 'lstatSync').returns({ + isFile: () => true, + isSymbolicLink: () => true, + } as fs.Stats); + + // set up a config service with different pytest args + const configServiceNew: IConfigurationService = ({ + getSettings: () => ({ + testing: { + pytestArgs: ['.', 'abc', 'xyz'], + cwd: expectedPath, + }, + }), + } as unknown) as IConfigurationService; + + // set up exec mock + deferred = createDeferred(); + execFactory = typeMoq.Mock.ofType(); + execFactory + .setup((x) => x.createActivatedEnvironment(typeMoq.It.isAny())) + .returns(() => { + deferred.resolve(); + return Promise.resolve(execService.object); + }); + + adapter = new PytestTestDiscoveryAdapter(testServer.object, configServiceNew, outputChannel.object); + adapter.discoverTests(uri, execFactory.object); + // add in await and trigger + await deferred.promise; + await deferred2.promise; + mockProc.trigger('close'); + + // verification + const expectedArgs = [ + '-m', + 'pytest', + '-p', + 'vscode_pytest', + '--collect-only', + '.', + 'abc', + 'xyz', + `--rootdir=${expectedPath}`, + ]; + execService.verify( + (x) => + x.execObservable( + expectedArgs, + typeMoq.It.is((options) => { + assert.deepEqual(options.env, expectedExtraVariables); + assert.equal(options.cwd, expectedPath); + assert.equal(options.throwOnStdErr, true); + return true; + }), + ), + typeMoq.Times.once(), + ); + }); }); diff --git a/extensions/positron-python/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts b/extensions/positron-python/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts index a097df65436..d2ab19368b2 100644 --- a/extensions/positron-python/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts +++ b/extensions/positron-python/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -171,7 +171,8 @@ suite('pytest test execution adapter', () => { const pathToPythonFiles = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); const pathToPythonScript = path.join(pathToPythonFiles, 'vscode_pytest', 'run_pytest_script.py'); - const expectedArgs = [pathToPythonScript, '--rootdir', myTestPath]; + const rootDirArg = `--rootdir=${myTestPath}`; + const expectedArgs = [pathToPythonScript, rootDirArg]; const expectedExtraVariables = { PYTHONPATH: pathToPythonFiles, TEST_UUID: 'uuid123', @@ -238,7 +239,7 @@ suite('pytest test execution adapter', () => { const pathToPythonFiles = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); const pathToPythonScript = path.join(pathToPythonFiles, 'vscode_pytest', 'run_pytest_script.py'); - const expectedArgs = [pathToPythonScript, '--rootdir', myTestPath]; + const expectedArgs = [pathToPythonScript, `--rootdir=${newCwd}`]; const expectedExtraVariables = { PYTHONPATH: pathToPythonFiles, TEST_UUID: 'uuid123', @@ -305,7 +306,7 @@ suite('pytest test execution adapter', () => { x.launchDebugger( typeMoq.It.is((launchOptions) => { assert.equal(launchOptions.cwd, uri.fsPath); - assert.deepEqual(launchOptions.args, ['--rootdir', myTestPath, '--capture', 'no']); + assert.deepEqual(launchOptions.args, [`--rootdir=${myTestPath}`, '--capture=no']); assert.equal(launchOptions.testProvider, 'pytest'); assert.equal(launchOptions.pytestPort, '12345'); assert.equal(launchOptions.pytestUUID, 'uuid123'); diff --git a/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts b/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts index 014261a4023..5bcf8dfa10c 100644 --- a/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts +++ b/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts @@ -9,6 +9,10 @@ import { ExtractJsonRPCData, parseJsonRPCHeadersAndData, splitTestNameWithRegex, + mapToArgs, + addArgIfNotExist, + argKeyExists, + argsToMap, } from '../../../client/testing/testController/common/utils'; suite('Test Controller Utils: JSON RPC', () => { @@ -158,4 +162,174 @@ ${data}${secondPayload}`; }); }); }); + suite('Test Controller Utils: Args Mapping', () => { + test('Converts map with mixed values to array of strings', async () => { + const inputMap = { + key1: 'value1', + key2: null, + key3: undefined, + key4: 'value4', + }; + const expectedOutput = ['key1=value1', 'key2', 'key4=value4']; + + const result = mapToArgs(inputMap); + + assert.deepStrictEqual(result, expectedOutput); + }); + + test('Returns an empty array for an empty map', async () => { + const inputMap = {}; + const expectedOutput: unknown[] = []; + + const result = mapToArgs(inputMap); + + assert.deepStrictEqual(result, expectedOutput); + }); + + test('Skips undefined values', async () => { + const inputMap = { + key1: undefined, + key2: undefined, + }; + const expectedOutput: unknown[] = []; + + const result = mapToArgs(inputMap); + + assert.deepStrictEqual(result, expectedOutput); + }); + + test('Handles null values correctly', async () => { + const inputMap = { + key1: null, + key2: null, + }; + const expectedOutput = ['key1', 'key2']; + + const result = mapToArgs(inputMap); + + assert.deepStrictEqual(result, expectedOutput); + }); + test('Adds new argument if it does not exist', () => { + const map = {}; + const argKey = 'newKey'; + const argValue = 'newValue'; + + const updatedMap = addArgIfNotExist(map, argKey, argValue); + + assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); + }); + + test('Does not overwrite existing argument', () => { + const map = { existingKey: 'existingValue' }; + const argKey = 'existingKey'; + const argValue = 'newValue'; + + const updatedMap = addArgIfNotExist(map, argKey, argValue); + + assert.deepStrictEqual(updatedMap, { [argKey]: 'existingValue' }); + }); + + test('Handles null value for new key', () => { + const map = {}; + const argKey = 'nullKey'; + const argValue = null; + + const updatedMap = addArgIfNotExist(map, argKey, argValue); + + assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); + }); + + test('Ignores addition if key exists with null value', () => { + const map = { nullKey: null }; + const argKey = 'nullKey'; + const argValue = 'newValue'; + + const updatedMap = addArgIfNotExist(map, argKey, argValue); + + assert.deepStrictEqual(updatedMap, { [argKey]: null }); + }); + + test('Accepts addition if key exists with undefined value', () => { + const map = { undefinedKey: undefined }; + const argKey = 'undefinedKey'; + const argValue = 'newValue'; + + // Attempting to add a key that is explicitly set to undefined + const updatedMap = addArgIfNotExist(map, argKey, argValue); + + // Expect the map to remain unchanged because the key exists as undefined + assert.strictEqual(map[argKey], argValue); + assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); + }); + test('Complex test for argKeyExists with various key types', () => { + const map = { + stringKey: 'stringValue', + nullKey: null, + // Note: not adding an 'undefinedKey' explicitly since it's not present and hence undefined by default + }; + + // Should return true for keys that are present, even with a null value + assert.strictEqual( + argKeyExists(map, 'stringKey'), + true, + "Failed to recognize 'stringKey' which has a string value.", + ); + assert.strictEqual( + argKeyExists(map, 'nullKey'), + true, + "Failed to recognize 'nullKey' which has a null value.", + ); + + // Should return false for keys that are not present + assert.strictEqual( + argKeyExists(map, 'undefinedKey'), + false, + "Incorrectly recognized 'undefinedKey' as existing.", + ); + }); + test('Converts array of strings with "=" into a map', () => { + const args = ['key1=value1', 'key2=value2']; + const expectedMap = { key1: 'value1', key2: 'value2' }; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + + test('Assigns null to keys without "="', () => { + const args = ['key1', 'key2']; + const expectedMap = { key1: null, key2: null }; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + + test('Handles mixed keys with and without "="', () => { + const args = ['key1=value1', 'key2']; + const expectedMap = { key1: 'value1', key2: null }; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + + test('Handles strings with multiple "=" characters', () => { + const args = ['key1=part1=part2']; + const expectedMap = { key1: 'part1=part2' }; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + + test('Returns an empty map for an empty input array', () => { + const args: ReadonlyArray = []; + const expectedMap = {}; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + }); });