Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doctesting with pytest fails on system install of sage #33521

Closed
kiwifb opened this issue Mar 17, 2022 · 24 comments
Closed

doctesting with pytest fails on system install of sage #33521

kiwifb opened this issue Mar 17, 2022 · 24 comments

Comments

@kiwifb
Copy link
Member

kiwifb commented Mar 17, 2022

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.

CC: @tobiasdiez @orlitzky @dimpase

Component: distribution

Author: Matthias Koeppe

Branch/Commit: 84d3d9e

Reviewer: François Bissey

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

@kiwifb kiwifb added this to the sage-9.6 milestone Mar 17, 2022
@kiwifb
Copy link
Member Author

kiwifb commented Mar 17, 2022

comment:1

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.

@kiwifb
Copy link
Member Author

kiwifb commented Mar 19, 2022

comment:4

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

@kiwifb
Copy link
Member Author

kiwifb commented Mar 22, 2022

comment:5

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.

@tobiasdiez
Copy link
Contributor

comment:6

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).

@tobiasdiez
Copy link
Contributor

comment:7

Replying to @kiwifb:

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.

@kiwifb
Copy link
Member Author

kiwifb commented Mar 22, 2022

comment:8

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2022

comment:9

Replying to @kiwifb:

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 23, 2022

Dependencies: #31924

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 23, 2022

comment:10

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

@kiwifb
Copy link
Member Author

kiwifb commented Mar 23, 2022

comment:11

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.

@tobiasdiez
Copy link
Contributor

comment:12

Replying to @kiwifb:

--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).

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2022

comment:13

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 25, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 25, 2022

Changed dependencies from #31924 to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 25, 2022

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 25, 2022

Commit: d0831c1

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 25, 2022

comment:16

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2022

Changed commit from d0831c1 to 84d3d9e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2022

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

@kiwifb
Copy link
Member Author

kiwifb commented Apr 12, 2022

comment:19

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.

@kiwifb
Copy link
Member Author

kiwifb commented Apr 12, 2022

comment:20

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

@kiwifb
Copy link
Member Author

kiwifb commented Apr 12, 2022

Reviewer: François Bissey

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 13, 2022

comment:21

Thank you!

@vbraun
Copy link
Member

vbraun commented Apr 21, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants