Skip to content
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

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

Open
dnicolodi opened this issue Jun 17, 2024 · 8 comments
Open

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

dnicolodi opened this issue Jun 17, 2024 · 8 comments
Labels
documentation Improvements or additions to documentation

Comments

@dnicolodi
Copy link
Member

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.

@dnicolodi dnicolodi added the documentation Improvements or additions to documentation label Jun 17, 2024
@rgommers
Copy link
Contributor

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
Copy link
Member Author

dnicolodi commented Jun 17, 2024

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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

@dnicolodi
Copy link
Member Author

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants