epxand test coverage and drop dead code for wrappers

Author: RonnyPfannschmidt

.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*$

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,

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()

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()