Skip to content

Commit d970068

Browse files
authored
support extra patching for doctest (#25591)
fixes #25469
1 parent cd5ecb9 commit d970068

File tree

7 files changed

+165
-6
lines changed

7 files changed

+165
-6
lines changed

.github/instructions/python-quality-checks.instructions.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,29 @@ nox --session install_python_libs
6969

7070
**Type errors in ignored files**: Legacy files in `pyproject.toml` ignore list—fix if working on them
7171

72+
## When Writing Tests
73+
74+
**Always format your test files before committing:**
75+
76+
```bash
77+
cd python_files
78+
ruff format tests/ # Format all test files
79+
# or format specific files:
80+
ruff format tests/unittestadapter/test_utils.py
81+
```
82+
83+
**Best practice workflow:**
84+
85+
1. Write your test code
86+
2. Run `ruff format` on the test files
87+
3. Run the tests to verify they pass
88+
4. Run `npm run check-python` to catch any remaining issues
89+
90+
This ensures your tests pass both functional checks and quality checks in CI.
91+
7292
## Learnings
7393

7494
- Always run `npm run check-python` before pushing to catch CI failures early (1)
7595
- Use `ruff check . --fix` to auto-fix most linting issues before manual review (1)
7696
- Pyright version must match CI (1.1.308) to avoid inconsistent results between local and CI runs (1)
97+
- Always run `ruff format` on test files after writing them to avoid formatting CI failures (1)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
"""
2+
Patched doctest module.
3+
This module's doctests will be patched to have proper IDs.
4+
5+
>>> 2 + 2
6+
4
7+
"""
8+
9+
10+
def example_function():
11+
"""
12+
Example function with doctest.
13+
14+
>>> example_function()
15+
'works'
16+
"""
17+
return "works"
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
"""
2+
Standard doctest module that should be blocked.
3+
This has a simple doctest with short ID.
4+
5+
>>> 2 + 2
6+
4
7+
"""
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
"""Test file with patched doctest integration that should work."""
4+
5+
import unittest
6+
import doctest
7+
import sys
8+
import doctest_patched_module
9+
10+
11+
# Patch DocTestCase to modify test IDs to be compatible with the extension
12+
original_init = doctest.DocTestCase.__init__
13+
14+
15+
def patched_init(self, test, optionflags=0, setUp=None, tearDown=None, checker=None):
16+
"""Patch to modify doctest names to have proper hierarchy."""
17+
if hasattr(test, 'name'):
18+
# Get module name
19+
module_hierarchy = test.name.split('.')
20+
module_name = module_hierarchy[0] if module_hierarchy else 'unknown'
21+
22+
# Reconstruct with proper formatting to have enough components
23+
# Format: module.file.class.function
24+
if test.filename.endswith('.py'):
25+
file_base = test.filename.split('/')[-1].replace('.py', '')
26+
test_name = test.name.split('.')[-1] if '.' in test.name else test.name
27+
# Create a properly formatted ID with enough components
28+
test.name = f"{module_name}.{file_base}._DocTests.{test_name}"
29+
30+
# Call original init
31+
original_init(self, test, optionflags, setUp, tearDown, checker)
32+
33+
34+
# Apply the patch
35+
doctest.DocTestCase.__init__ = patched_init
36+
37+
38+
def load_tests(loader, tests, ignore):
39+
"""
40+
Standard hook for unittest to load tests.
41+
This uses patched doctest to create compatible test IDs.
42+
"""
43+
tests.addTests(doctest.DocTestSuite(doctest_patched_module))
44+
return tests
45+
46+
47+
# Clean up the patch after loading
48+
def tearDownModule():
49+
"""Restore original DocTestCase.__init__"""
50+
doctest.DocTestCase.__init__ = original_init
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
"""Test file with standard doctest integration that should be blocked."""
4+
5+
import unittest
6+
import doctest
7+
import doctest_standard
8+
9+
10+
def load_tests(loader, tests, ignore):
11+
"""
12+
Standard hook for unittest to load tests.
13+
This uses standard doctest without any patching.
14+
"""
15+
tests.addTests(doctest.DocTestSuite(doctest_standard))
16+
return tests

python_files/tests/unittestadapter/test_utils.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,49 @@ def test_build_empty_tree() -> None:
291291
assert tests is not None
292292
assert tests.get("children") == []
293293
assert not errors
294+
295+
296+
def test_doctest_standard_blocked() -> None:
297+
"""Standard doctests with short IDs should be skipped with an error message."""
298+
start_dir = os.fsdecode(TEST_DATA_PATH)
299+
pattern = "test_doctest_standard*"
300+
301+
loader = unittest.TestLoader()
302+
suite = loader.discover(start_dir, pattern)
303+
tests, errors = build_test_tree(suite, start_dir)
304+
305+
# Should return a tree but with no test children (since doctests are skipped)
306+
assert tests is not None
307+
# Check that we got an error about doctests not being supported
308+
assert len(errors) > 0
309+
assert "Skipping doctest as it is not supported for the extension" in errors[0]
310+
311+
312+
def test_doctest_patched_works() -> None:
313+
"""Patched doctests with properly formatted IDs should be processed normally."""
314+
start_dir = os.fsdecode(TEST_DATA_PATH)
315+
pattern = "test_doctest_patched*"
316+
317+
loader = unittest.TestLoader()
318+
suite = loader.discover(start_dir, pattern)
319+
tests, errors = build_test_tree(suite, start_dir)
320+
321+
# Should successfully build a tree with the patched doctest
322+
assert tests is not None
323+
324+
# The patched doctests should have proper IDs and be included
325+
# We should find at least one test child (the doctests that were patched)
326+
def count_tests(node):
327+
"""Recursively count test nodes."""
328+
if node.get("type_") == "test":
329+
return 1
330+
count = 0
331+
for child in node.get("children", []):
332+
count += count_tests(child)
333+
return count
334+
335+
test_count = count_tests(tests)
336+
# We expect at least the module doctest and function doctest
337+
assert test_count > 0, "Patched doctests should be included in the tree"
338+
# Should not have doctest-related errors since they're properly formatted
339+
assert not any("doctest" in str(e).lower() for e in errors)

python_files/unittestadapter/pvsc_utils.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,6 @@ def build_test_tree(
203203
root = build_test_node(top_level_directory, directory_path.name, TestNodeTypeEnum.folder)
204204

205205
for test_case in get_test_case(suite):
206-
if isinstance(test_case, doctest.DocTestCase):
207-
print(
208-
"Skipping doctest as it is not supported for the extension. Test case: ", test_case
209-
)
210-
error = ["Skipping doctest as it is not supported for the extension."]
211-
continue
212206
test_id = test_case.id()
213207
if test_id.startswith("unittest.loader._FailedTest"):
214208
error.append(str(test_case._exception)) # type: ignore # noqa: SLF001
@@ -221,6 +215,14 @@ def build_test_tree(
221215
else:
222216
# Get the static test path components: filename, class name and function name.
223217
components = test_id.split(".")
218+
# Check if this is a doctest with insufficient components that would cause unpacking to fail
219+
if len(components) < 3 and isinstance(test_case, doctest.DocTestCase):
220+
print(
221+
"Skipping doctest as it is not supported for the extension. Test case: ",
222+
test_case,
223+
)
224+
error = ["Skipping doctest as it is not supported for the extension."]
225+
continue
224226
*folders, filename, class_name, function_name = components
225227
py_filename = f"{filename}.py"
226228

0 commit comments

Comments
 (0)