Skip to content

Export types of builtin fixtures for type annotations #8017

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 2 commits into from
Dec 5, 2020

Conversation

bluetech
Copy link
Member

@bluetech bluetech commented Nov 9, 2020

The first commit is a hack to fix the issue discussed here: #7469 (comment)

The second commit is a partial fix for #7469 -- only for the fixture types. If this is accepted I will work on the other items on the list, but the fixtures are the most urgent so I'm starting with them.

Please see the commit messages for the approach I took, particularly the "privacy" mechanism in the second commit.

@bluetech bluetech changed the title Export types of builtin fixture for type annotations Export types of builtin fixtures for type annotations Nov 9, 2020
@nicoddemus
Copy link
Member

Unfortunately the _ispytest solution is very verbose requiring many changes, but I don't have a better suggestion. 👍

Copy link
Member

@graingert graingert left a comment

Choose a reason for hiding this comment

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

I think you're missing several *, _ispytest: bool = False defaults

…itself

An upcoming commit wants to import from `_pytest.pytester` in the public
`pytest` module. This means that `_pytest.pytester` would start to get
imported during import time, which it hasn't up to now -- it was
imported by the plugin loader (if requested). When a plugin is loaded,
it is subjected to assertion rewriting, but only if the module isn't
imported yet, it issues a warning "Module already imported so cannot be
rewritten" and skips the rewriting. So we'd end up with the pytester
plugin not being rewritten, but it wants to be.

Absent better ideas, the solution here is to split the pytester
assertions to their own plugin (which will always only be imported by
the plugin loader) and exclude pytester itself from plugin rewriting.
In order to allow users to type annotate fixtures they request, the
types need to be imported from the `pytest` namespace. They are/were
always available to import from the `_pytest` namespace, but that is
not guaranteed to be stable.

These types are only exported for the purpose of typing. Specifically,
the following are *not* public:

- Construction (`__init__`)
- Subclassing
- staticmethods and classmethods

We try to combat them being used anyway by:

- Marking the classes as `@final` when possible (already done).

- Not documenting private stuff in the API Reference.

- Using `_`-prefixed names or marking as `:meta private:` for private
  stuff.

- Adding a keyword-only `_ispytest=False` to private constructors,
  warning if False, and changing pytest itself to pass True. In the
  future it will (hopefully) become a hard error.

Hopefully that will be enough.
@bluetech bluetech force-pushed the typing-public-fixtures branch from ee20d13 to f1e6fdc Compare November 13, 2020 09:27
@bluetech
Copy link
Member Author

I think you're missing several *, _ispytest: bool = False defaults

Whoops!

Thanks for the review. I updated the PR, you can see the changes here.

@bluetech
Copy link
Member Author

bluetech commented Dec 5, 2020

Merging per @nicoddemus's approval and replies to @graingert's comments. Let's see how the _is_pytest stuff works out!

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