sage -pytest

[mkoeppe]

Thin wrapper around pytest for running it in the Sage environment.

as suggested in #31924 comment:94

Similar to our existing wrapper sage -tox.

CC: @tornaria @tobiasdiez @soehms

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: 8825079

Reviewer: Tobias Diez

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

[mkoeppe]

Branch: u/mkoeppe/sage__pytest

[mkoeppe]

comment:2

Here's an attempt at this command, but I think I'll need some help here to make it actually work:

$ ./sage -pytest src/sage/numerical/
============================================================================================================ 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 0 items / 5 errors                                                                                                                                                                                                                

================================================================================================================== ERRORS ===================================================================================================================
______________________________________________________________________________________ ERROR collecting sage/numerical/backends/cvxopt_backend_test.py ______________________________________________________________________________________
ImportError while importing test module '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src/sage/numerical/backends/cvxopt_backend_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
src/sage/numerical/backends/cvxopt_backend_test.py:5: in <module>
    from sage.numerical.mip import MixedIntegerLinearProgram
sage/numerical/mip.pyx:242: in init sage.numerical.mip
    ???
sage/rings/integer_ring.pyx:1: in init sage.rings.integer_ring
    ???
sage/rings/integer.pyx:1: in init sage.rings.integer
    ???
sage/rings/rational.pyx:79: in init sage.rings.rational
    ???
E   ImportError: cannot import name ZZ

---

New commits:

9385501

src/bin/sage: Implement 'sage --pytest'

[mkoeppe]

Changed author from Matthias Koeppe to Matthias Koeppe, ...

[mkoeppe]

Commit: 9385501

[tobiasdiez]

comment:3

That looks like a typical cyclic import that relies on sage.all to be broken. It is also not specific to pytest as the following example fails with the same error (when run through ./sage src/test_module_load.py).

import importlib.util
spec = importlib.util.spec_from_file_location("module.name", "src/sage/numerical/backends/cvxopt_backend_test.py")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)

[tobiasdiez]

comment:4

Moreover, there are a few dozens of such import errors:

import importlib.util
import pathlib
for file in pathlib.Path().glob("src/sage/**/*.py"):
    try: 
        spec = importlib.util.spec_from_file_location("module.name", file)
        module = importlib.util.module_from_spec(spec)
        spec.loader.exec_module(module)
    except:
        print(file)

[mkoeppe]

comment:5

Can't you just add import sage.all in conftest.py?

[tobiasdiez]

comment:6

There is already a hook which adds sage.all, but that one is only executed after the collection phase.

[sagetrac-git]

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

abbb61f

src/conftest.py: import sage.all to avoid cyclic import errors

[sagetrac-git]

Changed commit from 9385501 to abbb61f

[mkoeppe]

Changed author from Matthias Koeppe, ... to Matthias Koeppe

[tobiasdiez]

comment:9

Ah that's what you had in mind. Seems to work indeed. Can you please add a comment that prevents pyright to complain about the unused import and add a link to #33580. Thanks!

[sagetrac-git]

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

9bb72ea	src/conftest.py: Add # type: ignore, add reference to trac ticket
2c16704	src/conftest.py: Remove outdated text

[sagetrac-git]

Changed commit from abbb61f to 2c16704

[tobiasdiez]

comment:12

$ ./sage -pytest
========================================================================================================= test session starts ==========================================================================================================
platform linux -- Python 3.8.10, pytest-7.0.1, pluggy-1.0.0
rootdir: /workspace/sagetrac-mirror/src
collected 0 items / 1 error                                                                                                                                                                                                            

================================================================================================================ ERRORS ================================================================================================================
____________________________________________________________________________________________________ ERROR collecting test session _____________________________________________________________________________________________________
local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/IPython/conftest.py:9: in <module>
    from .testing import tools
E   ModuleNotFoundError: No module named 'workspace'

should probably fallback to ./sage -pytest src

[sagetrac-git]

Changed commit from 2c16704 to 30642ba

[sagetrac-git]

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

30642ba

src/bin/sage: Handle 'sage -pytest' without file args

[mkoeppe]

comment:23

Replying to @tobiasdiez:

While playing around, I've also discovered the following error:

$ ./sage --pytest src/sage/ext_data/nbconvert/postprocess.py

This file is to be considered a data file; it's not a module of the Sage library

[tobiasdiez]

comment:24

Thanks for the follow-up. However, during the merge something seems to went wrong. The collect_ignore in the test config shouldn't be readded.

[tobiasdiez]

comment:25

Replying to @mkoeppe:

Replying to @tobiasdiez:

While playing around, I've also discovered the following error:

$ ./sage --pytest src/sage/ext_data/nbconvert/postprocess.py

This file is to be considered a data file; it's not a module of the Sage library

So you are saying it should be removed during collection? Then please do so by adding a filter in pytest_collect_file similar to #33560.

[sagetrac-git]

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

f0d5f84	src/conftest.py: import sage.all to avoid cyclic import errors
d881a5a	src/conftest.py: Add # type: ignore, add reference to trac ticket
1242e1f	src/conftest.py: Remove outdated text
0d9225d	src/bin/sage: Handle 'sage -pytest' without file args
05bc58f	src/tox.ini (pytest): Set --import-mode importlib here, not in src/bin/sage, src/bin/sage-runtests

[sagetrac-git]

Changed commit from 11219e5 to 05bc58f

[mkoeppe]

comment:27

Indeed. Thanks for catching the mistake in rebasing

[mkoeppe]

comment:28

Replying to @tobiasdiez:

So you are saying it should be removed during collection? Then please do so by adding a filter in pytest_collect_file similar to #33560.

The directories marked by the presence of a nodoctest file are ignored by the doctester.

$ find src -name nodoctest
src/sage/ext_data/nodoctest
src/sage/doctest/tests/nodoctest

Not sure if we want to use this mechanism somehow or just hardcode it in conftest.py

[tobiasdiez]

comment:30

Replying to @mkoeppe:

Replying to @tobiasdiez:

So you are saying it should be removed during collection? Then please do so by adding a filter in pytest_collect_file similar to #33560.

The directories marked by the presence of a nodoctest file are ignored by the doctester.

$ find src -name nodoctest
src/sage/ext_data/nodoctest
src/sage/doctest/tests/nodoctest

Not sure if we want to use this mechanism somehow or just hardcode it in conftest.py

I would say its simpler to exclude these two directories in conftest. As you already know, I'm not a big fan of such marker files in general.

[mkoeppe]

comment:31

OK, sounds good.

[mkoeppe]

comment:32

Given that ./sage --pytest src/sage/ext_data/nbconvert/ collects nothing, I don't think we need to do anything about ./sage --pytest src/sage/ext_data/nbconvert/postprocess.py.

[tobiasdiez]

comment:33

Why not?
It's the same issue as with sage -t and #33560. It works if you specify a folder and let pytest do the collection, but doesn't work correctly if you overwrite the collection by specifying the complete path.

[mkoeppe]

comment:34

Not the same because there is no existing practice of running pytest on that file.

[sagetrac-git]

Changed commit from 05bc58f to 8825079

[sagetrac-git]

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

8825079

src/doc/en/developer/tools.rst: Update section on pytest

[tobiasdiez]

comment:36

Replying to @mkoeppe:

Not the same because there is no existing practice of running pytest on that file.

That applies to Cython files as well, or not? We still exclude them in #33560.

[mkoeppe]

comment:37

There is very much existing practice of running sage -t on Cython files.

[mkoeppe]

comment:38

Let's get this in please

[tobiasdiez]

comment:39

Okay. I still think these folders with nodoctest files should be skipped; but that can be done in a follow-up ticket. (The sagebot issues don't seem to be related to this ticket.)

[mkoeppe]

comment:40

Thanks!

[vbraun]

Changed branch from u/mkoeppe/sage__pytest to 8825079