From c5277e4e9d374ca99d20d4f92fa42e17d48da678 Mon Sep 17 00:00:00 2001 From: Victor Makarov Date: Fri, 24 Mar 2023 08:29:08 +0300 Subject: [PATCH 1/4] Detailed error message on hook registration conflict --- src/pluggy/_hooks.py | 6 +++++- testing/test_hookcaller.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 79dd9cb4..9f230db3 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -337,7 +337,11 @@ def set_specification( specmodule_or_class: _Namespace, spec_opts: "_HookSpecOpts", ) -> None: - assert not self.has_spec() + if self.has_spec(): + raise RuntimeError( + f"Hook {self.spec.name!r} is already registered " + f"within namespace {self.spec.namespace}" + ) self.spec = HookSpec(specmodule_or_class, self.name, spec_opts) if spec_opts.get("historic"): self._call_history = [] diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index 5e69a003..a12a2ad1 100644 --- a/testing/test_hookcaller.py +++ b/testing/test_hookcaller.py @@ -405,3 +405,19 @@ def hello(self, arg: int) -> int: pm.register(Plugin2()) with pytest.raises(PluginValidationError): pm.check_pending() + + +def test_hook_conflict(pm: PluginManager) -> None: + class Api1: + @hookspec + def conflict(self) -> None: + pass + + class Api2: + @hookspec + def conflict(self) -> None: + pass + + pm.add_hookspecs(Api1) + with pytest.raises(RuntimeError): + pm.add_hookspecs(Api2) From c501a32b34f177a7cf1f0511bbaecc2110b237bb Mon Sep 17 00:00:00 2001 From: Victor Makarov Date: Fri, 24 Mar 2023 08:39:54 +0300 Subject: [PATCH 2/4] mypy fix --- src/pluggy/_hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 9f230db3..de4c7cf2 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -337,7 +337,7 @@ def set_specification( specmodule_or_class: _Namespace, spec_opts: "_HookSpecOpts", ) -> None: - if self.has_spec(): + if self.spec is not None: raise RuntimeError( f"Hook {self.spec.name!r} is already registered " f"within namespace {self.spec.namespace}" From aac593436a655905e77ce63dd6446d7503834e8d Mon Sep 17 00:00:00 2001 From: Victor Makarov Date: Fri, 24 Mar 2023 09:15:34 +0300 Subject: [PATCH 3/4] review fixes --- src/pluggy/_hooks.py | 2 +- testing/test_hookcaller.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index de4c7cf2..6fe5a6a0 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -338,7 +338,7 @@ def set_specification( spec_opts: "_HookSpecOpts", ) -> None: if self.spec is not None: - raise RuntimeError( + raise ValueError( f"Hook {self.spec.name!r} is already registered " f"within namespace {self.spec.namespace}" ) diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index a12a2ad1..5880cd9c 100644 --- a/testing/test_hookcaller.py +++ b/testing/test_hookcaller.py @@ -419,5 +419,9 @@ def conflict(self) -> None: pass pm.add_hookspecs(Api1) - with pytest.raises(RuntimeError): + with pytest.raises(ValueError) as exc: pm.add_hookspecs(Api2) + assert str(exc.value) == ( + "Hook 'conflict' is already registered within namespace " + ".Api1'>" + ) From 33a6936813009208a97e6b194216a3dbe7ab2e57 Mon Sep 17 00:00:00 2001 From: Victor Makarov Date: Fri, 24 Mar 2023 09:45:06 +0300 Subject: [PATCH 4/4] fix --- testing/test_hookcaller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index 5880cd9c..1f0c0a43 100644 --- a/testing/test_hookcaller.py +++ b/testing/test_hookcaller.py @@ -411,12 +411,12 @@ def test_hook_conflict(pm: PluginManager) -> None: class Api1: @hookspec def conflict(self) -> None: - pass + """Api1 hook""" class Api2: @hookspec def conflict(self) -> None: - pass + """Api2 hook""" pm.add_hookspecs(Api1) with pytest.raises(ValueError) as exc: