pytest - fix scope in which doctests are run

[mkoeppe]

(from #33546)

When running doctests via sage --pytest (added in #33546), there are many failed tests due to some issues with assumptions and/or symbolic variables and/or deprecation warnings. To see examples of these failures, run pytest on

src/sage/manifolds/differentiable/examples/sphere.py
src/sage/manifolds/utilities.py
src/sage/manifolds/chart.py

UPDATE - deprecation warnings are OK, thanks to 92bbf1d in comment:10

CC: @tobiasdiez

Component: doctest framework

Author: Dima Pasechnik, Tobias Diez

Branch/Commit: public/build/pytest-github-action @ 92bbf1d

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

[tobiasdiez]

New commits:

4290b11	Add pytest-xdist package
7df807e	Run pytest on github to see failing tests

[tobiasdiez]

Commit: 7df807e

[tobiasdiez]

Branch: public/build/pytest-github-action

[tobiasdiez]

Dependencies: #33825

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 7df807e to 5f78a38

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

d3fdd95	Cleanup vscode config
58e11d7	Fix checker and namespace injection
ff4056b	Merge branch 'develop' into public/tests/pytest_doctests
e62c29b	Cleanup imports
3ee9815	Add test that pytest correctly fails on example
f863428	Only run doctests in pytest when doctest-modules is specified
428a18c	Remove nonexisting paths from config
cc19e92	Specify importlib as import mode to be consistent
bb11eb9	Merge remote-tracking branch 'origin/public/tests/pytest_doctests' into public/build/pytest-github-action
5f78a38	Fix command

[tobiasdiez]

Changed dependencies from #33825 to #33825, #33546

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 5f78a38 to 9aa5a42

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

9aa5a42

Run in serial

[dimpase]

comment:7

the issue with deprecation warnings is due to pytest capturing warnings, see https://docs.pytest.org/en/stable/how-to/capture-warnings.html

Trying to disable this via

--- a/src/tox.ini
+++ b/src/tox.ini
@@ -232,7 +232,7 @@ exclude =
 [pytest]
 python_files = *_test.py
 norecursedirs = local prefix venv build pkgs .git src/doc src/bin
-addopts = --import-mode importlib
+addopts = --import-mode importlib -p no:warnings
 doctest_optionflags = NORMALIZE_WHITESPACE ELLIPSIS

 [coverage:run]

gives a bit more info:

...
/home/dimpase/work/software/sage/src/sage/manifolds/chart.py:492: DocTestFailure
__________________________ [doctest] sage.manifolds.chart.Chart.add_restrictions __________________________
748         means ``(x > y) and ((x != 0) or (y != 0)) and (z^2 < x)``.
749         If the list ``restrictions`` contains only one item, this
750         item can be passed as such, i.e. writing ``x > y`` instead
751         of the single element list ``[x > y]``.
752 
753         EXAMPLES::
754 
755             sage: M = Manifold(2, 'M', field='complex', structure='topological')
756             sage: X.<x,y> = M.chart()
757             sage: X.add_restrictions(abs(x) > 1)
Expected:
    doctest:warning...
    DeprecationWarning: Chart.add_restrictions is deprecated; provide the
    restrictions at the time of creating the chart
    See https://trac.sagemath.org/32102 for details.
Got nothing

/home/dimpase/work/software/sage/src/sage/manifolds/chart.py:757: DocTestFailure
------------------------------------------ Captured stderr call -------------------------------------------
<doctest sage.manifolds.chart.Chart.add_restrictions[2]>:1: DeprecationWarning: Chart.add_restrictions is deprecated; provide the restrictions at the time of creating the chart
See https://trac.sagemath.org/32102 for details.
  X.add_restrictions(abs(x) > Integer(1))
...

Note Captured stderr call - this does not arise with the unchanged tox.ini.

Perhaps the problem is in the way warning level is set in deprecation(), I don't know.

[dimpase]

comment:8

With vanilla python pytest still needs a bit of monkey-patching of the warning system, namely, warnings.showwarning. Note that Sage does it too, but in a trickier way.

# c.py
import warnings
dowarn = warnings.showwarning
def showwarn(message, category, filename, lineno, file=None, line=None):
    import sys
    dowarn(message, category, filename, lineno, sys.stdout, line)

warnings.showwarning = showwarn # use stdout instead of stderr

class tst:
    """
    >>> len(tst.__subclasses__()) >= 0
    True
    >>> import warnings
    >>> warnings.warn("foo")
    <doctest ...: UserWarning: foo
      warnings.warn("foo")
    """
    pass

at least the above works:

$ pytest --doctest-modules -p no:warnings c.py
========================================== test session starts ===========================================
platform linux -- Python 3.9.2, pytest-7.1.3, pluggy-0.13.0
rootdir: /tmp
collected 1 item                                                                                         

c.py .                                                                                             [100%]

=========================================== 1 passed in 0.03s ============================================

It's still not clear how to fix Sage's pytesting: I tried a similar hack with showwarning in src/sage/doctest/forker.py, with the one in comment:7, to no avail...

[dimpase]

comment:9

I've opened https://github.com/pytest-dev/pytest/discussions/10565 to check whether there is a better way than monkey-patching warnings.showwarning.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 9aa5a42 to 92bbf1d

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

497d67a	Run pytest on github to see failing tests
d9211e1	Fix command
d474c3a	Run in serial
92bbf1d	allow pytest to process deprecations in docstrings

[dimpase]

Description changed:

--- 
+++ 
@@ -9,4 +9,5 @@
 src/sage/manifolds/chart.py

+UPDATE - deprecation warnings are OK, thanks to 92bbf1d in comment:10

[dimpase]

comment:11

Rebased over latest 9.8.beta4. The last addition, 92bbf1d, allows pytest to process docstrings with deprecations. Thus, only src/sage/manifolds/utilities.py still fails with pytest, the other 2 files are fine.

[dimpase]

Changed dependencies from #33825, #33546 to none

[dimpase]

Author: Dima Pasechnik, ...

[mkoeppe]

Changed author from Dima Pasechnik, ... to Dima Pasechnik, Tobias Diez

[mkoeppe]

comment:16

https://github.com/sagemath/sagetrac-mirror/actions/runs/3643485912/jobs/6151758539 says "collected 0 items / 1 skipped"

[dimpase]

comment:17

Replying to Matthias Köppe:

https://github.com/sagemath/sagetrac-mirror/actions/runs/3643485912/jobs/6151758539 says "collected 0 items / 1 skipped"

not caused by the latest commit: without it, or with it, the same problem:

=========================================== test session starts ===========================================
platform linux -- Python 3.9.13, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/scratch/scratch2/dimpase/sage/sagetrac-mirror, configfile: tox.ini
plugins: xdist-3.1.0
collected 0 items / 1 error                                                                               

================================================= ERRORS ==================================================
______________________________________ ERROR collecting test session ______________________________________
/usr/lib64/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1030: in _gcd_import
    ???
<frozen importlib._bootstrap>:1007: in _find_and_load
    ???
<frozen importlib._bootstrap>:972: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:228: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1030: in _gcd_import
    ???
<frozen importlib._bootstrap>:1007: in _find_and_load
    ???
<frozen importlib._bootstrap>:986: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:680: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:850: in exec_module
    ???
<frozen importlib._bootstrap>:228: in _call_with_frames_removed
    ???
echelon/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/matplotlib/tests/__init__.py:6: in <module>
    raise IOError(
E   OSError: The baseline image directory does not exist. This is most likely because the test data is not installed. You may need to install matplotlib from source to get the test data.
========================================= short test summary info =========================================
ERROR  - OSError: The baseline image directory does not exist. This is most likely because the test data is not...
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================ 1 error in 5.96s =============================================

[dimpase]

comment:18

Google finds a numeber of complaints about this error with matplotlib, and the answer appears to be to follow https://matplotlib.org/stable/devel/development_setup.html to install matplotlib.

indeed, we apparently don't install baseline image directory, or install it improperly for pytest...

[dimpase]

comment:19

Why do we even try to run tests of matplotlib? Can they be suppressed?

[mkoeppe]

comment:20

Replying to Dima Pasechnik:

Why do we even try to run tests of matplotlib?

It's definitely not intended, it's supposed to run tests out of src/ only. Not sure what's happening there

[dimpase]

comment:21

Replying to Matthias Köppe:

Replying to Dima Pasechnik:

Why do we even try to run tests of matplotlib?

It's definitely not intended, it's supposed to run tests out of src/ only. Not sure what's happening there

well, it's late here, and

https://docs.pytest.org/en/7.2.x/reference/customize.html#initialization-determining-rootdir-and-configfile

is quite a long read...

[dimpase]

comment:22

Replying to Matthias Köppe:

Replying to Dima Pasechnik:

Why do we even try to run tests of matplotlib?

It's definitely not intended, it's supposed to run tests out of src/ only. Not sure what's happening there

some kind of trouble while discovering namespace packages to test.

$ ../sage -python -m coverage run -m pytest -c tox.ini --pyargs sage.misc
========================================== test session starts ===========================================
platform linux -- Python 3.9.2, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/dimpase/work/software/sage/src, configfile: tox.ini
plugins: xdist-3.1.0
collected 0 items                                                                                        

========================================= no tests ran in 0.01s ==========================================
ERROR: module or package not found: sage.misc (missing __init__.py?)

but

$ ../sage -python -m coverage run -m pytest -c tox.ini --pyargs sage.tests
========================================== test session starts ===========================================
platform linux -- Python 3.9.2, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/dimpase/work/software/sage/src, configfile: tox.ini
plugins: xdist-3.1.0
collected 32 items / 1 skipped                                                                           

sage/tests/books/computational-mathematics-with-sagemath/calculus_doctest.py F                     [  3%]
sage/tests/books/computational-mathematics-with-sagemath/combinat_doctest.py F                     [  6%]
sage/tests/books/computational-mathematics-with-sagemath/domaines_doctest.py .                     [  9%]
sage/tests/books/computational-mathematics-with-sagemath/float_doctest.py .                        [ 12%]
sage/tests/books/computational-mathematics-with-sagemath/graphique_doctest.py F                    [ 15%]
...

there is quite a bit on this in pytest github: https://github.com/pytest-dev/pytest/pull/1597 closing https://github.com/pytest-dev/pytest/issues/1567

[dimpase]

comment:23

Also note that sage --python -m pytest manipulates sys.path (adding the currect directory to it), and this results in an extra trouble: e.g. ./sage --pytest src/sage/misc/*.py works- collects a lot of tests, about 30% of failing tests, and a warning with a backtrace:

pytest.PytestCollectionWarning: cannot collect test class 'TestParent3.Element' because it has a __init__ constructor (from: sage/misc/test_nested_class.py)
doctest:warning
...

but ./sage --python -m pytest src/sage/misc/*.py only collects 4 tests, and errors with weird

================================================= ERRORS =================================================
_____________________________________ ERROR at setup of test_pickle ______________________________________
file /home/dimpase/work/software/sage/src/sage/misc/explain_pickle.py, line 2531
  def test_pickle(p, verbose_eval=False, pedantic=False, args=()):
E       fixture 'p' 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, testrun_uid, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, worker_id
>       use 'pytest --fixtures [testpath]' for help on them.

/home/dimpase/work/software/sage/src/sage/misc/explain_pickle.py:2531
__________________________________ ERROR at setup of test_add_commutes ___________________________________
file /home/dimpase/work/software/sage/src/sage/misc/random_testing.py, line 162
  @random_testing
  def test_add_commutes(trials, verbose=False):
E       fixture 'trials' 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, testrun_uid, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, worker_id
>       use 'pytest --fixtures [testpath]' for help on them.

/home/dimpase/work/software/sage/src/sage/misc/random_testing.py:162
...

[dimpase]

comment:24

And bad news from https://github.com/pytest-dev/pytest/issues/1567#issuecomment-1342418664 :

Pytest is unaware of pep420, nobody fixed that so far

[mkoeppe]

comment:25

Indeed, this is very bad.

[mkoeppe]

comment:26

The problem with PEP 420 implicit namespace packages for pytest and similar source discovery is that there is no clear way to find the root of a source tree.

In Sage, there is a solution for that -- we have marker files all.py and all__*.py that distinguish namespace packages from non-package directories. These are recognized in https://github.com/sagemath/sage-prod/blob/develop/src/sage/misc/package_dir.py#L132 (As Tobias pointed out in previous discussions, using such marker files was rejected during the PEP 420 design discussion; but our source tree already had these files, so we repurposed them for discovery.)

So a sage-specific solution could be to monkey-patch https://github.com/pytest-dev/pytest/blob/main/src/_pytest/_py/path.py so that it understands the Sage source layout.

For upstream pytest, perhaps an intermediate goal could be to have a documented hook that makes this monkey-patching easier.

[dimpase]

comment:27

Replying to Matthias Köppe:

The problem with PEP 420 implicit namespace packages for pytest and similar source discovery is that there is no clear way to find the root of a source tree.

In Sage, there is a solution for that -- we have marker files all.py and all__*.py that distinguish namespace packages from non-package directories. These are recognized in https://github.com/sagemath/sage-prod/blob/develop/src/sage/misc/package_dir.py#L132 (As Tobias pointed out in previous discussions, using such marker files was rejected during the PEP 420 design discussion; but our source tree already had these files, so we repurposed them for discovery.)

So a sage-specific solution could be to monkey-patch https://github.com/pytest-dev/pytest/blob/main/src/_pytest/_py/path.py so that it understands the Sage source layout.

please see the reply to https://github.com/pytest-dev/pytest/issues/10569#issuecomment-1344122839, basically, upstream says monkey-patching this will break, soon.

On https://github.com/pytest-dev/pytest/issues/10569 let us further discuss with the upstream how we can go forward here.

[mkoeppe]

comment:28

By the way, it's not just our doctests that are not run by pytest any more because of the PEP 420 issue. Also the pytest files *_test.py in src/sage/numerical/backends and src/sage/manifolds/differentiable are no longer being run.

[mkoeppe]

comment:29

Replying to Matthias Köppe:

So a sage-specific solution could be to monkey-patch https://github.com/pytest-dev/pytest/blob/main/src/_pytest/_py/path.py so that it understands the Sage source layout.

A more relevant place seems to be https://github.com/pytest-dev/pytest/blob/9fbd67dd4b1baa6889dbb2073c17b85da39f80d9/src/_pytest/python.py#L747 (Package.collect)

[dimpase]

comment:30

Anyway, should we move to sage --pytest from sage --python -m pytest - as the latter is playing fast and loose with sys.path ?

[mkoeppe]

comment:31

Yes, I think so