Code review

Author: nicoddemus

changelog/1367.feature.rst

@@ -6,13 +6,18 @@
 
 .. code-block:: python
 
-    def test(subtests):
-        for i in range(5):
-            with subtests.test(msg="custom message", i=i):
-                assert i % 2 == 0
+    def contains_docstring(p: Path) -> bool:
+        """Return True if the given Python file contains a top-level docstring."""
+        ...
 
 
-Each assert failure or error is caught by the context manager and reported individually.
+    def test_py_files_contain_docstring(subtests: pytest.Subtests) -> None:
+        for path in Path.cwd().glob("*.py"):
+            with subtests.test(path=str(path)):
+                assert contains_docstring(path)
+
+
+Each assert failure or error is caught by the context manager and reported individually, giving a clear picture of all files that are missing a docstring.
 
 In addition, :meth:`unittest.TestCase.subTest` is now also supported.
 

doc/en/how-to/subtests.rst

@@ -16,13 +16,15 @@ Subtests are an alternative to parametrization, particularly useful when the exa
 
 .. code-block:: python
 
-    # content of test_subtest.py
+    def contains_docstring(p: Path) -> bool:
+        """Return True if the given Python file contains a top-level docstring."""
+        ...
 
 
-    def test(subtests):
-        for i in range(5):
-            with subtests.test(msg="custom message", i=i):
-                assert i % 2 == 0
+    def test_py_files_contain_docstring(subtests: pytest.Subtests) -> None:
+        for path in Path.cwd().glob("*.py"):
+            with subtests.test(path=str(path)):
+                assert contains_docstring(path)
 
 Each assertion failure or error is caught by the context manager and reported individually:
 
@@ -38,13 +40,13 @@ outside the ``subtests.test`` block:
 
     def test(subtests):
         for i in range(5):
-            with subtests.test(msg="stage 1", i=i):
+            with subtests.test("stage 1", i=i):
                 assert i % 2 == 0
 
         assert func() == 10
 
         for i in range(10, 20):
-            with subtests.test(msg="stage 2", i=i):
+            with subtests.test("stage 2", i=i):
                 assert i % 2 == 0
 
 .. note::

src/_pytest/subtests.py

@@ -13,12 +13,12 @@
 import time
 from types import TracebackType
 from typing import Any
-from typing import Literal
 from typing import TYPE_CHECKING
 
 import pluggy
 
 from _pytest._code import ExceptionInfo
+from _pytest._io.saferepr import saferepr
 from _pytest.capture import CaptureFixture
 from _pytest.capture import FDCapture
 from _pytest.capture import SysCapture
@@ -83,11 +83,11 @@ def head_line(self) -> str:
 
     def _sub_test_description(self) -> str:
         parts = []
-        if isinstance(self.context.msg, str):
+        if self.context.msg is not None:
             parts.append(f"[{self.context.msg}]")
         if self.context.kwargs:
             params_desc = ", ".join(
-                f"{k}={v!r}" for (k, v) in sorted(self.context.kwargs.items())
+                f"{k}={saferepr(v)}" for (k, v) in self.context.kwargs.items()
             )
             parts.append(f"({params_desc})")
         return " ".join(parts) or "(<subtest>)"
@@ -106,8 +106,12 @@ def _from_json(cls, reportdict: dict[str, Any]) -> SubtestReport:
         return report
 
     @classmethod
-    def _from_test_report(cls, test_report: TestReport) -> SubtestReport:
-        return super()._from_json(test_report._to_json())
+    def _from_test_report(
+        cls, test_report: TestReport, context: SubtestContext
+    ) -> Self:
+        result = super()._from_json(test_report._to_json())
+        result.context = context
+        return result
 
 
 @fixture
@@ -121,8 +125,6 @@ def subtests(request: SubRequest) -> Subtests:
     return Subtests(request.node.ihook, suspend_capture_ctx, request, _ispytest=True)
 
 
-# Note: cannot use a dataclass here because Sphinx insists on showing up the __init__ method in the documentation,
-# even if we explicitly use :exclude-members: __init__.
 class Subtests:
     """Subtests fixture, enables declaring subtests inside test functions via the :meth:`test` method."""
 
@@ -178,12 +180,13 @@ class _SubTestContextManager:
     """
     Context manager for subtests, capturing exceptions raised inside the subtest scope and handling
     them through the pytest machinery.
-
-    Note: initially this logic was implemented directly in Subtests.test() as a @contextmanager, however
-    it is not possible to control the output fully when exiting from it due to an exception when
-    in --exitfirst mode, so this was refactored into an explicit context manager class (pytest-dev/pytest-subtests#134).
     """
 
+    # Note: initially the logic for this context manager was implemented directly
+    # in Subtests.test() as a @contextmanager, however, it is not possible to control the output fully when
+    # exiting from it due to an exception when in `--exitfirst` mode, so this was refactored into an
+    # explicit context manager class (pytest-dev/pytest-subtests#134).
+
     ihook: pluggy.HookRelay
     msg: str | None
     kwargs: dict[str, Any]
@@ -224,14 +227,21 @@ def __exit__(
         duration = precise_stop - self._precise_start
         stop = time.time()
 
-        call_info = make_call_info(
-            exc_info, start=self._start, stop=stop, duration=duration, when="call"
+        call_info = CallInfo[None](
+            None,
+            exc_info,
+            start=self._start,
+            stop=stop,
+            duration=duration,
+            when="call",
+            _ispytest=True,
         )
         report = self.ihook.pytest_runtest_makereport(
             item=self.request.node, call=call_info
         )
-        sub_report = SubtestReport._from_test_report(report)
-        sub_report.context = SubtestContext(msg=self.msg, kwargs=self.kwargs.copy())
+        sub_report = SubtestReport._from_test_report(
+            report, SubtestContext(msg=self.msg, kwargs=self.kwargs.copy())
+        )
 
         self._captured_output.update_report(sub_report)
         self._captured_logs.update_report(sub_report)
@@ -250,25 +260,6 @@ def __exit__(
         return True
 
 
-def make_call_info(
-    exc_info: ExceptionInfo[BaseException] | None,
-    *,
-    start: float,
-    stop: float,
-    duration: float,
-    when: Literal["collect", "setup", "call", "teardown"],
-) -> CallInfo[Any]:
-    return CallInfo(
-        None,
-        exc_info,
-        start=start,
-        stop=stop,
-        duration=duration,
-        when=when,
-        _ispytest=True,
-    )
-
-
 @contextmanager
 def capturing_output(request: SubRequest) -> Iterator[Captured]:
     option = request.config.getoption("capture", None)

src/_pytest/unittest.py

@@ -35,7 +35,6 @@
 from _pytest.python import Module
 from _pytest.runner import CallInfo
 from _pytest.runner import check_interactive_exception
-from _pytest.subtests import make_call_info
 from _pytest.subtests import SubtestContext
 from _pytest.subtests import SubtestReport
 
@@ -419,17 +418,21 @@ def addSubTest(
             case unreachable:
                 assert_never(unreachable)
 
-        call_info = make_call_info(
+        call_info = CallInfo[None](
+            None,
             exception_info,
             start=0,
             stop=0,
             duration=0,
             when="call",
+            _ispytest=True,
         )
         msg = test._message if isinstance(test._message, str) else None  # type: ignore[attr-defined]
         report = self.ihook.pytest_runtest_makereport(item=self, call=call_info)
-        sub_report = SubtestReport._from_test_report(report)
-        sub_report.context = SubtestContext(msg=msg, kwargs=dict(test.params))  # type: ignore[attr-defined]
+        sub_report = SubtestReport._from_test_report(
+            report,
+            SubtestContext(msg=msg, kwargs=dict(test.params)),  # type: ignore[attr-defined]
+        )
         self.ihook.pytest_runtest_logreport(report=sub_report)
         if check_interactive_exception(call_info, sub_report):
             self.ihook.pytest_exception_interact(

testing/test_subtests.py

@@ -220,7 +220,7 @@ def test_subtests_and_parametrization(pytester: pytest.Pytester) -> None:
         @pytest.mark.parametrize("x", [0, 1])
         def test_foo(subtests, x):
             for i in range(3):
-                with subtests.test(msg="custom", i=i):
+                with subtests.test("custom", i=i):
                     assert i % 2 == 0
             assert x == 0
     """