empty parameterset - enable opt to xfail #3044
Conversation
_pytest/mark.py
Outdated
| from pytest import UsageError | ||
| raise UsageError( | ||
| "empty_parameterset must be one of skip and xfail," | ||
| " insteat it is {!r}".format(empty_parameterset)) |
There was a problem hiding this comment.
insteat -> instead (but "but" would be a better match probably)
RonnyPfannschmidt
force-pushed
the
parameterset-empty-enable-xfail
branch
from
0d88125
to
156abd0
Compare
| fs, lineno = getfslineno(function) | ||
| reason = "got empty parameter set %r, function %s at %s:%d" % ( | ||
| argnames, function.__name__, fs, lineno) | ||
| return mark(reason=reason) |
There was a problem hiding this comment.
For requested_mark == 'xfail' this will be equivalent to MARK_GEN.xfail(run=False)(reason=reason)... is this right?
There was a problem hiding this comment.
Weird, so pytest.mark.xfail(run=False)(reason=reason) produces the correct result?
There was a problem hiding this comment.
@nicoddemus pytest marks support compounding addition of args/kwargs
mark.foo(1)(2)(2)(b=2) == mark.foo(1,2,3,b=3)
testing/test_mark.py
Outdated
| if mark is None: | ||
| # normalize to the requested name | ||
| mark = 'skip' | ||
| assert result_mark.name == mark |
There was a problem hiding this comment.
Please also check the reason attribute
RonnyPfannschmidt
force-pushed
the
parameterset-empty-enable-xfail
branch
from
04a2778
to
8979b2a
Compare
|
@nicoddemus changes are implemented, will create a followup issue wrt the deprecation/option change timeline |
There was a problem hiding this comment.
Thanks @RonnyPfannschmidt it will be great to get this into 3.4. Please take a look at my comments.
_pytest/mark.py
Outdated
| @@ -97,6 +94,20 @@ def _for_parameterize(cls, argnames, argvalues, function): | |||
| return argnames, parameters | |||
|
|
|||
|
|
|||
| def get_empty_parameterset_mark(config, argnames, function): | |||
| requested_mark = config.getini('empty_parameterset') | |||
There was a problem hiding this comment.
Now that I take a look another look at it, I think this option should be named empty_parameter_set_mark instead, what do you think?
There was a problem hiding this comment.
yup, a moment please for the fix
changelog/2527.feature
Outdated
| @@ -0,0 +1 @@ | |||
| introduce a pytest ini option to pick the mark for empty parametersets and allow to use xfail(run=False) | |||
There was a problem hiding this comment.
I think this can be improved to be more user-oriented:
Introduce ``empty_parameter_set_mark`` ini option to select which mark to apply when ``@pytest.mark.parametrize`` is given an empty set of parameters. Valid options are ``skip`` (default) and ``xfail``. Note that it is planned to change the default to ``xfail`` in future releases as this is considered less error prone.
doc/en/customize.rst
Outdated
|
|
||
| .. versionadded:: 3.4 | ||
|
|
||
| allows to pick the action for empty parametersets in parameterization |
There was a problem hiding this comment.
This should start with upper case: Allows ...
doc/en/customize.rst
Outdated
| * ``skip`` skips tests with a empty parameterset | ||
| * ``xfail`` marks tests with a empty parameterset as xfail(run=False) | ||
|
|
||
| The default is ``skip``, it will be shifted to xfail in future. |
There was a problem hiding this comment.
I think mentioning that skip is default in its own description is slightly better:
* ``skip`` skips tests with a empty parameter set (default)And use this paragraph to mention that we plan to change the default in a future release with a link to #3155
testing/test_mark.py
Outdated
| @@ -891,3 +891,26 @@ class TestMarkDecorator(object): | |||
| ]) | |||
| def test__eq__(self, lhs, rhs, expected): | |||
| assert (lhs == rhs) == expected | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize('mark', [None, 'skip', 'xfail']) | |||
There was a problem hiding this comment.
You check for the empty string in the code, should you add an empty string here too I suppose?
|
now it should actially pass, there was a small oversight on my part, i think i'd like to create a constant |
|
@RonnyPfannschmidt took the liberty of making a slight tweak to the docs in 169635e, hope you don't mind. As soon as CI passes we should merge this. 👍 |
closes #2527