sage -t --installed

[mkoeppe]

This new option is similar to --all but

• it tests all Sage modules installed (in SAGE_LIB) instead of SAGE_SRC. (This will be useful for testing modularized distributions such as Modularization of sagelib: Break out a separate package sagemath-standard-no-symbolics #32601.)
• it tests the installed Sage documentation (specifically, the .rst.txt files installed in html/**/_sources) instead of SAGE_DOC_SRC.

$ ./sage -tp --installed
too many failed tests, not using stored timings
Running doctests with ID 2022-02-23-21-52-47-11831639.
Using --optional=ccache,cryptominisat,debugpy,e_antic,homebrew,igraph,jupymake,lrslib,normaliz,pip,polytopes_db_4d,pycryptosat,pynormaliz,sage,sage_spkg
Features to be detected: [...] 
Doctesting all installed modules of the Sage library.
Sorting sources by runtime so that slower doctests are run first....
Doctesting 3522 files using 8 threads.
sage -t --random-seed=334799075370772165291245032923424842623 local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/groups/matrix_gps/heisenberg.py
    [32 tests, 1.83 s]
sage -t --random-seed=334799075370772165291245032923424842623 local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/combinat/crystals/alcove_path.py
    [293 tests, 4.16 s]
...

(Actually, because portions of namespace packages can be installed in separate locations, the variable sage.env.SAGE_LIB is no longer meaningful. Instead, we make use of the __path__ attribute of packages, https://python.readthedocs.io/en/stable/reference/import.html#module-path, which in the case of namespace packages is an iterable.)

This would also make sense for distribution packaging, which no longer would have to rely on the fallbacks SAGE_SRC -> SAGE_LIB and SAGE_DOC_SRC -> SAGE_DOC.

Part of Meta-ticket #33037.

CC: @kiwifb @antonio-rojas @tobiasdiez

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: b4d2b8e

Reviewer: François Bissey, Antonio Rojas

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

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Branch: u/mkoeppe/sage__t___all_installed

[mkoeppe]

Commit: de649be

[mkoeppe]

New commits:

de649be

src/sage/doctest/control.py, src/bin/sage-runtests: Implement sage -t --installed

[kiwifb]

comment:5

I think the move makes some sense especially in the context of the elimination of SAGE_LIB and potentially more variables as sage matures more in the future.

The only thing I am not sure is introducing a new option. Why not make it the default instead? What workflow would that prevent?

[mkoeppe]

comment:6

Replying to @kiwifb:

Why not make it the default instead? What workflow would that prevent?

I think the default is valuable because doctests can be written / updated and immediately tested without having to build the Sage library.

[kiwifb]

comment:7

Valid for pure python files but not .pyx, you need to build those before testing them. But on the numbers they probably are the minority.

[mkoeppe]

comment:8

Replying to @kiwifb:

Valid for pure python files but not .pyx, you need to build those before testing them.

When you are done changing code, you can still edit doctests and in this phase you don't need to recompile in order to tests the doctests.

[kiwifb]

comment:9

Replying to @mkoeppe:

Replying to @kiwifb:

Valid for pure python files but not .pyx, you need to build those before testing them.

When you are done changing code, you can still edit doctests and in this phase you don't need to recompile in order to tests the doctests.

More to the point, it is a pain in the workflow to recompile in that case. But you should explicitly test a file or directory in that case. But let's go with a developer focused for now.

We may want to suggest not to use --installed and --all at the same time for distros as it will try to add a non-existent path once we remove the default SAGE_SRC -> SAGE_LIB. Also the current --all will catch sage_setup, sage_docbuild and SAGE_DOC_SRC (which in distros will point to SAGE_DOC). But --installed doesn't do any of that. Should it?

[mkoeppe]

comment:10

Replying to @kiwifb:

We may want to suggest not to use --installed and --all at the same time

Yes, they are mutually exclusive options

[kiwifb]

comment:11

Replying to @mkoeppe:

Replying to @kiwifb:

We may want to suggest not to use --installed and --all at the same time

Yes, they are mutually exclusive options

I didn't pay enough attention to that part of the code. I see it now.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

5dd2dd2

src/sage/doctest/control.py: In sage -t --installed, also handle sage_setup, sage_docbuild

[sagetrac-git]

Changed commit from de649be to 5dd2dd2

[mkoeppe]

comment:13

Replying to @kiwifb:

Also the current --all will catch sage_setup, sage_docbuild and SAGE_DOC_SRC (which in distros will point to SAGE_DOC). But --installed doesn't do any of that. Should it?

I have added sage_setup, sage_docbuild but I'm not sure about SAGE_DOC_SRC because Sage does not install it; do you install it in the Gentoo package?

[kiwifb]

comment:14

No, I don't install SAGE_DOC_SRC. But as I said it points to the installed SAGE_DOC which effectively tests nothing. Because the point would be to test the .rst files, but all those are installed as .rst.txt for some reason. It would make sense to me to add SAGE_DOC for --installed if we could test those.

[mkoeppe]

comment:15

Ah, I see, in the _sources subdirectories

[mkoeppe]

comment:16

I looked at these files but they do not seem useful

[kiwifb]

comment:17

What else do you test in SAGE_DOC_SRC? conf.py files? Those do not have doctests either. If there is no value in testing a .rst.txt in SAGE_DOC, I am not sure there is value in testing the corresponding .rst in SAGE_DOC_SRC.

[kiwifb]

comment:18

None of these files have TESTS blocks but a few have stuff that does get tested

$ sage -t --long src/doc/en/thematic_tutorials/sandpile.rst
too many failed tests, not using stored timings
Running doctests with ID 2022-02-25-13-31-30-ea5e5e11.
Using --optional=pip,sage
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,pdftocairo,plantri,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.
sage -t --long --random-seed=249406959745805344364707629509298952414 src/doc/en/thematic_tutorials/sandpile.rst
    [705 tests, 12.50 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------

[mkoeppe]

comment:19

Yes, my mistake

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

07f37d0

src/sage/doctest/{control,sources}.py: Handle .rst.txt files like .rst files

[kiwifb]

comment:30

Well, I'll be going for the school run in 20-25 minutes, I'll try to wrap my head around what this commit is about before going.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

e80c135

src/sage/doctest/control.py: Use elif because options are exclusive

[sagetrac-git]

Changed commit from 1627cc9 to e80c135

[kiwifb]

comment:32

Good spotting that last commit. The previous commit is neat, it almost negates the need for having the --installed option. I can continue doing --all on distro and it will behave as --installed. It is only distinctive on vanilla sage where it does something different.

It feels like I won my first point about making it the default without you really conceding anything. Really well done. The code reorganisation around it is also quite good. It is all more clear about what each part does.

[mkoeppe]

comment:33

Replying to @kiwifb:

The previous commit is neat, it almost negates the need for having the --installed option.

Yes, for the Sage-on-distro use case, but not for my use case (doctesting a modularized subset distribution like sagemath-polyhedra).

[mkoeppe]

comment:36

Thank you!

[vbraun]

comment:37

Test fail

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b4d2b8e

src/sage/doctest/control.py: Update doctest output

[sagetrac-git]

Changed commit from e80c135 to b4d2b8e

[kiwifb]

comment:40

Apologies, that was very sloppy of me.

[mkoeppe]

comment:41

Patchbots have been on strike, hard to see failures.

[kiwifb]

comment:42

I have now run a full doctest on sage-on-gentoo with --installed. Because we have not acted yet on SAGE_SRC/SAGE_LIB/SAGE_DOC_SRC checking with --all was just business as usual. But nothing else came up (apart from the current lot of failures in #33141 and new giac issues that I have to inspect more closely).

[kiwifb]

comment:43

While it is unrelated to eliminating SAGE_* variables, there is something related to testing an installed sage that we need to address in a follow up. pytest doesn't work if you cannot write in the "rootdir" of what is tested.

There are two issues with that. First it may leave unwanted files all over the place. Second it fails miserably if you try to test something that is read-only.

============================= test session starts ==============================
platform linux -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
rootdir: /usr
plugins: hypothesis-6.38.0
collected 0 items

=============================== warnings summary ===============================
../../usr/lib/python3.10/site-packages/_pytest/cacheprovider.py:433
  /usr/lib/python3.10/site-packages/_pytest/cacheprovider.py:433: PytestCacheWarning: could not create cache path /usr/.pytest_cache/v/cache/nodeids
    config.cache.set("cache/nodeids", sorted(self.cached_nodeids))

../../usr/lib/python3.10/site-packages/_pytest/stepwise.py:52
  /usr/lib/python3.10/site-packages/_pytest/stepwise.py:52: PytestCacheWarning: could not create cache path /usr/.pytest_cache/v/cache/stepwise
    session.config.cache.set(STEPWISE_CACHE_DIR, [])

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

There seem to be some option to change where the cache is created (-o cache_dir=$somelocation) but I have so far failed to integrate it successfully in sage-runtest when pytest is called.

[kiwifb]

comment:45

Follow up for pytest at #33521.

[antonio-rojas]

Changed reviewer from François Bissey to François Bissey, Antonio Rojas

[antonio-rojas]

comment:46

Working fine here.

[mkoeppe]

comment:47

Thanks all for the review!

[vbraun]

Changed branch from u/mkoeppe/sage__t___all_installed to b4d2b8e