[tests] start fixing the current plugin [part 1] (#12089)

Author: picnixz

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
 --------------
@@ -101,6 +106,13 @@ Bugs fixed
 
 Testing
 -------
+* #11285: :func:`!pytest.mark.sphinx` and :class:`!sphinx.testing.util.SphinxTestApp`
+  accept *warningiserror*, *keep_going* and *verbosity* as keyword arguments.
+  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.
 
 * pytest: report the result of ``test_run_epubcheck`` as ``skipped`` instead of
   ``success`` when Java and/or the ``epubcheck.jar`` code are not available.

sphinx/testing/fixtures.py

@@ -20,8 +20,12 @@
 
 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.',
 ]
@@ -45,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
 
@@ -163,15 +167,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,29 +77,74 @@ 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
+    # see https://github.com/sphinx-doc/sphinx/pull/12089 for the
+    # discussion on how the signature of this class should be used
 
     def __init__(
         self,
+        /,  # to allow 'self' as an extras
         buildername: str = 'html',
         srcdir: Path | None = None,
-        builddir: Path | None = None,
-        freshenv: bool = False,
-        confoverrides: dict | None = None,
-        status: IO | None = None,
-        warning: IO | None = None,
+        builddir: Path | None = None,  # extra constructor argument
+        freshenv: bool = False,  # argument is not in the same order as in the superclass
+        confoverrides: dict[str, Any] | None = None,
+        status: StringIO | None = None,
+        warning: StringIO | None = None,
         tags: list[str] | None = None,
-        docutils_conf: str | None = None,
+        docutils_conf: str | None = None,  # extra constructor argument
         parallel: int = 0,
+        # additional arguments at the end to keep the signature
+        verbosity: int = 0,  # argument is not in the same order as in the superclass
+        keep_going: bool = False,
+        warningiserror: bool = False,  # argument is not in the same order as in the superclass
+        # 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:
             self.docutils_conf_path.write_text(docutils_conf, encoding='utf8')
@@ -112,17 +161,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()