mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
118 stars 59 forks source link

Document how to use pytest in packages using meson-python #642

Open dnicolodi opened 1 week ago

dnicolodi commented 1 week ago

There are two common directory layouts used by Python packages: tests source code alongside the Python source code, or tests source code in a dedicated package top-level directory. Some packages install the tests and some do not. AFAIU pytest 8.0 and later do not allow to have the tests files alongside the Python source code and execute them without installing them.

Example, given a package source code structure looking like:

meson.build
package/
   __init__.py
   _foo.pyx
   foo.py
   foo_test.py
pyproject.toml

The package needs to be installed to build the Cython extension module:

python -m pip install --no-build-isolation -e .

However, if foo_test.py is not installed, running pytest package/ results in tests loading errors because pytest imports pacakage.fo_test, which puts the package folder in the source directory in the modules cache, and it does not contain the _foo extension module. With pytest version previous to the 8.0 release, using the --import-mode=importlib option solved the issue. However, this is not true for pytest 8.0 and later.

The only solution I've found is to install the tests alongside the package. Then pytest package/ works as expected.

If a project does not want to have the tests as part of the distributed wheels, it needs either to add a meson build option that disables installing the tests and set it when building wheels for redistribution and not for development builds, or it needs to use install tags to filter the tests out and use -Cinstall-args=--tags=python-runtime,tests in development builds. I don't like the latter solution much, because it requires listing all the tags to be installed.

The pytest issue is not meson-python specific, it is more specific to packages using extension modules, but it is complex enough that it may be worth a mention somewhere in the documentation. Maybe we need a Development with meson-python section in the documentation.

rgommers commented 1 week ago

That does seem like a good idea - this causes confusion regularly.

Related: I happen to have just written some docs on this topic for SciPy: http://scipy.github.io/devdocs/building/redistributable_binaries.html#stripping-the-test-suite-from-a-wheel-or-installed-package.

dnicolodi commented 1 week ago

I was honestly expecting someone to tell me that the issue is me being thick, not a limitation of pytest. The unittest standard library module does not have the same problem. In the example package above,

python -m unittest discover -p '*_test.py' package/

works without problems... One more reason why I'm not a big fan of pytest.

eli-schwartz commented 1 week ago

I'm not familiar with the internal implementation details of pytest or unittest.

Purely looking at how the imports work, given a mymodule/ directory and a tests/ directory, you just need them to be usable independently in any way shape or form, which is always the case so no problems. If you use mymodule/tests/ then the individual files inside mymodule/tests/*.py could have things like from mymodule.tests.util import frobnicate and then you are dependent on the exact structure, no ifs ands or buts. If the tests are freestanding but nested then they could in principle be treated as "add ['mymodule/', 'mymodule/tests/'] to sys.path and process each file separately".

IMO it's a really bad idea to have the test import names be "mymodule.tests" and a testsuite loader should otherwise be as unopinionated as it can get away with.

If you do the bad idea though, then you need the entire built/installed/editable version of the project to also build/install/editable-install the tests. A common issue there is that setuptools copies the entire installable project to build/ (and then copies it again into the wheel) but if the tests aren't installable then they don't get copied -- hence adding the build/lib/ directory to the import path means that that copy is preferred for importing, but cannot import* the tests.

Mix that with compiled extensions and you cannot use the build/ version (no tests), nor the original sources (no compiled modules). And thus people started using python setup.py build_ext --inplace.

dnicolodi commented 1 week ago

I don't want to defend the practice of having tests in the same directory as the Python source. I'm quite indifferent about it: I see the value of having the tests next to the code, and I see how segregating the tests in their own top level directory makes things easier to keep organized.

However, there is not mymodule/tests/ directory in this example. This is what numpy uses, but pytest breaks even if the test module is not in a subpackage. I haven't verified that this is the case looking at the source code, but the symptoms are compatible with pytest looking at a test file names mymodule/foo_test.py and using importing it as mymodule.foo_test in some contrived way that makes mymodule not being imported in the process.

eli-schwartz commented 1 week ago

Fascinating. Since foo_test.py doesn't depend on the structure (I daresay it doesn't do import mymodule.bar_test) there is no principled reason it should be unable to collect tests from a bare file. But if pytest itself does import mymodule.foo_test in order to collect the tests, that would be potentially very bad...

Maybe unittest tries to assume the test files are individual/standalone but pytest doesn't.

eli-schwartz commented 1 week ago

Is it possible to add an editable-install style loader that tricks pytest into seeing them all as importable using a merged structure?

dnicolodi commented 1 week ago

I think it would be possible, but it is much easier to just install the tests alongside the code, or to move them in a different directory structure.

dnicolodi commented 1 week ago

Maybe unittest tries to assume the test files are individual/standalone but pytest doesn't.

I believe that pytest loads test code in the way it does to allow some of its "magic" functions. The most likely culprit is the auto loading of conftests.py. I don't have time to argue with the pytest maintainers that the use case described here is a valid use case that deserves better support.