Opened 11 months ago

Closed 9 months ago

#33521 closed defect (fixed)

doctesting with pytest fails on system install of sage

Reported by: François Bissey Owned by:
Priority: blocker Milestone: sage-9.6
Component: distribution Keywords:
Cc: Tobias Diez, Michael Orlitzky, Dima Pasechnik Merged in:
Authors: Matthias Koeppe Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 84d3d9e (Commits, GitHub, GitLab) Commit: 84d3d9e5ca7b6d15355743f94c6c917359136998
Dependencies: Stopgaps:

Status badges

Description

pytest assumes the directory containing the code to be tested is writable. This is not currently the case on sage-on-distro were most of the time testing is done after installing on the system.

Here is a typical result

============================= 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

pytest can be started with some option to change the location of the cache directory -o cache_dir=.... pytest is currently called from sage-runtest and this is where we may want to apply any changes.

Change History (23)

comment:1 Changed 11 months ago by François Bissey

I have tried inserting the right option in sage-runtest with variation of

    try:
        exit_code_pytest = 0
        import pytest
        pytest_options = ["-o cache_dir=/tmp", "--import-mode", "importlib"]
        if args.verbose:
            pytest_options.append("-v")
        exit_code_pytest = pytest.main(pytest_options + args.filenames)
        if exit_code_pytest == 5:
            # Exit code 5 means there were no test files, pass in this case
            exit_code_pytest = 0

without success. Suggestions welcome.

comment:2 Changed 11 months ago by Matthias Köppe

Cc: Tobias Diez added

comment:3 Changed 10 months ago by Matthias Köppe

Priority: majorcritical

comment:4 Changed 10 months ago by François Bissey

I think if we include #33531 it can join the list of less urgent stuff to fix about pytest.

comment:5 Changed 10 months ago by François Bissey

I just realised it may make more sense to just disable pytest for sage -t --installed. Running pytest relies on the presence of src/conftest.py which is not installed (not sure it would make sense to install it). So, a system install of sage would run pytest without fixtures and ignore list - unless you manually point to a copy.

comment:6 Changed 10 months ago by Tobias Diez

I'm not sure about the exact use case of --installed but if you completely disable pytest in this situation then users no longer run the existing pytests (like the ones for the symplectic form).

comment:7 in reply to:  1 Changed 10 months ago by Tobias Diez

Replying to fbissey:

I have tried inserting the right option in sage-runtest with variation of

    try:
        exit_code_pytest = 0
        import pytest
        pytest_options = ["-o cache_dir=/tmp", "--import-mode", "importlib"]
        if args.verbose:
            pytest_options.append("-v")
        exit_code_pytest = pytest.main(pytest_options + args.filenames)
        if exit_code_pytest == 5:
            # Exit code 5 means there were no test files, pass in this case
            exit_code_pytest = 0

without success. Suggestions welcome.

Can you please try running pytest with --rootdir SAGE_SRC and/or -c SAGE_SRC/tox.ini.

comment:8 Changed 10 months ago by François Bissey

--installed is for running against a sage install as opposed to running on the source. pytest default on running on source. As for your latest suggestion, in the normal use case for this ticket, SAGE_SRC is usually pointing to SAGE_LIB and tox.ini may not be available.

comment:9 in reply to:  5 Changed 10 months ago by Matthias Köppe

Replying to fbissey:

I just realised it may make more sense to just disable pytest for sage -t --installed. Running pytest relies on the presence of src/conftest.py which is not installed (not sure it would make sense to install it). So, a system install of sage would run pytest without fixtures and ignore list - unless you manually point to a copy.

Yes, I agree

comment:10 Changed 10 months ago by Matthias Köppe

Dependencies: #31924

Setting #31924 as a dependency because it modifies src/bin/sage-runtests in the same place that needs modifying here.

comment:11 Changed 10 months ago by François Bissey

Need to get on with that ticket. Not sure how long it will take me to get to it, I am trying to diagnose flaky RAM on my main dev machine. Compilations breaking in random places is a good signal of issue.

comment:12 in reply to:  8 Changed 10 months ago by Tobias Diez

Replying to fbissey:

--installed is for running against a sage install as opposed to running on the source. pytest default on running on source. As for your latest suggestion, in the normal use case for this ticket, SAGE_SRC is usually pointing to SAGE_LIB and tox.ini may not be available.

Thanks for the explanation. Pytest is able to test everything that can be imported using importlib, so testing the existing sage install should be possible as well. However, the "normal" setup would be to run pytest on the source code of the tests but use the installed version of the package (this is for example how pytest works in the context of tox environments).

Maybe its worth a try to distribute/install the tox.ini file as well, or export the pytest config to pytest.ini or pyproject.toml and distribute that?

I cannot judge if it would be okay to skip pytest for --installed. That probably depends on the context when --installed is used. You definitely skip the existing pytests in that case. (Admittedly, this is not a huge lost atm).

comment:13 Changed 10 months ago by Matthias Köppe

There are two major use cases for --installed: In downstream distributions and for testing modularized distribution packages. For the latter, an example is #32601. In this context, there is no problem with providing the pytest configuration, as the source is available.

comment:14 Changed 10 months ago by Matthias Köppe

Authors: Matthias Koeppe
Dependencies: #31924

comment:15 Changed 10 months ago by Matthias Köppe

Branch: u/mkoeppe/doctesting_with_pytest_fails_on_system_install_of_sage

comment:16 Changed 10 months ago by Matthias Köppe

Commit: d0831c194b0c2d9aeabd1302fa6b75bfecbbe33d
Status: newneeds_review

Here's an attempt at a solution following comment:5


New commits:

d0831c1src/bin/sage-runtests: Do not run pytest if the pytest configuration is not available

comment:17 Changed 10 months ago by git

Commit: d0831c194b0c2d9aeabd1302fa6b75bfecbbe33d84d3d9e5ca7b6d15355743f94c6c917359136998

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

7c46c08src/bin/sage-runtests: Do not run pytest if the pytest configuration is not available
84d3d9esrc/bin/sage-runtests: Clarify where pytest would have to be installed

comment:18 Changed 10 months ago by Matthias Köppe

Cc: Michael Orlitzky Dima Pasechnik added

comment:19 Changed 10 months ago by François Bissey

I have been busy the last couple of weeks and completely forgotten about this ticket. I will try to do a proper review later today.

comment:20 Changed 10 months ago by François Bissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

It works as expected for me in sage-on-gentoo. pytest are not run on system install lacking the right configuration files anymore.

comment:21 Changed 10 months ago by Matthias Köppe

Thank you!

comment:22 Changed 9 months ago by Matthias Köppe

Priority: criticalblocker

comment:23 Changed 9 months ago by Volker Braun

Branch: u/mkoeppe/doctesting_with_pytest_fails_on_system_install_of_sage84d3d9e5ca7b6d15355743f94c6c917359136998
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.