sage -t: Do not run pytest on individual Python files unless they match the pytest file pattern

[mkoeppe]

sage -t is broken since #31003/#31103 because

• pytest is configured to look for methods prefixed with test_ and treats them as pytest test functions/methods, which are then executed. However, sage's code base contains some functions in its codebase that match this pattern without being pytests.
• pytest (#31003) is configured to only discover *_test.py files; but if one uses sage -t with file arguments, this will override it and lead to many errors.

$ ./sage -t  src/sage/databases/*.py 
too many failed tests, not using stored timings
Running doctests with ID 2021-06-07-11-04-14-3c1f8784.
Using --optional=4ti2,bliss,build,database_knotinfo,dochtml,e_antic,homebrew,jupymake,latte_int,lidia,lrslib,normaliz,pip,sage,sage_spkg
Doctesting 16 files.
sage -t --random-seed=0 src/sage/databases/__init__.py
    [0 tests, 0.00 s]
sage -t --random-seed=0 src/sage/databases/all.py
    [5 tests, 0.10 s]
sage -t --random-seed=0 src/sage/databases/conway.py
    [42 tests, 0.10 s]
sage -t --random-seed=0 src/sage/databases/cremona.py
    [133 tests, 0.31 s]
sage -t --random-seed=0 src/sage/databases/cunningham_tables.py
    [0 tests, 0.00 s]
sage -t --random-seed=0 src/sage/databases/db_class_polynomials.py
    [7 tests, 0.01 s]
sage -t --random-seed=0 src/sage/databases/db_modular_polynomials.py
    [13 tests, 0.01 s]
sage -t --random-seed=0 src/sage/databases/findstat.py
    [122 tests, 0.25 s]
sage -t --random-seed=0 src/sage/databases/jones.py
    [8 tests, 0.05 s]
sage -t --random-seed=0 src/sage/databases/knotinfo_db.py
    [97 tests, 1.80 s]
sage -t --random-seed=0 src/sage/databases/odlyzko.py
    [0 tests, 0.00 s]
sage -t --random-seed=0 src/sage/databases/oeis.py
    [134 tests, 0.91 s]
sage -t --random-seed=0 src/sage/databases/sloane.py
    [0 tests, 0.00 s]
sage -t --random-seed=0 src/sage/databases/sql_db.py
    [293 tests, 0.27 s]
sage -t --random-seed=0 src/sage/databases/stein_watkins.py
    [12 tests, 0.01 s]
sage -t --random-seed=0 src/sage/databases/symbolic_data.py
    [0 tests, 0.00 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 4.6 seconds
    cpu time: 3.9 seconds
    cumulative wall time: 3.8 seconds
============================================================================== test session starts ===============================================================================
platform darwin -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
collected 0 items / 7 errors                                                                                                                                                     

===================================================================================== ERRORS =====================================================================================
_____________________________________________________________________ ERROR collecting sage/databases/all.py _____________________________________________________________________
ImportError while importing test module '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/databases/all.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/databases/all.py:51: in <module>
    from .sql_db import SQLQuery, SQLDatabase
E   ImportError: attempted relative import with no known parent package
_____________________________________________________________________ ERROR collecting sage/databases/all.py _____________________________________________________________________
ImportError while importing test module '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/databases/all.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/databases/all.py:51: in <module>
    from .sql_db import SQLQuery, SQLDatabase
E   ImportError: attempted relative import with no known parent package
___________________________________________________________________ ERROR collecting sage/databases/cremona.py ___________________________________________________________________
ImportError while importing test module '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/databases/cremona.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/databases/cremona.py:52: in <module>
    from .sql_db import SQLDatabase, verify_column
E   ImportError: attempted relative import with no known parent package
____________________________________________________________ ERROR collecting sage/databases/db_class_polynomials.py _____________________________________________________________
ImportError while importing test module '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/databases/db_class_polynomials.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/databases/db_class_polynomials.py:14: in <module>
    from .db_modular_polynomials import _dbz_to_integers
E   ImportError: attempted relative import with no known parent package
__________________________________________________________________ ERROR collecting sage/databases/findstat.py ___________________________________________________________________
src/sage/databases/findstat.py:329: in <module>
    class FindStat(UniqueRepresentation, SageObject):
sage/misc/nested_class.pyx:318: in sage.misc.nested_class.NestedClassMetaclass.__init__
    ???
E   KeyError: 'findstat'
_________________________________________________________________ ERROR collecting sage/databases/knotinfo_db.py _________________________________________________________________
src/sage/databases/knotinfo_db.py:330: in <module>
    class KnotInfoDataBase(SageObject, UniqueRepresentation):
sage/misc/nested_class.pyx:318: in sage.misc.nested_class.NestedClassMetaclass.__init__
    ???
E   KeyError: 'knotinfo_db'
____________________________________________________________________ ERROR collecting sage/databases/oeis.py _____________________________________________________________________
src/sage/databases/oeis.py:651: in <module>
    class OEISSequence(SageObject, UniqueRepresentation):
sage/misc/nested_class.pyx:318: in sage.misc.nested_class.NestedClassMetaclass.__init__
    ???
E   KeyError: 'oeis'
============================================================================ short test summary info =============================================================================
ERROR src/sage/databases/all.py
ERROR src/sage/databases/all.py
ERROR src/sage/databases/cremona.py
ERROR src/sage/databases/db_class_polynomials.py
ERROR src/sage/databases/findstat.py - KeyError: 'findstat'
ERROR src/sage/databases/knotinfo_db.py - KeyError: 'knotinfo_db'
ERROR src/sage/databases/oeis.py - KeyError: 'oeis'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 7 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

Related report in https://groups.google.com/g/sage-devel/c/SE_A2Jw5Kko/m/KQpA9GbjBQAJ:

=============================================================== ERRORS
================================================================
_________________________________________ ERROR collecting
sage/structure/sage_object_test.py
_________________________________________
ImportError while importing test module
'/home/john/sage/src/sage/structure/sage_object_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/structure/sage_object_test.py:3: in <module>
from .sage_object import SageObject
E ImportError: attempted relative import with no known parent package
======================================================= short test
summary info =======================================================
ERROR src/sage/structure/sage_object_test.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error
during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

In this ticket, we fix it by passing names of regular files to pytest only if they match the pytest pattern (ending with _test.py).

(The dependency #32975 renamed the files in the Sage sources that used this naming scheme but were not pytest files.)

Example:

$ ./sage -tp src/sage/numerical/backends src/sage/symbolic/expression.pyx src/sage/manifolds/differentiable/symplectic_form_test.py 
too many failed tests, not using stored timings
Running doctests with ID 2022-03-22-11-30-32-fd280468.
Using --optional=4ti2,buckygen,ccache,cryptominisat,debugpy,e_antic,gap_packages,homebrew,igraph,jupymake,latte_int,libsemigroups,lidia,lrslib,meataxe,normaliz,pip,polytopes_db_4d,pynormaliz,sage,sage_spkg
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Sorting sources by runtime so that slower doctests are run first....
Doctesting 27 files using 8 threads.
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/interactivelp_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/generic_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_graph_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/matrix_sdp_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/cvxopt_sdp_backend.pyx
    [52 tests, 0.08 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/generic_sdp_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/matrix_sdp_backend.pyx
    [87 tests, 0.14 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_graph_backend.pyx
    [193 tests, 0.17 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/ppl_backend.pyx
    [221 tests, 0.25 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/logging_backend.py
    [45 tests, 0.05 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_exact_backend.pxd
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/generic_sdp_backend.pyx
    [37 tests, 0.07 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/manifolds/differentiable/symplectic_form_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/interactivelp_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/cvxopt_backend.pyx
    [25 tests, 0.05 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_exact_backend.pyx
    [24 tests, 0.03 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/cvxopt_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/ppl_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/__init__.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/generic_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_exact_backend_test.py
    [0 tests, 0.00 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/generic_backend.pyx
    [96 tests, 0.59 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/interactivelp_backend.pyx
    [266 tests, 3.09 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/numerical/backends/glpk_backend.pyx
    [592 tests, 3.08 s]
sage -t --random-seed=156872034187085869352407545599456758260 src/sage/symbolic/expression.pyx
    [3070 tests, 31.83 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 32.6 seconds
    cpu time: 39.3 seconds
    cumulative wall time: 39.4 seconds
Features detected for doctesting: sage.rings.number_field
============================================================================================================ test session starts ============================================================================================================
platform darwin -- Python 3.7.8, pytest-7.1.1, pluggy-1.0.0
rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
collected 32 items                                                                                                                                                                                                                          

src/sage/numerical/backends/cvxopt_backend_test.py ..                                                                                                                                                                                 [  6%]
src/sage/numerical/backends/glpk_backend_test.py ..                                                                                                                                                                                   [ 12%]
src/sage/numerical/backends/glpk_exact_backend_test.py ..                                                                                                                                                                             [ 18%]
src/sage/numerical/backends/interactivelp_backend_test.py ..                                                                                                                                                                          [ 25%]
src/sage/numerical/backends/ppl_backend_test.py ..                                                                                                                                                                                    [ 31%]
src/sage/manifolds/differentiable/symplectic_form_test.py ......................                                                                                                                                                      [100%]

=======================================================================================

CC: @tobiasdiez @soehms @JohnCremona @dimpase @saraedum @isuruf @tscrim

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: 1214e0c

Reviewer: Sebastian Oehms

Issue created by migration from https://trac.sagemath.org/ticket/31924

[mkoeppe]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-pytest is configured to only discover `*_test.py` files; but if one uses `sage -t` with file arguments, this will override it and lead to many errors:
+pytest (#31003) is configured to only discover `*_test.py` files; but if one uses `sage -t` with file arguments, this will override it and lead to many errors:

$ ./sage -t src/sage/databases/*.py @@ -106,3 +106,27 @@ ERROR src/sage/databases/oeis.py - KeyError: 'oeis' !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 7 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

+
+Related report in https://groups.google.com/g/sage-devel/c/SE_A2Jw5Kko/m/KQpA9GbjBQAJ:
+
+```
+=============================================================== ERRORS
+================================================================
+_________________________________________ ERROR collecting
+sage/structure/sage_object_test.py
+_________________________________________
+ImportError while importing test module
+'/home/john/sage/src/sage/structure/sage_object_test.py'.
+Hint: make sure your test modules/packages have valid Python names.
+Traceback:
+src/sage/structure/sage_object_test.py:3: in <module>
+from .sage_object import SageObject
+E ImportError: attempted relative import with no known parent package
+======================================================= short test
+summary info =======================================================
+ERROR src/sage/structure/sage_object_test.py
+!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error
+during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+```
+
+

[soehms]

comment:3

It seems that there is no way to tell pytest to ignore non matching files from the argument list on throwing these error messages.

So, is there anything standing against just doing it like this?

diff --git a/src/bin/sage-runtests b/src/bin/sage-runtests
index 16d3295..e852853 100755
--- a/src/bin/sage-runtests
+++ b/src/bin/sage-runtests
@@ -143,15 +143,16 @@ if __name__ == "__main__":
     DC = DocTestController(options, args)
     err = DC.run()

-    try:
-        exit_code_pytest = 0
-        import pytest
-        pytest_options = ["--import-mode", "importlib"]
-        if options.verbose:
-            pytest_options.append("-v")
-        exit_code_pytest = pytest.main(pytest_options + args)
-    except ModuleNotFoundError:
-        print("Pytest is not installed, skip checking tests that rely on it.")
+    exit_code_pytest = 0
+    if not args:
+        try:
+            import pytest
+            pytest_options = ["--import-mode", "importlib"]
+            if options.verbose:
+                pytest_options.append("-v")
+            exit_code_pytest = pytest.main(pytest_options + args)
+        except ModuleNotFoundError:
+            print("Pytest is not installed, skip checking tests that rely on it.")

[mkoeppe]

comment:4

Directory arguments work fine, it's just regular file arguments that cause the trouble.

[soehms]

comment:5

Replying to @mkoeppe:

Directory arguments work fine, it's just regular file arguments that cause the trouble.

I see! I will have a look at it next week!

[tobiasdiez]

comment:6

I think the problem is that in pytest.main(pytest_options + args) the argument -t src/sage/databases/*.py is appended, but the correct format for pytest to only test a single file would be without the -t, i.e. pytest src/sage/databases/*.py should work.

[soehms]

comment:7

Replying to @tobiasdiez:

I think the problem is that in pytest.main(pytest_options + args) the argument -t src/sage/databases/*.py is appended, but the correct format for pytest to only test a single file would be without the -t, i.e. pytest src/sage/databases/*.py should work.

I cannot verify this on my environment:

~/devel/sage$ ./local/bin/pytest src/sage/databases/knotinfo_db.py
======================================================================================================================================== test session starts ========================================================================================================================================
platform linux -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/sebastian/devel/sage/src, configfile: tox.ini
collected 0 items / 1 error

============================================================================================================================================== ERRORS ===============================================================================================================================================
__________________________________________________________________________________________________________________________ ERROR collecting sage/databases/knotinfo_db.py ___________________________________________________________________________________________________________________________
ImportError while importing test module '/home/sebastian/devel/sage/src/sage/databases/knotinfo_db.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
local/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
src/sage/databases/knotinfo_db.py:31: in <module>
    from sage.structure.sage_object import SageObject
src/sage/structure/__init__.py:2: in <module>
    import sage.structure.element
E   ModuleNotFoundError: No module named 'sage.structure.element'
====================================================================================================================================== short test summary info ======================================================================================================================================
ERROR src/sage/databases/knotinfo_db.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================================================================================================= 1 error in 0.29s ==========================================================================================================================================

[tobiasdiez]

comment:8

Okay, then I misread the exception message. Thanks for the investigation.

So the problem is that pytest doesn't find the compiled cython modules. I think the common advice is to use the editable install. Another solution that might work is to install and run pytest from the same virtual environment where sage is installed in. Finally, there are some hacks to add the sage path in sys.path as outlined here: https://github.com/pytest-dev/pytest/issues/2421#issuecomment-403724503

[soehms]

comment:9

Replying to @tobiasdiez:

So the problem is that pytest doesn't find the compiled cython modules.

Why does it find them for directory arguments?

I think the common advice is to use the editable install. Another solution that might work is to install and run pytest from the same virtual environment where sage is installed in.

How can we achieve one of these options?

Finally, there are some hacks to add the sage path in sys.path as outlined here: https://github.com/pytest- dev/pytest/issues/2421#issuecomment-403724503

I don't think that it is worth to do that (citation from the comment: And do not forget to remove this before publishing to PyPI).

[tobiasdiez]

comment:10

Replying to @soehms:

Replying to @tobiasdiez:

So the problem is that pytest doesn't find the compiled cython modules.

Why does it find them for directory arguments?

Good question. I have no idea. Maybe the directory argument is in a wrong format so that pytest actually doesn't try to load any test files?

I think the common advice is to use the editable install. Another solution that might work is to install and run pytest from the same virtual environment where sage is installed in.

How can we achieve one of these options?

The editable install is possible since #31377 and should just work. I'm not familiar enough with the "usual" sage build to be able to recommend a solution that works with it. In the end, you need to specify the sage install directory as part of the PYTHON_PATH so that pytest is able to find the compiled cython modules. Maybe @mkoeppe has an idea.

[mkoeppe]

comment:11

Replying to @soehms:

Replying to @tobiasdiez:

So the problem is that pytest doesn't find the compiled cython modules.

Why does it find them for directory arguments?

I don't think it does. What is happening, if I'm not mistaken, is simply that pytest, as configured in src/tox.ini, only runs for Python files matching the pattern *_test.py; modified with the exclusions declared in src/conftest.py. When you pass the names of individual Python files to it, the pattern matching is overridden.

Hence my suggestion of a workaround -- filter out Python file name arguments before passing them to pytest when invoked via sage -t.

[tobiasdiez]

comment:12

Yes, that would work as a workaround. However, the underlying problem is still that pytest cannot find cython modules and I think this should be fixed as well (since otherwise you cannot have a pytest test file that has similar imports as knotinfo_db.py)

[mkoeppe]

comment:13

Replying to @tobiasdiez:

the underlying problem is still that pytest cannot find cython modules and I think this should be fixed as well (since otherwise you cannot have a pytest test file that has similar imports as knotinfo_db.py)

Yes, I agree, but that would be another ticket, as this is not "critical" for Sage 9.4.

[soehms]

comment:14

Replying to @tobiasdiez:

The editable install is possible since #31377 and should just work. I'm not familiar enough with the "usual" sage build to be able to recommend a solution that works with it. In the end, you need to specify the sage install directory as part of the PYTHON_PATH so that pytest is able to find the compiled cython modules. Maybe @mkoeppe has an idea.

After building Sage with ./configure --enable-editable the issue still shows. What could I have done wrong?

[soehms]

comment:15

Replying to @mkoeppe:

Replying to @soehms:

Replying to @tobiasdiez:

So the problem is that pytest doesn't find the compiled cython modules.

Why does it find them for directory arguments?

I don't think it does. What is happening, if I'm not mistaken, is simply that pytest, as configured in src/tox.ini, only runs for Python files matching the pattern *_test.py; modified with the exclusions declared in src/conftest.py. When you pass the names of individual Python files to it, the pattern matching is overridden.

First, I thought the same. But, the following output made me doubt: collected 0 items / 1 error.

Hence my suggestion of a workaround -- filter out Python file name arguments before passing them to pytest when invoked via sage -t.

Passing an empty argument list, causes an error, as well:

:~/devel/sage$ sage -t --new
Running doctests with ID 2021-07-08-09-15-06-93fe9beb.
Git branch: develop
Using --optional=build,debian,dochtml,pip,sage,sage_spkg
Doctesting files changed since last git commit
No files to doctest
======================================================================================================================================== test session starts ========================================================================================================================================
platform linux -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/sebastian/devel/sage
collected 0 items / 1 error

============================================================================================================================================== ERRORS ===============================================================================================================================================
___________________________________________________________________________________________________________________________________ ERROR collecting test session ___________________________________________________________________________________________________________________________________
local/lib/python3.9/site-packages/IPython/conftest.py:9: in <module>
    from .testing import tools
E   ImportError: attempted relative import with no known parent package
====================================================================================================================================== short test summary info ======================================================================================================================================
ERROR  - ImportError: attempted relative import with no known parent package

The same output shows with ./local/bin/pytest --import-mode importlib

[mkoeppe]

comment:16

Replying to @soehms:

Passing an empty argument list, causes an error, as well

OK then, that's another case in which sage -t should not invoke pytest.

[soehms]

comment:17

I would implement the workaround suggested in comment 11. But at the moment I'm sticking at the following question: What is the best way to share the pattern *_test.py between src/tox.ini and sage-runtests?

[tobiasdiez]

comment:21

Thinking about this again, I was wondering what is the desired behavior of the -t <files> argument? The following seems reasonable to me:

• If <files> is a folder, then test all "testable" files in that folder. Here "testable" takes e.g. the file extension (py, rst) into account; and also the pre-defined filter for pytest (_test.py ending).
• If <files> is a path to a file (or a glob), then test exactly those files without taking the above "testable" criteria into account. One might argue that one should also intersect with "testable" but I guess its better to trust the user that he knows why he wants exactly these files to be tested.

This also seems to be the default behavior of pytest (i.e. run pytest <files> with either a folder or a fixed path). It also seems to be that sage-runtests is behaving exactly like this, so I don't know if the test discovery should really be changed.

According to this point of view, the reported error is not really a problem with the pytest test discovery but more so an issue with loading these files as modules. If the import would work, pytest would try to find pytest tests in these files, which don't exist, so it would simply not collect anything.

[soehms]

comment:22

Replying to @tobiasdiez:

According to this point of view, the reported error is not really a problem with the pytest test discovery but more so an issue with loading these files as modules. If the import would work, pytest would try to find pytest tests in these files, which don't exist, so it would simply not collect anything.

To you mean to replace relative import statements by absolute ones all over the source code? Would this solve the problem?

BTW: Did you note that the pytest call in sage-runtests is broken since 9.5.beta3 (see this comment in the according release management thread):

Traceback (most recent call last):
  File "/home/sebastian/devel/sage/src/bin/sage-runtests", line 159, in <module>
    exit_code_pytest = pytest.main(pytest_options + args)
TypeError: can only concatenate list (not "Namespace") to list

Probably this is due to #32332.

[tobiasdiez]

comment:23

I don't have much experience with relative imports and namespace imports. But yes, converting relative imports to absolute ones should fix the issue.

[tornaria]

comment:24

I get a doctest failure in src/sage/tests/cmdline.py that seems to be related.

sage -t --random-seed=156938138878576468116341825690522372403 src/sage/tests/cmdline.py
**********************************************************************
File "src/sage/tests/cmdline.py", line 325, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    4
**********************************************************************
File "src/sage/tests/cmdline.py", line 330, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    5
**********************************************************************
1 item had failures:
   2 of 221 in sage.tests.cmdline.test_executable
    [220 tests, 2 failures, 48.13 s]
----------------------------------------------------------------------

The second failure gets fixed after https://github.com/sagemath/sage-prod/issues/32892 but not the first one.

Here's a standalone example of the issue:

$ cat my_script.sage                                             
# -*- coding: utf-8 -*-
'''This is a test file.
And I am its doctest'''
def my_add(a):
    '''
    Add 2 to a.

        EXAMPLES::

            sage: my_add(2)
            4
        '''
    return a + 2
$ ./sage -t my_script.sage ; echo $?                             
Running doctests with ID 2021-11-22-16-48-40-b731cf3e.
Git branch: develop
Using --optional=build,pip,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg,void
Doctesting 1 file.
sage -t --warn-long 36.1 --random-seed=215445574591070240478383051094230654766 my_script.sage
    [1 test, 0.00 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.0 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds
============================================================================ test session starts =============================================================================
platform linux -- Python 3.9.7, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /usr/lib/sage-9.5.beta7
collected 0 items                                                                                                                                                            

=========================================================================== no tests ran in 0.00s ============================================================================
ERROR: not found: /usr/lib/sage-9.5.beta7/my_script.sage
(no name '/usr/lib/sage-9.5.beta7/my_script.sage' in any of [])

4

What I think is going on: pytest is called by

exit_code_pytest = pytest.main(pytest_options + args.filenames)

where args.filenames = ['my_script.sage']. But pytest is skipping my_script.sage since it doesn't match patterns *.py, *.txt or *.rst (or something like that). That's the reason for the "ERROR: not found" in there, and the error code is 4 (pytest command line usage error).

I don't know what's a proper way to fix this. Maybe filter out the list of files in args.filenames so that only the files that pytest likes get through (not *.sage, anyway). Warning: when args.filenames is not empty but it becomes empty after filtering, the call to pytest must be skipped otherwise pytest just tests everything recursively, which is not what we want.

[soehms]

comment:25

Replying to @tobiasdiez:

I don't have much experience with relative imports and namespace imports. But yes, converting relative imports to absolute ones should fix the issue.

Sounds like we should make a script that replaces all relative import statements by an absolute one (all over the source tree). Are we sure that there are no such import statement being intentional (by whatever reason)?

[soehms]

comment:26

Replying to @tornaria:

I don't know what's a proper way to fix this. Maybe filter out the list of files in args.filenames so that only the files that pytest likes get through (not *.sage, anyway). Warning: when args.filenames is not empty but it becomes empty after filtering, the call to pytest must be skipped otherwise pytest just tests everything recursively, which is not what we want.

What is the difference to the suggestion made in comment:11 ff? Do you have a hint to the question I asked in comment:17 ?

[tornaria]

comment:27

Replying to @soehms:

What is the difference to the suggestion made in comment:11 ff?

I don't see any difference.

Do you have a hint to the question I asked in comment:17 ?

No idea.

I only installed pytest because of this message at the end of testing: "Pytest is not installed, skip checking tests that rely on it." I've since learned that I shouldn't. Maybe it's better to remove this warning until this is fixed? AFAICT there are very few tests (if any) that require pytest.

[dimpase]

comment:28

Replying to @tobiasdiez:

Okay, then I misread the exception message. Thanks for the investigation.

So the problem is that pytest doesn't find the compiled cython modules. I think the common advice is to use the editable install. Another solution that might work is to install and run pytest from the same virtual environment where sage is installed in. Finally, there are some hacks to add the sage path in sys.path as outlined here: https://github.com/pytest-dev/pytest/issues/2421#issuecomment-403724503

there is pytest-cython which allows one to run pytest on cython files, like pytest --doctest-cython ... - I found out about it while working on #25009, which led to https://github.com/dimpase/primecountpy (to be moved to https://github.com/sagemath/primecountpy later).

[fchapoton]

comment:29

bump to 9.6

[tobiasdiez]

comment:30

This issue should be fixed by #32975.

[mkoeppe]

comment:31

... which is stalled in "needs_work".

[tornaria]

comment:32

Replying to @tobiasdiez:

This issue should be fixed by #32975.

That ticket seems completely unrelated to the issue here.

[tobiasdiez]

comment:33

I don't exactly remember why #32975 fixes this issue, but for me the command from the ticket description works on top of that ticket. I think the problem was the collect_ignore patterns in the test config that has been removed in #32975.

[mkoeppe]

Dependencies: #32975

[mkoeppe]

comment:35

With #32975, the failures in the ticket description are fixed.

[mkoeppe]

comment:36

However, related to comment:24:

$ ./sage -tp src/sage/tests/cmdline.py
too many failed tests, not using stored timings
Running doctests with ID 2022-03-20-11-46-21-1dc8fb11.
Using --optional=4ti2,buckygen,ccache,cryptominisat,debugpy,e_antic,gap_packages,homebrew,igraph,jupymake,latte_int,libsemigroups,lidia,lrslib,meataxe,normaliz,pip,polytopes_db_4d,pynormaliz,sage,sage_spkg,texttable
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 1 file using 8 threads.
sage -t --random-seed=23402257756506608859254049173870803147 src/sage/tests/cmdline.py
**********************************************************************
File "src/sage/tests/cmdline.py", line 312, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    4
**********************************************************************
1 item had failures:
   1 of 218 in sage.tests.cmdline.test_executable
    [217 tests, 1 failure, 67.30 s]
----------------------------------------------------------------------
sage -t --random-seed=23402257756506608859254049173870803147 src/sage/tests/cmdline.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 67.4 seconds
    cpu time: 0.3 seconds
    cumulative wall time: 67.3 seconds
Features detected for doctesting: pandoc
=================================================================================== test session starts ===================================================================================
platform darwin -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
rootdir: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src, configfile: tox.ini
collected 1 item                                                                                                                                                                          

src/sage/tests/cmdline.py E                                                                                                                                                         [100%]

========================================================================================= ERRORS ==========================================================================================
____________________________________________________________________________ ERROR at setup of test_executable ____________________________________________________________________________
file /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py, line 61
  def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False, **kwds):
E       fixture 'args' not found
>       available fixtures: add_imports, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.

/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/tests/cmdline.py:61
================================================================================= short test summary info =================================================================================
ERROR src/sage/tests/cmdline.py::test_executable
==================================================================================== 1 error in 0.01s =====================================================================================

[tobiasdiez]

comment:37

Not sure how to approach fixing the error that pytesting tests/cmdline.py is not working. What is happening here is the following:

• The -t argument overwrites pytests check that test files have to end on _test.py (this is expected behavior and is needed if you want to run pytests specified in a custom test file)
• Pytest is then searching for test methods in cmdline.py and finds test_executable as, by default, pytest will consider any function prefixed with test as a test.

Would it be okay to rename test_executable to say run_executable so that it is no longer discovered by pytest?

[mkoeppe]

comment:38

$ git grep '^def test' src/sage reveals lots of files that would need changing. I think we need a better solution than that.

[mkoeppe]

comment:39

I think this really needs the change that the ticket title says: "sage -t: Do not run pytest on individual files" (... unless they match the pytest file pattern...)

[tobiasdiez]

comment:41

Replying to @mkoeppe:

$ git grep '^def test' src/sage reveals lots of files that would need changing. I think we need a better solution than that.

Yeah, there are quite a few but also not sooo super many (around 60 hits). One could also skip these methods, https://docs.pytest.org/en/6.2.x/skipping.html#skip.

I think this really needs the change that the ticket title says: "sage -t: Do not run pytest on individual files" (... unless they match the pytest file pattern...)

I'm not a big fan of this. The longterm goal is to use pytest also for the doctests (replacing sages custom doctest runner) and for this doctests and pytests should be treated in the same way.

[mkoeppe]

comment:42

Replying to @tobiasdiez:

Replying to @mkoeppe:

$ git grep '^def test' src/sage reveals lots of files that would need changing. I think we need a better solution than that.

Yeah, there are quite a few but also not sooo super many (around 60 hits). One could also skip these methods, https://docs.pytest.org/en/6.2.x/skipping.html#skip.

If skipping is done with the pytest decorators, we would somehow have to have a version of this decorator that becomes a no-op if pytest is not installed.

[mkoeppe]

comment:43

Replying to @tobiasdiez:

Replying to @mkoeppe:

$ git grep '^def test' src/sage reveals lots of files that would need changing. I think we need a better solution than that.

Yeah, there are quite a few but also not sooo super many (around 60 hits). One could also skip these methods, https://docs.pytest.org/en/6.2.x/skipping.html#skip.

I think it's fine to rename them if they are only used by doctests in the same module. Then we might argue that it was not really intended to be in the public interface and should have been named _test....

[mkoeppe]

comment:45

Replying to @tobiasdiez:

I think this really needs the change that the ticket title says: "sage -t: Do not run pytest on individual files" (... unless they match the pytest file pattern...)

I'm not a big fan of this. The longterm goal is to use pytest also for the doctests (replacing sages custom doctest runner) and for this doctests and pytests should be treated in the same way.

That's fine to keep in mind for the long term. But we really need to restore a non-broken doctesting facility. I marked this as "critical" 9 months ago and we released 9.4 and 9.5 with it. It's overdue to fix it.

[mkoeppe]

Branch: u/mkoeppe/sagetdo_not_run_pytest_on_individual_python_files_unless_they_match_the_pytest_file_pattern

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Commit: 16c1b69

[mkoeppe]

New commits:

5851f01	Improve doctest interaction with pytest
e717715	Merge remote-tracking branch 'origin/develop' into public/tests/doctests_pytest
9c5d5a9	Fix docs
0c7bf17	Add documentation for test:pytest
01f376c	Use endswith to test for pytest files
54e9314	Merge #32975
16c1b69	src/bin/sage-runtests: Do not run pytest on individual Python files unless they match the pytest file pattern

[mkoeppe]

comment:48

Replying to @mkoeppe:

Replying to @tobiasdiez:

The longterm goal is to use pytest also for the doctests (replacing sages custom doctest runner) and for this doctests and pytests should be treated in the same way.

That's fine to keep in mind for the long term.

I've opened #33546 so that we don't forget.

[tornaria]

comment:49

Looks good to me, I will test and report back.

I assume I only have to test the last commit (16c1b69) since the other commits have been tested (and indeed already merged) as part of #32975.

[mkoeppe]

comment:50

Yes

[tornaria]

comment:51

Mmmmm... why this:

        filenames = [f for f in args.filenames
                     if f.endswith("_test.py") or not f.endswith(".py")]

instead of just

        filenames = [f for f in args.filenames if f.endswith("_test.py")]

We only want to run pytest on *_test.pyfiles, right? What am I missing?

[mkoeppe]

comment:52

We also want to run it on directory names.

[tornaria]

comment:53

Replying to @mkoeppe:

We also want to run it on directory names.

Sure, but not on *.pyx, *.pxd, *.rst, etc.

[mkoeppe]

comment:54

Yes, I have a better version in a second