-
-
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
Mark expose nontransfered marks via pytestmark property #2517
Changes from 5 commits
bdec2c8
64ae6ae
19b12b2
c791895
1d92601
23d016f
b0b6c35
8d5f287
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,11 @@ | |
""" | ||
from __future__ import absolute_import, division, print_function | ||
|
||
|
||
class RemovedInPytest4_0Warning(DeprecationWarning): | ||
"warning class for features removed in pytest 4.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor nitpick, please use triple strings for docstrings. 😁 |
||
|
||
|
||
MAIN_STR_ARGS = 'passing a string to pytest.main() is deprecated, ' \ | ||
'pass a list of arguments instead.' | ||
|
||
|
@@ -22,3 +27,6 @@ | |
GETFUNCARGVALUE = "use of getfuncargvalue is deprecated, use getfixturevalue" | ||
|
||
RESULT_LOG = '--result-log is deprecated and scheduled for removal in pytest 4.0' | ||
|
||
MARK_INFO_ATTRIBUTE = RemovedInPytest4_0Warning( | ||
"Markinfo attributes are deprecated, please iterate the mark Collection") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be documented on the docs, I think. Users who see this probably won't know what this means. I also suggest to add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no new code for this yet ^^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? Actually, now that I read it again, I'm not sure what you mean by "please iterate the mark Collection"... oh do you mean the marks in the collected item? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. markinfo objects are iterables, atm the iteration yields markinfo objects as wel @hpk42 i wonder if we can make it return the new mark type instead |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,20 @@ | |
from __future__ import absolute_import, division, print_function | ||
|
||
import inspect | ||
import warnings | ||
from collections import namedtuple | ||
from operator import attrgetter | ||
from .compat import imap | ||
from .deprecated import MARK_INFO_ATTRIBUTE | ||
|
||
def alias(name, warning=None): | ||
getter = attrgetter(name) | ||
|
||
def alias(name): | ||
return property(attrgetter(name), doc='alias for ' + name) | ||
def warned(self): | ||
warnings.warn(warning, stacklevel=2) | ||
return getter(self) | ||
|
||
return property(getter if warning is None else warned, doc='alias for ' + name) | ||
|
||
|
||
class ParameterSet(namedtuple('ParameterSet', 'values, marks, id')): | ||
|
@@ -329,30 +336,40 @@ def __call__(self, *args, **kwargs): | |
is_class = inspect.isclass(func) | ||
if len(args) == 1 and (istestfunc(func) or is_class): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion to improve readability here:
I find a good practice to always assert for a known condition in a if condition1:
...
elif condition2:
...
else:
assert condition3
... |
||
if is_class: | ||
if hasattr(func, 'pytestmark'): | ||
mark_list = func.pytestmark | ||
if not isinstance(mark_list, list): | ||
mark_list = [mark_list] | ||
# always work on a copy to avoid updating pytestmark | ||
# from a superclass by accident | ||
mark_list = mark_list + [self] | ||
func.pytestmark = mark_list | ||
else: | ||
func.pytestmark = [self] | ||
apply_mark(func, self.mark) | ||
else: | ||
holder = getattr(func, self.name, None) | ||
if holder is None: | ||
holder = MarkInfo(self.mark) | ||
setattr(func, self.name, holder) | ||
else: | ||
holder.add_mark(self.mark) | ||
apply_legacy_mark(func, self.mark) | ||
apply_mark(func, self.mark) | ||
return func | ||
|
||
mark = Mark(self.name, args, kwargs) | ||
return self.__class__(self.mark.combined_with(mark)) | ||
|
||
|
||
def apply_mark(obj, mark): | ||
assert isinstance(mark, Mark), mark | ||
"""applies a marker to an object, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docstring should be the first statement here (before the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to review the PR, the code, the tests and use the debugger to finally get a picture of what's going on. 😅 OK IIUC Perhaps the docstring of both methods should describe exactly what's going on? Suggestions: def apply_mark(obj, mark):
"""
Sets or updates the ``pytestmark`` attribute on ``obj`` with the given ``mark`` object.
In the future (see issue X), this is the only effect ``@pytest.mark`` will have, and the
mark transfering from classes and modules to test functions will be done on
``pytest.Item`` objects instead.
"""
def update_legacy_mark_info(obj, mark):
"""
Updates or creates a ``MarkInfo`` as an attribute of the given object.
This is how markers are transfered from classes and modules to test functions in the legacy way,
which is planned to be phased out.
""" Also, we are now introducing a public way to get "pristine" (non-inherited) markers on functions by using a machanism which is already used elsewhere, the I suggest we take this opportunity to introduce a proper public API which hides this detail, like def get_pristine_marks(obj):
"""
Returns pytest marks applied directly to the given object, without any inheritance from
parent classes decorated with ``@pytest.mark`` or parent modules with the ``pytestmark``
attribute.
""" Then we don't have to "promise" that we will be supporting a On the other hand now that I think about it, What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pytestmark will stay, no need to change it im against a get_pristine_marks function, - the marks api people use to get/interact with marks sould be an new object on item that we will provide after all marks in and from parameterization are to be considered as well and those are a pain to get There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was actually suggesting using Side topic: usually when things decorate user code, it might be a better to use a public API to manipulate what's being decorated instead of making the decorated attribute public. For example the That was the general feeling that I had when proposing to use an API to access the list of unmerged marks: it would allow us to use an internal ugly name, possibly change this in the future to store that information somewhere without breaking user expectations, and even do some more work instead of just returning an internal attribute of the object. But as I said, let's move forward with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (sorry for all the internal rambling) |
||
makrer transfers only update legacy markinfo objects | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: marker |
||
""" | ||
mark_list = getattr(obj, 'pytestmark', []) | ||
|
||
if not isinstance(mark_list, list): | ||
mark_list = [mark_list] | ||
# always work on a copy to avoid updating pytestmark | ||
# from a superclass by accident | ||
mark_list = mark_list + [mark] | ||
obj.pytestmark = mark_list | ||
|
||
|
||
def apply_legacy_mark(func, mark): | ||
if not isinstance(mark, Mark): | ||
raise TypeError("got {mark!r} instead of a Mark".format(mark=mark)) | ||
holder = getattr(func, mark.name, None) | ||
if holder is None: | ||
holder = MarkInfo(mark) | ||
setattr(func, mark.name, holder) | ||
else: | ||
holder.add_mark(mark) | ||
|
||
|
||
class Mark(namedtuple('Mark', 'name, args, kwargs')): | ||
|
@@ -371,9 +388,9 @@ def __init__(self, mark): | |
self.combined = mark | ||
self._marks = [mark] | ||
|
||
name = alias('combined.name') | ||
args = alias('combined.args') | ||
kwargs = alias('combined.kwargs') | ||
name = alias('combined.name', warning=MARK_INFO_ATTRIBUTE) | ||
args = alias('combined.args', warning=MARK_INFO_ATTRIBUTE) | ||
kwargs = alias('combined.kwargs', warning=MARK_INFO_ATTRIBUTE) | ||
|
||
def __repr__(self): | ||
return "<MarkInfo {0!r}>".format(self.combined) | ||
|
@@ -389,3 +406,32 @@ def __iter__(self): | |
|
||
|
||
MARK_GEN = MarkGenerator() | ||
|
||
|
||
def _marked(func, mark): | ||
""" Returns True if :func: is already marked with :mark:, False otherwise. | ||
This can happen if marker is applied to class and the test file is | ||
invoked more than once. | ||
""" | ||
try: | ||
func_mark = getattr(func, mark.name) | ||
except AttributeError: | ||
return False | ||
return mark.args == func_mark.args and mark.kwargs == func_mark.kwargs | ||
|
||
|
||
def transfer_markers(funcobj, cls, mod): | ||
""" | ||
transfer legacy markers to the function level marminfo objects | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: |
||
this one is a major fsckup for mark breakages | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be nice to expand on this here; the details are scattered through multiple issues, here would be a good place to summarize them IMO |
||
""" | ||
for obj in (cls, mod): | ||
mark_list = getattr(obj, 'pytestmark', []) | ||
|
||
if not isinstance(mark_list, list): | ||
mark_list = [mark_list] | ||
|
||
for mark in mark_list: | ||
mark = getattr(mark, 'mark', mark) # unpack MarkDecorator | ||
if not _marked(funcobj, mark): | ||
apply_legacy_mark(funcobj, mark) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
safe_str, getlocation, enum, | ||
) | ||
from _pytest.runner import fail | ||
|
||
from _pytest.mark import transfer_markers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe just empty line after imports? |
||
cutdir1 = py.path.local(pluggy.__file__.rstrip("oc")) | ||
cutdir2 = py.path.local(_pytest.__file__).dirpath() | ||
cutdir3 = py.path.local(py.__file__).dirpath() | ||
|
@@ -361,35 +361,6 @@ def _genfunctions(self, name, funcobj): | |
) | ||
|
||
|
||
def _marked(func, mark): | ||
""" Returns True if :func: is already marked with :mark:, False otherwise. | ||
This can happen if marker is applied to class and the test file is | ||
invoked more than once. | ||
""" | ||
try: | ||
func_mark = getattr(func, mark.name) | ||
except AttributeError: | ||
return False | ||
return mark.args == func_mark.args and mark.kwargs == func_mark.kwargs | ||
|
||
|
||
def transfer_markers(funcobj, cls, mod): | ||
# XXX this should rather be code in the mark plugin or the mark | ||
# plugin should merge with the python plugin. | ||
for holder in (cls, mod): | ||
try: | ||
pytestmark = holder.pytestmark | ||
except AttributeError: | ||
continue | ||
if isinstance(pytestmark, list): | ||
for mark in pytestmark: | ||
if not _marked(funcobj, mark): | ||
mark(funcobj) | ||
else: | ||
if not _marked(funcobj, pytestmark): | ||
pytestmark(funcobj) | ||
|
||
|
||
class Module(main.File, PyCollector): | ||
""" Collector for test classes and functions. """ | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
store unmeshed marks on functions pytestmark attribute | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be more descriptive to end users:
Also we should definitely document this properly somewhere on the docs and make it official part of the API. |
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 think just
RemovedPytest4Warning
is enough, no need to add the_0
in my opinion.