From 4577b459a9043ba283e0d7a39beeddd627cc54a0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 20 Jan 2024 12:58:24 +0200 Subject: [PATCH 1/2] hooks: add comment describing `_hookimpls`'s format/invariants. --- src/pluggy/_hooks.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 916ca704..ed72ed2f 100644 --- a/src/pluggy/_hooks.py +++ b/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. From 443fee6d7a30ca694aa2eec77bde4aea9356efa0 Mon Sep 17 00:00:00 2001 From: Vishu Tupili Date: Wed, 13 Dec 2023 13:37:09 -0500 Subject: [PATCH 2/2] hooks: fix `call_extra` extra methods getting ordered before everything else when there is at least one wrapper impl. Fix #441. Regressed in 63b7e908b4b22c30d86cd2cff240b3b7aa6da596 - pluggy 1.1.0. [ran: reworked] --- src/pluggy/_hooks.py | 9 ++--- testing/test_hookcaller.py | 72 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index ed72ed2f..6623c147 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -548,10 +548,11 @@ def call_extra( hookimpl = HookImpl(None, "", 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) diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index 88ed1316..9eb6f979 100644 --- a/testing/test_hookcaller.py +++ b/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 " ".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", + ]