-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Allow staticmethods to be detected as test functions #2531
Conversation
testing/python/collect.py
Outdated
@@ -143,6 +143,18 @@ class test(object): | |||
"*collected 0*", | |||
]) | |||
|
|||
def test_staticmethid(self, testdir): |
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.
test_static_method or test_staticmethod would be better
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.
Spelling fix: argh, yes. Underscores: happy to change it but I was copying the decorator name and the method just below named test_setup_teardown_class_as_classmethod
(for the @classmethod
decorator)
Thanks! This should go to |
Definitely. 👍 Also, please rename the changelog entry to |
Changed it to a feature and changed the target branch to |
Thanks! |
Seems to fail with "py26" (restarted out of current habit). |
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 like it and i'd like to hear other opinions
python 2.6 needs support code - we do have a helper for that |
Sorry I didn't try the fix on 2.6 earlier. The 2.6 failure is because 2.6 staticmethods are different -- they don't have the @RonnyPfannschmidt can you point me to the 2.6 helper-code you mentioned? |
i was wrong, the helper cant apply because the functionality cant work on python 2.6 i propose intentionally leaving it crippled on python2.6 and skipping the tests there + adding a comment the problem is that staticmethod on python26 doesn't expose one can abuse |
By the way, are staticmethods always guaranteed to have a |
@The-Compiler as far as i can tell its supported on all python versions that are not EOL |
i propose simply issuing a warning if the attribute is not present |
Thanks for all the comments! I pushed a new version where I attempt to implement @RonnyPfannschmidt's suggestion: if it looks like a test, and it's a static method, but we can't find a |
_pytest/python.py
Outdated
self.warn(code="C2", message="cannot collect static method %r because it is not a function (always the case in Python 2.6)" % name) | ||
return ( | ||
safe_getattr(obj, "__call__", False) and fixtures.getfixturemarker(obj) is None | ||
) |
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.
There should probably be a return False
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.
Thanks a lot! Just asking for two small changes which once addressed it can be merged IMHO.
testing/python/collect.py
Outdated
result = testdir.runpytest() | ||
if sys.version_info < (2,7): | ||
# in 2.6, the code to handle static methods doesn't work | ||
result.stdout.fnmatch_lines(["*collected 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.
Please improve matching here:
if sys.version_info < (2, 7):
result.stdout.fnmatch_lines([
"*collected 0 items*",
"*cannot collect static method*",
])
else:
result.stdout.fnmatch_lines([
"*collected 1*",
"*1 passed in*",
])
This way we ensure that we are showing the appropriate warning and that we are collecting and running it in Py27+.
return False | ||
return ( | ||
safe_getattr(obj, "__call__", False) and fixtures.getfixturemarker(obj) is None | ||
) |
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.
For consistency please return False
in case you don't execute the block inside if self.funcnamefilter(name) or self.isnosetest(obj):
.
I'll push the requested changes on Thursday. (I love how quickly this got reviewed but I'm away from my editor)
|
Sure thing, no rush, and thanks again! |
Allow a class method decorated `@staticmethod` to be collected as a test function (if it meets the usual criteria). This feature will not work in Python 2.6 -- static methods will still be ignored there.
thanks, well done 👍 |
Thank you! This was a fun PR. You're all doing an awesome job of making contributions easy! |
Allow a class method decorated
@staticmethod
to be collected as a test function(if it meets the usual criteria).
Fixes #2528
bugfix
,vendor
,doc
ortrivial
fixes, targetmaster
; for removals or features targetfeatures
;AUTHORS
;