From 3875ea55ce4949fc89d8875ed5ef7275d2d2f890 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 21 Dec 2024 11:13:41 +0100 Subject: [PATCH 01/16] fix #544: Correctly pass StopIteration trough wrappers Raising a StopIteration in a generator triggers a RuntimeError. If the RuntimeError of a generator has the passed in StopIteration as cause resume with that StopIteration as normal exception instead of failing with the RuntimeError. --- src/pluggy/_callers.py | 181 +++++++++++++++++------------------------ 1 file changed, 74 insertions(+), 107 deletions(-) diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index f88b92e0..db00be78 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -6,10 +6,9 @@ from collections.abc import Generator from collections.abc import Mapping -from collections.abc import Sequence from typing import cast from typing import NoReturn -from typing import Union +from typing import Sequence import warnings from ._hooks import HookImpl @@ -20,10 +19,33 @@ # Need to distinguish between old- and new-style hook wrappers. # Wrapping with a tuple is the fastest type-safe way I found to do it. -Teardown = Union[ - tuple[Generator[None, Result[object], None], HookImpl], - Generator[None, object, object], -] +Teardown = Generator[None, object, object] + + +def run_legacy_hookwrapper( + hook_impl: HookImpl, hook_name: str, args: Sequence[object] +) -> Teardown: + teardown: Teardown = cast(Teardown, hook_impl.function(*args)) + try: + next(teardown) + except StopIteration: + _raise_wrapfail(teardown, "did not yield") + try: + res = yield + result = Result(res, None) + except BaseException as exc: + result = Result(None, exc) + try: + teardown.send(result) + except StopIteration: + return result.get_result() + except BaseException as e: + _warn_teardown_exception(hook_name, hook_impl, e) + raise + else: + _raise_wrapfail(teardown, "has second yield") + finally: + teardown.close() def _raise_wrapfail( @@ -45,7 +67,7 @@ def _warn_teardown_exception( msg += f"Plugin: {hook_impl.plugin_name}, Hook: {hook_name}\n" msg += f"{type(e).__name__}: {e}\n" msg += "For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning" # noqa: E501 - warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=5) + warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=6) def _multicall( @@ -62,7 +84,6 @@ def _multicall( __tracebackhide__ = True results: list[object] = [] exception = None - only_new_style_wrappers = True try: # run impl and wrapper setup functions in a loop teardowns: list[Teardown] = [] try: @@ -77,16 +98,17 @@ def _multicall( ) if hook_impl.hookwrapper: - only_new_style_wrappers = False try: # If this cast is not valid, a type error is raised below, # which is the desired response. - res = hook_impl.function(*args) - wrapper_gen = cast(Generator[None, Result[object], None], res) - next(wrapper_gen) # first yield - teardowns.append((wrapper_gen, hook_impl)) + function_gen = run_legacy_hookwrapper( + hook_impl, hook_name, args + ) + + next(function_gen) # first yield + teardowns.append(function_gen) except StopIteration: - _raise_wrapfail(wrapper_gen, "did not yield") + _raise_wrapfail(function_gen, "did not yield") elif hook_impl.wrapper: try: # If this cast is not valid, a type error is raised below, @@ -106,99 +128,44 @@ def _multicall( except BaseException as exc: exception = exc finally: - # Fast path - only new-style wrappers, no Result. - if only_new_style_wrappers: - if firstresult: # first result hooks return a single value - result = results[0] if results else None - else: - result = results - - # run all wrapper post-yield blocks - for teardown in reversed(teardowns): - try: - if exception is not None: - try: - teardown.throw(exception) # type: ignore[union-attr] - except RuntimeError as re: - # StopIteration from generator causes RuntimeError - # even for coroutine usage - see #544 - if ( - isinstance(exception, StopIteration) - and re.__cause__ is exception - ): - teardown.close() # type: ignore[union-attr] - continue - else: - raise - else: - teardown.send(result) # type: ignore[union-attr] - # Following is unreachable for a well behaved hook wrapper. - # Try to force finalizers otherwise postponed till GC action. - # Note: close() may raise if generator handles GeneratorExit. - teardown.close() # type: ignore[union-attr] - except StopIteration as si: - result = si.value - exception = None - continue - except BaseException as e: - exception = e - continue - _raise_wrapfail(teardown, "has second yield") # type: ignore[arg-type] - - if exception is not None: - raise exception - else: - return result - - # Slow path - need to support old-style wrappers. + if firstresult: # first result hooks return a single value + result = results[0] if results else None else: - if firstresult: # first result hooks return a single value - outcome: Result[object | list[object]] = Result( - results[0] if results else None, exception - ) - else: - outcome = Result(results, exception) - - # run all wrapper post-yield blocks - for teardown in reversed(teardowns): - if isinstance(teardown, tuple): - try: - teardown[0].send(outcome) - except StopIteration: - pass - except BaseException as e: - _warn_teardown_exception(hook_name, teardown[1], e) - raise - else: - _raise_wrapfail(teardown[0], "has second yield") - else: + result = results + + # run all wrapper post-yield blocks + for teardown in reversed(teardowns): + try: + if exception is not None: try: - if outcome._exception is not None: - try: - teardown.throw(outcome._exception) - except RuntimeError as re: - # StopIteration from generator causes RuntimeError - # even for coroutine usage - see #544 - if ( - isinstance(outcome._exception, StopIteration) - and re.__cause__ is outcome._exception - ): - teardown.close() - continue - else: - raise + teardown.throw(exception) + except RuntimeError as re: + # StopIteration from generator causes RuntimeError + # even for coroutine usage - see #544 + if ( + isinstance(exception, StopIteration) + and re.__cause__ is exception + ): + teardown.close() + continue else: - teardown.send(outcome._result) - # Following is unreachable for a well behaved hook wrapper. - # Try to force finalizers otherwise postponed till GC action. - # Note: close() may raise if generator handles GeneratorExit. - teardown.close() - except StopIteration as si: - outcome.force_result(si.value) - continue - except BaseException as e: - outcome.force_exception(e) - continue - _raise_wrapfail(teardown, "has second yield") - - return outcome.get_result() + raise + else: + teardown.send(result) + # Following is unreachable for a well behaved hook wrapper. + # Try to force finalizers otherwise postponed till GC action. + # Note: close() may raise if generator handles GeneratorExit. + teardown.close() + except StopIteration as si: + result = si.value + exception = None + continue + except BaseException as e: + exception = e + continue + _raise_wrapfail(teardown, "has second yield") + + if exception is not None: + raise exception + else: + return result From 85e1d742c616c24ca3e8c82461cde09cf62e6426 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Mon, 4 Nov 2024 17:31:00 +0100 Subject: [PATCH 02/16] WIP: move legacy hook result trigger outside of the except block --- src/pluggy/_callers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index db00be78..afceb6ee 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -38,7 +38,7 @@ def run_legacy_hookwrapper( try: teardown.send(result) except StopIteration: - return result.get_result() + pass except BaseException as e: _warn_teardown_exception(hook_name, hook_impl, e) raise @@ -46,6 +46,7 @@ def run_legacy_hookwrapper( _raise_wrapfail(teardown, "has second yield") finally: teardown.close() + return result.get_result() def _raise_wrapfail( From 85a6c242bad5c6bcb1c9ff4239e12d4e271afe3f Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 30 Nov 2024 16:26:31 +0100 Subject: [PATCH 03/16] expand ruff linting --- pyproject.toml | 8 +++++++- scripts/release.py | 10 ++++++---- scripts/towncrier-draft-to-file.py | 2 +- src/pluggy/_callers.py | 6 ++---- src/pluggy/_hooks.py | 3 ++- testing/benchmark.py | 10 ++++++---- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3baa556c..7e7033f3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,8 +48,14 @@ package-data = {"pluggy" = ["py.typed"]} [tool.ruff.lint] extend-select = [ "I", # isort - "UP", + "F","E", "W", + "UP", "ANN", ] +extend-ignore = ["ANN101", "ANN102", "ANN401"] + +[tool.ruff.lint.extend-per-file-ignores] +"testing/*.py" = ["ANN001", "ANN002", "ANN003", "ANN201", "ANN202","ANN204" ,] +"docs/*.py" = ["ANN001", "ANN002", "ANN003", "ANN201", "ANN202","ANN204" ,] [tool.ruff.lint.isort] force-single-line = true diff --git a/scripts/release.py b/scripts/release.py index 879d35df..84dc4675 100644 --- a/scripts/release.py +++ b/scripts/release.py @@ -12,7 +12,7 @@ from git import Repo -def create_branch(version): +def create_branch(version: str) -> Repo: """Create a fresh branch from upstream/main""" repo = Repo.init(".") if repo.is_dirty(untracked_files=True): @@ -36,7 +36,7 @@ def get_upstream(repo: Repo) -> Remote: raise RuntimeError("could not find pytest-dev/pluggy remote") -def pre_release(version): +def pre_release(version: str) -> None: """Generates new docs, release announcements and creates a local tag.""" create_branch(version) changelog(version, write_out=True) @@ -47,7 +47,7 @@ def pre_release(version): print(f"{Fore.GREEN}Please push your branch to your fork and open a PR.") -def changelog(version, write_out=False): +def changelog(version: str, write_out: bool = False) -> None: if write_out: addopts = [] else: @@ -56,7 +56,7 @@ def changelog(version, write_out=False): check_call(["towncrier", "build", "--yes", "--version", version] + addopts) -def main(): +def main() -> int: init(autoreset=True) parser = argparse.ArgumentParser() parser.add_argument("version", help="Release version") @@ -66,6 +66,8 @@ def main(): except RuntimeError as e: print(f"{Fore.RED}ERROR: {e}") return 1 + else: + return 0 if __name__ == "__main__": diff --git a/scripts/towncrier-draft-to-file.py b/scripts/towncrier-draft-to-file.py index a47caa8f..63b9434c 100644 --- a/scripts/towncrier-draft-to-file.py +++ b/scripts/towncrier-draft-to-file.py @@ -2,7 +2,7 @@ import sys -def main(): +def main() -> int: """ Platform agnostic wrapper script for towncrier. Fixes the issue (pytest#7251) where windows users are unable to natively diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index afceb6ee..e147b25b 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -6,9 +6,9 @@ from collections.abc import Generator from collections.abc import Mapping +from collections.abc import Sequence from typing import cast from typing import NoReturn -from typing import Sequence import warnings from ._hooks import HookImpl @@ -50,9 +50,7 @@ def run_legacy_hookwrapper( def _raise_wrapfail( - wrap_controller: ( - Generator[None, Result[object], None] | Generator[None, object, object] - ), + wrap_controller: Generator[None, object, object], msg: str, ) -> NoReturn: co = wrap_controller.gi_code # type: ignore[union-attr] diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index a7c5f17a..3b07a433 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -483,7 +483,8 @@ def _verify_all_args_are_provided(self, kwargs: Mapping[str, object]) -> None: notincall = ", ".join( repr(argname) for argname in self.spec.argnames - # Avoid self.spec.argnames - kwargs.keys() - doesn't preserve order. + # Avoid self.spec.argnames - kwargs.keys() + # it doesn't preserve order. if argname not in kwargs.keys() ) warnings.warn( diff --git a/testing/benchmark.py b/testing/benchmark.py index d13e50aa..cc3be4eb 100644 --- a/testing/benchmark.py +++ b/testing/benchmark.py @@ -2,6 +2,8 @@ Benchmarking and performance tests. """ +from typing import Any + import pytest from pluggy import HookimplMarker @@ -26,16 +28,16 @@ def wrapper(arg1, arg2, arg3): @pytest.fixture(params=[10, 100], ids="hooks={}".format) -def hooks(request): +def hooks(request: Any) -> list[object]: return [hook for i in range(request.param)] @pytest.fixture(params=[10, 100], ids="wrappers={}".format) -def wrappers(request): +def wrappers(request: Any) -> list[object]: return [wrapper for i in range(request.param)] -def test_hook_and_wrappers_speed(benchmark, hooks, wrappers): +def test_hook_and_wrappers_speed(benchmark, hooks, wrappers) -> None: def setup(): hook_name = "foo" hook_impls = [] @@ -65,7 +67,7 @@ def setup(): (100, 100, 0), ], ) -def test_call_hook(benchmark, plugins, wrappers, nesting): +def test_call_hook(benchmark, plugins, wrappers, nesting) -> None: pm = PluginManager("example") class HookSpec: From 5a1297356847937323494deed67bcabe0b88a1f0 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 16 Mar 2025 20:58:59 +0100 Subject: [PATCH 04/16] implment review: bettern name for hookwrapper helper --- src/pluggy/_callers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index e147b25b..ab3bbe73 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -22,9 +22,14 @@ Teardown = Generator[None, object, object] -def run_legacy_hookwrapper( +def run_old_style_hookwrapper( hook_impl: HookImpl, hook_name: str, args: Sequence[object] ) -> Teardown: + """ + backward compatibility wrapper to run a old style hookwrapper as a wrapper + """ + + teardown: Teardown = cast(Teardown, hook_impl.function(*args)) try: next(teardown) @@ -100,7 +105,7 @@ def _multicall( try: # If this cast is not valid, a type error is raised below, # which is the desired response. - function_gen = run_legacy_hookwrapper( + function_gen = run_old_style_hookwrapper( hook_impl, hook_name, args ) From 25af101747d61479fd31f507a4122da377150db7 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 16 Mar 2025 21:01:31 +0100 Subject: [PATCH 05/16] fixup: correct type ignore comment --- src/pluggy/_callers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index ab3bbe73..82b803f1 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -29,7 +29,6 @@ def run_old_style_hookwrapper( backward compatibility wrapper to run a old style hookwrapper as a wrapper """ - teardown: Teardown = cast(Teardown, hook_impl.function(*args)) try: next(teardown) @@ -58,7 +57,7 @@ def _raise_wrapfail( wrap_controller: Generator[None, object, object], msg: str, ) -> NoReturn: - co = wrap_controller.gi_code # type: ignore[union-attr] + co = wrap_controller.gi_code # type: ignore[attr-defined] raise RuntimeError( f"wrap_controller at {co.co_name!r} {co.co_filename}:{co.co_firstlineno} {msg}" ) From 2f428b93072c43702ab648e21b0f58fc32b7934e Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 9 Apr 2025 21:48:22 +0200 Subject: [PATCH 06/16] chore: add coverage to testing extra --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 7e7033f3..6884ce78 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ requires-python = ">=3.9" dynamic = ["version"] [project.optional-dependencies] dev = ["pre-commit", "tox"] -testing = ["pytest", "pytest-benchmark"] +testing = ["pytest", "pytest-benchmark", "coverage"] [tool.setuptools] packages = ["pluggy"] From 6b044a673b8447cc1184d91e45f5aa0c278cc68b Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 9 Apr 2025 21:48:44 +0200 Subject: [PATCH 07/16] chore: drop deprecated ruff rules from config --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 6884ce78..f69f7ff1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,7 +51,7 @@ extend-select = [ "F","E", "W", "UP", "ANN", ] -extend-ignore = ["ANN101", "ANN102", "ANN401"] +extend-ignore = ["ANN401"] [tool.ruff.lint.extend-per-file-ignores] "testing/*.py" = ["ANN001", "ANN002", "ANN003", "ANN201", "ANN202","ANN204" ,] From 3202c881e60d18f8ae6c7bb886de4cc354bc33be Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 9 Apr 2025 21:49:19 +0200 Subject: [PATCH 08/16] chore: coverage: expand default ignores to pass lines and ellipsis definitions --- .coveragerc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.coveragerc b/.coveragerc index 6801ce8b..10bbd40b 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,6 +1,7 @@ [run] include = pluggy/* + src/pluggy/* testing/* */lib/python*/site-packages/pluggy/* */pypy*/site-packages/pluggy/* @@ -27,3 +28,7 @@ exclude_lines = # Ignore coverage on lines solely with `...` ^\s*\.\.\.\s*$ + # ignore coverage on ruff line continued + ^\s*def.*:\ \.\.\.\s*$ + # ignore coverage on pass lines + ^\s*passs*$ From 5d1ffb9a769b2dbeaaab0d05455c66aa7aa5ecf4 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 9 Apr 2025 21:49:46 +0200 Subject: [PATCH 09/16] chore: expand coverage by pragmas and test changes --- src/pluggy/__init__.py | 9 +-------- src/pluggy/_manager.py | 6 +++--- testing/conftest.py | 2 +- testing/test_details.py | 33 +++++++++++++++++++++++---------- testing/test_helpers.py | 8 ++++---- testing/test_hookcaller.py | 28 ++++++++++++++-------------- testing/test_invocations.py | 2 +- testing/test_multicall.py | 10 +++++----- testing/test_pluginmanager.py | 23 +++++++++++++---------- 9 files changed, 65 insertions(+), 56 deletions(-) diff --git a/src/pluggy/__init__.py b/src/pluggy/__init__.py index 36ce1680..8a651f49 100644 --- a/src/pluggy/__init__.py +++ b/src/pluggy/__init__.py @@ -1,10 +1,3 @@ -try: - from ._version import version as __version__ -except ImportError: - # broken installation, we don't even try - # unknown only works because we do poor mans version compare - __version__ = "unknown" - __all__ = [ "__version__", "PluginManager", @@ -21,7 +14,6 @@ "PluggyWarning", "PluggyTeardownRaisedWarning", ] - from ._hooks import HookCaller from ._hooks import HookImpl from ._hooks import HookimplMarker @@ -33,5 +25,6 @@ from ._manager import PluginValidationError from ._result import HookCallError from ._result import Result +from ._version import version as __version__ from ._warnings import PluggyTeardownRaisedWarning from ._warnings import PluggyWarning diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index 2f57270d..cd7fbc21 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -188,11 +188,11 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None res: HookimplOpts | None = getattr( method, self.project_name + "_impl", None ) - except Exception: - res = {} # type: ignore[assignment] + except Exception: #pragma: no cover + res = {} # type: ignore[assignment] #pragma: no cover if res is not None and not isinstance(res, dict): # false positive - res = None # type:ignore[unreachable] + res = None # type:ignore[unreachable] #pragma: no cover return res def unregister( diff --git a/testing/conftest.py b/testing/conftest.py index 8842bd72..1f0682f4 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -14,7 +14,7 @@ def he_pm(request, pm: PluginManager) -> PluginManager: class Hooks: @hookspec def he_method1(self, arg: int) -> int: - return arg + 1 + return arg + 1 # pragma: no cover pm.add_hookspecs(request.param(Hooks)) return pm diff --git a/testing/test_details.py b/testing/test_details.py index 9b68a081..7f85f8c2 100644 --- a/testing/test_details.py +++ b/testing/test_details.py @@ -1,3 +1,5 @@ +from importlib.metadata import distribution + import pytest from pluggy import HookimplMarker @@ -20,7 +22,7 @@ def parse_hookimpl_opts(self, module_or_class, name): class Plugin: def x1meth(self): - pass + pass # pragma: no cover @hookimpl(hookwrapper=True, tryfirst=True) def x1meth2(self): @@ -33,15 +35,15 @@ def x1meth3(self): class Spec: @hookspec def x1meth(self): - pass + pass # pragma: no cover @hookspec def x1meth2(self): - pass + pass # pragma: no cover @hookspec def x1meth3(self): - pass + pass # pragma: no cover pm = MyPluginManager(hookspec.project_name) pm.register(Plugin()) @@ -75,12 +77,12 @@ def test_warn_when_deprecated_specified(recwarn) -> None: class Spec: @hookspec(warn_on_impl=warning) def foo(self): - pass + pass # pragma: no cover class Plugin: @hookimpl def foo(self): - pass + pass # pragma: no cover pm = PluginManager(hookspec.project_name) pm.add_hookspecs(Spec) @@ -136,10 +138,12 @@ def __getattr__(self, x): raise Exception("can't touch me") class Module: - pass + x: DontTouchMe module = Module() - module.x = DontTouchMe() # type: ignore[attr-defined] + module.x = DontTouchMe() + with pytest.raises(Exception, match="touch me"): + module.x.broken pm = PluginManager(hookspec.project_name) # register() would raise an error @@ -154,11 +158,11 @@ def test_not_all_arguments_are_provided_issues_a_warning(pm: PluginManager) -> N class Spec: @hookspec def hello(self, arg1, arg2): - pass + pass # pragma: no cover @hookspec(historic=True) def herstory(self, arg1, arg2): - pass + pass # pragma: no cover pm.add_hookspecs(Spec) @@ -189,3 +193,12 @@ def myhook(self): assert repr(pm.hook.myhook.get_hookimpls()[0]) == ( f"" ) + + +def test_dist_facade_list_attributes() -> None: + from pluggy._manager import DistFacade + + fc = DistFacade(distribution("pluggy")) + res = dir(fc) + assert res == sorted(res) + assert set(res) - set(dir(fc._dist)) == {"_dist", "project_name"} diff --git a/testing/test_helpers.py b/testing/test_helpers.py index 4fe26a57..b74c70e7 100644 --- a/testing/test_helpers.py +++ b/testing/test_helpers.py @@ -10,15 +10,15 @@ def test_varnames() -> None: def f(x) -> None: - i = 3 # noqa + i = 3 # noqa #pragma: no cover class A: def f(self, y) -> None: - pass + pass # pragma: no cover class B: def __call__(self, z) -> None: - pass + pass # pragma: no cover assert varnames(f) == (("x",), ()) assert varnames(A().f) == (("y",), ()) @@ -96,7 +96,7 @@ def test_varnames_decorator() -> None: def my_decorator(func: F) -> F: @wraps(func) def wrapper(*args, **kwargs): - return func(*args, **kwargs) + return func(*args, **kwargs) # pragma: no cover return cast(F, wrapper) diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index 032d5c7b..f6ca9577 100644 --- a/testing/test_hookcaller.py +++ b/testing/test_hookcaller.py @@ -158,23 +158,23 @@ def he_method1_b() -> None: def test_adding_wrappers_ordering(hc: HookCaller, addmeth: AddMeth) -> None: @addmeth(hookwrapper=True) def he_method1(): - yield + yield # pragma: no cover @addmeth(wrapper=True) def he_method1_fun(): - yield + yield # pragma: no cover @addmeth() def he_method1_middle(): - return + return # pragma: no cover @addmeth(hookwrapper=True) def he_method3_fun(): - yield + yield # pragma: no cover @addmeth(hookwrapper=True) def he_method3(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [ he_method1_middle, @@ -188,15 +188,15 @@ def he_method3(): def test_adding_wrappers_ordering_tryfirst(hc: HookCaller, addmeth: AddMeth) -> None: @addmeth(hookwrapper=True, tryfirst=True) def he_method1(): - yield + yield # pragma: no cover @addmeth(hookwrapper=True) def he_method2(): - yield + yield # pragma: no cover @addmeth(wrapper=True, tryfirst=True) def he_method3(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [he_method2, he_method1, he_method3] @@ -206,7 +206,7 @@ def test_adding_wrappers_complex(hc: HookCaller, addmeth: AddMeth) -> None: @addmeth(hookwrapper=True, trylast=True) def m1(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [m1] @@ -227,7 +227,7 @@ def m4() -> None: ... @addmeth(wrapper=True, tryfirst=True) def m5(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [m3, m2, m1, m4, m5] @@ -243,7 +243,7 @@ def m7() -> None: ... @addmeth(wrapper=True) def m8(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [m3, m2, m7, m6, m1, m4, m8, m5] @@ -264,7 +264,7 @@ def m11() -> None: ... @addmeth(wrapper=True) def m12(): - yield + yield # pragma: no cover assert funcs(hc.get_hookimpls()) == [ m9, @@ -405,7 +405,7 @@ def hello(self, arg: object) -> None: class Plugin: @hookimpl(specname="hello") def foo(self, arg: int, too, many, args) -> int: - return arg + 1 + return arg + 1 # pragma: no cover with pytest.raises(PluginValidationError): pm.register(Plugin()) @@ -415,7 +415,7 @@ def foo(self, arg: int, too, many, args) -> int: class Plugin2: @hookimpl(specname="bar") def hello(self, arg: int) -> int: - return arg + 1 + return arg + 1 # pragma: no cover pm.register(Plugin2()) with pytest.raises(PluginValidationError): diff --git a/testing/test_invocations.py b/testing/test_invocations.py index 779a1ce3..cef301d9 100644 --- a/testing/test_invocations.py +++ b/testing/test_invocations.py @@ -133,7 +133,7 @@ def hello(self, arg): class Plugin1: @hookimpl def hello(self, arg): - return arg + 1 + return arg + 1 # pragma: no cover class Plugin2: @hookimpl diff --git a/testing/test_multicall.py b/testing/test_multicall.py index c7ed0235..304c12f8 100644 --- a/testing/test_multicall.py +++ b/testing/test_multicall.py @@ -55,7 +55,7 @@ def f(x, z=1): def test_tags_call_error() -> None: @hookimpl def f(x): - return x + return x # pragma: no cover with pytest.raises(HookCallError): MC([f], {}) @@ -331,7 +331,7 @@ def m1(): raise finally: out.append("m1 finish") - return result + return result # pragma: no cover @hookimpl def m2(): @@ -358,7 +358,7 @@ def m2(): @hookimpl(wrapper=True) def m3(): yield - return 10 + return 10 # pragma: no cover @hookimpl(wrapper=True) def m4(): @@ -384,7 +384,7 @@ def m1(): out.append("m1 init") try: yield - out.append("m1 unreachable") + out.append("m1 unreachable") # pragma: no cover except BaseException: out.append("m1 teardown") raise @@ -459,7 +459,7 @@ def m2(): out.append("m2 init") try: yield - out.append("m2 unreachable") + out.append("m2 unreachable") # pragma: no cover except ValueError: out.append("m2 suppress") return 22 diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 46c96780..86ea72f8 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -65,9 +65,11 @@ class A: def __getattr__(self, name): if name[0] != "_": return 42 - raise AttributeError() + raise AttributeError(name) a = A() + a.test + he_pm.register(a) assert not he_pm.get_hookcallers(a) @@ -126,7 +128,7 @@ def test_register_mismatch_method(he_pm: PluginManager) -> None: class hello: @hookimpl def he_method_notexists(self): - pass + pass # pragma: no cover plugin = hello() @@ -140,7 +142,7 @@ def test_register_mismatch_arg(he_pm: PluginManager) -> None: class hello: @hookimpl def he_method1(self, qlwkje): - pass + pass # pragma: no cover plugin = hello() @@ -180,7 +182,7 @@ def he_method1(self): def test_register(pm: PluginManager) -> None: class MyPlugin: @hookimpl - def he_method1(self): ... + def he_method1(self): ... # pragma: no cover my = MyPlugin() pm.register(my) @@ -210,8 +212,7 @@ def he_method1(self, arg): class Hooks: @hookspec - def he_method1(self, arg): - pass + def he_method1(self, arg): ... pm.add_hookspecs(Hooks) # assert not pm._unverified_hooks @@ -224,8 +225,7 @@ def he_method1(self, arg): def test_register_historic(pm: PluginManager) -> None: class Hooks: @hookspec(historic=True) - def he_method1(self, arg): - pass + def he_method1(self, arg): ... pm.add_hookspecs(Hooks) @@ -373,7 +373,7 @@ def he_method1(self, arg): class Plugin: @hookimpl(hookwrapper=True) def he_method1(self, arg): - out.append(arg) + out.append(arg) # pragma: no cover with pytest.raises(PluginValidationError): pm.register(Plugin()) @@ -390,7 +390,7 @@ def he_method1(self, arg): class Plugin: @hookimpl(wrapper=True) def he_method1(self, arg): - yield + yield # pragma: no cover with pytest.raises(PluginValidationError): pm.register(Plugin()) @@ -425,6 +425,9 @@ def he_method1(self, arg): 0 / 0 pm.register(Plugin1()) + with pytest.raises(ZeroDivisionError): + pm.hook.he_method1(arg="works") + with pytest.raises(HookCallError): with pytest.warns(UserWarning): pm.hook.he_method1() From d0ab3611f662ec5c8809fee50b304ec7af672365 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 9 Apr 2025 22:51:58 +0200 Subject: [PATCH 10/16] add test to cover nonspec hook arg missing --- src/pluggy/_manager.py | 2 +- testing/test_pluginmanager.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index cd7fbc21..51912386 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -188,7 +188,7 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None res: HookimplOpts | None = getattr( method, self.project_name + "_impl", None ) - except Exception: #pragma: no cover + except Exception: # pragma: no cover res = {} # type: ignore[assignment] #pragma: no cover if res is not None and not isinstance(res, dict): # false positive diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 86ea72f8..14691bd7 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -757,3 +757,18 @@ def configure(self) -> int: result = [] pm.hook.configure.call_historic(result.append) assert result == [4, 5, 3, 2, 1, 6] + + +def test_check_pending_nonspec_hook( + pm: PluginManager, +) -> None: + hookimpl = HookimplMarker("example") + + class Plugin: + @hookimpl + def a_hook(self, param): + pass + + pm.register(Plugin()) + with pytest.raises(HookCallError, match="hook call must provide argument 'param'"): + pm.hook.a_hook() From eb70fb9b484022da2587ee5967bdbb39a75cd38c Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 10 Apr 2025 11:45:18 +0200 Subject: [PATCH 11/16] epxand test coverage and drop dead code for wrappers --- .coveragerc | 1 + src/pluggy/_callers.py | 21 +++++++------------ testing/test_details.py | 42 +++++++++++++++++++++++++++++++++++++ testing/test_invocations.py | 41 ++++++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/.coveragerc b/.coveragerc index 10bbd40b..b79041a4 100644 --- a/.coveragerc +++ b/.coveragerc @@ -30,5 +30,6 @@ exclude_lines = ^\s*\.\.\.\s*$ # ignore coverage on ruff line continued ^\s*def.*:\ \.\.\.\s*$ + .*: ...$ # ignore coverage on pass lines ^\s*passs*$ diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index 82b803f1..472d5dd0 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -93,25 +93,20 @@ def _multicall( for hook_impl in reversed(hook_impls): try: args = [caller_kwargs[argname] for argname in hook_impl.argnames] - except KeyError: - for argname in hook_impl.argnames: + except KeyError as e: + # coverage bug - this is tested + for argname in hook_impl.argnames: # pragma: no cover if argname not in caller_kwargs: raise HookCallError( f"hook call must provide argument {argname!r}" - ) + ) from e if hook_impl.hookwrapper: - try: - # If this cast is not valid, a type error is raised below, - # which is the desired response. - function_gen = run_old_style_hookwrapper( - hook_impl, hook_name, args - ) + function_gen = run_old_style_hookwrapper(hook_impl, hook_name, args) + + next(function_gen) # first yield + teardowns.append(function_gen) - next(function_gen) # first yield - teardowns.append(function_gen) - except StopIteration: - _raise_wrapfail(function_gen, "did not yield") elif hook_impl.wrapper: try: # If this cast is not valid, a type error is raised below, diff --git a/testing/test_details.py b/testing/test_details.py index 7f85f8c2..de79d536 100644 --- a/testing/test_details.py +++ b/testing/test_details.py @@ -2,6 +2,7 @@ import pytest +import pluggy from pluggy import HookimplMarker from pluggy import HookspecMarker from pluggy import PluginManager @@ -202,3 +203,44 @@ def test_dist_facade_list_attributes() -> None: res = dir(fc) assert res == sorted(res) assert set(res) - set(dir(fc._dist)) == {"_dist", "project_name"} + + +def test_hookimpl_disallow_invalid_combination() -> None: + decorator = hookspec(historic=True, firstresult=True) + with pytest.raises(ValueError, match="cannot have a historic firstresult hook"): + decorator(any) + + +def test_hook_nonspec_call(pm: PluginManager) -> None: + class Plugin: + @hookimpl + def a_hook(self, passed: str, missing: int) -> None: + pass + + pm.register(Plugin()) + with pytest.raises( + pluggy.HookCallError, match="hook call must provide argument 'missing'" + ): + pm.hook.a_hook(passed="a") + pm.hook.a_hook(passed="a", missing="ok") + + +def test_wrapper_runtimeerror_passtrough(pm: PluginManager) -> None: + """ + ensure runtime-error passes trough a wrapper in case of exceptions + """ + + class Fail: + @hookimpl + def fail_late(self): + raise RuntimeError("this is personal") + + class Plugin: + @hookimpl(wrapper=True) + def fail_late(self): + yield + + pm.register(Plugin()) + pm.register(Fail()) + with pytest.raises(RuntimeError, match="this is personal"): + pm.hook.fail_late() diff --git a/testing/test_invocations.py b/testing/test_invocations.py index cef301d9..e24af750 100644 --- a/testing/test_invocations.py +++ b/testing/test_invocations.py @@ -1,4 +1,5 @@ from collections.abc import Iterator +from typing import Any import pytest @@ -326,3 +327,43 @@ def hello(self): pm.register(Plugin3()) res = pm.hook.hello() assert [y for x in res for y in x] == [2, 3, 1] + + +@pytest.mark.parametrize( + "kind", + [ + pytest.param(hookimpl(wrapper=True), id="wrapper"), + pytest.param(hookimpl(hookwrapper=True), id="legacy-wrapper"), + ], +) +def test_wrappers_yield_twice_fails(pm: PluginManager, kind: Any) -> None: + class Plugin: + @kind + def wrap(self): + yield + yield + + pm.register(Plugin()) + with pytest.raises( + RuntimeError, match="wrap_controller at 'wrap'.* has second yield" + ): + pm.hook.wrap() + + +@pytest.mark.parametrize( + "kind", + [ + pytest.param(hookimpl(wrapper=True), id="wrapper"), + pytest.param(hookimpl(hookwrapper=True), id="legacy-wrapper"), + ], +) +def test_wrappers_yield_never_fails(pm: PluginManager, kind: Any) -> None: + class Plugin: + @kind + def wrap(self): + if False: + yield # type: ignore[unreachable] + + pm.register(Plugin()) + with pytest.raises(RuntimeError, match="wrap_controller at 'wrap'.* did not yield"): + pm.hook.wrap() From cfa7ed15797c8b85477f35043132de4d861f4945 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 10 Apr 2025 12:20:06 +0200 Subject: [PATCH 12/16] add test to cover unregistration of a blocked plugin --- testing/test_pluginmanager.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 14691bd7..2a5e929e 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -201,6 +201,18 @@ def he_method1(self): ... # pragma: no cover pm.unregister(my) +def test_unregister_blocked(pm: PluginManager) -> None: + class Plugin: + pass + + p = Plugin() + pm.set_blocked("error") + pm.register(p, "error") + # bloked plugins can be unregistred many times atm + pm.unregister(p, "error") + pm.unregister(p, "error") + + def test_register_unknown_hooks(pm: PluginManager) -> None: class Plugin1: @hookimpl From 6a417de0f060e421008ab9ebbc066111a4bc6705 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 10 Apr 2025 12:31:47 +0200 Subject: [PATCH 13/16] add coverage for optional hook validation and error cases --- src/pluggy/_manager.py | 19 ++++++++++--------- testing/test_pluginmanager.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index 51912386..ff1e3ce6 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -378,15 +378,16 @@ def check_pending(self) -> None: hook specification are optional, otherwise raise :exc:`PluginValidationError`.""" for name in self.hook.__dict__: - if name[0] != "_": - hook: HookCaller = getattr(self.hook, name) - if not hook.has_spec(): - for hookimpl in hook.get_hookimpls(): - if not hookimpl.optionalhook: - raise PluginValidationError( - hookimpl.plugin, - f"unknown hook {name!r} in plugin {hookimpl.plugin!r}", - ) + if name[0] == "_": + continue + hook: HookCaller = getattr(self.hook, name) + if not hook.has_spec(): + for hookimpl in hook.get_hookimpls(): + if not hookimpl.optionalhook: + raise PluginValidationError( + hookimpl.plugin, + f"unknown hook {name!r} in plugin {hookimpl.plugin!r}", + ) def load_setuptools_entrypoints(self, group: str, name: str | None = None) -> int: """Load modules from querying the specified setuptools ``group``. diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 2a5e929e..f80b1b55 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -771,6 +771,31 @@ def configure(self) -> int: assert result == [4, 5, 3, 2, 1, 6] +def test_check_pending_skips_underscore(pm: PluginManager) -> None: + # todo: determine what we want to do with the namespace + class Plugin: + @hookimpl + def _problem(self): + pass + + pm.register(Plugin()) + pm.hook._problem() + pm.check_pending() + + +def test_check_pending_optionalhook( + pm: PluginManager, +) -> None: + class Plugin: + @hookimpl(optionalhook=True) + def a_hook(self, param): + pass + + pm.register(Plugin()) + pm.hook.a_hook(param=1) + pm.check_pending() + + def test_check_pending_nonspec_hook( pm: PluginManager, ) -> None: @@ -784,3 +809,8 @@ def a_hook(self, param): pm.register(Plugin()) with pytest.raises(HookCallError, match="hook call must provide argument 'param'"): pm.hook.a_hook() + + with pytest.raises( + PluginValidationError, match="unknown hook 'a_hook' in plugin .*" + ): + pm.check_pending() From d4437645b3c537939cae8a091694b0cc40896071 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 10 Apr 2025 12:41:35 +0200 Subject: [PATCH 14/16] add no cover for special varnames cases --- src/pluggy/_hooks.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 3b07a433..97fef0d7 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -300,12 +300,12 @@ def varnames(func: object) -> tuple[tuple[str, ...], tuple[str, ...]]: if inspect.isclass(func): try: func = func.__init__ - except AttributeError: + except AttributeError: # pragma: no cover - pypy special case return (), () elif not inspect.isroutine(func): # callable object? try: func = getattr(func, "__call__", func) - except Exception: + except Exception: # pragma: no cover - pypy special case return (), () try: @@ -313,7 +313,7 @@ def varnames(func: object) -> tuple[tuple[str, ...], tuple[str, ...]]: sig = inspect.signature( func.__func__ if inspect.ismethod(func) else func # type:ignore[arg-type] ) - except TypeError: + except TypeError: # pragma: no cover return (), () _valid_param_kinds = ( @@ -345,7 +345,7 @@ def varnames(func: object) -> tuple[tuple[str, ...], tuple[str, ...]]: # pypy3 uses "obj" instead of "self" for default dunder methods if not _PYPY: implicit_names: tuple[str, ...] = ("self",) - else: + else: # pragma: no cover implicit_names = ("self", "obj") if args: qualname: str = getattr(func, "__qualname__", "") From 4be0c554ea22ca01c7dc30ecccac413a412c1ea7 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 9 May 2025 15:55:57 +0200 Subject: [PATCH 15/16] add changelog --- changelog/573.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/573.bugfix.rst diff --git a/changelog/573.bugfix.rst b/changelog/573.bugfix.rst new file mode 100644 index 00000000..5ff31add --- /dev/null +++ b/changelog/573.bugfix.rst @@ -0,0 +1 @@ +Fix python 3.14 SyntaxWrror by rearranging code. \ No newline at end of file From 1f4872e8cb2547dacea155a27f94f75e7e8f66ae Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 9 May 2025 13:57:40 +0000 Subject: [PATCH 16/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- changelog/573.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/573.bugfix.rst b/changelog/573.bugfix.rst index 5ff31add..26b05de8 100644 --- a/changelog/573.bugfix.rst +++ b/changelog/573.bugfix.rst @@ -1 +1 @@ -Fix python 3.14 SyntaxWrror by rearranging code. \ No newline at end of file +Fix python 3.14 SyntaxWrror by rearranging code.