From 4f90f81183b0d9c844e293beb81af77e8bfccdcd Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 10 Jul 2025 10:31:25 -0700 Subject: [PATCH 1/4] support --black arg for pytest --- .../pytest-json-test-builder.instructions.md | 126 ++++++++++++++++++ build/test-requirements.txt | 1 + .../.data/2496-black-formatter/app.py | 6 + .../.data/2496-black-formatter/test_app.py | 14 ++ .../expected_discovery_test_output.py | 99 ++++++++++++++ .../tests/pytestadapter/test_discovery.py | 1 + python_files/vscode_pytest/__init__.py | 2 - .../testing/testController/common/utils.ts | 10 +- 8 files changed, 256 insertions(+), 3 deletions(-) create mode 100644 .github/instructions/pytest-json-test-builder.instructions.md create mode 100644 python_files/tests/pytestadapter/.data/2496-black-formatter/app.py create mode 100644 python_files/tests/pytestadapter/.data/2496-black-formatter/test_app.py diff --git a/.github/instructions/pytest-json-test-builder.instructions.md b/.github/instructions/pytest-json-test-builder.instructions.md new file mode 100644 index 000000000000..436bce0c9cd8 --- /dev/null +++ b/.github/instructions/pytest-json-test-builder.instructions.md @@ -0,0 +1,126 @@ +--- +applyTo: 'python_files/tests/pytestadapter/test_discovery.py' +description: 'A guide for adding new tests for pytest discovery and JSON formatting in the test_pytest_collect suite.' +--- + +# How to Add New Pytest Discovery Tests + +This guide explains how to add new tests for pytest discovery and JSON formatting in the `test_pytest_collect` suite. Follow these steps to ensure your tests are consistent and correct. + +--- + +## 1. Add Your Test File + +- Place your new test file/files in the appropriate subfolder under: + ``` + python_files/tests/pytestadapter/.data/ + ``` +- Organize folders and files to match the structure you want to test. For example, to test nested folders, create the corresponding directory structure. +- In your test file, mark each test function with a comment: + ```python + def test_function(): # test_marker--test_function + ... + ``` + +**Root Node Matching:** + +- The root node in your expected output must match the folder or file you pass to pytest discovery. For example, if you run discovery on a subfolder, the root `"name"`, `"path"`, and `"id_"` in your expected output should be that subfolder, not the parent `.data` folder. +- Only use `.data` as the root if you are running discovery on the entire `.data` folder. + +**Example:** +If you run: + +```python +helpers.runner([os.fspath(TEST_DATA_PATH / "myfolder"), "--collect-only"]) +``` + +then your expected output root should be: + +```python +{ + "name": "myfolder", + "path": os.fspath(TEST_DATA_PATH / "myfolder"), + "type_": "folder", + ... +} +``` + +--- + +## 2. Update `expected_discovery_test_output.py` + +- Open `expected_discovery_test_output.py` in the same test suite. +- Add a new expected output dictionary for your test file, following the format of existing entries. +- Use the helper functions and path conventions: + - Use `os.fspath()` for all paths. + - Use `find_test_line_number("function_name", file_path)` for the `lineno` field. + - Use `get_absolute_test_id("relative_path::function_name", file_path)` for `id_` and `runID`. + - Always use current path concatenation (e.g., `TEST_DATA_PATH / "your_folder" / "your_file.py"`). + - Create new constants as needed to keep the code clean and maintainable. + +**Important:** + +- Do **not** read the entire `expected_discovery_test_output.py` file if you only need to add or reference a single constant. This file is very large; prefer searching for the relevant section or appending to the end. + +**Example:** +If you run discovery on a subfolder: + +```python +helpers.runner([os.fspath(TEST_DATA_PATH / "myfolder"), "--collect-only"]) +``` + +then your expected output root should be: + +```python +myfolder_path = TEST_DATA_PATH / "myfolder" +my_expected_output = { + "name": "myfolder", + "path": os.fspath(myfolder_path), + "type_": "folder", + ... +} +``` + +- Add a comment above your dictionary describing the structure, as in the existing examples. + +--- + +## 3. Add Your Test to `test_discovery.py` + +- In `test_discovery.py`, add your new test as a parameterized case to the main `test_pytest_collect` function. Do **not** create a standalone test function for new discovery cases. +- Reference your new expected output constant from `expected_discovery_test_output.py`. + +**Example:** + +```python +@pytest.mark.parametrize( + ("file", "expected_const"), + [ + ("myfolder", my_expected_output), + # ... other cases ... + ], +) +def test_pytest_collect(file, expected_const): + ... +``` + +--- + +## 4. Run and Verify + +- Run the test suite to ensure your new test is discovered and passes. +- If the test fails, check your expected output dictionary for path or structure mismatches. + +--- + +## 5. Tips + +- Always use the helper functions for line numbers and IDs. +- Match the folder/file structure in `.data` to the expected JSON structure. +- Use comments to document the expected output structure for clarity. +- Ensure all `"path"` and `"id_"` fields in your expected output match exactly what pytest returns, including absolute paths and root node structure. + +--- + +**Reference:** +See `expected_discovery_test_output.py` for more examples and formatting. Use search or jump to the end of the file to avoid reading the entire file when possible. diff --git a/build/test-requirements.txt b/build/test-requirements.txt index df9fd2b08c6e..6d64ff72ac7f 100644 --- a/build/test-requirements.txt +++ b/build/test-requirements.txt @@ -39,3 +39,4 @@ pytest-describe # for pytest-ruff related tests pytest-ruff +pytest-black diff --git a/python_files/tests/pytestadapter/.data/2496-black-formatter/app.py b/python_files/tests/pytestadapter/.data/2496-black-formatter/app.py new file mode 100644 index 000000000000..3b474e9d911e --- /dev/null +++ b/python_files/tests/pytestadapter/.data/2496-black-formatter/app.py @@ -0,0 +1,6 @@ +def add(a, b): + return a + b + + +def subtract(a, b): + return a - b diff --git a/python_files/tests/pytestadapter/.data/2496-black-formatter/test_app.py b/python_files/tests/pytestadapter/.data/2496-black-formatter/test_app.py new file mode 100644 index 000000000000..ef4398feb786 --- /dev/null +++ b/python_files/tests/pytestadapter/.data/2496-black-formatter/test_app.py @@ -0,0 +1,14 @@ +import pytest +from app import add, subtract + + +def test_add(): # test_marker--test_add + assert add(2, 3) == 5 + assert add(-1, 1) == 0 + assert add(0, 0) == 0 + + +def test_subtract(): # test_marker--test_subtract + assert subtract(5, 3) == 2 + assert subtract(0, 0) == 0 + assert subtract(-1, -1) == 0 diff --git a/python_files/tests/pytestadapter/expected_discovery_test_output.py b/python_files/tests/pytestadapter/expected_discovery_test_output.py index 13bb6ee983cf..31bd71df74fb 100644 --- a/python_files/tests/pytestadapter/expected_discovery_test_output.py +++ b/python_files/tests/pytestadapter/expected_discovery_test_output.py @@ -1719,3 +1719,102 @@ ], "id_": TEST_DATA_PATH_STR, } + +# This is the expected output for the 2496-black-formatter folder when run with black plugin +# └── .data +# └── 2496-black-formatter +# └── app.py +# └── black +# └── test_app.py +# └── black +# └── test_add +# └── test_subtract +black_formatter_folder_path = TEST_DATA_PATH / "2496-black-formatter" +black_app_path = black_formatter_folder_path / "app.py" +black_test_app_path = black_formatter_folder_path / "test_app.py" +black_formatter_expected_output = { + "name": "2496-black-formatter", + "path": os.fspath(black_formatter_folder_path), + "type_": "folder", + "id_": os.fspath(black_formatter_folder_path), + "children": [ + { + "name": "app.py", + "path": os.fspath(black_app_path), + "type_": "file", + "id_": os.fspath(black_app_path), + "children": [ + { + "name": "black", + "path": os.fspath(black_app_path), + "lineno": "0", + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/app.py::black", + black_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/app.py::black", + black_app_path, + ), + } + ], + }, + { + "name": "test_app.py", + "path": os.fspath(black_test_app_path), + "type_": "file", + "id_": os.fspath(black_test_app_path), + "children": [ + { + "name": "black", + "path": os.fspath(black_test_app_path), + "lineno": "0", + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::black", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::black", + black_test_app_path, + ), + }, + { + "name": "test_add", + "path": os.fspath(black_test_app_path), + "lineno": find_test_line_number( + "test_add", + black_test_app_path, + ), + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_add", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_add", + black_test_app_path, + ), + }, + { + "name": "test_subtract", + "path": os.fspath(black_test_app_path), + "lineno": find_test_line_number( + "test_subtract", + black_test_app_path, + ), + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_subtract", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_subtract", + black_test_app_path, + ), + }, + ], + }, + ], +} diff --git a/python_files/tests/pytestadapter/test_discovery.py b/python_files/tests/pytestadapter/test_discovery.py index 4f9fe3eb19ac..1d43dcb02f7a 100644 --- a/python_files/tests/pytestadapter/test_discovery.py +++ b/python_files/tests/pytestadapter/test_discovery.py @@ -169,6 +169,7 @@ def test_parameterized_error_collect(): "pytest_describe_plugin" + os.path.sep + "nested_describe.py", expected_discovery_test_output.expected_nested_describe_output, ), + ("2496-black-formatter", expected_discovery_test_output.black_formatter_expected_output), ], ) def test_pytest_collect(file, expected_const): diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index de396d8520ef..106ffdcdd4bb 100644 --- a/python_files/vscode_pytest/__init__.py +++ b/python_files/vscode_pytest/__init__.py @@ -18,8 +18,6 @@ from pluggy import Result -USES_PYTEST_DESCRIBE = False - with contextlib.suppress(ImportError): from pytest_describe.plugin import DescribeBlock diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index c624ef034cf1..f0ce8ebeff8d 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -209,7 +209,15 @@ export function populateTestTree( let range: Range | undefined; if (child.lineno) { - range = new Range(new Position(Number(child.lineno) - 1, 0), new Position(Number(child.lineno), 0)); + // if lineno is '0' + if (Number(child.lineno) === 0) { + range = new Range(new Position(0, 0), new Position(0, 0)); + } else { + range = new Range( + new Position(Number(child.lineno) - 1, 0), + new Position(Number(child.lineno), 0), + ); + } } testItem.canResolveChildren = false; testItem.range = range; From acb3bad4f8c800879a84f8ad699e1111ca0ede29 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 10 Jul 2025 10:32:29 -0700 Subject: [PATCH 2/4] fix error --- python_files/vscode_pytest/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index 106ffdcdd4bb..72eaa7a787d5 100644 --- a/python_files/vscode_pytest/__init__.py +++ b/python_files/vscode_pytest/__init__.py @@ -17,6 +17,7 @@ if TYPE_CHECKING: from pluggy import Result +USES_PYTEST_DESCRIBE = False with contextlib.suppress(ImportError): from pytest_describe.plugin import DescribeBlock From 369a6c05a34478e92a9462ddd3b02442129247bd Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 10 Jul 2025 10:33:07 -0700 Subject: [PATCH 3/4] clarity --- src/client/testing/testController/common/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index f0ce8ebeff8d..f4647d20666d 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -209,7 +209,6 @@ export function populateTestTree( let range: Range | undefined; if (child.lineno) { - // if lineno is '0' if (Number(child.lineno) === 0) { range = new Range(new Position(0, 0), new Position(0, 0)); } else { From 55961e2fa74bd9511deb67524dd34603bf54db59 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 10 Jul 2025 11:05:13 -0700 Subject: [PATCH 4/4] fix test --- .../expected_discovery_test_output.py | 152 +++++++++--------- .../tests/pytestadapter/test_discovery.py | 39 +++-- 2 files changed, 110 insertions(+), 81 deletions(-) diff --git a/python_files/tests/pytestadapter/expected_discovery_test_output.py b/python_files/tests/pytestadapter/expected_discovery_test_output.py index 31bd71df74fb..e00db5d660a3 100644 --- a/python_files/tests/pytestadapter/expected_discovery_test_output.py +++ b/python_files/tests/pytestadapter/expected_discovery_test_output.py @@ -1733,88 +1733,96 @@ black_app_path = black_formatter_folder_path / "app.py" black_test_app_path = black_formatter_folder_path / "test_app.py" black_formatter_expected_output = { - "name": "2496-black-formatter", - "path": os.fspath(black_formatter_folder_path), + "name": ".data", + "path": TEST_DATA_PATH_STR, "type_": "folder", - "id_": os.fspath(black_formatter_folder_path), "children": [ { - "name": "app.py", - "path": os.fspath(black_app_path), - "type_": "file", - "id_": os.fspath(black_app_path), + "name": "2496-black-formatter", + "path": os.fspath(black_formatter_folder_path), + "type_": "folder", + "id_": os.fspath(black_formatter_folder_path), "children": [ { - "name": "black", + "name": "app.py", "path": os.fspath(black_app_path), - "lineno": "0", - "type_": "test", - "id_": get_absolute_test_id( - "2496-black-formatter/app.py::black", - black_app_path, - ), - "runID": get_absolute_test_id( - "2496-black-formatter/app.py::black", - black_app_path, - ), - } - ], - }, - { - "name": "test_app.py", - "path": os.fspath(black_test_app_path), - "type_": "file", - "id_": os.fspath(black_test_app_path), - "children": [ - { - "name": "black", - "path": os.fspath(black_test_app_path), - "lineno": "0", - "type_": "test", - "id_": get_absolute_test_id( - "2496-black-formatter/test_app.py::black", - black_test_app_path, - ), - "runID": get_absolute_test_id( - "2496-black-formatter/test_app.py::black", - black_test_app_path, - ), - }, - { - "name": "test_add", - "path": os.fspath(black_test_app_path), - "lineno": find_test_line_number( - "test_add", - black_test_app_path, - ), - "type_": "test", - "id_": get_absolute_test_id( - "2496-black-formatter/test_app.py::test_add", - black_test_app_path, - ), - "runID": get_absolute_test_id( - "2496-black-formatter/test_app.py::test_add", - black_test_app_path, - ), + "type_": "file", + "id_": os.fspath(black_app_path), + "children": [ + { + "name": "black", + "path": os.fspath(black_app_path), + "lineno": "0", + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/app.py::black", + black_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/app.py::black", + black_app_path, + ), + } + ], }, { - "name": "test_subtract", + "name": "test_app.py", "path": os.fspath(black_test_app_path), - "lineno": find_test_line_number( - "test_subtract", - black_test_app_path, - ), - "type_": "test", - "id_": get_absolute_test_id( - "2496-black-formatter/test_app.py::test_subtract", - black_test_app_path, - ), - "runID": get_absolute_test_id( - "2496-black-formatter/test_app.py::test_subtract", - black_test_app_path, - ), + "type_": "file", + "id_": os.fspath(black_test_app_path), + "children": [ + { + "name": "black", + "path": os.fspath(black_test_app_path), + "lineno": "0", + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::black", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::black", + black_test_app_path, + ), + }, + { + "name": "test_add", + "path": os.fspath(black_test_app_path), + "lineno": find_test_line_number( + "test_add", + black_test_app_path, + ), + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_add", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_add", + black_test_app_path, + ), + }, + { + "name": "test_subtract", + "path": os.fspath(black_test_app_path), + "lineno": find_test_line_number( + "test_subtract", + black_test_app_path, + ), + "type_": "test", + "id_": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_subtract", + black_test_app_path, + ), + "runID": get_absolute_test_id( + "2496-black-formatter/test_app.py::test_subtract", + black_test_app_path, + ), + }, + ], }, ], - }, + } ], + "id_": TEST_DATA_PATH_STR, } diff --git a/python_files/tests/pytestadapter/test_discovery.py b/python_files/tests/pytestadapter/test_discovery.py index 1d43dcb02f7a..842ee3c7c707 100644 --- a/python_files/tests/pytestadapter/test_discovery.py +++ b/python_files/tests/pytestadapter/test_discovery.py @@ -169,7 +169,6 @@ def test_parameterized_error_collect(): "pytest_describe_plugin" + os.path.sep + "nested_describe.py", expected_discovery_test_output.expected_nested_describe_output, ), - ("2496-black-formatter", expected_discovery_test_output.black_formatter_expected_output), ], ) def test_pytest_collect(file, expected_const): @@ -338,15 +337,37 @@ def test_config_sub_folder(): assert tests.get("name") == "config_sub_folder" -def test_ruff_plugin(): - """Here the session node will be a subfolder of the workspace root and the test are in another subfolder. +@pytest.mark.parametrize( + ("file", "expected_const", "extra_arg"), + [ + ( + "folder_with_script", + expected_discovery_test_output.ruff_test_expected_output, + "--ruff", + ), + ( + "2496-black-formatter", + expected_discovery_test_output.black_formatter_expected_output, + "--black", + ), + ], +) +def test_plugin_collect(file, expected_const, extra_arg): + """Test pytest discovery on a folder with a plugin argument (e.g., --ruff, --black). - This tests checks to see if test node path are under the session node and if so the - session node is correctly updated to the common path. + Uses variables from expected_discovery_test_output.py to store the expected + dictionary return. Only handles discovery and therefore already contains the arg + --collect-only. All test discovery will succeed, be in the correct cwd, and match + expected test output. + + Keyword arguments: + file -- a string with the file or folder to run pytest discovery on. + expected_const -- the expected output from running pytest discovery on the file. + extra_arg -- the extra plugin argument to pass (e.g., --ruff, --black) """ - file_path = helpers.TEST_DATA_PATH / "folder_with_script" + file_path = helpers.TEST_DATA_PATH / file actual = helpers.runner( - [os.fspath(file_path), "--collect-only", "--ruff"], + [os.fspath(file_path), "--collect-only", extra_arg], ) assert actual @@ -360,8 +381,8 @@ def test_ruff_plugin(): assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH) assert is_same_tree( actual_item.get("tests"), - expected_discovery_test_output.ruff_test_expected_output, + expected_const, ["id_", "lineno", "name", "runID"], ), ( - f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_discovery_test_output.ruff_test_expected_output, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}" + f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_const, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}" )