Code review

Author: nicoddemus

changelog/1367.feature.rst

@@ -1,6 +1,6 @@
 **Support for subtests** has been added.
 
-:ref:`subtests <subtests>` are an alternative to parametrization, useful in situations where test setup is expensive or the parametrization values are not all known at collection time.
+:ref:`subtests <subtests>` are an alternative to parametrization, useful in situations where the parametrization values are not all known at collection time.
 
 **Example**
 

doc/en/how-to/subtests.rst

@@ -11,7 +11,7 @@ How to use subtests
 
 pytest allows for grouping assertions within a normal test, known as *subtests*.
 
-Subtests are an alternative to parametrization, particularly useful when test setup is expensive or when the exact parametrization values are not known at collection time.
+Subtests are an alternative to parametrization, particularly useful when the exact parametrization values are not known at collection time.
 
 
 .. code-block:: python
@@ -55,11 +55,11 @@ outside the ``subtests.test`` block:
 Typing
 ------
 
-:class:`pytest.SubTests` is exported so it can be used in type annotations:
+:class:`pytest.Subtests` is exported so it can be used in type annotations:
 
 .. code-block:: python
 
-    def test(subtests: pytest.SubTests) -> None: ...
+    def test(subtests: pytest.Subtests) -> None: ...
 
 .. _parametrize_vs_subtests:
 

src/_pytest/subtests.py

@@ -1,7 +1,8 @@
+"""Builtin plugin that adds subtests support."""
+
 from __future__ import annotations
 
 from collections.abc import Callable
-from collections.abc import Generator
 from collections.abc import Iterator
 from collections.abc import Mapping
 from contextlib import AbstractContextManager
@@ -10,10 +11,12 @@
 from contextlib import nullcontext
 import dataclasses
 import time
+from types import TracebackType
 from typing import Any
-from typing import TYPE_CHECKING
+from typing import Literal
 
 import pluggy
+from typing_extensions import Self
 
 from _pytest._code import ExceptionInfo
 from _pytest.capture import CaptureFixture
@@ -22,6 +25,7 @@
 from _pytest.config import Config
 from _pytest.config import hookimpl
 from _pytest.config.argparsing import Parser
+from _pytest.deprecated import check_ispytest
 from _pytest.fixtures import fixture
 from _pytest.fixtures import SubRequest
 from _pytest.logging import catching_logs
@@ -31,11 +35,6 @@
 from _pytest.runner import check_interactive_exception
 
 
-if TYPE_CHECKING:
-    from types import TracebackType
-    from typing import Literal
-
-
 def pytest_addoption(parser: Parser) -> None:
     group = parser.getgroup("subtests")
     group.addoption(
@@ -54,24 +53,31 @@ def pytest_addoption(parser: Parser) -> None:
     )
 
 
-@dataclasses.dataclass
-class SubTestContext:
-    """The values passed to SubTests.test() that are included in the test report."""
+@dataclasses.dataclass(frozen=True, slots=True, kw_only=True)
+class SubtestContext:
+    """The values passed to Subtests.test() that are included in the test report."""
 
     msg: str | None
-    kwargs: dict[str, Any]
+    kwargs: Mapping[str, Any]
+
+    def _to_json(self) -> dict[str, Any]:
+        return dataclasses.asdict(self)
+
+    @classmethod
+    def _from_json(cls, d: dict[str, Any]) -> Self:
+        return cls(msg=d["msg"], kwargs=d["kwargs"])
 
 
 @dataclasses.dataclass(init=False)
-class SubTestReport(TestReport):
-    context: SubTestContext
+class SubtestReport(TestReport):
+    context: SubtestContext
 
     @property
     def head_line(self) -> str:
         _, _, domain = self.location
-        return f"{domain} {self.sub_test_description()}"
+        return f"{domain} {self._sub_test_description()}"
 
-    def sub_test_description(self) -> str:
+    def _sub_test_description(self) -> str:
         parts = []
         if isinstance(self.context.msg, str):
             parts.append(f"[{self.context.msg}]")
@@ -86,45 +92,45 @@ def _to_json(self) -> dict[str, Any]:
         data = super()._to_json()
         del data["context"]
         data["_report_type"] = "SubTestReport"
-        data["_subtest.context"] = dataclasses.asdict(self.context)
+        data["_subtest.context"] = self.context._to_json()
         return data
 
     @classmethod
-    def _from_json(cls, reportdict: dict[str, Any]) -> SubTestReport:
+    def _from_json(cls, reportdict: dict[str, Any]) -> SubtestReport:
         report = super()._from_json(reportdict)
-        context_data = reportdict["_subtest.context"]
-        report.context = SubTestContext(
-            msg=context_data["msg"], kwargs=context_data["kwargs"]
-        )
+        report.context = SubtestContext._from_json(reportdict["_subtest.context"])
         return report
 
     @classmethod
-    def _from_test_report(cls, test_report: TestReport) -> SubTestReport:
+    def _from_test_report(cls, test_report: TestReport) -> SubtestReport:
         return super()._from_json(test_report._to_json())
 
 
 @fixture
-def subtests(request: SubRequest) -> Generator[SubTests, None, None]:
+def subtests(request: SubRequest) -> Subtests:
     """Provides subtests functionality."""
     capmam = request.node.config.pluginmanager.get_plugin("capturemanager")
     if capmam is not None:
         suspend_capture_ctx = capmam.global_and_fixture_disabled
     else:
         suspend_capture_ctx = nullcontext
-    yield SubTests(request.node.ihook, suspend_capture_ctx, request)
+    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:
+class Subtests:
     """Subtests fixture, enables declaring subtests inside test functions via the :meth:`test` method."""
 
     def __init__(
         self,
         ihook: pluggy.HookRelay,
         suspend_capture_ctx: Callable[[], AbstractContextManager[None]],
         request: SubRequest,
+        *,
+        _ispytest: bool = False,
     ) -> None:
+        check_ispytest(_ispytest)
         self._ihook = ihook
         self._suspend_capture_ctx = suspend_capture_ctx
         self._request = request
@@ -169,7 +175,7 @@ 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
+    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).
     """
@@ -220,8 +226,8 @@ def __exit__(
         report = self.ihook.pytest_runtest_makereport(
             item=self.request.node, call=call_info
         )
-        sub_report = SubTestReport._from_test_report(report)
-        sub_report.context = SubTestContext(self.msg, self.kwargs.copy())
+        sub_report = SubtestReport._from_test_report(report)
+        sub_report.context = SubtestContext(msg=self.msg, kwargs=self.kwargs.copy())
 
         self._captured_output.update_report(sub_report)
         self._captured_logs.update_report(sub_report)
@@ -330,14 +336,14 @@ def update_report(self, report: TestReport) -> None:
 
 
 def pytest_report_to_serializable(report: TestReport) -> dict[str, Any] | None:
-    if isinstance(report, SubTestReport):
+    if isinstance(report, SubtestReport):
         return report._to_json()
     return None
 
 
-def pytest_report_from_serializable(data: dict[str, Any]) -> SubTestReport | None:
+def pytest_report_from_serializable(data: dict[str, Any]) -> SubtestReport | None:
     if data.get("_report_type") == "SubTestReport":
-        return SubTestReport._from_json(data)
+        return SubtestReport._from_json(data)
     return None
 
 
@@ -346,11 +352,11 @@ def pytest_report_teststatus(
     report: TestReport,
     config: Config,
 ) -> tuple[str, str, str | Mapping[str, bool]] | None:
-    if report.when != "call" or not isinstance(report, SubTestReport):
+    if report.when != "call" or not isinstance(report, SubtestReport):
         return None
 
     outcome = report.outcome
-    description = report.sub_test_description()
+    description = report._sub_test_description()
     no_output = ("", "", "")
 
     if hasattr(report, "wasxfail"):

src/_pytest/unittest.py

@@ -36,8 +36,8 @@
 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
+from _pytest.subtests import SubtestContext
+from _pytest.subtests import SubtestReport
 
 
 if sys.version_info[:2] < (3, 11):
@@ -428,8 +428,8 @@ def addSubTest(
         )
         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, dict(test.params))  # type: ignore[attr-defined]
+        sub_report = SubtestReport._from_test_report(report)
+        sub_report.context = 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(

src/pytest/__init__.py

@@ -71,7 +71,7 @@
 from _pytest.runner import CallInfo
 from _pytest.stash import Stash
 from _pytest.stash import StashKey
-from _pytest.subtests import SubTests
+from _pytest.subtests import Subtests
 from _pytest.terminal import TerminalReporter
 from _pytest.terminal import TestShortLogReport
 from _pytest.tmpdir import TempPathFactory
@@ -149,7 +149,7 @@
     "Session",
     "Stash",
     "StashKey",
-    "SubTests",
+    "Subtests",
     "TempPathFactory",
     "TempdirFactory",
     "TerminalReporter",

testing/test_subtests.py

@@ -138,10 +138,10 @@ def test_typing_exported(
     ) -> None:
         pytester.makepyfile(
             """
-            from pytest import SubTests
+            from pytest import Subtests
 
-            def test_typing_exported(subtests: SubTests) -> None:
-                assert isinstance(subtests, SubTests)
+            def test_typing_exported(subtests: Subtests) -> None:
+                assert isinstance(subtests, Subtests)
             """
         )
         if mode == "normal":
@@ -212,8 +212,33 @@ def test_foo(subtests):
         )
 
 
-class TestSubTest:
-    """Test.subTest functionality."""
+def test_subtests_and_parametrization(pytester: pytest.Pytester) -> None:
+    pytester.makepyfile(
+        """
+        import pytest
+
+        @pytest.mark.parametrize("x", [0, 1])
+        def test_foo(subtests, x):
+            for i in range(3):
+                with subtests.test(msg="custom", i=i):
+                    assert i % 2 == 0
+            assert x == 0
+    """
+    )
+    result = pytester.runpytest("-v", "--no-subtests-reports")
+    result.stdout.fnmatch_lines(
+        [
+            "test_subtests_and_parametrization.py::test_foo[[]0[]] [[]custom[]] (i=1) SUBFAIL*[[] 50%[]]",
+            "test_subtests_and_parametrization.py::test_foo[[]0[]] PASSED              *[[] 50%[]]",
+            "test_subtests_and_parametrization.py::test_foo[[]1[]] [[]custom[]] (i=1) SUBFAIL *[[]100%[]]",
+            "test_subtests_and_parametrization.py::test_foo[[]1[]] FAILED              *[[]100%[]]",
+            "* 3 failed, 1 passed in *",
+        ]
+    )
+
+
+class TestUnittestSubTest:
+    """Test unittest.TestCase.subTest functionality."""
 
     @pytest.fixture
     def simple_script(self, pytester: pytest.Pytester) -> Path: