-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Refactor direct fixture call warning to avoid incompatibility with plugins #3754
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
Refactor direct fixture call warning to avoid incompatibility with plugins #3754
Conversation
changelog/3747.bugfix.rst
Outdated
@@ -0,0 +1 @@ | |||
Fix compatibility problem with plugins and the the warning code issued by fixture functions when they are called directly. |
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.
drive-by-nit: "the the"
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!
Hmmm 2 failing tests on py27, I will take a look at them tomorrow if the current approach is considered sound by others. |
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.
the approach is sound, lets take a look at __wrapped__
to see if that fixes the issue
@@ -249,6 +249,21 @@ def get_real_func(obj): | |||
return obj | |||
|
|||
|
|||
def get_real_method(obj, holder): |
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 needs to handle __wrapped__
on python2
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.
Hmm get_real_func
already handles __wrapped__
. Can you clarify this a bit?
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.
@nicoddemus my understanding is that method descriptors are not exposing it readily, i will investigate in a bit
…ugins This refactors the code so we have the real function object right during collection. This avoids having to unwrap it later and lose attached information such as "async" functions. Fix pytest-dev#3747
1228b41
to
c6b11b9
Compare
# during fixture definition we wrap the original fixture function | ||
# to issue a warning if called directly, so here we unwrap it in order to not emit the warning | ||
# when pytest itself calls the fixture function | ||
if six.PY2 and unittest: |
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 was the best I could come with; unbound methods are a headache to deal 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.
cant wait for the python2 drop ^^
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.
You and me both! 😁
This refactors the code so we have the real function object right during
collection. This avoids having to unwrap it later and lose attached information
such as "async" functions.
I've tested this with
pytest-asyncio
and it fixes pytest-dev/pytest-asyncio#89.Fix #3747, Fix #3720
Fix pytest-dev/pytest-asyncio#89