--- 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.
Previously the error report would have all sections glued together:
- The assertion representation
- The error explanation
- The full diff
This makes it hard to see at a glance where which one starts and ends.
One of the representation (dataclasses, tuples, attrs) does display a
newlines at the start already.
Let's add a newlines before the error explanation and before the full
diff, so we get an easier to read report.
This has one disadvantage: we get one line less in the least verbose
mode, where the output gets truncated.
When running `tox -e py-lsof` I get a deluge of this warning:
```
src/pytest/.tox/py-lsof-numpy-pexpect/lib/python3.11/site-packages/_pytest/pytester.py:130: EncodingWarning: UTF-8 Mode affects locale.getpreferredencoding(). Consider locale.getencoding() instead.
```
Use `locale.getencoding` instead.
The normal default pretty printer is not great when objects are nested
and it can get hard to read the diff.
Instead, provide a pretty printer that behaves more like when json get
indented, which allows for smaller, more meaningful differences, at
the expense of a slightly longer diff.
This does not touch the other places where the pretty printer is used,
and only updated the full diff one.
We already have the AlwaysDispatchingPrettyPrinter override of the
default pretty printer. In order to make more in depth changes, we
need to copy the upstream version in, as it doesn't lend itself well to
being extended.
This does a verbatime copy, adding provenance information at the top.
The current method as the following problem, described by Sadra
Barikbin:
The tests that request both `pytester` and `monkeypatch` and use
`monkeypatch.chdir` without context, relying on `monkeypatch`'s teardown
to restore cwd. This doesn't work because the following sequence of
actions take place:
- `monkeypatch` is set up.
- `pytester` is set up. It saves the original cwd and changes it to a
new one dedicated to the test function.
- Test function calls `monkeypatch.chdir()` without context.
`monkeypatch` saves cwd, which is not the original one, before
changing it.
- `pytester` is torn down. It restores the cwd to the original one.
- `monkeypatch` is torn down. It restores cwd to what it has saved.
The solution here is to have pytester use `monkeypatch.chdir()` itself,
then everything is handled correctly.
