empty parameterset - enable opt to xfail by RonnyPfannschmidt · Pull Request #3044 · pytest-dev/pytest

RonnyPfannschmidt · 2017-12-17T11:52:28Z

The-Compiler · 2017-12-17T12:08:30Z

+        from pytest import UsageError
+        raise UsageError(
+            "empty_parameterset must be one of skip and xfail,"
+            " insteat it is {!r}".format(empty_parameterset))


insteat -> instead (but "but" would be a better match probably)

coveralls · 2017-12-17T14:48:39Z

Coverage decreased (-0.0004%) to 92.595% when pulling 2b62717 on RonnyPfannschmidt:parameterset-empty-enable-xfail into d872791 on pytest-dev:features.

nicoddemus

Looks like a great start, thanks!

Besides the few comments I made, I think we still need to add the documentation for this feature in customize.rst.

nicoddemus · 2017-12-25T23:47:18Z

+    fs, lineno = getfslineno(function)
+    reason = "got empty parameter set %r, function %s at %s:%d" % (
+        argnames, function.__name__, fs, lineno)
+    return mark(reason=reason)


For requested_mark == 'xfail' this will be equivalent to MARK_GEN.xfail(run=False)(reason=reason)... is this right?

Weird, so pytest.mark.xfail(run=False)(reason=reason) produces the correct result?

@nicoddemus pytest marks support compounding addition of args/kwargs

mark.foo(1)(2)(2)(b=2) == mark.foo(1,2,3,b=3)

nicoddemus · 2017-12-25T23:50:28Z

+    if mark is None:
+        # normalize to the requested name
+        mark = 'skip'
+    assert result_mark.name == mark


Please also check the reason attribute

coveralls · 2017-12-26T00:09:47Z

Coverage increased (+0.04%) to 92.669% when pulling 169635e on RonnyPfannschmidt:parameterset-empty-enable-xfail into b8be339 on pytest-dev:features.

…t action

RonnyPfannschmidt · 2018-01-26T10:27:36Z

@nicoddemus changes are implemented, will create a followup issue wrt the deprecation/option change timeline

nicoddemus

Thanks @RonnyPfannschmidt it will be great to get this into 3.4. Please take a look at my comments.

nicoddemus · 2018-01-26T10:36:04Z



+def get_empty_parameterset_mark(config, argnames, function):
+    requested_mark = config.getini('empty_parameterset')


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?

yup, a moment please for the fix

nicoddemus · 2018-01-26T10:39:27Z

@@ -0,0 +1 @@
+introduce a pytest ini option to pick the mark for empty parametersets and allow to use xfail(run=False)


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.

nicoddemus · 2018-01-26T10:40:16Z

+
+	.. versionadded:: 3.4
+
+	allows to pick the action for empty parametersets in parameterization


This should start with upper case: Allows ...

nicoddemus · 2018-01-26T10:41:39Z

+	* ``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.


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

nicoddemus · 2018-01-26T10:42:26Z

        assert (lhs == rhs) == expected
+
+
+@pytest.mark.parametrize('mark', [None, 'skip', 'xfail'])


You check for the empty string in the code, should you add an empty string here too I suppose?

nicoddemus

Thanks, LGTM!

RonnyPfannschmidt · 2018-01-26T11:06:35Z

now it should actially pass, there was a small oversight on my part, i think i'd like to create a constant

nicoddemus · 2018-01-27T13:04:22Z

@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. 👍

The-Compiler reviewed Dec 17, 2017

View reviewed changes

RonnyPfannschmidt force-pushed the parameterset-empty-enable-xfail branch from 0d88125 to 156abd0 Compare December 17, 2017 12:14

nicoddemus requested changes Dec 25, 2017

View reviewed changes

RonnyPfannschmidt added 5 commits January 26, 2018 11:10

parameterset: refactor marking empty parametersets

bf2c10c

fix pytest-dev#2527 - introduce a option to pic the empty parameterse…

37b41de

…t action

fix empty parameterset tests by mocking a config object

7f83605

exted empty parameterset check with reason test

d4c11e5

document empty_parameterset in customize.rst

8979b2a

RonnyPfannschmidt force-pushed the parameterset-empty-enable-xfail branch from 04a2778 to 8979b2a Compare January 26, 2018 10:26

RonnyPfannschmidt mentioned this pull request Jan 26, 2018

plan and implement the change of empty_parameterset values #3155

Closed

nicoddemus requested changes Jan 26, 2018

View reviewed changes

RonnyPfannschmidt added 2 commits January 26, 2018 11:56

s/empty_parameterset/empty_parameter_set_mark

d550c33

enhance docs for empty_parameter_set_mark according to review comments

77de45c

nicoddemus approved these changes Jan 26, 2018

View reviewed changes

correct testing and usage of the empty_parameter_set_mark config option

a54cd4c

RonnyPfannschmidt and others added 2 commits January 26, 2018 12:12

use a constant to sort out repeated use of the EMPTY_PARAMETERSET_OPTION

17a1ed5

Move example of empty_parameter_set_mark closer to the options

169635e

nicoddemus merged commit b3247c1 into pytest-dev:features Jan 27, 2018

RonnyPfannschmidt deleted the parameterset-empty-enable-xfail branch February 21, 2018 14:25



		def get_empty_parameterset_mark(config, argnames, function):
		requested_mark = config.getini('empty_parameterset')

		@@ -0,0 +1 @@
		introduce a pytest ini option to pick the mark for empty parametersets and allow to use xfail(run=False) No newline at end of file


		.. versionadded:: 3.4

		allows to pick the action for empty parametersets in parameterization

		assert (lhs == rhs) == expected


		@pytest.mark.parametrize('mark', [None, 'skip', 'xfail'])

Uh oh!

Conversation

RonnyPfannschmidt commented Dec 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 17, 2017

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jan 26, 2018

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

RonnyPfannschmidt commented Jan 26, 2018

Uh oh!

nicoddemus commented Jan 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Dec 26, 2017 •

edited

Loading