Skip to content

Type annotate pytest.mark.{skip,skipif,xfail,parametrize} #7379

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

Merged
merged 1 commit into from
Jun 21, 2020

Conversation

bluetech
Copy link
Member

These marks are some of the most used user-facing APIs in pytest, but because they are marks, which accept any arguments and are evaluated dynamically, there aren't any places to type them like regular functions.

This PR does a little cheating by special-casing them inside MarkGenerator. This is a gross breach of abstraction and other bad stuff, in particular it is not generalizable to 3rd-party marks (at least, if accessed through pytest.mark). However, due to the pervasiveness of these marks I think it is worth it, until/if we find a better solution.

I typed the marks as complete functions, i.e. building "partial" marks (like get_empty_parameterset_mark in the diff does) is not allowed. I think that's fine for these marks at least.

@bluetech bluetech force-pushed the typing-builtin-marks branch from c39eee6 to 00f6a7e Compare June 16, 2020 19:13
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.

Thanks! I agree the workaround is worth it, and only affects typing, so it shouldn't introduce unexpected problems.

FTR there is some relation with #5424.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Can users opt out of this, as this introduces partial mark incompatible typing

@bluetech
Copy link
Member Author

Can users opt out of this, as this introduces partial mark incompatible typing

As currently written, they can't. Mypy doesn't support functools.partial so I'm pretty sure there isn't a proper way currently to type partial marks.

It is possible for the user to "cheat" by using e.g. getattr(pytest.mark, "xfail") which will give them a MarkDecorator instead of the specific _XfailMarkDecorator subtype, but that of course is not obvious nor recommended.

@nicoddemus
Copy link
Member

introduces partial mark incompatible typing

What's that exactly, is it related to functools.partial as @bluetech commented? Can we see an example?

I ask because this is in line with the work started in #5424, where one of the outcomes (at least in my mind) is that we would not generate the builtin marks dynamically anymore, but would have proper signatures declared, very similar to what we have here.

@RonnyPfannschmidt
Copy link
Member

@bluetech i believe its possible to type this correct as long as the actual return type supports a overridden call and a override with_params (so stored partial markers could be applied correctly to classes and be expanded at usage sites and still type correct

@bluetech
Copy link
Member Author

Let me clarify the partial mark comment. There is a minor issue there but it's mostly a red herring, I didn't understand it quite right.

This patch only affects the mark-generator (pytest.mark.XXX) step, after the mark is created (pytest.mark.XXX(...)) it becomes just a generic Mark which you can do anything you want with.

So the difference is, that for the builtin marks, the pytest.mark.XXX.__call__ becomes typed:

  1. Cannot provide extra args/kwargs that are unexpected
  2. Must provide all required args/kwargs

(1) I think is fine, shouldn't do that. If for some reason want to provide extra args, can add them as a second step after the mark is created, e.g. pytest.mark.skip("reason")(extra="foo").

(2) is actually mostly a non-issue because out of all of pytest's builtin marks, only pytest.parametrize has required arguments. And I think it's pretty weird for someone to write pytest.mark.parametrize("key")(["value1", "value2"]). But maybe that's a thing?

@RonnyPfannschmidt mentioned with_args, I haven't noticed it before, but because I don't override it it can also be used as a workaround: pytest.mark.parametrize.with_args("key")(["value1", "value2"]).

@bluetech
Copy link
Member Author

BTW there are a few builtin marks I forgot (looking at pytest --markers), I might add (some of) them:

  • pytest.mark.filterwarnings
  • pytest.mark.usefixtures
  • pytest.mark.tryfirst
  • pytest.mark.trylast

@RonnyPfannschmidt
Copy link
Member

Thanks for elaborating the details, with that my misunderstanding is cleared

@bluetech bluetech force-pushed the typing-builtin-marks branch from 00f6a7e to 0188cf2 Compare June 21, 2020 17:13
@bluetech bluetech force-pushed the typing-builtin-marks branch from 0188cf2 to b3fb5a2 Compare June 21, 2020 17:19
@bluetech
Copy link
Member Author

I added typing also for filterwarnings and usefixtures.

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.

3 participants