--pyargs not used if -k is used

[oscarbenjamin]

I'm seeing this problem with a src-layout in both an editable install and also a plain spin test. If I pass -k through to pytest then --pyargs is not also passed:

$ spin test -- -k test_mfpz_mod_poly
...
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -m pytest -k test_mfpz_mod_poly
======================================== test session starts ========================================
platform linux -- Python 3.11.3, pytest-7.4.3, pluggy-1.2.0
rootdir: /home/oscar/current/active/python-flint
plugins: hypothesis-6.84.2, cov-4.1.0, split-0.8.1, timeout-2.1.0, doctestplus-1.0.0, xdist-3.3.1
collected 49 items / 1 error / 49 deselected / 0 selected                                           

============================================== ERRORS ===============================================
____________________________ ERROR collecting src/flint/test/test_all.py ____________________________
import file mismatch:
imported module 'flint.test.test_all' has this __file__ attribute:
  /home/oscar/current/active/python-flint/build-install/usr/lib/python3.11/site-packages/flint/test/test_all.py
which is not the same as the test file we want to collect:
  /home/oscar/current/active/python-flint/src/flint/test/test_all.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules
====================================== short test summary info ======================================
ERROR src/flint/test/test_all.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
================================== 49 deselected, 1 error in 0.35s ==================================

A plain spin test works because it uses --pyargs:

$ spin test
...
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -m pytest --pyargs flint
======================================== test session starts ========================================
platform linux -- Python 3.11.3, pytest-7.4.3, pluggy-1.2.0
rootdir: /home/oscar/current/active/python-flint
plugins: hypothesis-6.84.2, cov-4.1.0, split-0.8.1, timeout-2.1.0, doctestplus-1.0.0, xdist-3.3.1
collected 49 items                                                                                  

build-install/usr/lib/python3.11/site-packages/flint/test/test_all.py ....................... [ 46%]
..........................                                                                    [100%]

======================================== 49 passed in 1.16s =========================================

[stefanv]

🤔

    if tests and "--pyargs" not in pytest_args:
        pytest_args = ("--pyargs", tests) + pytest_args

Yeah, there's the culprit. Although tests are supposed to be set. Will figure out what's going on and submit a fix.

[stefanv]

The fix is going to be slightly more restrictive in some cases, since

spin test -- -k test_mfpz_mod_poly example_pkg/tests/test_core.py

will need to be invoked as

spin test -t example_pkg/tests/test_core.py -- -k test_mfpz_mod_poly

There's no other way to really know when the pytest arguments are intended to specify tests.

[oscarbenjamin]

will need to be invoked as

spin test -t example_pkg/tests/test_core.py -- -k test_mfpz_mod_poly

I think when using --pyargs you should use module syntax:

spin test -t example_pkg.tests.test_core -- -k test_fmpz_mod_poly

Probably if spin is going to intercept the -t argument it should also provide a more direct way to do something more or less equivalent to pytest's -k argument anyway though rather than using --.

It should not usually be necessary to combine -k and -t though.

[oscarbenjamin]

Note that this also works:

$ spin test -t flint.test.test_all::test_fmpz_mod_poly
...
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -m pytest --pyargs flint.test.test_all::test_fmpz_mod_poly
======================================== test session starts ========================================
platform linux -- Python 3.11.3, pytest-7.4.3, pluggy-1.2.0
rootdir: /home/oscar/current/active/python-flint
plugins: hypothesis-6.84.2, cov-4.1.0, split-0.8.1, timeout-2.1.0, doctestplus-1.0.0, xdist-3.3.1
collected 1 item                                                                                    

src/flint/test/test_all.py .                                                                  [100%]

========================================= 1 passed in 1.02s =========================================

[stefanv]

I think when using --pyargs you should use module syntax:

pytest handles that fine, although it may be abusing the syntax. What is the correct syntax for "look on the system path for the module, but interpret what I give you as a file path". --pyargs filepath seems to fit that behavior, but there may be a more direct way.

I've marked #205 as draft, since the following doesn't work:

spin test -t example_pkg/tests/test_core.py -- -k test_mfpz_mod_poly

I'd need to expand the test suite as well to make sure we capture all the cases correctly. If you have time to help extricate and understand all the use-cases (file-vs-module test specification, test specification directly or via pytest args, src vs non-source layout, editable vs meson-build-path), I'd appreciate it.

[oscarbenjamin]

I think that there are really just two cases that matter:

• in-package tests
• out-of-package tests

For in-package tests you should always use the module's import name and it should always be discovered via sys.path. Then spin makes the package importable or there is an editable install and either way it works via the import system. It does not matter whether a src layout is used if the mechanism for locating the tests is via the import system which is also the same mechanism as for locating the library.

For out-of-package tests the path to the test file needs to be given somehow and then it should maybe be treated as a standalone file. If the tests are themselves a package (separate from the library package) then if they are supposed to be installed by meson this is really just in-package tests. If they are in a package but not supposed to be installed then again they should be located by import module name but somehow spin/pytest should be configured to add the relevant base directory to sys.path.

I can't really think of a case where it makes sense to use file paths rather than module names besides a single file test script that is run like python test.py. If the tests are in a module that is supposed to be imported then that module should always be specified as an importable name relative to sys.path.

[stefanv]

I can't really think of a case where it makes sense to use file paths rather than module names besides a single file test script that is run like python test.py. If the tests are in a module that is supposed to be imported then that module should always be specified as an importable name relative to sys.path.

This is one of the most interesting parts of this project; learning developers' different work styles!

E.g., I exclusively use the file-path mode of invoking tests, because I can TAB-complete at the prompt to find the filename I want to run.

[oscarbenjamin]

I exclusively use the file-path mode of invoking tests, because I can TAB-complete at the prompt to find the filename I want to run.

Yeah, I know and I do that as well with sympy.

The thing is that it doesn't work in a src layout and if you think about it then really every nontrivial project should use the src layout (even if it is pure Python and doesn't really have a "build step"). A lot of legacy codebases don't use src layout and it's not clear whether it is worth changing them. We should strive to make src-layout-plus-out-of-tree-build a first class citizen in new tooling and new projects though.

Of course spin could have a completion interface like other CLI tools and that could discover things more intelligently than just by looking at the file system. Then you could complete the test function names as well as the module names and the interpretation of the arguments would always be well defined and unambiguous.

This also works in a src layout btw:

$ spin test -t src/flint/test/test_all.py::test_fmpz_mod_poly

That one is useful because that is what pytest outputs at the end of the test report if e.g. the test fails:

$ spin test 
...
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -m pytest --pyargs flint
...
src/flint/test/test_all.py:1850: ZeroDivisionError
====================================== short test summary info ======================================
FAILED src/flint/test/test_all.py::test_fmpz_mod_poly - ZeroDivisionError: division by zero
=================================== 1 failed, 48 passed in 1.39s ====================================

But how does pytest figure out how to import that file?

It is doing some strange magic to make it work by guessing which directory is the base of the import structure. That guessing should not be necessary and results from trying to make things convenient but then making them also poorly defined at the same time.

Really pytest should output the module name here rather than a file path (especially since we used --pyargs).

[stefanv]

Yes, strange magic 👻 We could potentially detect when src/ is the test prefix and modify the test invocation accordingly.

[stefanv]

Even more simply, we could detect when path/to/file.py exists, and then turn it into a module. BUT, what about packages that do not ship their tests?

[oscarbenjamin]

BUT, what about packages that do not ship their tests?

I think this is the more important question for spin. Other cases are handled fine by just using the module import name.

If the tests are not shipped then that presumably means they are not in the build directory and therefore spin test needs to make them importable somehow. I expect that this is just something that should be configured somehow with spin but I don't know any good example projects to look at and see what would be needed.

[stefanv]

For now, I consider this expected behavior: if you specify additional args to pytest, we do not mess with those args. However, if you use -t to specify tests, everything should still work as expected. See #213.

[stefanv]

Closed by #213