From f087c1ee7415200428fd0cbe9b12b550da6e6172 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 30 Aug 2017 14:43:31 -0400 Subject: [PATCH 1/4] Add a test which verifies firstresult wrappers This is to expose the regression from issue #71. Wrappers must return a single value not a list. --- testing/test_hookrelay.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testing/test_hookrelay.py b/testing/test_hookrelay.py index 3653151b..46ffadae 100644 --- a/testing/test_hookrelay.py +++ b/testing/test_hookrelay.py @@ -87,9 +87,17 @@ class Plugin3(object): def hello(self, arg): return None + class Plugin4(object): + @hookimpl(hookwrapper=True) + def hello(self, arg): + assert arg == 3 + outcome = yield + assert outcome.get_result() == 2 + pm.register(Plugin1()) # discarded - not the last registered plugin pm.register(Plugin2()) # used as result pm.register(Plugin3()) # None result is ignored + pm.register(Plugin4()) # hookwrapper should get same non-list result res = pm.hook.hello(arg=3) assert res == 2 From dc1d67d00c02cb7848a967a88a1d8260c4c88f15 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 30 Aug 2017 14:48:39 -0400 Subject: [PATCH 2/4] Always force a single value for firstresult hooks Ensures that wrappers receive a single value instead of a list. Fixes #71 --- pluggy/callers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pluggy/callers.py b/pluggy/callers.py index c5934b96..9b9d305f 100644 --- a/pluggy/callers.py +++ b/pluggy/callers.py @@ -97,6 +97,10 @@ def execute(self): excinfo = sys.exc_info() finally: outcome = _Result(results, excinfo) + if firstresult: # first result hooks return a single value + results = outcome.get_result() + result = results[0] if results else None + outcome.force_result(result) # run all wrapper post-yield blocks for gen in reversed(teardowns): @@ -106,10 +110,6 @@ def execute(self): except StopIteration: pass - if firstresult: - result = outcome.get_result() - return result[0] if result else None - return outcome.get_result() def __repr__(self): From 9c7246ffd8e9f366af9b30fa114a77e64a81bf11 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 30 Aug 2017 16:17:49 -0400 Subject: [PATCH 3/4] Simplify outcome generation Avoids forcing a result with `firstresult` hooks. Thanks to @RonnyPfannschmidt for the nice simplification. --- pluggy/callers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pluggy/callers.py b/pluggy/callers.py index 9b9d305f..642bbcc2 100644 --- a/pluggy/callers.py +++ b/pluggy/callers.py @@ -96,11 +96,10 @@ def execute(self): except BaseException: excinfo = sys.exc_info() finally: - outcome = _Result(results, excinfo) if firstresult: # first result hooks return a single value - results = outcome.get_result() - result = results[0] if results else None - outcome.force_result(result) + outcome = _Result(results[0] if results else None, excinfo) + else: + outcome = _Result(results, excinfo) # run all wrapper post-yield blocks for gen in reversed(teardowns): From 5c6846f0a9632015e99661515d40f0f523350573 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 30 Aug 2017 16:53:56 -0400 Subject: [PATCH 4/4] Test forcing a result in a wrapper --- testing/test_hookrelay.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/testing/test_hookrelay.py b/testing/test_hookrelay.py index 46ffadae..7d9b6f52 100644 --- a/testing/test_hookrelay.py +++ b/testing/test_hookrelay.py @@ -102,6 +102,41 @@ def hello(self, arg): assert res == 2 +def test_firstresult_force_result(pm): + """Verify forcing a result in a wrapper. + """ + class Api(object): + @hookspec(firstresult=True) + def hello(self, arg): + "api hook 1" + + pm.add_hookspecs(Api) + + class Plugin1(object): + @hookimpl + def hello(self, arg): + return arg + 1 + + class Plugin2(object): + @hookimpl(hookwrapper=True) + def hello(self, arg): + assert arg == 3 + outcome = yield + assert outcome.get_result() == 4 + outcome.force_result(0) + + class Plugin3(object): + @hookimpl + def hello(self, arg): + return None + + pm.register(Plugin1()) + pm.register(Plugin2()) # wrapper + pm.register(Plugin3()) # ignored since returns None + res = pm.hook.hello(arg=3) + assert res == 0 # this result is forced and not a list + + def test_firstresult_returns_none(pm): """If None results are returned by underlying implementations ensure the multi-call loop returns a None value.