Skip to content

Commit

Permalink
hooks: fix plugins registering other plugins in a hook
Browse files Browse the repository at this point in the history
when the other plugins implement the same hook itself.

Fix #438.
Regressed in 63b7e90 - pluggy 1.1.0.

Went with the simple solution described in the issue for now, basically
undoing most of the rationale for 63b7e90 (though I
think it's still better to have a single `_hookimpls` list).
  • Loading branch information
bluetech committed Jan 20, 2024
1 parent cc36605 commit 7aef3e6
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/pluggy/_hooks.py
Expand Up @@ -497,7 +497,8 @@ def __call__(self, **kwargs: object) -> Any:
), "Cannot directly call a historic hook - use call_historic instead."
self._verify_all_args_are_provided(kwargs)
firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
# Copy because plugins may register other plugins during iteration (#438).
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)

def call_historic(
self,
Expand All @@ -518,7 +519,8 @@ def call_historic(
self._call_history.append((kwargs, result_callback))
# Historizing hooks don't return results.
# Remember firstresult isn't compatible with historic.
res = self._hookexec(self.name, self._hookimpls, kwargs, False)
# Copy because plugins may register other plugins during iteration (#438).
res = self._hookexec(self.name, self._hookimpls.copy(), kwargs, False)
if result_callback is None:
return
if isinstance(res, list):
Expand Down
76 changes: 76 additions & 0 deletions testing/test_pluginmanager.py
Expand Up @@ -675,3 +675,79 @@ def he_method1(self):
assert saveindent[0] > indent
finally:
undo()


@pytest.mark.parametrize("historic", [False, True])
def test_register_while_calling(
pm: PluginManager,
historic: bool,
) -> None:
"""Test that registering an impl of a hook while it is being called does
not affect the call itself, only later calls.
For historic hooks however, the hook is called immediately on registration.
Regression test for #438.
"""
hookspec = HookspecMarker("example")

class Hooks:
@hookspec(historic=historic)
def configure(self) -> int:
raise NotImplementedError()

class Plugin1:
@hookimpl
def configure(self) -> int:
return 1

class Plugin2:
def __init__(self) -> None:
self.already_registered = False

@hookimpl
def configure(self) -> int:
if not self.already_registered:
pm.register(Plugin4())
pm.register(Plugin5())
pm.register(Plugin6())
self.already_registered = True
return 2

class Plugin3:
@hookimpl
def configure(self) -> int:
return 3

class Plugin4:
@hookimpl(tryfirst=True)
def configure(self) -> int:
return 4

class Plugin5:
@hookimpl()
def configure(self) -> int:
return 5

class Plugin6:
@hookimpl(trylast=True)
def configure(self) -> int:
return 6

pm.add_hookspecs(Hooks)
pm.register(Plugin1())
pm.register(Plugin2())
pm.register(Plugin3())

if not historic:
result = pm.hook.configure()
assert result == [3, 2, 1]
result = pm.hook.configure()
assert result == [4, 5, 3, 2, 1, 6]
else:
result = []
pm.hook.configure.call_historic(result.append)
assert result == [4, 5, 6, 3, 2, 1]
result = []
pm.hook.configure.call_historic(result.append)
assert result == [4, 5, 3, 2, 1, 6]

0 comments on commit 7aef3e6

Please sign in to comment.