Firstresult madness #72
Conversation
This is to expose the regression from issue pytest-dev#71. Wrappers must return a single value not a list.
Ensures that wrappers receive a single value instead of a list. Fixes pytest-dev#71
| @@ -87,9 +87,17 @@ class Plugin3(object): | |||
| def hello(self, arg): | |||
| return None | |||
|
|
|||
| class Plugin4(object): | |||
There was a problem hiding this comment.
Perhaps we should add a new test which also changes the outcome result to make sure?
I plan to test this branch in pytest later today as well.
There was a problem hiding this comment.
@nicoddemus yes good call.
I'll add a force result test.
Not at all, thanks for tackling this so quickly. |
pluggy/callers.py
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
i believe it would be more clear to construct the result differently in the firstresult case
if firstresult:
outcome = _Result(results[0] if results else None, excinfo)
else:
outcome = _Result(results, excinfo)There was a problem hiding this comment.
thanks ronny that's way cleaner 👍
Avoids forcing a result with `firstresult` hooks. Thanks to @RonnyPfannschmidt for the nice simplification.
|
@RonnyPfannschmidt added your cleaner code. Let me know if we need more. |
|
Tested with |
I think this best solves pytest-dev/pytest#2730 and #71 and is pretty clear.
I'm of course open to a better way if you guys think of one.
Sorry about the delay...