Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to new-style pluggy hook wrappers #11123

Merged
merged 1 commit into from
Jul 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/11122.improvement.rst
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this should be considered an improvement? Feels more like an internal change that does not affect users, so perhaps trivial would be more appropriate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has some behavioral changes in exception handling, however mostly I want plugin authors to see it, so let's keep it as improvement.

Original file line number Diff line number Diff line change
@@ -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.
14 changes: 8 additions & 6 deletions doc/en/example/simple.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:

Expand Down Expand Up @@ -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):
Expand Down
57 changes: 33 additions & 24 deletions doc/en/how-to/writing_hook_functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:

Expand All @@ -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:

Expand Down Expand Up @@ -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:

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion doc/en/requirements.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 7 additions & 6 deletions src/_pytest/assertion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
33 changes: 17 additions & 16 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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"
)
Expand All @@ -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:
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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"):
Expand All @@ -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] = {}
Expand All @@ -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]

Expand Down
49 changes: 29 additions & 20 deletions src/_pytest/capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
Loading