ruff is faster and handle everything we had prior.
isort configuration done based on the indication from
https://github.com/astral-sh/ruff/issues/4670, previousely based on
reorder-python-import (#11896)
flake8-docstrings was a wrapper around pydocstyle (now archived) that
explicitly asks to use ruff in https://github.com/PyCQA/pydocstyle/pull/658.
flake8-typing-import is useful mainly for project that support python 3.7
and the one useful check will be implemented in https://github.com/astral-sh/ruff/issues/2302
We need to keep blacken-doc because ruff does not handle detection
of python code inside .md and .rst. The direct link to the repo is
now used to avoid a redirection.
Manual fixes:
- Lines that became too long
- % formatting that was not done automatically
- type: ignore that were moved around
- noqa of hard to fix issues (UP031 generally)
- fmt: off and fmt: on that is not really identical
between black and ruff
- autofix re-order in pre-commit from faster to slower
Co-authored-by: Ran Benita <ran@unusedvar.com>
--- Current main
In current main (before pervious commit), calls to gethookproxy/ihook
are the trigger for loading non-initial conftests. This basically means
that conftest loading is done almost as a random side-effect,
uncontrolled and very non-obvious. And it also dashes any hope of making
gethookproxy faster (gethookproxy shows up prominently in pytest
profiles).
I've wanted to improve this for a while, #11268 was the latest step
towards that.
--- PR before change
In this PR, I ran into a problem.
Previously, Session and Package did all of the directory traversals
inside of their collect, which loaded the conftests as a side effect. If
the conftest loading failed, it will occur inside of the collect() and
cause it to be reported as a failure.
Now I've changed things such that Session.collect and Package.collect no
longer recurse, but just collect their immediate descendants, and
genitems does the recursive expansion work.
The problem though is that genitems() doesn't run inside of specific
collector's collect context. So when it loads a conftest, and the
conftest loading fails, the exception isn't handled by any CollectReport
and causes an internal error instead.
The way I've fixed this problem is by loading the conftests eagerly in a
pytest_collect_directory post-wrapper, but only during genitems to make
sure the directory is actually selected.
This solution in turn caused the conftests to be collected too early;
specifically, the plugins are loaded during the parent's collect(), one
after the other as the directory entries are collected. So when the
ihook is hoisted out of the loop, new plugins are loaded inside the
loop, and due to the way the hook proxy works, they are added to the
ihook even though they're supposed to be scoped to the child collectors.
So no hoisting.
--- PR after change
Now I've come up with a better solution: since now the collection tree
actually reflects the filesystem tree, what we really want is to load
the conftest of a directory right before we run its collect(). A
conftest can affect a directory's collect() (e.g. with a
pytest_ignore_collect hookimpl), but it cannot affect how the directory
node itself is collected. So I just moved the conftest loading to be
done right before calling collect, but still inside the CollectReport
context.
This allows the hoisting, and also removes conftest loading from
gethookproxy since it's no longer necessary. And it will probably enable
further cleanups. So I'm happy with it.
Since pytest now requires Python>=3.7, we can use the stdlib attrs
clone, dataclasses, instead of the OG package.
attrs is still somewhat nicer than dataclasses and has some extra
functionality, but for pytest usage there's not really a justification
IMO to impose the extra dependency on users when a standard alternative
exists.
SetupState maintains its own state, so it can store the exception
itself, instead of using the node's store, which is better avoided when
possible.
This also reduces the lifetime of the reference-cycle-inducing exception
objects which is never a bad thing.
The assertion ensures that when `addfinalizer(finalizer, node)` is
called, the node is in the stack. This then would ensure that the
finalization is actually properly executed properly during the node's
teardown. Anything else indicates something is wrong.
Previous commits fixed all of the tests which previously failed this, so
can be reenabeld now.
When the stack is empty, the finalizers which are supposed to be
attached to nodes in the stack really ought to be empty as well. So the
code here is dead. If this doesn't happen, the assert will trigger.
It is not very clear why this code exists -- we are not running any
unittest or nose code during collection, and really these frameworks
don't have the concept of collection at all, and just raising these
exceptions at e.g. the module level would cause an error. So unless I'm
missing something, I don't think anyone is using this.
Deprecate it so we can eventually clear up this code and keep unittest
more tightly restricted to its plugin.
The type cannot be constructed directly, but is exported for use in type
annotations, since it is reachable through existing public API.
This also documents `from_call` as public, because at least
pytest-forked uses it, so we must treat it as public already anyway.
This indicates at least for people using type checkers that these
classes are not designed for inheritance and we make no stability
guarantees regarding inheritance of them.
Currently this doesn't show up in the docs. Sphinx does actually support
`@final`, however it only works when imported directly from `typing`,
while we import from `_pytest.compat`.
In the future there might also be a `@sealed` decorator which would
cover some more cases.
This prevents referring to a generic type without filling in its generic
type parameters.
The FixtureDef typing might need some more refining in the future.
While working on improving the documentation of the
`pytest_runtest_setup` hook, I came up with this text:
> Called to perform the setup phase of the test item.
>
> The default implementation runs ``setup()`` on item and all of its
> parents (which haven't been setup yet). This includes obtaining the
> values of fixtures required by the item (which haven't been obtained
> yet).
But upon closer inspection I noticed this line at the start of
`SetupState.prepare` (which is what does the actual work for
`pytest_runtest_setup`):
self._teardown_towards(needed_collectors)
which implies that the setup phase of one item might trigger teardowns
of *previous* items. This complicates the simple explanation. It also
seems like a completely undesirable thing to do, because it breaks
isolation between tests -- e.g. a failed teardown of one item shouldn't
cause the failure of some other items just because it happens to run
after it.
So the first thing I tried was to remove that line and see if anything
breaks -- nothing did. At least pytest's own test suite runs fine. So
maybe it's just dead code?
Pierre Sassoulas
Bruno Oliveira
Ben Brown
Ran Benita
Zac Hatfield-Dodds
Hugo van Kemenade
Anthony Sottile
Sorin Sbarnea
Lukas Geiger