Merge pull request #472 from bluetech/call-extra-ordering-fix

Fix `call_extra` call ordering regression in pluggy 1.1.0

src/pluggy/_hooks.py

@@ -389,6 +389,13 @@ def __init__(
         #: Name of the hook getting called.
         self.name: Final = name
         self._hookexec: Final = hook_execute
+        # The hookimpls list. The caller iterates it *in reverse*. Format:
+        # 1. trylast nonwrappers
+        # 2. nonwrappers
+        # 3. tryfirst nonwrappers
+        # 4. trylast wrappers
+        # 5. wrappers
+        # 6. tryfirst wrappers
         self._hookimpls: Final[list[HookImpl]] = []
         self._call_history: _CallHistory | None = None
         # TODO: Document, or make private.
@@ -541,10 +548,11 @@ def call_extra(
             hookimpl = HookImpl(None, "<temp>", method, opts)
             # Find last non-tryfirst nonwrapper method.
             i = len(hookimpls) - 1
-            while (
-                i >= 0
-                and hookimpls[i].tryfirst
-                and not (hookimpls[i].hookwrapper or hookimpls[i].wrapper)
+            while i >= 0 and (
+                # Skip wrappers.
+                (hookimpls[i].hookwrapper or hookimpls[i].wrapper)
+                # Skip tryfirst nonwrappers.
+                or hookimpls[i].tryfirst
             ):
                 i -= 1
             hookimpls.insert(i + 1, hookimpl)

testing/test_hookcaller.py

@@ -1,4 +1,5 @@
 from typing import Callable
+from typing import Generator
 from typing import List
 from typing import Sequence
 from typing import TypeVar
@@ -448,3 +449,74 @@ def conflict(self) -> None:
         "Hook 'conflict' is already registered within namespace "
         "<class 'test_hookcaller.test_hook_conflict.<locals>.Api1'>"
     )
+
+
+def test_call_extra_hook_order(hc: HookCaller, addmeth: AddMeth) -> None:
+    """Ensure that call_extra is calling hooks in the right order."""
+    order = []
+
+    @addmeth(tryfirst=True)
+    def method1() -> str:
+        order.append("1")
+        return "1"
+
+    @addmeth()
+    def method2() -> str:
+        order.append("2")
+        return "2"
+
+    @addmeth(trylast=True)
+    def method3() -> str:
+        order.append("3")
+        return "3"
+
+    @addmeth(wrapper=True, tryfirst=True)
+    def method4() -> Generator[None, str, str]:
+        order.append("4pre")
+        result = yield
+        order.append("4post")
+        return result
+
+    @addmeth(wrapper=True)
+    def method5() -> Generator[None, str, str]:
+        order.append("5pre")
+        result = yield
+        order.append("5post")
+        return result
+
+    @addmeth(wrapper=True, trylast=True)
+    def method6() -> Generator[None, str, str]:
+        order.append("6pre")
+        result = yield
+        order.append("6post")
+        return result
+
+    def extra1() -> str:
+        order.append("extra1")
+        return "extra1"
+
+    def extra2() -> str:
+        order.append("extra2")
+        return "extra2"
+
+    result = hc.call_extra([extra1, extra2], {"arg": "test"})
+    assert order == [
+        "4pre",
+        "5pre",
+        "6pre",
+        "1",
+        "extra2",
+        "extra1",
+        "2",
+        "3",
+        "6post",
+        "5post",
+        "4post",
+    ]
+    assert result == [
+        "1",
+        "extra2",
+        "extra1",
+        "2",
+        "3",
+    ]