[tests] add public properties SphinxTestApp.status and SphinxTestApp.warning

CHANGES.rst

@@ -13,6 +13,11 @@ Deprecated
 * #11693: Support for old-style :file:`Makefile` and :file:`make.bat` output
   in :program:`sphinx-quickstart`, and the associated options :option:`!-M`,
   :option:`!-m`, :option:`!--no-use-make-mode`, and :option:`!--use-make-mode`.
+* #11285: Direct access to :attr:`!sphinx.testing.util.SphinxTestApp._status`
+  or :attr:`!sphinx.testing.util.SphinxTestApp._warning` is deprecated. Use
+  the public properties :attr:`!sphinx.testing.util.SphinxTestApp.status`
+  and :attr:`!sphinx.testing.util.SphinxTestApp.warning` instead.
+  Patch by Bénédikt Tran.
 
 Features added
 --------------
@@ -99,6 +104,19 @@ Bugs fixed
 
 Testing
 -------
+* #11285: :func:`!pytest.mark.sphinx` requires keyword arguments, except for
+  the builder name which can still be given as the first positional argument.
+  Patch by Bénédikt Tran.
+* #11285: :func:`!pytest.mark.sphinx` accepts *warningiserror*, *keep_going*
+  and *verbosity* as additional keyword arguments.
+  Patch by Bénédikt Tran.
+* #11285: :class:`!sphinx.testing.util.SphinxTestApp` *srcdir* argument is
+  now mandatory (previously, this was checked with an assertion).
+  Patch by Bénédikt Tran.
+* #11285: :class:`!sphinx.testing.util.SphinxTestApp` *status* and *warning*
+  arguments are checked to be :class:`io.StringIO` objects (the public API
+  incorrectly assumed this without checking it).
+  Patch by Bénédikt Tran.
 
 Release 7.2.6 (released Sep 13, 2023)
 =====================================

sphinx/testing/fixtures.py

@@ -7,7 +7,7 @@
 import sys
 from collections import namedtuple
 from io import StringIO
-from typing import TYPE_CHECKING, Any, Callable
+from typing import TYPE_CHECKING
 
 import pytest
 
@@ -16,11 +16,16 @@
 if TYPE_CHECKING:
     from collections.abc import Generator
     from pathlib import Path
+    from typing import Any, Callable
 
 DEFAULT_ENABLED_MARKERS = [
     (
-        'sphinx(builder, testroot=None, freshenv=False, confoverrides=None, tags=None, '
-        'docutils_conf=None, parallel=0): arguments to initialize the sphinx test application.'
+        'sphinx('
+        'buildername="html", /, *, '
+        'testroot="root", confoverrides=None, freshenv=False, '
+        'warningiserror=False, tags=None, verbosity=0, parallel=0, '
+        'keep_going=False, builddir=None, docutils_conf=None'
+        '): arguments to initialize the sphinx test application.'
     ),
     'test_params(shared_result=...): test parameters.',
 ]
@@ -44,8 +49,8 @@ def store(self, key: str, app_: SphinxTestApp) -> Any:
         if key in self.cache:
             return
         data = {
-            'status': app_._status.getvalue(),
-            'warning': app_._warning.getvalue(),
+            'status': app_.status.getvalue(),
+            'warning': app_.warning.getvalue(),
         }
         self.cache[key] = data
 
@@ -153,15 +158,15 @@ def status(app: SphinxTestApp) -> StringIO:
     """
     Back-compatibility for testing with previous @with_app decorator
     """
-    return app._status
+    return app.status
 
 
 @pytest.fixture()
 def warning(app: SphinxTestApp) -> StringIO:
     """
     Back-compatibility for testing with previous @with_app decorator
     """
-    return app._warning
+    return app.warning
 
 
 @pytest.fixture()

sphinx/testing/util.py

@@ -1,12 +1,15 @@
 """Sphinx test suite utilities"""
+
 from __future__ import annotations
 
 import contextlib
 import os
 import re
 import sys
 import warnings
-from typing import IO, TYPE_CHECKING, Any
+from io import StringIO
+from types import MappingProxyType
+from typing import TYPE_CHECKING
 from xml.etree import ElementTree
 
 from docutils import nodes
@@ -18,8 +21,9 @@
 from sphinx.util.docutils import additional_nodes
 
 if TYPE_CHECKING:
-    from io import StringIO
+    from collections.abc import Mapping
     from pathlib import Path
+    from typing import Any
 
     from docutils.nodes import Node
 
@@ -73,28 +77,75 @@ def etree_parse(path: str) -> Any:
 
 
 class SphinxTestApp(sphinx.application.Sphinx):
-    """
-    A subclass of :class:`Sphinx` that runs on the test root, with some
-    better default values for the initialization parameters.
+    """A subclass of :class:`~sphinx.application.Sphinx` for tests.
+
+    The constructor uses some better default values for the initialization
+    parameters and supports arbitrary keywords stored in the :attr:`extras`
+    read-only mapping.
+
+    It is recommended to use::
+
+        @pytest.mark.sphinx('html')
+        def test(app):
+            app = ...
+
+    instead of::
+
+        def test():
+            app = SphinxTestApp('html', srcdir=srcdir)
+
+    In the former case, the 'app' fixture takes care of setting the source
+    directory, whereas in the latter, the user must provide it themselves.
     """
 
-    _status: StringIO
-    _warning: StringIO
+    # Allow the builder name to be passed as a keyword argument
+    # but only make it positional-only for ``pytest.mark.sphinx``
+    # so that an exception can be raised if the constructor is
+    # directly called and multiple values for the builder name
+    # are given.
 
     def __init__(
         self,
+        /,
         buildername: str = 'html',
-        srcdir: Path | None = None,
-        builddir: Path | None = None,
+        *,
+        srcdir: Path,
+        confoverrides: dict[str, Any] | None = None,
+        status: StringIO | None = None,
+        warning: StringIO | None = None,
         freshenv: bool = False,
-        confoverrides: dict | None = None,
-        status: IO | None = None,
-        warning: IO | None = None,
+        warningiserror: bool = False,
         tags: list[str] | None = None,
-        docutils_conf: str | None = None,
+        verbosity: int = 0,
         parallel: int = 0,
+        keep_going: bool = False,
+        # extra constructor arguments
+        builddir: Path | None = None,
+        docutils_conf: str | None = None,
+        # unknown keyword arguments
+        **extras: Any,
     ) -> None:
-        assert srcdir is not None
+        if verbosity == -1:
+            quiet = True
+            verbosity = 0
+        else:
+            quiet = False
+
+        if status is None:
+            # ensure that :attr:`status` is a StringIO and not sys.stdout
+            # but allow the stream to be /dev/null by passing verbosity=-1
+            status = None if quiet else StringIO()
+        elif not isinstance(status, StringIO):
+            err = "%r must be an io.StringIO object, got: %s" % ('status', type(status))
+            raise TypeError(err)
+
+        if warning is None:
+            # ensure that :attr:`warning` is a StringIO and not sys.stderr
+            # but allow the stream to be /dev/null by passing verbosity=-1
+            warning = None if quiet else StringIO()
+        elif not isinstance(warning, StringIO):
+            err = '%r must be an io.StringIO object, got: %s' % ('warning', type(warning))
+            raise TypeError(err)
 
         self.docutils_conf_path = srcdir / 'docutils.conf'
         if docutils_conf is not None:
@@ -112,17 +163,35 @@ def __init__(
             confoverrides = {}
 
         self._saved_path = sys.path.copy()
+        self.extras: Mapping[str, Any] = MappingProxyType(extras)
+        """Extras keyword arguments."""
 
         try:
             super().__init__(
-                srcdir, confdir, outdir, doctreedir,
-                buildername, confoverrides, status, warning, freshenv,
-                warningiserror=False, tags=tags, parallel=parallel,
+                srcdir, confdir, outdir, doctreedir, buildername,
+                confoverrides=confoverrides, status=status, warning=warning,
+                freshenv=freshenv, warningiserror=warningiserror, tags=tags,
+                verbosity=verbosity, parallel=parallel, keep_going=keep_going,
+                pdb=False,
             )
         except Exception:
             self.cleanup()
             raise
 
+    @property
+    def status(self) -> StringIO:
+        """The in-memory text I/O for the application status messages."""
+        # sphinx.application.Sphinx uses StringIO for a quiet stream
+        assert isinstance(self._status, StringIO)
+        return self._status
+
+    @property
+    def warning(self) -> StringIO:
+        """The in-memory text I/O for the application warning messages."""
+        # sphinx.application.Sphinx uses StringIO for a quiet stream
+        assert isinstance(self._warning, StringIO)
+        return self._warning
+
     def cleanup(self, doctrees: bool = False) -> None:
         sys.path[:] = self._saved_path
         _clean_up_global_state()