SphinxTestApp builds in _build, not separated build folder

[adamgranthendry]

Describe the bug

When running sphinx-quickstart, users may choose to put build files in a folder build separate from source. However, when running unit tests, SphinxTestApp automatically builds in source/_build, which creates two sets of build files. This is evident from the source:

class SphinxTestApp(application.Sphinx):
    """
    A subclass of :class:`Sphinx` that runs on the test root, with some
    better default values for the initialization parameters.
    """
    _status: StringIO = None
    _warning: StringIO = None

    def __init__(self, buildername: str = 'html', srcdir: path = None, freshenv: bool = False,
                 confoverrides: Dict = None, status: IO = None, warning: IO = None,
                 tags: List[str] = None, docutilsconf: str = None, parallel: int = 0) -> None:

        if docutilsconf is not None:
            (srcdir / 'docutils.conf').write_text(docutilsconf)

        builddir = srcdir / '_build'

How to Reproduce

> sphinx-quickstart
...
Enter the root path for documentation.
> Root path for the documentation [.]:

You have two options for placing the build directory for Sphinx output.
Either, you use a directory "_build" within the root path, or you separate
"source" and "build" directories within the root path.
> Separate source and build directories (y/n) [n]: y
...

Then, when running a test, files are built to source/_build instead of the desired build folder.

Expected behavior

sphinx uses the environment variable ${BUILDDIR} (written out to Makefile and make.bat) set when running sphinx-quickstart as the folder to put build files in.

Your project

N/A

Screenshots

OS

Windows 10 Professional x64, Build 1909

Python version

3.8.10

Sphinx version

4.1.2

Sphinx extensions

N/A

Extra tools

N/A

Additional context

NOTE: If this was intentional, then users who do build under the source directory have their build files overwritten with each unit test, and then this is still a bug.

[tk0miya]

SphinxTestApp is used for testing sphinx itself and its extensions. And it does not relate to sphinx-quickstart at all. So I don't understand what you want. Please let me know why do you want to separate the source and destination directories? I'd like to know how do you use the testing class on your script.

[adamgranthendry]

@tk0miya Please refer to this PyGotham2018, this PyOhio2018, and this PyBay2019 tutorial on Customizing Sphinx by Paul Everitt. sphinx provides a pytest plugin, which he shows can be used for integration testing of documentation. This is a common usage for developers (at least those required to integration test their documentation).

The issue is sphinx creates two sets of build files in different directories when SphinxTestApp is used if it does not look to see where the user set to have build files placed. This is redundant, inefficient, and leads the developer to think where the build folder was set is not working properly.

The build directory should not be hard coded, or at the very least, the constructor should have an argument the developer can set to set where to put build files.

[jakobandersen]

If I understand correctly from the tutorial(s) the starting point is that you import sphinx.testing.fixtures?
I don't think that's part of that public API for Sphinx, hence there is no guarantee that behaviour won't change over time.

[adamgranthendry]

@jakobandersen I actually import from sphinx.testing.util, but I do pass sphinx.testing.fixtures to my plugins list:

conftest.py:

"""Configuration file for `pytest` tests
"""

from typing import Any, Callable, Generator, List, Sequence, Union
from unittest.mock import patch

import pytest

from _pytest.fixtures import SubRequest
from bs4 import BeautifulSoup

from sphinx.testing.path import path
from sphinx.testing.util import SphinxTestApp

pytest_plugins: Union[str, Sequence[str]] = ["sphinx.testing.fixtures",]
"""A ``pytest`` global variable that registers plugins for use in testing."""

# Exclude 'roots' dirs for pytest test collector
collect_ignore: List[str] = ["roots", "__init__.py"]
"""A ``pytest`` global variable that excludes directories or modules from being collected by the ``pytest`` runner"""


@pytest.fixture()
def rootdir() -> Generator[str, None, None]:
    """Root directory for sphinx tests.

    Yields:
        Generator[str, None, None]: Generator that yields test root directories
    """
    yield path(".\\docs\\source").abspath()


@pytest.fixture()
def content(
    make_app: Callable[..., Any], rootdir: str
) -> Generator[SphinxTestApp, None, None]:
    """Content of generated html pages.

    Args:
        make_app (Callable[..., Any]): Function that creates a sphinx app for testing purposes
        rootdir (str): Test root directory

    Yields:
        Generator[SphinxTestApp, None, None]: Generator that yields sphinx apps for testing purposes
    """
    app = make_app("html", rootdir)
    app.build()
    yield app


@pytest.fixture()
def page(
    content: SphinxTestApp, request: SubRequest
) -> Generator[BeautifulSoup, None, None]:
    """Generated HTML page of docs

    Args:
        content (SphinxTestApp): A sphinx app for testing purposes
        request (SubRequest): A sub request for handling getting a fixture from a test function/fixture

    Yields:
        Generator[BeautifulSoup, None, None]: Generator of beautiful soup web scraper instances using the html5lib parser
    """
    page_name = request.param
    page_content = (content.outdir / page_name).read_text()

    yield BeautifulSoup(page_content, "html5lib")

test_index_page.py:

"""Test the documentation index page
"""

from typing import Any, Callable, TypeVar

import pytest

pytestmark = pytest.mark.sphinx("html")

Fixture = TypeVar("Fixture", bound=Callable[..., Any])
FixtureFactory = Callable[[Fixture], Fixture]


@pytest.mark.parametrize(
    "page",
    [
        "index.html",
    ],
    indirect=True,
)
def test_index(page):
    """Test the index page to our documentation

    Args:
        page ([type]): [description]
    """
    expected_title = "Main Page"
    return_value = page.find("title").contents[0].strip()
    assert return_value == expected_title

[adamgranthendry]

@jakobandersen How is testing done for sphinx in general? It is a problem that the build folders aren't aligned, so this must be changed somewhere. It could very simply be fixed by adding an argument to SphinxTestApp.__init__(). You already provide an argument for srcdir. If you simply provide an argument for builddir, the developer can use this argument if behavior changes over time.

Right now, I am literally monkey-patching this by changing

builddir = srcdir / '_build'

to

builddir = srcdir.parent / "build"

inside sphinx.testing.util.SphinxTestApp, but this is an inelegant solution. If I could pass buildir directly to sphinx.testing.fixtures.make_app directly, that would solve all my problems. make_app simply wraps SphinxTestApp, passing it *args and **kwargs, so you would just need to add an argument builddir: path = None to SphinxTestApp.__init__():

@pytest.fixture()
def make_app(test_params: Dict, monkeypatch: Any) -> Generator[Callable, None, None]:
    """
    provides make_app function to initialize SphinxTestApp instance.
    if you want to initialize 'app' in your test function. please use this
    instead of using SphinxTestApp class directory.
    """
    monkeypatch.setattr('sphinx.application.abspath', lambda x: x)

    apps = []
    syspath = sys.path[:]

    def make(*args, **kwargs):
        status, warning = StringIO(), StringIO()
        kwargs.setdefault('status', status)
        kwargs.setdefault('warning', warning)
        app_: Any = SphinxTestApp(*args, **kwargs)
        apps.append(app_)
        if test_params['shared_result']:
            app_ = SphinxTestAppWrapperForSkipBuilding(app_)
        return app_
    yield make

    sys.path[:] = syspath
    for app_ in reversed(apps):  # clean up applications from the new ones
        app_.cleanup()

It's a one-line change.

[astrojuanlu]

tl;dr: (1) we might want to replace sphinx.testing.path.path by stdlib pathlib, (2) it's not clear what's considered "public API" of Sphinx, (3) there is not an official, blessed way of testing Sphinx extensions.

Just for the record, I watched @pauleveritt 2018-2019 talks on customizing Sphinx mentioned by @adamgranthendry, which are based on https://github.com/pauleveritt/customizing_sphinx, and I have some remarks:

• It uses sphinx.testing.path.path, but I'm wondering if that's essentially what https://docs.python.org/3/library/pathlib.html does these days. Given that Sphinx now requires Python 3.6+, perhaps it should be removed?
• It uses sphinx.testing.fixtures, although by quickly looking at https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/testing/fixtures.py it's not clear to me what fixtures are being used (@pauleveritt not sure if you still maintain that repo, would be cool to make that a bit more explicit).
• SphinxTestApp is not mentioned there.

This leaves me wondering if there should be a more clear way of designating what is considered "public API" and what is not (see also our recent discussion on sphinx-dev). Apart from "what is documented", perhaps it should be in a private namespace (_testing or something like that) to better convey that it's not meant to be used.

On the other hand, none of the tutorials on Sphinx extensions nor the overview mention how Sphinx extensions should be tested, so I think it's natural that people try to find their way. @jakobandersen @tk0miya do you have any thoughts on how this should be done, perhaps in the form of some raw notes that we can turn into actual docs?

[adamgranthendry]

@astrojuanlu +1 on replacing sphinx.testing.path.path with pathlib. I agree that it was probably done as it predates pathlib. Also, watching Get Your Resources Faster, with importlib.resources, PyCon 2018 by Barry Warsaw, using pathlib can present problems when packaging docs into a zip file. We might want to consider importlib.resources instead.

[tk0miya]

I've not watched the pointed videos yet. But I understand you'd like to test your extensions using SphinxTestApp. I was confused because you mentioned sphinx-quickstart in the first report.

I think there is no special reason to fix the build directory. So +1 for add a new argument to pass it from the caller.

[tk0miya]

(1) we might want to replace sphinx.testing.path.path by stdlib pathlib

It would be nice if we can migrate to the pathlib. Our path module has some additional methods to it. So we need to find the way to migrate.

Anyway, it's another topic.

(2) it's not clear what's considered "public API" of Sphinx

We called the APIs described on our document are "public APIs":
https://www.sphinx-doc.org/en/master/extdev/index.html

(3) there is not an official, blessed way of testing Sphinx extensions.

Indeed. It has been requested #7008. But not progressed yet...

[tk0miya]

I posted #9549 now. Could you try it please?