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

More doctests fix in sage_docbuild #33538

Closed
kiwifb opened this issue Mar 21, 2022 · 9 comments
Closed

More doctests fix in sage_docbuild #33538

kiwifb opened this issue Mar 21, 2022 · 9 comments

Comments

@kiwifb
Copy link
Member

kiwifb commented Mar 21, 2022

A fix for a couple of doctest went missing from #33141.

sage -t --long --random-seed=22919418159437026337026400262276501942 /usr/lib/python3.10/site-packages/sage_docbuild/__init__.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage_docbuild/__init__.py", line 110, in sage_docbuild.builder_helper
Failed example:
    try:  # optional - sagemath_doc_html
        build_many(build_ref_doc, [("docname", "en", "html", {})])
    except Exception as E:
        "Non-exception during docbuild: abort pool operation" in str(E)
Expected:
    True
Got:
    False
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage_docbuild/__init__.py", line 211, in sage_docbuild.DocBuilder._doctrees_dir
Failed example:
    b._doctrees_dir()             # optional - sagemath_doc_html
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.9/site-packages/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_docbuild.DocBuilder._doctrees_dir[2]>", line 1, in <module>
        b._doctrees_dir()             # optional - sagemath_doc_html
      File "/usr/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 210, in _doctrees_dir
        os.makedirs(d, exist_ok=True)
      File "/usr/lib/python3.9/os.py", line 215, in makedirs
        makedirs(head, exist_ok=exist_ok)
      File "/usr/lib/python3.9/os.py", line 215, in makedirs
        makedirs(head, exist_ok=exist_ok)
      File "/usr/lib/python3.9/os.py", line 225, in makedirs
        mkdir(name, mode)
    PermissionError: [Errno 13] Permission denied: '/usr/share/doc/sage-doc-9999/doctrees'

Doctesting this file in is made more robust, and distro friendly by allowing and making SAGE_DOC to be a random temporary directory at the start of the doctesting of the file.

To allow SAGE_DOC to be set randomly for doctesting we cannot let it be imported globally in the whole file. Functions that uses it need to import it explicitly.

Depends on #33141

CC: @jhpalmieri

Component: distribution

Author: François Bissey

Branch/Commit: 3569408

Reviewer: John Palmieri

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

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

kiwifb commented Mar 21, 2022

New commits:

07545e9Fix sage_setup doctests
aa971bdFix doctest in sage_docbuild/__init__.py. Most of the test rely on location that are writable or the presence of documentation sources.
54bd2f5Merge branch 'develop' into distro_doctests
6c1dc29replace optional build tag with sage_spkg
18308fcget rid of the last optional build tag
e2c9eeaMerge branch 'develop' into distro_doctests
5382264make doctest in find_extra_files more robust by sorting the result so that the order is stable.
3569408Make SAGE_DOC random in doctests from sage_docbuild/__init__.py

@kiwifb
Copy link
Member Author

kiwifb commented Mar 21, 2022

Branch: u/fbissey/trac33538

@kiwifb
Copy link
Member Author

kiwifb commented Mar 21, 2022

Commit: 3569408

@jhpalmieri
Copy link
Member

comment:3

Would it be too clumsy to conditionally import SAGE_DOC at the top of the file if it is not yet defined? That would require the doctests to set the environment variable SAGE_DOC before importing from sage_docbuild, but at the moment there seems to be only one affected doctest (or at least SAGE_DOC is only defined in one testing block), while the current branch requires five new from sage.env import SAGE_DOC statements.

@kiwifb
Copy link
Member Author

kiwifb commented Mar 27, 2022

comment:4

I tried various approaches. I am not sure about importing conditionally. Would you look at os.getenv? While I only defined SAGE_DOC once, that definition is carried to all subsequent doctests. And it affects the results of at least another doctest apart from the obvious one, the one at line 211 as stated in the description.

Now that I look at it with a bit of distance, there is one thing that could be interesting to try. Define SAGE_DOC in the doctest before the first import of sage_docbuild and see if that sticks.

@jhpalmieri
Copy link
Member

comment:5

Having thought about it more, I think it's too fragile to have doctests depend on whether SAGE_DOC is defined before the relevant import statements. Let's leave the branch as it is — I don't have any better ideas for how to fix this.

Tests pass on Sage built from scratch, and I trust that they fix problems, or at least don't cause any new ones, for distros.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@kiwifb
Copy link
Member Author

kiwifb commented Mar 28, 2022

comment:6

Thank you.

@vbraun
Copy link
Member

vbraun commented Apr 2, 2022

Changed branch from u/fbissey/trac33538 to 3569408

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

3 participants