-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update config._getconftest_pathlist for pytest-dev #157
Conversation
Something else need changing to be compatible with pytest-dev
|
pytest_doctestplus/plugin.py
Outdated
@@ -203,10 +210,10 @@ def collect(self): | |||
self.fspath) | |||
else: | |||
try: | |||
module = self.fspath.pyimport() | |||
module = fspath.pyimport() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change broke something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that method doesn't exist on Path
objects, so I will need to reimplement it I guess.
except TypeError: | ||
# Pytest 7.0+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not doing version check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot about it ;). And this is a bit of a mess, some changes are in 6.3 which is not released yet, and some will be in 7.0. And dev is currently 6.3.devXXX though it includes the 7.0 changes apparently.
So I added PYTEST_GT_6_3 after but I'm still not sure if that's a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean try/except is easy to do, distinguishing 6.3 and 7.0 is currently not obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, maybe we can merge as is but open issue to revisit in the future because you don't want the stable pytest to keep hitting the except
block.
Hah, me too! 😸 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Ref pytest-dev/pytest#7870
Close #156