fix #2540, introduce mark.with_args#2600
Conversation
RonnyPfannschmidt
force-pushed
the
mark_explicit_params
branch
2 times, most recently
from
86a1cb1 to
0e3d220
Compare
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
Looks like a good-enough solution to the problem, nice!
This needs a CHANGELOG entry and some docs
| class SomeClass(object): | ||
| pass | ||
|
|
||
| assert pytest.mark.fun(some_function) is some_function |
There was a problem hiding this comment.
We should also verify that the mark is correctly assigned I think
There was a problem hiding this comment.
other tests do that
| pass | ||
|
|
||
| assert pytest.mark.fun(some_function) is some_function | ||
| assert pytest.mark.fun.with_args(some_function) is not some_function |
There was a problem hiding this comment.
Probably safer to test that it is a MarkInfo object
There was a problem hiding this comment.
i merely want to assert return value behavior, the test as is is sufficient and minimal
There was a problem hiding this comment.
i merely want to assert return value behavior, the test as is is sufficient and minimal
Until someone refactors the code and this starts to return not some_function but also not a MarkInfo, and then unexpected breakage occurs. 😜
But OK then.
There was a problem hiding this comment.
uhm, other tests should catch that - if all tests test all the things we overcover and make change hard
There was a problem hiding this comment.
I might be misunderstanding the change then, because IIUC pytest.mark.fun.with_args(some_function) is a new feature (so no other tests use it yet) and we expect it to return a MarkInfo, but we are not making that assertion anywhere.
But no biggie really, if you think this is fine no worries.
There was a problem hiding this comment.
i ensured mark.__call__ is using it ^^ after all its an extraction of the tail of that function
There was a problem hiding this comment.
Oh right right, sorry for the noise. 😁
RonnyPfannschmidt
force-pushed
the
mark_explicit_params
branch
from
0e3d220 to
65b2de1
Compare
| @@ -0,0 +1 @@ | |||
| Introduce ``mark.with_args`` in order to allow passing functions/classes as sole argument to marks. No newline at end of file | |||
There was a problem hiding this comment.
Shouldn't we add some docs as well, preferably with an example?
3 participants
this extracts the bit about adding params and exposes it in a unconditional way