Merge pull request #11123 from bluetech/new-style-wrappers

Switch to new-style pluggy hook wrappers

Author: bluetech

changelog/11122.improvement.rst

@@ -0,0 +1,6 @@
+``pluggy>=1.2.0`` is now required.
+
+pytest now uses "new-style" hook wrappers internally, available since pluggy 1.2.0.
+See `pluggy's 1.2.0 changelog <https://pluggy.readthedocs.io/en/latest/changelog.html#pluggy-1-2-0-2023-06-21>`_ and the :ref:`updated docs <hookwrapper>` for details.
+
+Plugins which want to use new-style wrappers can do so if they require this version of pytest or later.

doc/en/example/simple.rst

@@ -808,11 +808,10 @@ case we just write some information out to a ``failures`` file:
     import pytest
 
 
-    @pytest.hookimpl(tryfirst=True, hookwrapper=True)
+    @pytest.hookimpl(wrapper=True, tryfirst=True)
     def pytest_runtest_makereport(item, call):
         # execute all other hooks to obtain the report object
-        outcome = yield
-        rep = outcome.get_result()
+        rep = yield
 
         # we only look at actual failing test calls, not setup/teardown
         if rep.when == "call" and rep.failed:
@@ -826,6 +825,8 @@ case we just write some information out to a ``failures`` file:
 
                 f.write(rep.nodeid + extra + "\n")
 
+        return rep
+
 
 if you then have failing tests:
 
@@ -899,16 +900,17 @@ here is a little example implemented via a local plugin:
     phase_report_key = StashKey[Dict[str, CollectReport]]()
 
 
-    @pytest.hookimpl(tryfirst=True, hookwrapper=True)
+    @pytest.hookimpl(wrapper=True, tryfirst=True)
     def pytest_runtest_makereport(item, call):
         # execute all other hooks to obtain the report object
-        outcome = yield
-        rep = outcome.get_result()
+        rep = yield
 
         # store test results for each phase of a call, which can
         # be "setup", "call", "teardown"
         item.stash.setdefault(phase_report_key, {})[rep.when] = rep
 
+        return rep
+
 
     @pytest.fixture
     def something(request):

doc/en/how-to/writing_hook_functions.rst

@@ -56,7 +56,7 @@ The remaining hook functions will not be called in this case.
 
 .. _`hookwrapper`:
 
-hookwrapper: executing around other hooks
+hook wrappers: executing around other hooks
 -------------------------------------------------
 
 .. currentmodule:: _pytest.core
@@ -69,10 +69,8 @@ which yields exactly once. When pytest invokes hooks it first executes
 hook wrappers and passes the same arguments as to the regular hooks.
 
 At the yield point of the hook wrapper pytest will execute the next hook
-implementations and return their result to the yield point in the form of
-a :py:class:`Result <pluggy._Result>` instance which encapsulates a result or
-exception info.  The yield point itself will thus typically not raise
-exceptions (unless there are bugs).
+implementations and return their result to the yield point, or will
+propagate an exception if they raised.
 
 Here is an example definition of a hook wrapper:
 
@@ -81,26 +79,35 @@ Here is an example definition of a hook wrapper:
     import pytest
 
 
-    @pytest.hookimpl(hookwrapper=True)
+    @pytest.hookimpl(wrapper=True)
     def pytest_pyfunc_call(pyfuncitem):
         do_something_before_next_hook_executes()
 
-        outcome = yield
-        # outcome.excinfo may be None or a (cls, val, tb) tuple
+        # If the outcome is an exception, will raise the exception.
+        res = yield
 
-        res = outcome.get_result()  # will raise if outcome was exception
+        new_res = post_process_result(res)
 
-        post_process_result(res)
+        # Override the return value to the plugin system.
+        return new_res
 
-        outcome.force_result(new_res)  # to override the return value to the plugin system
+The hook wrapper needs to return a result for the hook, or raise an exception.
 
-Note that hook wrappers don't return results themselves, they merely
-perform tracing or other side effects around the actual hook implementations.
-If the result of the underlying hook is a mutable object, they may modify
-that result but it's probably better to avoid it.
+In many cases, the wrapper only needs to perform tracing or other side effects
+around the actual hook implementations, in which case it can return the result
+value of the ``yield``. The simplest (though useless) hook wrapper is
+``return (yield)``.
+
+In other cases, the wrapper wants the adjust or adapt the result, in which case
+it can return a new value. If the result of the underlying hook is a mutable
+object, the wrapper may modify that result, but it's probably better to avoid it.
+
+If the hook implementation failed with an exception, the wrapper can handle that
+exception using a ``try-catch-finally`` around the ``yield``, by propagating it,
+supressing it, or raising a different exception entirely.
 
 For more information, consult the
-:ref:`pluggy documentation about hookwrappers <pluggy:hookwrappers>`.
+:ref:`pluggy documentation about hook wrappers <pluggy:hookwrappers>`.
 
 .. _plugin-hookorder:
 
@@ -130,11 +137,14 @@ after others, i.e.  the position in the ``N``-sized list of functions:
 
 
     # Plugin 3
-    @pytest.hookimpl(hookwrapper=True)
+    @pytest.hookimpl(wrapper=True)
     def pytest_collection_modifyitems(items):
         # will execute even before the tryfirst one above!
-        outcome = yield
-        # will execute after all non-hookwrappers executed
+        try:
+            return (yield)
+        finally:
+            # will execute after all non-wrappers executed
+            ...
 
 Here is the order of execution:
 
@@ -149,12 +159,11 @@ Here is the order of execution:
    Plugin1).
 
 4. Plugin3's pytest_collection_modifyitems then executing the code after the yield
-   point.  The yield receives a :py:class:`Result <pluggy._Result>` instance which encapsulates
-   the result from calling the non-wrappers.  Wrappers shall not modify the result.
+   point.  The yield receives the result from calling the non-wrappers, or raises
+   an exception if the non-wrappers raised.
 
-It's possible to use ``tryfirst`` and ``trylast`` also in conjunction with
-``hookwrapper=True`` in which case it will influence the ordering of hookwrappers
-among each other.
+It's possible to use ``tryfirst`` and ``trylast`` also on hook wrappers
+in which case it will influence the ordering of hook wrappers among each other.
 
 
 Declaring new hooks

doc/en/requirements.txt

@@ -1,5 +1,5 @@
 pallets-sphinx-themes
-pluggy>=1.0
+pluggy>=1.2.0
 pygments-pytest>=2.3.0
 sphinx-removed-in>=0.2.0
 sphinx>=5,<6

setup.cfg

@@ -46,7 +46,7 @@ py_modules = py
 install_requires =
     iniconfig
     packaging
-    pluggy>=0.12,<2.0
+    pluggy>=1.2.0,<2.0
     colorama;sys_platform=="win32"
     exceptiongroup>=1.0.0rc8;python_version<"3.11"
     tomli>=1.0.0;python_version<"3.11"

src/_pytest/assertion/__init__.py

@@ -112,8 +112,8 @@ def pytest_collection(session: "Session") -> None:
             assertstate.hook.set_session(session)
 
 
-@hookimpl(tryfirst=True, hookwrapper=True)
-def pytest_runtest_protocol(item: Item) -> Generator[None, None, None]:
+@hookimpl(wrapper=True, tryfirst=True)
+def pytest_runtest_protocol(item: Item) -> Generator[None, object, object]:
     """Setup the pytest_assertrepr_compare and pytest_assertion_pass hooks.
 
     The rewrite module will use util._reprcompare if it exists to use custom
@@ -162,10 +162,11 @@ def call_assertion_pass_hook(lineno: int, orig: str, expl: str) -> None:
 
         util._assertion_pass = call_assertion_pass_hook
 
-    yield
-
-    util._reprcompare, util._assertion_pass = saved_assert_hooks
-    util._config = None
+    try:
+        return (yield)
+    finally:
+        util._reprcompare, util._assertion_pass = saved_assert_hooks
+        util._config = None
 
 
 def pytest_sessionfinish(session: "Session") -> None:

src/_pytest/cacheprovider.py

@@ -217,12 +217,12 @@ def __init__(self, lfplugin: "LFPlugin") -> None:
         self.lfplugin = lfplugin
         self._collected_at_least_one_failure = False
 
-    @hookimpl(hookwrapper=True)
-    def pytest_make_collect_report(self, collector: nodes.Collector):
+    @hookimpl(wrapper=True)
+    def pytest_make_collect_report(
+        self, collector: nodes.Collector
+    ) -> Generator[None, CollectReport, CollectReport]:
+        res = yield
         if isinstance(collector, (Session, Package)):
-            out = yield
-            res: CollectReport = out.get_result()
-
             # Sort any lf-paths to the beginning.
             lf_paths = self.lfplugin._last_failed_paths
 
@@ -240,19 +240,16 @@ def sort_key(node: Union[nodes.Item, nodes.Collector]) -> bool:
                 key=sort_key,
                 reverse=True,
             )
-            return
 
         elif isinstance(collector, File):
             if collector.path in self.lfplugin._last_failed_paths:
-                out = yield
-                res = out.get_result()
                 result = res.result
                 lastfailed = self.lfplugin.lastfailed
 
                 # Only filter with known failures.
                 if not self._collected_at_least_one_failure:
                     if not any(x.nodeid in lastfailed for x in result):
-                        return
+                        return res
                     self.lfplugin.config.pluginmanager.register(
                         LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
                     )
@@ -268,8 +265,8 @@ def sort_key(node: Union[nodes.Item, nodes.Collector]) -> bool:
                     # Keep all sub-collectors.
                     or isinstance(x, nodes.Collector)
                 ]
-                return
-        yield
+
+        return res
 
 
 class LFPluginCollSkipfiles:
@@ -342,14 +339,14 @@ def pytest_collectreport(self, report: CollectReport) -> None:
         else:
             self.lastfailed[report.nodeid] = True
 
-    @hookimpl(hookwrapper=True, tryfirst=True)
+    @hookimpl(wrapper=True, tryfirst=True)
     def pytest_collection_modifyitems(
         self, config: Config, items: List[nodes.Item]
     ) -> Generator[None, None, None]:
-        yield
+        res = yield
 
         if not self.active:
-            return
+            return res
 
         if self.lastfailed:
             previously_failed = []
@@ -394,6 +391,8 @@ def pytest_collection_modifyitems(
             else:
                 self._report_status += "not deselecting items."
 
+        return res
+
     def pytest_sessionfinish(self, session: Session) -> None:
         config = self.config
         if config.getoption("cacheshow") or hasattr(config, "workerinput"):
@@ -414,11 +413,11 @@ def __init__(self, config: Config) -> None:
         assert config.cache is not None
         self.cached_nodeids = set(config.cache.get("cache/nodeids", []))
 
-    @hookimpl(hookwrapper=True, tryfirst=True)
+    @hookimpl(wrapper=True, tryfirst=True)
     def pytest_collection_modifyitems(
         self, items: List[nodes.Item]
     ) -> Generator[None, None, None]:
-        yield
+        res = yield
 
         if self.active:
             new_items: Dict[str, nodes.Item] = {}
@@ -436,6 +435,8 @@ def pytest_collection_modifyitems(
         else:
             self.cached_nodeids.update(item.nodeid for item in items)
 
+        return res
+
     def _get_increasing_order(self, items: Iterable[nodes.Item]) -> List[nodes.Item]:
         return sorted(items, key=lambda item: item.path.stat().st_mtime, reverse=True)  # type: ignore[no-any-return]
 

src/_pytest/capture.py

@@ -36,6 +36,7 @@
 from _pytest.nodes import Collector
 from _pytest.nodes import File
 from _pytest.nodes import Item
+from _pytest.reports import CollectReport
 
 _CaptureMethod = Literal["fd", "sys", "no", "tee-sys"]
 
@@ -130,8 +131,8 @@ def _reopen_stdio(f, mode):
     sys.stderr = _reopen_stdio(sys.stderr, "wb")
 
 
-@hookimpl(hookwrapper=True)
-def pytest_load_initial_conftests(early_config: Config):
+@hookimpl(wrapper=True)
+def pytest_load_initial_conftests(early_config: Config) -> Generator[None, None, None]:
     ns = early_config.known_args_namespace
     if ns.capture == "fd":
         _windowsconsoleio_workaround(sys.stdout)
@@ -145,12 +146,16 @@ def pytest_load_initial_conftests(early_config: Config):
 
     # Finally trigger conftest loading but while capturing (issue #93).
     capman.start_global_capturing()
-    outcome = yield
-    capman.suspend_global_capture()
-    if outcome.excinfo is not None:
+    try:
+        try:
+            yield
+        finally:
+            capman.suspend_global_capture()
+    except BaseException:
         out, err = capman.read_global_capture()
         sys.stdout.write(out)
         sys.stderr.write(err)
+        raise
 
 
 # IO Helpers.
@@ -841,41 +846,45 @@ def item_capture(self, when: str, item: Item) -> Generator[None, None, None]:
             self.deactivate_fixture()
             self.suspend_global_capture(in_=False)
 
-        out, err = self.read_global_capture()
-        item.add_report_section(when, "stdout", out)
-        item.add_report_section(when, "stderr", err)
+            out, err = self.read_global_capture()
+            item.add_report_section(when, "stdout", out)
+            item.add_report_section(when, "stderr", err)
 
     # Hooks
 
-    @hookimpl(hookwrapper=True)
-    def pytest_make_collect_report(self, collector: Collector):
+    @hookimpl(wrapper=True)
+    def pytest_make_collect_report(
+        self, collector: Collector
+    ) -> Generator[None, CollectReport, CollectReport]:
         if isinstance(collector, File):
             self.resume_global_capture()
-            outcome = yield
-            self.suspend_global_capture()
+            try:
+                rep = yield
+            finally:
+                self.suspend_global_capture()
             out, err = self.read_global_capture()
-            rep = outcome.get_result()
             if out:
                 rep.sections.append(("Captured stdout", out))
             if err:
                 rep.sections.append(("Captured stderr", err))
         else:
-            yield
+            rep = yield
+        return rep
 
-    @hookimpl(hookwrapper=True)
+    @hookimpl(wrapper=True)
     def pytest_runtest_setup(self, item: Item) -> Generator[None, None, None]:
         with self.item_capture("setup", item):
-            yield
+            return (yield)
 
-    @hookimpl(hookwrapper=True)
+    @hookimpl(wrapper=True)
     def pytest_runtest_call(self, item: Item) -> Generator[None, None, None]:
         with self.item_capture("call", item):
-            yield
+            return (yield)
 
-    @hookimpl(hookwrapper=True)
+    @hookimpl(wrapper=True)
     def pytest_runtest_teardown(self, item: Item) -> Generator[None, None, None]:
         with self.item_capture("teardown", item):
-            yield
+            return (yield)
 
     @hookimpl(tryfirst=True)
     def pytest_keyboard_interrupt(self) -> None: