From f7e0e29267ea6e09c3ed530bbd3501e7134b21e6 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:39:47 -0800 Subject: [PATCH 1/2] support extra patching for doctest --- .../.data/doctest_patched_module.py | 17 +++++++ .../unittestadapter/.data/doctest_standard.py | 7 +++ .../.data/test_doctest_patched.py | 50 +++++++++++++++++++ .../.data/test_doctest_standard.py | 16 ++++++ .../tests/unittestadapter/test_utils.py | 45 +++++++++++++++++ python_files/unittestadapter/pvsc_utils.py | 13 ++--- 6 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 python_files/tests/unittestadapter/.data/doctest_patched_module.py create mode 100644 python_files/tests/unittestadapter/.data/doctest_standard.py create mode 100644 python_files/tests/unittestadapter/.data/test_doctest_patched.py create mode 100644 python_files/tests/unittestadapter/.data/test_doctest_standard.py diff --git a/python_files/tests/unittestadapter/.data/doctest_patched_module.py b/python_files/tests/unittestadapter/.data/doctest_patched_module.py new file mode 100644 index 000000000000..636c5320b6d6 --- /dev/null +++ b/python_files/tests/unittestadapter/.data/doctest_patched_module.py @@ -0,0 +1,17 @@ +""" +Patched doctest module. +This module's doctests will be patched to have proper IDs. + +>>> 2 + 2 +4 +""" + + +def example_function(): + """ + Example function with doctest. + + >>> example_function() + 'works' + """ + return "works" diff --git a/python_files/tests/unittestadapter/.data/doctest_standard.py b/python_files/tests/unittestadapter/.data/doctest_standard.py new file mode 100644 index 000000000000..52a10aa46a7f --- /dev/null +++ b/python_files/tests/unittestadapter/.data/doctest_standard.py @@ -0,0 +1,7 @@ +""" +Standard doctest module that should be blocked. +This has a simple doctest with short ID. + +>>> 2 + 2 +4 +""" diff --git a/python_files/tests/unittestadapter/.data/test_doctest_patched.py b/python_files/tests/unittestadapter/.data/test_doctest_patched.py new file mode 100644 index 000000000000..3a719c7139ca --- /dev/null +++ b/python_files/tests/unittestadapter/.data/test_doctest_patched.py @@ -0,0 +1,50 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""Test file with patched doctest integration that should work.""" + +import unittest +import doctest +import sys +import doctest_patched_module + + +# Patch DocTestCase to modify test IDs to be compatible with the extension +original_init = doctest.DocTestCase.__init__ + + +def patched_init(self, test, optionflags=0, setUp=None, tearDown=None, checker=None): + """Patch to modify doctest names to have proper hierarchy.""" + if hasattr(test, 'name'): + # Get module name + module_hierarchy = test.name.split('.') + module_name = module_hierarchy[0] if module_hierarchy else 'unknown' + + # Reconstruct with proper formatting to have enough components + # Format: module.file.class.function + if test.filename.endswith('.py'): + file_base = test.filename.split('/')[-1].replace('.py', '') + test_name = test.name.split('.')[-1] if '.' in test.name else test.name + # Create a properly formatted ID with enough components + test.name = f"{module_name}.{file_base}._DocTests.{test_name}" + + # Call original init + original_init(self, test, optionflags, setUp, tearDown, checker) + + +# Apply the patch +doctest.DocTestCase.__init__ = patched_init + + +def load_tests(loader, tests, ignore): + """ + Standard hook for unittest to load tests. + This uses patched doctest to create compatible test IDs. + """ + tests.addTests(doctest.DocTestSuite(doctest_patched_module)) + return tests + + +# Clean up the patch after loading +def tearDownModule(): + """Restore original DocTestCase.__init__""" + doctest.DocTestCase.__init__ = original_init diff --git a/python_files/tests/unittestadapter/.data/test_doctest_standard.py b/python_files/tests/unittestadapter/.data/test_doctest_standard.py new file mode 100644 index 000000000000..f5dba1209b98 --- /dev/null +++ b/python_files/tests/unittestadapter/.data/test_doctest_standard.py @@ -0,0 +1,16 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""Test file with standard doctest integration that should be blocked.""" + +import unittest +import doctest +import doctest_standard + + +def load_tests(loader, tests, ignore): + """ + Standard hook for unittest to load tests. + This uses standard doctest without any patching. + """ + tests.addTests(doctest.DocTestSuite(doctest_standard)) + return tests diff --git a/python_files/tests/unittestadapter/test_utils.py b/python_files/tests/unittestadapter/test_utils.py index 390b0779a44d..fcfdad78bb03 100644 --- a/python_files/tests/unittestadapter/test_utils.py +++ b/python_files/tests/unittestadapter/test_utils.py @@ -291,3 +291,48 @@ def test_build_empty_tree() -> None: assert tests is not None assert tests.get("children") == [] assert not errors + + +def test_doctest_standard_blocked() -> None: + """Standard doctests with short IDs should be skipped with an error message.""" + start_dir = os.fsdecode(TEST_DATA_PATH) + pattern = "test_doctest_standard*" + + loader = unittest.TestLoader() + suite = loader.discover(start_dir, pattern) + tests, errors = build_test_tree(suite, start_dir) + + # Should return a tree but with no test children (since doctests are skipped) + assert tests is not None + # Check that we got an error about doctests not being supported + assert len(errors) > 0 + assert "Skipping doctest as it is not supported for the extension" in errors[0] + + +def test_doctest_patched_works() -> None: + """Patched doctests with properly formatted IDs should be processed normally.""" + start_dir = os.fsdecode(TEST_DATA_PATH) + pattern = "test_doctest_patched*" + + loader = unittest.TestLoader() + suite = loader.discover(start_dir, pattern) + tests, errors = build_test_tree(suite, start_dir) + + # Should successfully build a tree with the patched doctest + assert tests is not None + # The patched doctests should have proper IDs and be included + # We should find at least one test child (the doctests that were patched) + def count_tests(node): + """Recursively count test nodes.""" + if node.get("type_") == "test": + return 1 + count = 0 + for child in node.get("children", []): + count += count_tests(child) + return count + + test_count = count_tests(tests) + # We expect at least the module doctest and function doctest + assert test_count > 0, "Patched doctests should be included in the tree" + # Should not have doctest-related errors since they're properly formatted + assert not any("doctest" in str(e).lower() for e in errors) diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index 09a5ec9f3be5..49a1ea1b18ae 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -203,12 +203,6 @@ def build_test_tree( root = build_test_node(top_level_directory, directory_path.name, TestNodeTypeEnum.folder) for test_case in get_test_case(suite): - if isinstance(test_case, doctest.DocTestCase): - print( - "Skipping doctest as it is not supported for the extension. Test case: ", test_case - ) - error = ["Skipping doctest as it is not supported for the extension."] - continue test_id = test_case.id() if test_id.startswith("unittest.loader._FailedTest"): error.append(str(test_case._exception)) # type: ignore # noqa: SLF001 @@ -221,6 +215,13 @@ def build_test_tree( else: # Get the static test path components: filename, class name and function name. components = test_id.split(".") + # Check if this is a doctest with insufficient components that would cause unpacking to fail + if len(components) < 3 and isinstance(test_case, doctest.DocTestCase): + print( + "Skipping doctest as it is not supported for the extension. Test case: ", test_case + ) + error = ["Skipping doctest as it is not supported for the extension."] + continue *folders, filename, class_name, function_name = components py_filename = f"{filename}.py" From 1d76931d0701ba1a64064aa049a25b16a433858e Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:03:29 -0800 Subject: [PATCH 2/2] linting --- .../python-quality-checks.instructions.md | 21 +++++++++++++++++++ .../tests/unittestadapter/test_utils.py | 1 + python_files/unittestadapter/pvsc_utils.py | 3 ++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/instructions/python-quality-checks.instructions.md b/.github/instructions/python-quality-checks.instructions.md index d07699965cc2..48f37529dfbc 100644 --- a/.github/instructions/python-quality-checks.instructions.md +++ b/.github/instructions/python-quality-checks.instructions.md @@ -69,8 +69,29 @@ nox --session install_python_libs **Type errors in ignored files**: Legacy files in `pyproject.toml` ignore list—fix if working on them +## When Writing Tests + +**Always format your test files before committing:** + +```bash +cd python_files +ruff format tests/ # Format all test files +# or format specific files: +ruff format tests/unittestadapter/test_utils.py +``` + +**Best practice workflow:** + +1. Write your test code +2. Run `ruff format` on the test files +3. Run the tests to verify they pass +4. Run `npm run check-python` to catch any remaining issues + +This ensures your tests pass both functional checks and quality checks in CI. + ## Learnings - Always run `npm run check-python` before pushing to catch CI failures early (1) - Use `ruff check . --fix` to auto-fix most linting issues before manual review (1) - Pyright version must match CI (1.1.308) to avoid inconsistent results between local and CI runs (1) +- Always run `ruff format` on test files after writing them to avoid formatting CI failures (1) diff --git a/python_files/tests/unittestadapter/test_utils.py b/python_files/tests/unittestadapter/test_utils.py index fcfdad78bb03..dc8a81175e70 100644 --- a/python_files/tests/unittestadapter/test_utils.py +++ b/python_files/tests/unittestadapter/test_utils.py @@ -320,6 +320,7 @@ def test_doctest_patched_works() -> None: # Should successfully build a tree with the patched doctest assert tests is not None + # The patched doctests should have proper IDs and be included # We should find at least one test child (the doctests that were patched) def count_tests(node): diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index 49a1ea1b18ae..e9d7bc092992 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -218,7 +218,8 @@ def build_test_tree( # Check if this is a doctest with insufficient components that would cause unpacking to fail if len(components) < 3 and isinstance(test_case, doctest.DocTestCase): print( - "Skipping doctest as it is not supported for the extension. Test case: ", test_case + "Skipping doctest as it is not supported for the extension. Test case: ", + test_case, ) error = ["Skipping doctest as it is not supported for the extension."] continue