From fadb6b10cb15d2a8ce336cee307dcb3ff64680bd Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 17:12:42 +0100 Subject: [PATCH 1/4] Stop exiting early with ``--fail-on-warnings``; add ``--exception-on-warning`` (#12743) Co-authored-by: Jeremy Maitin-Shepard --- .github/workflows/builddoc.yml | 1 - CHANGES.rst | 8 +++ doc/internals/contributing.rst | 2 +- doc/man/sphinx-build.rst | 32 +++++++-- doc/usage/extensions/autodoc.rst | 4 ++ sphinx/application.py | 58 ++++++++-------- sphinx/builders/__init__.py | 5 +- sphinx/builders/linkcheck.py | 4 +- sphinx/cmd/build.py | 21 ++++-- sphinx/ext/autodoc/__init__.py | 23 ++++--- sphinx/ext/autodoc/importer.py | 15 ++--- sphinx/ext/autosummary/generate.py | 2 +- sphinx/ext/coverage.py | 8 +-- sphinx/ext/doctest.py | 2 +- sphinx/testing/fixtures.py | 2 +- sphinx/testing/util.py | 7 +- sphinx/util/logging.py | 78 ++++++---------------- tests/test_builders/test_build_warnings.py | 18 +++++ tests/test_util/test_util_logging.py | 45 ------------- tox.ini | 2 +- 20 files changed, 160 insertions(+), 177 deletions(-) diff --git a/.github/workflows/builddoc.yml b/.github/workflows/builddoc.yml index 7fa7b96f3f9..5c1736d3c5b 100644 --- a/.github/workflows/builddoc.yml +++ b/.github/workflows/builddoc.yml @@ -44,4 +44,3 @@ jobs: --jobs=auto --show-traceback --fail-on-warning - --keep-going diff --git a/CHANGES.rst b/CHANGES.rst index c2c74365e3e..7de6ab68dfe 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -30,6 +30,14 @@ Features added output files. * #12474: Support type-dependent search result highlighting via CSS. Patch by Tim Hoffmann. +* #12743: No longer exit on the first warning when + :option:`--fail-on-warning ` is used. + Instead, exit with a non-zero status if any warnings were generated + during the build. + Patch by Adam Turner. +* #12743: Add :option:`sphinx-build --exception-on-warning`, + to raise an exception when warnings are emitted during the build. + Patch by Adam Turner and Jeremy Maitin-Shepard. Bugs fixed ---------- diff --git a/doc/internals/contributing.rst b/doc/internals/contributing.rst index 0f387341658..0407a1a36b5 100644 --- a/doc/internals/contributing.rst +++ b/doc/internals/contributing.rst @@ -263,7 +263,7 @@ To build the documentation, run the following command: .. code-block:: shell - sphinx-build -M html ./doc ./build/sphinx --fail-on-warning --keep-going + sphinx-build -M html ./doc ./build/sphinx --fail-on-warning This will parse the Sphinx documentation's source files and generate HTML for you to preview in :file:`build/sphinx/html`. diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index a8a21f8ce93..a9e3a09cbfd 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -43,7 +43,7 @@ Options the source and output directories, before any other options are passed. For example:: - sphinx-build -M html ./source ./build --fail-on-warning --keep-going + sphinx-build -M html ./source ./build --fail-on-warning The *make-mode* provides the same build functionality as a default :ref:`Makefile or Make.bat `, @@ -253,20 +253,35 @@ Options .. option:: -W, --fail-on-warning - Turn warnings into errors. This means that the build stops at the first - warning and ``sphinx-build`` exits with exit status 1. + Turn warnings into errors. + This means that :program:`sphinx-build` exits with exit status 1 + if any warnings are generated during the build. .. versionchanged:: 7.3 Add ``--fail-on-warning`` long option. + .. versionchanged:: 8.1 + :program:`sphinx-build` no longer exits on the first warning, + but instead runs the entire build and exits with exit status 1 + if any warnings were generated. + This behaviour was previously enabled with :option:`--keep-going`. .. option:: --keep-going - Only applicable whilst using :option:`--fail-on-warning`, - which by default exits :program:`sphinx-build` on the first warning. + From Sphinx 8.1, :option:`!--keep-going` is always enabled. + Previously, it was only applicable whilst using :option:`--fail-on-warning`, + which by default exited :program:`sphinx-build` on the first warning. Using :option:`!--keep-going` runs :program:`!sphinx-build` to completion and exits with exit status 1 if errors are encountered. .. versionadded:: 1.8 + .. versionchanged:: 8.1 + :program:`sphinx-build` no longer exits on the first warning, + meaning that in effect :option:`!--fail-on-warning` is always enabled. + The option is retained for compatibility, but may be removed at some + later date. + + .. xref RemovedInSphinx10Warning: deprecate this option in Sphinx 10 + or no earlier than 2026-01-01. .. option:: -T, --show-traceback @@ -287,6 +302,13 @@ Options .. versionchanged:: 7.3 Add ``--pdb`` long option. +.. option:: --exception-on-warning + + Raise an exception when a warning is emitted during the build. + This can be useful in combination with :option:`--pdb` to debug warnings. + + .. versionadded:: 8.1 + .. option:: -h, --help, --version Display usage summary or Sphinx version. diff --git a/doc/usage/extensions/autodoc.rst b/doc/usage/extensions/autodoc.rst index 7495d3a9681..289218f8093 100644 --- a/doc/usage/extensions/autodoc.rst +++ b/doc/usage/extensions/autodoc.rst @@ -770,6 +770,10 @@ There are also config values that you can set: If ``False`` is given, autodoc forcedly suppresses the error if the imported module emits warnings. By default, ``True``. + .. versionchanged:: 8.1 + This option now has no effect as :option:`!--fail-on-warning` + no longer exits early. + .. confval:: autodoc_inherit_docstrings This value controls the docstrings inheritance. diff --git a/sphinx/application.py b/sphinx/application.py index a1589fb230c..142e1929400 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -41,6 +41,8 @@ from sphinx.util.tags import Tags if TYPE_CHECKING: + from typing import Final + from docutils import nodes from docutils.nodes import Element, Node from docutils.parsers import Parser @@ -134,7 +136,7 @@ class Sphinx: :ivar outdir: Directory for storing build documents. """ - warningiserror: bool + warningiserror: Final = False _warncount: int def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[str] | None, @@ -144,7 +146,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), verbosity: int = 0, parallel: int = 0, keep_going: bool = False, - pdb: bool = False) -> None: + pdb: bool = False, exception_on_warning: bool = False) -> None: """Initialize the Sphinx application. :param srcdir: The path to the source directory. @@ -163,8 +165,9 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st :param verbosity: The verbosity level. :param parallel: The maximum number of parallel jobs to use when reading/writing documents. - :param keep_going: If true, continue processing when an error occurs. + :param keep_going: Unused. :param pdb: If true, enable the Python debugger on an exception. + :param exception_on_warning: If true, raise an exception on warnings. """ self.phase = BuildPhase.INITIALIZATION self.verbosity = verbosity @@ -203,12 +206,10 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st else: self._warning = warning self._warncount = 0 - self.keep_going = warningiserror and keep_going - if self.keep_going: - self.warningiserror = False - else: - self.warningiserror = warningiserror + self.keep_going = bool(warningiserror) # Unused + self._fail_on_warnings = bool(warningiserror) self.pdb = pdb + self._exception_on_warning = exception_on_warning logging.setup(self, self._status, self._warning) self.events = EventManager(self) @@ -386,26 +387,31 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> self.events.emit('build-finished', err) raise - if self._warncount and self.keep_going: - self.statuscode = 1 - - status = (__('succeeded') if self.statuscode == 0 - else __('finished with problems')) - if self._warncount: - if self.warningiserror: - if self._warncount == 1: - msg = __('build %s, %s warning (with warnings treated as errors).') - else: - msg = __('build %s, %s warnings (with warnings treated as errors).') + if self._warncount == 0: + if self.statuscode != 0: + logger.info(bold(__('build finished with problems.'))) else: - if self._warncount == 1: - msg = __('build %s, %s warning.') - else: - msg = __('build %s, %s warnings.') - - logger.info(bold(msg), status, self._warncount) + logger.info(bold(__('build succeeded.'))) + elif self._warncount == 1: + if self._fail_on_warnings: + self.statuscode = 1 + msg = __('build finished with problems, 1 warning ' + '(with warnings treated as errors).') + elif self.statuscode != 0: + msg = __('build finished with problems, 1 warning.') + else: + msg = __('build succeeded, 1 warning.') + logger.info(bold(msg)) else: - logger.info(bold(__('build %s.')), status) + if self._fail_on_warnings: + self.statuscode = 1 + msg = __('build finished with problems, %s warnings ' + '(with warnings treated as errors).') + elif self.statuscode != 0: + msg = __('build finished with problems, %s warnings.') + else: + msg = __('build succeeded, %s warnings.') + logger.info(bold(msg), self._warncount) if self.statuscode == 0 and self.builder.epilog: logger.info('') diff --git a/sphinx/builders/__init__.py b/sphinx/builders/__init__.py index 3c5313afe41..76e7e230cdc 100644 --- a/sphinx/builders/__init__.py +++ b/sphinx/builders/__init__.py @@ -6,6 +6,7 @@ import pickle import re import time +from contextlib import nullcontext from os import path from typing import TYPE_CHECKING, Any, Literal, final @@ -327,7 +328,7 @@ def build( logger.info(bold(__('building [%s]: ')) + summary, self.name) # while reading, collect all warnings from docutils - with logging.pending_warnings(): + with nullcontext() if self.app._exception_on_warning else logging.pending_warnings(): updated_docnames = set(self.read()) doccount = len(updated_docnames) @@ -627,7 +628,7 @@ def write( self._write_serial(sorted(docnames)) def _write_serial(self, docnames: Sequence[str]) -> None: - with logging.pending_warnings(): + with nullcontext() if self.app._exception_on_warning else logging.pending_warnings(): for docname in status_iterator(docnames, __('writing output... '), "darkgreen", len(docnames), self.app.verbosity): self.app.phase = BuildPhase.RESOLVING diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index 5352b25936b..e9b07164eaa 100644 --- a/sphinx/builders/linkcheck.py +++ b/sphinx/builders/linkcheck.py @@ -106,7 +106,7 @@ def process_result(self, result: CheckResult) -> None: elif result.status == 'working': logger.info(darkgreen('ok ') + result.uri + result.message) elif result.status == 'timeout': - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning('timeout ' + result.uri + result.message, location=(result.docname, result.lineno)) else: @@ -115,7 +115,7 @@ def process_result(self, result: CheckResult) -> None: result.uri + ': ' + result.message) self.timed_out_hyperlinks += 1 elif result.status == 'broken': - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(__('broken link: %s (%s)'), result.uri, result.message, location=(result.docname, result.lineno)) else: diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index 02fd99a8ccc..1773fa670a7 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -201,12 +201,14 @@ def get_parser() -> argparse.ArgumentParser: help=__('write warnings (and errors) to given file')) group.add_argument('--fail-on-warning', '-W', action='store_true', dest='warningiserror', help=__('turn warnings into errors')) - group.add_argument('--keep-going', action='store_true', dest='keep_going', - help=__("with --fail-on-warning, keep going when getting warnings")) + group.add_argument('--keep-going', action='store_true', help=argparse.SUPPRESS) group.add_argument('--show-traceback', '-T', action='store_true', dest='traceback', help=__('show full traceback on exception')) group.add_argument('--pdb', '-P', action='store_true', dest='pdb', help=__('run Pdb on exception')) + group.add_argument('--exception-on-warning', action='store_true', + dest='exception_on_warning', + help=__('raise an exception on warnings')) return parser @@ -329,11 +331,16 @@ def build_main(argv: Sequence[str]) -> int: try: confdir = args.confdir or args.sourcedir with patch_docutils(confdir), docutils_namespace(): - app = Sphinx(args.sourcedir, args.confdir, args.outputdir, - args.doctreedir, args.builder, args.confoverrides, args.status, - args.warning, args.freshenv, args.warningiserror, - args.tags, args.verbosity, args.jobs, args.keep_going, - args.pdb) + app = Sphinx( + srcdir=args.sourcedir, confdir=args.confdir, + outdir=args.outputdir, doctreedir=args.doctreedir, + buildername=args.builder, confoverrides=args.confoverrides, + status=args.status, warning=args.warning, + freshenv=args.freshenv, warningiserror=args.warningiserror, + tags=args.tags, + verbosity=args.verbosity, parallel=args.jobs, keep_going=False, + pdb=args.pdb, exception_on_warning=args.exception_on_warning, + ) app.build(args.force_all, args.filenames) return app.statuscode except (Exception, KeyboardInterrupt) as exc: diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index e318d83a00e..28559e0eccc 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -415,9 +415,10 @@ def import_object(self, raiseerror: bool = False) -> bool: """ with mock(self.config.autodoc_mock_imports): try: - ret = import_object(self.modname, self.objpath, self.objtype, - attrgetter=self.get_attr, - warningiserror=self.config.autodoc_warningiserror) + ret = import_object( + self.modname, self.objpath, self.objtype, + attrgetter=self.get_attr, + ) self.module, self.parent, self.object_name, self.object = ret if ismock(self.object): self.object = undecorate(self.object) @@ -1960,7 +1961,7 @@ def import_object(self, raiseerror: bool = False) -> bool: # annotation only instance variable (PEP-526) try: with mock(self.config.autodoc_mock_imports): - parent = import_module(self.modname, self.config.autodoc_warningiserror) + parent = import_module(self.modname) annotations = get_type_hints(parent, None, self.config.autodoc_type_aliases, include_extras=True) @@ -2455,9 +2456,10 @@ def import_object(self, raiseerror: bool = False) -> bool: except ImportError as exc: try: with mock(self.config.autodoc_mock_imports): - ret = import_object(self.modname, self.objpath[:-1], 'class', - attrgetter=self.get_attr, # type: ignore[attr-defined] - warningiserror=self.config.autodoc_warningiserror) + ret = import_object( + self.modname, self.objpath[:-1], 'class', + attrgetter=self.get_attr, # type: ignore[attr-defined] + ) parent = ret[3] if self.is_runtime_instance_attribute(parent): self.object = self.RUNTIME_INSTANCE_ATTRIBUTE @@ -2509,9 +2511,10 @@ def import_object(self, raiseerror: bool = False) -> bool: return super().import_object(raiseerror=True) # type: ignore[misc] except ImportError as exc: try: - ret = import_object(self.modname, self.objpath[:-1], 'class', - attrgetter=self.get_attr, # type: ignore[attr-defined] - warningiserror=self.config.autodoc_warningiserror) + ret = import_object( + self.modname, self.objpath[:-1], 'class', + attrgetter=self.get_attr, # type: ignore[attr-defined] + ) parent = ret[3] if self.is_uninitialized_instance_attribute(parent): self.object = UNINITIALIZED_ATTR diff --git a/sphinx/ext/autodoc/importer.py b/sphinx/ext/autodoc/importer.py index dd28146b0dd..ebdaa984888 100644 --- a/sphinx/ext/autodoc/importer.py +++ b/sphinx/ext/autodoc/importer.py @@ -137,24 +137,22 @@ def unmangle(subject: Any, name: str) -> str | None: return name -def import_module(modname: str, warningiserror: bool = False) -> Any: +def import_module(modname: str) -> Any: """Call importlib.import_module(modname), convert exceptions to ImportError.""" try: - with logging.skip_warningiserror(not warningiserror): - return importlib.import_module(modname) + return importlib.import_module(modname) except BaseException as exc: # Importing modules may cause any side effects, including # SystemExit, so we need to catch all errors. raise ImportError(exc, traceback.format_exc()) from exc -def _reload_module(module: ModuleType, warningiserror: bool = False) -> Any: +def _reload_module(module: ModuleType) -> Any: """ Call importlib.reload(module), convert exceptions to ImportError """ try: - with logging.skip_warningiserror(not warningiserror): - return importlib.reload(module) + return importlib.reload(module) except BaseException as exc: # Importing modules may cause any side effects, including # SystemExit, so we need to catch all errors. @@ -162,8 +160,7 @@ def _reload_module(module: ModuleType, warningiserror: bool = False) -> Any: def import_object(modname: str, objpath: list[str], objtype: str = '', - attrgetter: Callable[[Any, str], Any] = safe_getattr, - warningiserror: bool = False) -> Any: + attrgetter: Callable[[Any, str], Any] = safe_getattr) -> Any: if objpath: logger.debug('[autodoc] from %s import %s', modname, '.'.join(objpath)) else: @@ -176,7 +173,7 @@ def import_object(modname: str, objpath: list[str], objtype: str = '', while module is None: try: original_module_names = frozenset(sys.modules) - module = import_module(modname, warningiserror=warningiserror) + module = import_module(modname) if os.environ.get('SPHINX_AUTODOC_RELOAD_MODULES'): new_modules = [m for m in sys.modules if m not in original_module_names] # Try reloading modules with ``typing.TYPE_CHECKING == True``. diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py index bed3e5b8a37..b6f31be9b3b 100644 --- a/sphinx/ext/autosummary/generate.py +++ b/sphinx/ext/autosummary/generate.py @@ -71,7 +71,7 @@ def __init__(self, translator: NullTranslations) -> None: self.translator = translator self.verbosity = 0 self._warncount = 0 - self.warningiserror = False + self._exception_on_warning = False self.config.add('autosummary_context', {}, 'env', ()) self.config.add('autosummary_filename_map', {}, 'env', ()) diff --git a/sphinx/ext/coverage.py b/sphinx/ext/coverage.py index 5ecc1670959..c075e954883 100644 --- a/sphinx/ext/coverage.py +++ b/sphinx/ext/coverage.py @@ -241,7 +241,7 @@ def write_c_coverage(self) -> None: for typ, name in sorted(undoc): op.write(' * %-50s [%9s]\n' % (name, typ)) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(__('undocumented c api: %s [%s] in file %s'), name, typ, filename) else: @@ -423,7 +423,7 @@ def write_py_coverage(self) -> None: op.write('Functions:\n') op.writelines(' * %s\n' % x for x in undoc['funcs']) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: for func in undoc['funcs']: logger.warning( __('undocumented python function: %s :: %s'), @@ -440,7 +440,7 @@ def write_py_coverage(self) -> None: if not methods: op.write(' * %s\n' % class_name) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning( __('undocumented python class: %s :: %s'), name, class_name) @@ -452,7 +452,7 @@ def write_py_coverage(self) -> None: op.write(' * %s -- missing methods:\n\n' % class_name) op.writelines(' - %s\n' % x for x in methods) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: for meth in methods: logger.warning( __('undocumented python method:' diff --git a/sphinx/ext/doctest.py b/sphinx/ext/doctest.py index 19f01f4cab6..ba451208a5e 100644 --- a/sphinx/ext/doctest.py +++ b/sphinx/ext/doctest.py @@ -322,7 +322,7 @@ def _out(self, text: str) -> None: self.outfile.write(text) def _warn_out(self, text: str) -> None: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(text) else: logger.info(text, nonl=True) diff --git a/sphinx/testing/fixtures.py b/sphinx/testing/fixtures.py index 7e7811e8907..899fea6b613 100644 --- a/sphinx/testing/fixtures.py +++ b/sphinx/testing/fixtures.py @@ -28,7 +28,7 @@ 'testroot="root", srcdir=None, ' 'confoverrides=None, freshenv=False, ' 'warningiserror=False, tags=None, verbosity=0, parallel=0, ' - 'keep_going=False, builddir=None, docutils_conf=None' + 'builddir=None, docutils_conf=None' '): arguments to initialize the sphinx test application.' ), 'test_params(shared_result=...): test parameters.', diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index f5a750a16ab..54c3a6f66e0 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -117,8 +117,9 @@ def __init__( 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 + pdb: bool = False, + exception_on_warning: bool = False, # unknown keyword arguments **extras: Any, ) -> None: @@ -170,8 +171,8 @@ def __init__( 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, + verbosity=verbosity, parallel=parallel, + pdb=pdb, exception_on_warning=exception_on_warning, ) except Exception: self.cleanup() diff --git a/sphinx/util/logging.py b/sphinx/util/logging.py index 4816bcbbe00..804ef62ccb4 100644 --- a/sphinx/util/logging.py +++ b/sphinx/util/logging.py @@ -5,7 +5,7 @@ import logging import logging.handlers from collections import defaultdict -from contextlib import contextmanager +from contextlib import contextmanager, nullcontext from typing import IO, TYPE_CHECKING, Any from docutils import nodes @@ -17,6 +17,7 @@ if TYPE_CHECKING: from collections.abc import Iterator, Sequence, Set + from typing import NoReturn from docutils.nodes import Node @@ -322,24 +323,7 @@ def pending_logging() -> Iterator[MemoryHandler]: memhandler.flushTo(logger) -@contextmanager -def skip_warningiserror(skip: bool = True) -> Iterator[None]: - """Context manager to skip WarningIsErrorFilter temporarily.""" - logger = logging.getLogger(NAMESPACE) - - if skip is False: - yield - else: - try: - disabler = DisableWarningIsErrorFilter() - for handler in logger.handlers: - # use internal method; filters.insert() directly to install disabler - # before WarningIsErrorFilter - handler.filters.insert(0, disabler) - yield - finally: - for handler in logger.handlers: - handler.removeFilter(disabler) +skip_warningiserror = nullcontext # Deprecate in Sphinx 10 @contextmanager @@ -407,6 +391,21 @@ def filter(self, record: logging.LogRecord) -> bool: return record.levelno < logging.WARNING +class _RaiseOnWarningFilter(logging.Filter): + """Raise exception if a warning is emitted.""" + + def filter(self, record: logging.LogRecord) -> NoReturn: + try: + message = record.msg % record.args + except (TypeError, ValueError): + message = record.msg # use record.msg itself + if location := getattr(record, 'location', ''): + message = f"{location}:{message}" + if record.exc_info is not None: + raise SphinxWarning(message) from record.exc_info[1] + raise SphinxWarning(message) + + def is_suppressed_warning( warning_type: str, sub_type: str, suppress_warnings: Set[str] | Sequence[str], ) -> bool: @@ -445,44 +444,6 @@ def filter(self, record: logging.LogRecord) -> bool: return True -class WarningIsErrorFilter(logging.Filter): - """Raise exception if warning emitted.""" - - def __init__(self, app: Sphinx) -> None: - self.app = app - super().__init__() - - def filter(self, record: logging.LogRecord) -> bool: - if getattr(record, 'skip_warningsiserror', False): - # disabled by DisableWarningIsErrorFilter - return True - elif self.app.warningiserror: - location = getattr(record, 'location', '') - try: - message = record.msg % record.args - except (TypeError, ValueError): - message = record.msg # use record.msg itself - - if location: - exc = SphinxWarning(location + ":" + str(message)) - else: - exc = SphinxWarning(message) - if record.exc_info is not None: - raise exc from record.exc_info[1] - else: - raise exc - else: - return True - - -class DisableWarningIsErrorFilter(logging.Filter): - """Disable WarningIsErrorFilter if this filter installed.""" - - def filter(self, record: logging.LogRecord) -> bool: - record.skip_warningsiserror = True - return True - - class MessagePrefixFilter(logging.Filter): """Prepend prefix to all log records.""" @@ -653,9 +614,10 @@ def setup(app: Sphinx, status: IO, warning: IO) -> None: info_handler.setFormatter(ColorizeFormatter()) warning_handler = WarningStreamHandler(SafeEncodingWriter(warning)) + if app._exception_on_warning: + warning_handler.addFilter(_RaiseOnWarningFilter()) warning_handler.addFilter(WarningSuppressor(app)) warning_handler.addFilter(WarningLogRecordTranslator(app)) - warning_handler.addFilter(WarningIsErrorFilter(app)) warning_handler.addFilter(OnceFilter()) warning_handler.setLevel(logging.WARNING) warning_handler.setFormatter(ColorizeFormatter()) diff --git a/tests/test_builders/test_build_warnings.py b/tests/test_builders/test_build_warnings.py index 7489ed9fd44..c89e33b93d5 100644 --- a/tests/test_builders/test_build_warnings.py +++ b/tests/test_builders/test_build_warnings.py @@ -1,9 +1,11 @@ import os import re import sys +import traceback import pytest +from sphinx.errors import SphinxError from sphinx.util.console import strip_colors ENV_WARNINGS = """\ @@ -71,6 +73,22 @@ def test_html_warnings(app): _check_warnings(warnings_exp, app.warning.getvalue()) +@pytest.mark.sphinx( + 'html', + testroot='warnings', + freshenv=True, + exception_on_warning=True, +) +def test_html_warnings_exception_on_warning(app): + try: + app.build(force_all=True) + pytest.fail('Expected an exception to be raised') + except SphinxError: + tb = traceback.format_exc() + assert 'unindent_warning' in tb + assert 'pending_warnings' not in tb + + @pytest.mark.sphinx( 'latex', testroot='warnings', diff --git a/tests/test_util/test_util_logging.py b/tests/test_util/test_util_logging.py index 0191aca986b..9d3e5023aea 100644 --- a/tests/test_util/test_util_logging.py +++ b/tests/test_util/test_util_logging.py @@ -7,7 +7,6 @@ import pytest from docutils import nodes -from sphinx.errors import SphinxWarning from sphinx.util import logging, osutil from sphinx.util.console import colorize, strip_colors from sphinx.util.logging import is_suppressed_warning, prefixed_warnings @@ -176,25 +175,6 @@ def test_suppress_warnings(app): assert app._warncount == 8 -@pytest.mark.sphinx('html', testroot='root') -def test_warningiserror(app): - logging.setup(app, app.status, app.warning) - logger = logging.getLogger(__name__) - - # if False, warning is not error - app.warningiserror = False - logger.warning('message') - - # if True, warning raises SphinxWarning exception - app.warningiserror = True - with pytest.raises(SphinxWarning): - logger.warning('message: %s', 'arg') - - # message contains format string (refs: #4070) - with pytest.raises(SphinxWarning): - logger.warning('%s') - - @pytest.mark.sphinx('html', testroot='root') def test_info_location(app): logging.setup(app, app.status, app.warning) @@ -356,31 +336,6 @@ def write(self, object): assert app.status.getvalue() == 'unicode ?...\n' -@pytest.mark.sphinx('html', testroot='root') -def test_skip_warningiserror(app): - logging.setup(app, app.status, app.warning) - logger = logging.getLogger(__name__) - - app.warningiserror = True - with logging.skip_warningiserror(): - logger.warning('message') - - # if False, warning raises SphinxWarning exception - with logging.skip_warningiserror(False): # NoQA: SIM117 - with pytest.raises(SphinxWarning): - logger.warning('message') - - # It also works during pending_warnings. - with logging.pending_warnings(): # NoQA: SIM117 - with logging.skip_warningiserror(): - logger.warning('message') - - with pytest.raises(SphinxWarning): # NoQA: PT012,SIM117 - with logging.pending_warnings(): - with logging.skip_warningiserror(False): - logger.warning('message') - - @pytest.mark.sphinx('html', testroot='root') def test_prefixed_warnings(app): logging.setup(app, app.status, app.warning) diff --git a/tox.ini b/tox.ini index b6f1daac1f4..13b43827b39 100644 --- a/tox.ini +++ b/tox.ini @@ -48,7 +48,7 @@ extras = docs commands = python -c "import shutil; shutil.rmtree('./build/sphinx', ignore_errors=True) if '{env:CLEAN:}' else None" - sphinx-build -M {env:BUILDER:html} ./doc ./build/sphinx -nW --keep-going {posargs} + sphinx-build -M {env:BUILDER:html} ./doc ./build/sphinx --fail-on-warning {posargs} [testenv:docs-live] description = From 444cc04444b40fcb8d786e87f223fd755c513ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20B?= <2589111+jfbu@users.noreply.github.com> Date: Tue, 13 Aug 2024 20:37:05 +0200 Subject: [PATCH 2/4] LaTeX: Lift a potential limitation of \sphinxincludegraphics --- sphinx/texinputs/sphinxlatexgraphics.sty | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sphinx/texinputs/sphinxlatexgraphics.sty b/sphinx/texinputs/sphinxlatexgraphics.sty index f30101382c4..f0c7c25f0ca 100644 --- a/sphinx/texinputs/sphinxlatexgraphics.sty +++ b/sphinx/texinputs/sphinxlatexgraphics.sty @@ -1,7 +1,7 @@ %% GRAPHICS % % change this info string if making any custom modification -\ProvidesPackage{sphinxlatexgraphics}[2021/01/27 graphics] +\ProvidesPackage{sphinxlatexgraphics}[2024/08/13 v8.1.0 graphics] % Provides support for this output mark-up from Sphinx latex writer: % @@ -84,7 +84,8 @@ \ifin@ \setbox\spx@image@box \hbox{\includegraphics - [%#1,% contained only width and/or height and overruled anyhow + [#1,% contains only width and/or height which are overruled next + % but in future may contain page=N hence must be kept width=\spx@image@requiredwidth,height=\spx@image@requiredheight]% {#2}}% % \includegraphics does not set box dimensions to the exactly From eb6ff0a25861dce03fb064b144423c220183565a Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 14 Aug 2024 20:45:33 +0100 Subject: [PATCH 3/4] Increase test independence by using the basic theme (#12776) --- sphinx/testing/util.py | 10 ++++ tests/test_builders/test_build_html.py | 60 +++++++++++-------- tests/test_builders/test_build_html_assets.py | 1 - .../test_build_html_highlight.py | 2 +- tests/test_domains/test_domain_std.py | 4 +- tests/test_intl/test_intl.py | 1 + 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index 54c3a6f66e0..9cf77d30439 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -178,6 +178,16 @@ def __init__( self.cleanup() raise + def _init_builder(self) -> None: + # override the default theme to 'basic' rather than 'alabaster' + # for test independence + + if 'html_theme' in self.config._overrides: + pass # respect overrides + elif 'html_theme' in self.config and self.config.html_theme == 'alabaster': + self.config.html_theme = self.config._overrides.get('html_theme', 'basic') + super()._init_builder() + @property def status(self) -> StringIO: """The in-memory text I/O for the application status messages.""" diff --git a/tests/test_builders/test_build_html.py b/tests/test_builders/test_build_html.py index 31fc1981dc5..2c4601849c9 100644 --- a/tests/test_builders/test_build_html.py +++ b/tests/test_builders/test_build_html.py @@ -367,15 +367,12 @@ def test_html_style(app): @pytest.mark.sphinx( 'html', testroot='basic', - # alabaster changed default sidebars in 1.0.0 confoverrides={ 'html_sidebars': { '**': [ - 'about.html', - 'navigation.html', - 'relations.html', - 'searchbox.html', - 'donate.html', + 'localtoc.html', + 'searchfield.html', + 'sourcelink.html', ] } }, @@ -386,45 +383,58 @@ def test_html_sidebar(app): # default for alabaster app.build(force_all=True) result = (app.outdir / 'index.html').read_text(encoding='utf8') + # layout.html assert '