pytest
pytest copied to clipboard
pytest-dev
Typing and public API
pytest's "official" public API is exported by the pytest package, and everything else is defined in the _pytest package.
Currently, pytest only contains APIs that users need to use directly: functions to be called, decorators, classes to be instantiated or subclasssed, globals. Other objects, which the user doesn't interact with directly, are not exported.
Typing extends the range of cases in which some object needs to be referred to by name. As long as the user can somehow reach an object, directly or indirectly, through some public API, they need to be able to refer to its type by name, due to type annotations.
To come up with a list of reachable but unexported types, I used the following categories:
-
Fixtures: built-in fixtures that are dependency-injected. To benefit from the types the user needs to type annotate them, for example:
def test_it(monkeypatch: MonkeyPatch) -> None: ... -
Hooks: the user needs to be able to type annotate the arguments and return values of any hook.
-
Transitive: types that are referred to transitively by some other public API, e.g. return value of a function, method, context manager, public attribute etc.
See below for the list I came up with.
Questions for discussion:
-
Is this a problem we want to fix?
-
Most of these types are classes, that are not intended to be instantiated and/or subclassed by users, only internally.
2.1. Do we want to enforce this? 2.2. How do we enforce this?
-
How do we want to officialy declare something as public API?
-
Which types from the list below are we actually ready to export?
-
Is there a category or specific types I missed?
I'll add my own thoughts on these questions in a separate reply.
Fixtures:
- [X]
_pytest.fixtures.FixtureRequest->pytest.FixtureRequest - [ ]
_pytest.fixtures.SubRequest - [x]
_pytest.cacheprovider.Cache->pytest.Cache - [x]
_pytest.capture.CaptureFixture->pytest.CaptureFixture - [x]
_pytest.logging.LogCaptureFixture->pytest.LogCaptureFixture - [x]
_pytest.pytester.Pytester->pytest.Pytester - [x]
_pytest.pytester.Testdir->pytest.Testdir - [x]
_pytest.tmpdir.TempdirFactory->pytest.TempdirFactory - [x]
_pytest.tmpdir.TempPathFactory->pytest.TempPathFactory - [x]
_pytest.monkeypatch.MonkeyPatch->pytest.MonkeyPatch - [x]
_pytest.recwarn.WarningsRecorder->pytest.WarningsRecorder
Hooks:
- [x]
_pytest.config.Config->pytest.Config - [x]
_pytest.config.PytestPluginManager->pytest.PytestPluginManager - [x]
_pytest.config.argparsing.Parser->pytest.Parser - [x]
_pytest.reports.CollectReport->pytest.CollectReport - [x] ~~
_pytest.python.PyCollector~~ (Made private #9264) - [x]
_pytest.python.Metafunc->pytest.Metafunc - [x]
_pytest.runner.CallInfo->pytest.CallInfo - [x]
_pytest.reports.TestReport->pytest.TestReport - [ ]
_pytest.fixtures.SubRequest - [x]
_pytest.fixtures.FixtureDef - [ ]
_pytest.terminal.TerminalReporter - [ ]
_pytest._code.code.ExceptionRepr - [x]
_pytest.code.ExceptionInfo->pytest.ExceptionInfo
API:
- [ ]
_pytest.mark.structures.ParameterSet
Transitive:
- [ ]
_pytest.fixtures.FuncFixtureInfo(thruMetafunc) - [ ]
_pytest.fixtures.FixtureFunctionMarker - [x]
_pytest.mark.structures.MarkGenerator->pytest.MarkGenerator - [x]
_pytest.mark.structures.MarkDecorator->pytest.MarkDecorator - [x]
_pytest.mark.structures.Mark->pytest.Mark - [x] ~~
_pytest.mark.structures.NodeKeywords(thruNode)~~ - only exposed as aMutableMappinginstead. - [ ]
_pytest.code._code.TerminalRepr - [ ]
_pytest.python.CallSpec2 - [ ]
_pytest.python_api.ApproxBase - [x] ~~
_pytest.python_api.RaisesContext~~ - Not worth exposing, just a context manager. - [x]
_pytest.recwarn.WarningsRecorder->pytest.WarningsRecorder - [ ]
_pytest._io.TerminalWriter - [x]
_pytest.config.argparsing.OptionGroup->pytest.OptionGroup - [x]
_pytest.pytester.ParsedCall->pytest.RecordedHookCall - [x]
_pytest.pytester.HookRecorder->pytest.HookRecorder - [x]
_pytest.pytester.RunResult->pytest.RunResult - [x]
_pytest.pytester.LineMatcher->pytest.LineMatcher
Here are my thoughts on the questions:
- Is this a problem we want to fix?
Yes; otherwise users can't utilize types in these cases. Also, if we don't, people will be importing from _pytest anyway, which we don't want to happen.
More generally, I think it will be really good for us to clearly delineate what we consider public API and what is private. Currently it is somewhat murky which makes internal refactorings difficult.
2.1. Do we want to enforce this?
I think we want to enforce it, because otherwise instantiation and subclassing are implicitly assumed to be allowed, and then we are locked into supporting them even when we don't want to.
2.2. How do we enforce this?
My current intention is to steal trio's approach and see how it goes.
- How do we want to officialy declare something as public API?
I think there should be three criteria:
- It is exported in
pytest. - It doesn't have a
_or__prefix. - It is documented in the reference.
By "it" I mean both the types themselves, and their contents (methods, attributes, etc.).
Some work will be required to align reality with these requirements...
- Which types from the list below are we actually ready to export?
The fixtures in the list look good to export, except maybe _pytest.fixtures.FixtureRequest / _pytest.fixtures.SubRequest which I'm not super clear about myself.
Stuff in the hooks needs to be exported; some require care however, and we might decide to leave unexported in the first pass.
Similar for the "Transitive" ones.
I think we can discuss the specifics in future PRs.
- Is there a category or specific types I missed?
That's for others to answer :)
Thanks @bluetech for writing up this issue with so much details. :+1:
My thoughts:
Most of these types are classes, that are not intended to be instantiated and/or subclassed by users, only internally.
2.1. Do we want to enforce this? 2.2. How do we enforce this?
I think we want to enforce it, because otherwise instantiation and subclassing are implicitly assumed to be allowed, and then we are locked into supporting them even when we don't want to.
I agree that we don't want to allow instantiation and subclassing, specially because we might want to change __init__ methods later to include additional parameters to support new features.
An idea would be to implement declare ABCs, and only export the ABCs. This I believe would remove the need to export the actual type, and hence the __init__ method. It would also be a double-checking of the public API, because changing only the implementation would upset the type-checker and bring to light unintentional changes in the API.
The downside is that we would need to implement ABCs for all the objects you listed here, which is a bunch of work.
My current intention is to steal trio's approach and see how it goes.
This looks very similar to the work done by @RonnyPfannschmidt in the Node.from_parent endeavor.
By "it" I mean both the types themselves, and their contents (methods, attributes, etc.).
Those criteria look good to me!
An idea would be to implement declare ABCs, and only export the ABCs
Doing this would be nice in that it would really highlight public vs. private. However I don't think we want to go this way for these reasons:
- It doesn't prevent subclassing; in fact an ABC probably encourages it.
- It requires a lot of duplication, scattered code and funny-looking "Impl" classes. I fear our code will look more like Java than Python :)
- I'm not sure it's very practical because to gain the benefit we would need to to change all arguments/return values to the ABCs, but often our internal code uses public APIs and then would have to cast or such in order to access the private fields.
- Not very newbie friendly.
However I don't think we want to go this way for these reasons
All good points, I had a hunch it was a lot of work but hand't thought through the details, but you confirmed it. 😁
So I agree that ABCs is a no-go.
How about having a export module for types which has an explicitly spec that anything not exported somewhere else is only intended for type declaration and won't have public constructors for now
An idea would be to implement declare ABCs, and only export the protocol definition. https://www.python.org/dev/peps/pep-0544/
What about using protocols instead of ABCs? And we only expose the protocols publicly 🤔 That could even be separated into a typing module:
from pytest.typing import MonkeyPatch, TempPathFactory
This way the fixtures have a very explicit public API, however the implementation does not have to inherit from it. Having the typing namespace makes it explicit that it's not meant for instiation. E.g.:
# pytest/typing.py
class Notset:
def __repr__(self) -> str:
return "<notset>"
notset = Notset()
K = TypeVar("K")
V = TypeVar("V")
class MonkeyPatch(Protocol):
@contextmanager
def context(self) -> Generator["MonkeyPatch", None, None]:
...
@overload
def setattr(self, target: str, name: object, value: Notset = ..., raising: bool = ...) -> None:
...
@overload
def setattr(self, target: object, name: str, value: object, raising: bool = ...) -> None:
...
def setattr(
self, target: Union[str, object], name: Union[object, str], value: object = ..., raising: bool = ...,
) -> None:
...
def delattr(self, target: Union[object, str], name: Union[str, Notset] = notset, raising: bool = ...,) -> None:
...
def setitem(self, dic: MutableMapping[K, V], name: K, value: V) -> None:
...
def delitem(self, dic: MutableMapping[K, V], name: K, raising: bool = ...) -> None:
...
I like the idea of protocols as a public api, it gives us a little bit more flexibility on the implementation
the tricky parts become though:
(1) how to ensure we stay in line with the protocols (typing tests?) (2) if we use the protocols internally, we'd probably have to cast to the concrete types occasionally to use the internal methods? this feels like it defeats some of the purposes of typing in the first place 🤔
I don't think you need to cast protocols, they should be replaceable automatically. You can test it by merging the protocol and implementation via retype into a temporary folder 🤔and running type check on that 🤔
@gaborbernat the problem would be if we internally defined things like:
def internal_method(x: ProtocolType) -> ...
x.other_internal_method()
that would not type check because other_internal_method is not on the public protocol
The fixtures in the list look good to export, except maybe
_pytest.fixtures.FixtureRequest/_pytest.fixtures.SubRequestwhich I'm not super clear about myself.
I believe _pytest.fixtures.SubRequest is what I would like to import for https://github.com/pytest-dev/pytest/issues/3342#issuecomment-665579927:
import pytest
@pytest.fixture(params=["a"])
def example_fixture(request) -> str: # << type hint for request on this line
return request.param
@asottile
(1) how to ensure we stay in line with the protocols (typing tests?)
@altendky has come up with a neat trick with a decorator for each protocol
this works today
from typing import *
import attr
class Eggs: pass
class HamProtocol(Protocol):
spam: Eggs
HamProtocolT = TypeVar("HamProtocolT", bound=HamProtocol)
def implements_ham(cls: Type[HamProtocolT]) -> Type[HamProtocolT]:
return cls
@implements_ham
@attr.s
class ConcreteHam:
spam: Eggs
It's a bit of boilerplate so would be quite nice, and I assume a few dozen lines of code to do, as a mypy plugin:
from protocol_check import implements
@implements(HamProtocol)
@attr.frozen
class ConcreteHam:
spam: Eggs
mypy would be the easiest way, I would not want to litter the actual code with decorators though
@asottile, I'm pretty new with mypy. What would be a more fitting way to handle this?
something like this? maybe not quite correct (couldn't decide on Type[Protocol] vs Callable[..., Protocol] -- I think the former is not correct according to the protocol PEP though mypy seems to allow it
impl.py
- FooConcrete proto,py
- FooProtocol tests/types.py
from typing import Callabe
t: Callable[..., FooProtocol]: FooConcrete
I think you meant an = instead of the last :?
Maybe I just haven't fully converted myself yet but in my head this still seems like a thing that would be nice to put right at the class definition. I think it is useful that protocols don't require explicit marking of implementing classes but there does seem to be value in such marking sometimes. Sure, a test works, but it seems more like dealing with a situation than a thing to actually want. The little decorator I use for now isn't nice in that it isn't implemented within annotations, so it definitely has its own issues.
I think this is roughly what you mean, albeit all in one file here. The options that actually instantiate the class definitely give the best errors though with any required arguments they are more annoying to write.
https://mypy-play.net/?mypy=latest&python=3.8&gist=4c73ed75bb613aca54dd07f5bdfb7ba3
import typing
class P(typing.Protocol):
def m(self, s: str) -> None:
return
class C:
def m(self, s: str) -> None:
return
class D:
def m(self, s: int) -> None:
return
if typing.TYPE_CHECKING:
la: typing.List[typing.Callable[..., P]] = [C, D]
lb: typing.List[P] = [C(), D()]
c: typing.Callable[..., P] = C
d: typing.Callable[..., P] = D
cc: P = C()
dd: P = D()
main.py:16: error: List item 1 has incompatible type "Type[D]"; expected "Callable[..., P]"
main.py:17: error: List item 1 has incompatible type "D"; expected "P"
main.py:17: note: Following member(s) of "D" have conflicts:
main.py:17: note: Expected:
main.py:17: note: def m(self, s: str) -> None
main.py:17: note: Got:
main.py:17: note: def m(self, s: int) -> None
main.py:20: error: Incompatible types in assignment (expression has type "Type[D]", variable has type "Callable[..., P]")
main.py:23: error: Incompatible types in assignment (expression has type "D", variable has type "P")
main.py:23: note: Following member(s) of "D" have conflicts:
main.py:23: note: Expected:
main.py:23: note: def m(self, s: str) -> None
main.py:23: note: Got:
main.py:23: note: def m(self, s: int) -> None
Found 4 errors in 1 file (checked 1 source file)
I just had a look at this, for https://github.com/twisted/pydoctor/pull/220#discussion_r502417550, would you consider access to:
def test_maple_trees(caplog):
records: Sequence[logging.LogRecord] = caplog.handler.records
reset: Callable[[], None] = caplog.handler.resest
as public?
Effectively if it can be reached from a public object caplog without accessing underscored attributes or attributes we explicitly made private in some manner, then it should be considered public, just because we really don't have much choice in the matter. We can't break caplog.handler.records for example without a deprecation. This case I seem to have missed in my "Transitive" list though.
BTW, I am working on a proposal for this. I will be starting with just the fixture types. The @final annotation I think sufficiently covers the subclassing angle. What remains are private __init__s and factory methods. We need to find a way to mark them private. My current inclination is to add a *, _pytest_private: bool=False kw-only param to each such init/factory method. If it is false, then a warning is issued, which says "this is a private API, can break at any release, etc.", which optionally will become a hard error in the future. Pytest internal code itself would have some _pytest_private=True boilerplate. I like this better than the metaclass approach, or the protocols approach. It plays better with Python and typing, and still allows plugin to access internals if they really want, but forces them to visibly opt-in to that.
I've prepared a branch with a proposal for publishing the fixture types: https://github.com/bluetech/pytest/commits/typing-public-fixtures
However I ran into an issue with adding pytest.Pytester: the pytester module wants to be assert-rewritten, because it contains a few assert functions like assertoutcomes. This means it needs to be loaded lazily by pytest. If it is loaded before pytest imports it as a plugin, then we get a "Module already imported so cannot be rewritten" warning and the asserts aren't rewritten.
I'll try to come up with some way around it. My first thought was to just load it lazily with module __getattr__ but that's Python>=3.7 and kinda hacky.
If the problem is a few functions, perhaps we might move the functions to another module, say _pytest/pytester_impl.py, and import those functions locally.
Example, currently we have Pytester.assertoutcome:
def assertoutcome(self, passed: int = 0, skipped: int = 0, failed: int = 0) -> None:
__tracebackhide__ = True
outcomes = self.listoutcomes()
realpassed, realskipped, realfailed = outcomes
obtained = {
"passed": len(realpassed),
"skipped": len(realskipped),
"failed": len(realfailed),
}
expected = {"passed": passed, "skipped": skipped, "failed": failed}
assert obtained == expected, outcomes
Becomes:
def assertoutcome(self, passed: int = 0, skipped: int = 0, failed: int = 0) -> None:
from _pytest.pytester_impl import assertoutcome
assertoutcome(self, passed=passed, skipped=skipped, failed=failed)
It is not as clean as I would like because _pytest.pytester_impl functions would need to receive a Pytester as argument, but that might not be that bad.
However, this might happen with any builtin fixture that uses assert (I'm surprised there aren't others).
Another idea is to instead expose a separate public namespace, say pytest.fixtures; this module would just import from the internal modules and define an __all__ attribute exposing them. That would mean the pytest module doesn't need to import them anymore.
I also wonder if we could provide something where plugins could be written in a way that makes assertion description work without needing to rewrite
You could do it with:
- Macros, https://www.python.org/dev/peps/pep-0638/
- Or @asottile 's file encoding hack
- Or
pytest/__init__.pycould set an import hook - Or you could run assertion rewriting in setup.py so the files are shipped like that
- Or an @pytest.rewrite that replaces the
__code__object of the decorated function, likepatchy.patch
I also wonder if we could provide something where plugins could be written in a way that makes assertion description work without needing to rewrite
Hmmm all I can think of is to provide an assert_ function that plugins can use... anything involving plain assert statements would require rewriting.
For now I went with @nicoddemus suggestion of separating just the assertions to a separate plugin. In addition to what I mentioned previously, other things I considered were:
- Removing the need for rewriting by doing it manually -- too much duplication, doesn't handle various hooks that are injected and such.
- Splitting just a general
assert_function - I don't think it would work because the AST introspects the local vars and names and such? -
pytest.fixtures- would be subject to same problem if user somehow doesfrom pytest.fixtures import Pytesterbefore pytest is running. Also less ergonomic.
The separate plugin is a hack but not too bad...
Could I request that _pytest.mark.structures.ParameterSet be added to the list of types to expose publicly? Anything depending on knowing the return type of pytest.param() will need to use it. Thanks!
Can you add pytest.mark.usefixtures() in the list? I'm getting this error in Pyright:
Untyped function decorator obscures type of function; ignoring decorator (reportUntypedFunctionDecorator)
Or should I open a new issue?
that one is typed -- I wonder why pyright doesn't notice it: https://github.com/pytest-dev/pytest/blob/6a5d47a243d2ddbf92fca5e807cf1324d60cabb1/src/_pytest/mark/structures.py#L462-L464