pytest in sage is broken, please make it really optional #33531
Description
- pytest support was introduced in Add minimal pytest configuration #31003 and merged on the basis that it was a no-op
- Later in sage.numerical.backends: Replace use of TestSuite by pytest #31103 some code was added at the end of
src/bin/sage-runteststo run pytest (see 719e442) - This code is broken in many ways, some that have been fixed (Fix exit code for pytest in case of no matched files #32892) and some that have not (sage -t: Do not run pytest on individual Python files unless they match the pytest file pattern #31924, also #31924 comment:23)
- The bug in sage -t: Do not run pytest on individual Python files unless they match the pytest file pattern #31924 is a PITA and the only way to avoid it is to patch out the code that runs pytest in `sage-runtests
- Result is: if pytest is not installed, we get an annoying warning that induces to install it, but if pytest is installed then doctesting is broken (as in we get scary warnings at the end and a non-zero exit code, on top of a 3s overhead)
$ time sage -t src/sage/all.py ; echo $?
Running doctests with ID 2022-03-19-12-07-01-0e558614.
Using --optional=build,pip,sage,sage_spkg,void
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,pandoc,pdf2svg,plantri,pynormaliz,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.plot,sage.rings.number_field,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 1 file.
sage -t --warn-long 60.2 --random-seed=67815784290108767013007019285883868229 src/sage/all.py
[13 tests, 0.75 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.8 seconds
cpu time: 0.7 seconds
cumulative wall time: 0.7 seconds
Features detected for doctesting:
============================================================================ test session starts =============================================================================
platform linux -- Python 3.10.3, pytest-6.2.5, py-1.10.0, pluggy-0.13.1
rootdir: /opt/sage/sage-git/develop/src, configfile: tox.ini
plugins: anyio-3.5.0, xonsh-0.11.0
collected 0 items / 1 error
=================================================================================== ERRORS ===================================================================================
________________________________________________________________________ ERROR collecting sage/all.py ________________________________________________________________________
src/sage/all.py:315: in <module>
sage.misc.lazy_import.finish_startup()
sage/misc/lazy_import.pyx:87: in sage.misc.lazy_import.finish_startup (build/cythonized/sage/misc/lazy_import.c:1995)
???
sage/misc/lazy_import.pyx:103: in sage.misc.lazy_import.finish_startup (build/cythonized/sage/misc/lazy_import.c:1929)
???
E AssertionError: finish_startup() must be called exactly once
========================================================================== short test summary info ===========================================================================
ERROR src/sage/all.py - AssertionError: finish_startup() must be called exactly once
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================================== 1 error in 0.34s ==============================================================================
real 0m6.917s
user 0m6.496s
sys 0m0.486s
2
Expected behaviour (this is what happens when pytest is patched out in sage-runtests):
$ time sage -t src/sage/all.py ; echo $?
Running doctests with ID 2022-03-19-12-07-20-a46d44e6.
Using --optional=build,pip,sage,sage_spkg,void
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,pandoc,pdf2svg,plantri,pynormaliz,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.plot,sage.rings.number_field,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 1 file.
sage -t --warn-long 60.2 --random-seed=339422732397923267068138499972932430419 src/sage/all.py
[13 tests, 0.75 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.8 seconds
cpu time: 0.8 seconds
cumulative wall time: 0.8 seconds
Features detected for doctesting:
real 0m4.115s
user 0m3.783s
sys 0m0.323s
0
My proposal:
- add a feature flag "pytest" and make running pytest conditional to that
- do NOT add pytest to the default flags until all issues are fixed and it is known to work ok (at the very least sage -t: Do not run pytest on individual Python files unless they match the pytest file pattern #31924 should be fixed)
In fact pytest should only run when there's some file *_test.py in the list of files, and only in those files. I think DC already traversed the filesystem in search of files, so instead of having pytest traverse the filesystem again, just filter the list of files that DC obtained for *_test.py and run pytest just on those and only if there's some.
Even if/after pytest integration is fixed, there are advantages to place it behind a feature flag so there's an easy way to run doctests skipping pytest (just add --optional=sage or whatever else one wants to test, leaving out pytest)
Depends on #31924
CC: @tobiasdiez @mkoeppe
Component: doctest framework
Reviewer: Gonzalo Tornaría
Issue created by migration from https://trac.sagemath.org/ticket/33531