Skip to content
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

fix #2540, introduce mark.with_args #2600

Merged

Conversation

RonnyPfannschmidt
Copy link
Member

this extracts the bit about adding params and exposes it in a unconditional way

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the mark_explicit_params branch 2 times, most recently from 86a1cb1 to 0e3d220 Compare July 21, 2017 06:30
@pytest-dev pytest-dev deleted a comment from coveralls Jul 21, 2017
@pytest-dev pytest-dev deleted a comment from coveralls Jul 21, 2017
@pytest-dev pytest-dev deleted a comment from coveralls Jul 21, 2017
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also verify that the mark is correctly assigned I think

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other tests do that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

pass

assert pytest.mark.fun(some_function) is some_function
assert pytest.mark.fun.with_args(some_function) is not some_function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably safer to test that it is a MarkInfo object

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i merely want to assert return value behavior, the test as is is sufficient and minimal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm, other tests should catch that - if all tests test all the things we overcover and make change hard

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i ensured mark.__call__ is using it ^^ after all its an extraction of the tail of that function

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right right, sorry for the noise. 😁

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 90.81% when pulling 65b2de1 on RonnyPfannschmidt:mark_explicit_params into cbceef2 on pytest-dev:features.

@nicoddemus nicoddemus merged commit 6461dc9 into pytest-dev:features Jul 21, 2017
@@ -0,0 +1 @@
Introduce ``mark.with_args`` in order to allow passing functions/classes as sole argument to marks.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add some docs as well, preferably with an example?

@RonnyPfannschmidt RonnyPfannschmidt deleted the mark_explicit_params branch November 13, 2017 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants