-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Remove support for applying marks to values in parametrize #4571
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
Remove support for applying marks to values in parametrize #4571
Conversation
"equal to the number of names ({})".format( | ||
param.values, argnames | ||
) | ||
msg = ( |
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.
Improved this message, it now looks like this:
_________________________________________ ERROR collecting test-param-mark.py _________________________________________
test-param-mark.py::test_increment: in "parametrize" the number of names (2):
('n', 'expected')
must be equal to the number of values (3):
(1, 2, 4)
@@ -82,39 +81,23 @@ def param(cls, *values, **kw): | |||
return cls(values, marks, id_) | |||
|
|||
@classmethod | |||
def extract_from(cls, parameterset, belonging_definition, legacy_force_tuple=False): | |||
def extract_from(cls, parameterset, force_tuple=False): |
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.
that parameter name shouldnt be changed
also taking away belonging definition is a breaking change
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.
Hmmm ParameterSet
is not public API and is not exposed in the pytest.
namespace, so I don't think this is a breaking change.
About the parameter name I renamed it because I don't understand why it was named legacy
, it seems like it is part of the normal workflow...
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.
actually right, that one is a internal one, we should make it a normal function, so its not accidentially exposed on instances
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.
That's a good point, but I think that's not even necessary because one can't even access a ParameterSet
instance, even from hooks (I think).
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 its exposed as return value of pytest.param
Codecov Report
@@ Coverage Diff @@
## features #4571 +/- ##
============================================
- Coverage 95.74% 95.73% -0.02%
============================================
Files 111 111
Lines 24888 24863 -25
Branches 2452 2449 -3
============================================
- Hits 23829 23802 -27
- Misses 746 749 +3
+ Partials 313 312 -1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## features #4571 +/- ##
============================================
- Coverage 95.74% 95.73% -0.02%
============================================
Files 111 111
Lines 24888 24863 -25
Branches 2452 2449 -3
============================================
- Hits 23829 23802 -27
- Misses 746 749 +3
+ Partials 313 312 -1
Continue to review full report at Codecov.
|
Fix #3082