Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace raise StopIteration with return - adhere to Pep 479 #2160

Closed
goodboy opened this issue Dec 24, 2016 · 7 comments
Closed

Replace raise StopIteration with return - adhere to Pep 479 #2160

goodboy opened this issue Dec 24, 2016 · 7 comments

Comments

@goodboy
Copy link
Member

@goodboy goodboy commented Dec 24, 2016

As per pytest-dev/pluggy#38 and pep 479, it seems that some pluggy hook wrappers implemented in pytest are still using the soon to be deprecated raise StopIteration mechanism to signal early generator termination.

An example would be pytest_pycollect_makeitem.

As indicated in the pep all such usage should be changed to simple returns in Py3.6+.

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Dec 24, 2016

its impossible to fix this as of now due to support for old python versions

@goodboy
Copy link
Member Author

@goodboy goodboy commented Dec 25, 2016

@RonnyPfannschmidt I'm not sure that's true?

Supposedly all code can be made backwards compatible even in Py2.6 as per the original generator's Pep 255. Maybe I'm forgetting something though?

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Dec 26, 2016

@tgoodlet indeed, the example looks perfectly valid to convert, i may have been overly pessimistic
we should go and try at least, perhaps we can convert everything just fine

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Dec 27, 2016

Unless I'm missing something (and that's a possibility since I didn't read PEP-479 thoroughly), can we just replace the raise StopIteration by a return?

This patch seems to work fine in Py2.6, 2.7 and 3.5:

diff --git a/_pytest/python.py b/_pytest/python.py
index e46f2f1..2973d43 100644
--- a/_pytest/python.py
+++ b/_pytest/python.py
@@ -174,7 +174,7 @@ def pytest_pycollect_makeitem(collector, name, obj):
     outcome = yield
     res = outcome.get_result()
     if res is not None:
-        raise StopIteration
+        return
     # nothing was collected elsewhere, let's do it here
     if isclass(obj):
         if collector.istestclass(obj, name):
@goodboy
Copy link
Member Author

@goodboy goodboy commented Dec 28, 2016

@RonnyPfannschmidt Sounds good :)
@nicoddemus Yep you've got it.

You guys want me to make a PR?

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Dec 28, 2016

Please go ahead. 😁 Thanks!

@tiran
Copy link

@tiran tiran commented Feb 1, 2017

The problem is causing test failures in Custodia on Python 3.6. The test turn all deprecation warnings into fatal failures. For Custodia I put a band-aid on the issue and filter out deprecation warnings from '_pytest\..*'.

latchset/custodia#105 (comment)

This was referenced Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants