From d405fd18b9c4489f91f5e4264f0a1b4b34f1d1e9 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 8 May 2024 13:33:43 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Default=20to=20warning=20f?= =?UTF-8?q?or=20missing=20needextend=20ID=20(#1066)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than defaulting to "strict" mode for unknown IDs in the `needextend` directives, whereby the entire build is excepted, The default is now changed to "non-strict" mode, and this emits a warning rather than an info log. --- docs/directives/needextend.rst | 8 ++- sphinx_needs/config.py | 2 +- sphinx_needs/directives/needextend.py | 52 +++++++++++++------ .../conf.py | 0 .../index.rst | 10 ++-- tests/test_needextend.py | 43 +++++---------- tests/test_needs_builder.py | 2 +- 7 files changed, 59 insertions(+), 58 deletions(-) rename tests/doc_test/{doc_needextend_strict => doc_needextend_unknown_id}/conf.py (100%) rename tests/doc_test/{doc_needextend_strict => doc_needextend_unknown_id}/index.rst (65%) diff --git a/docs/directives/needextend.rst b/docs/directives/needextend.rst index 9b7b89e09..bc606e299 100644 --- a/docs/directives/needextend.rst +++ b/docs/directives/needextend.rst @@ -81,22 +81,20 @@ Options strict ~~~~~~ -The purpose of the ``:strict:`` option is to handle whether an exception gets thrown -or a log-info message gets written if there is no need object to match the ``needextend's`` -required argument (e.g. an ID). +The purpose of the ``:strict:`` option is to handle whether an exception gets thrown or a warning log message gets written, if there is no need object to match the ``needextend's`` required argument (e.g. an ID). If you set the ``:strict:`` option value to ``true``, ``needextend`` raises an exception because the given ID does not exist, and the build stops. If you set the ``:strict:`` option value to ``false``, -``needextend`` logs an info-level message in the console, and the build continues. +``needextend`` logs an warning-level message in the console, and the build continues. Allowed values: * true or * false -Default: true +Default: false .. note:: diff --git a/sphinx_needs/config.py b/sphinx_needs/config.py index e401afcc1..1a71f6ab8 100644 --- a/sphinx_needs/config.py +++ b/sphinx_needs/config.py @@ -289,7 +289,7 @@ def __setattr__(self, name: str, value: Any) -> None: ) """Added to options on need directives, and key on need data items, for use by ``needgantt``""" needextend_strict: bool = field( - default=True, metadata={"rebuild": "html", "types": (bool,)} + default=False, metadata={"rebuild": "html", "types": (bool,)} ) statuses: list[dict[str, str]] = field( default_factory=list, metadata={"rebuild": "html", "types": ()} diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index b123da3c6..7c61ddfd4 100644 --- a/sphinx_needs/directives/needextend.py +++ b/sphinx_needs/directives/needextend.py @@ -48,13 +48,11 @@ def run(self) -> Sequence[nodes.Node]: f"Filter of needextend must be set. See {env.docname}:{self.lineno}" ) - strict_option = self.options.get( - "strict", str(NeedsSphinxConfig(self.env.app.config).needextend_strict) - ) - strict = True - if strict_option.upper() == "TRUE": + strict = NeedsSphinxConfig(self.env.app.config).needextend_strict + strict_option: str = self.options.get("strict", "").upper() + if strict_option == "TRUE": strict = True - elif strict_option.upper() == "FALSE": + elif strict_option == "FALSE": strict = False data = SphinxNeedsData(env).get_or_create_extends() @@ -91,19 +89,43 @@ def extend_needs_data( needs_config.id_regex, need_filter ): # an unknown ID - error = f"Provided id {need_filter} for needextend does not exist." + error = f"Provided id {need_filter!r} for needextend does not exist. [needs.extend]" if current_needextend["strict"]: raise NeedsInvalidFilter(error) else: - logger.info(error) - continue + logger.warning( + error, + type="needs", + subtype="extend", + location=( + current_needextend["docname"], + current_needextend["lineno"], + ), + ) + continue else: - found_needs = filter_needs( - all_needs.values(), - needs_config, - need_filter, - location=(current_needextend["docname"], current_needextend["lineno"]), - ) + # a filter string + try: + found_needs = filter_needs( + all_needs.values(), + needs_config, + need_filter, + location=( + current_needextend["docname"], + current_needextend["lineno"], + ), + ) + except NeedsInvalidFilter as e: + logger.warning( + f"Invalid filter {need_filter!r}: {e} [needs.extend]", + type="needs", + subtype="extend", + location=( + current_needextend["docname"], + current_needextend["lineno"], + ), + ) + continue for found_need in found_needs: # Work in the stored needs, not on the search result diff --git a/tests/doc_test/doc_needextend_strict/conf.py b/tests/doc_test/doc_needextend_unknown_id/conf.py similarity index 100% rename from tests/doc_test/doc_needextend_strict/conf.py rename to tests/doc_test/doc_needextend_unknown_id/conf.py diff --git a/tests/doc_test/doc_needextend_strict/index.rst b/tests/doc_test/doc_needextend_unknown_id/index.rst similarity index 65% rename from tests/doc_test/doc_needextend_strict/index.rst rename to tests/doc_test/doc_needextend_unknown_id/index.rst index f67f0f1dc..9e87e0279 100644 --- a/tests/doc_test/doc_needextend_strict/index.rst +++ b/tests/doc_test/doc_needextend_unknown_id/index.rst @@ -1,5 +1,5 @@ -needextend strict-mode disabled -=============================== +needextend unknown id +===================== .. story:: needextend Example 3 :id: extend_test_003 @@ -16,9 +16,5 @@ needextend strict-mode disabled .. needextend:: extend_test_003 :links: extend_test_004 -.. needextend:: strict_disable_extend_test +.. needextend:: unknown_id :status: open - :strict: false - -.. needextend:: strict_enable_extend_test - :status: closed diff --git a/tests/test_needextend.py b/tests/test_needextend.py index 266655c1d..8f5bde96b 100644 --- a/tests/test_needextend.py +++ b/tests/test_needextend.py @@ -1,9 +1,9 @@ import json -import sys from pathlib import Path import pytest from sphinx.application import Sphinx +from sphinx.util.console import strip_colors from syrupy.filters import props @@ -45,38 +45,23 @@ def test_doc_needextend_html(test_app: Sphinx, snapshot): @pytest.mark.parametrize( "test_app", - [{"buildername": "html", "srcdir": "doc_test/doc_needextend_strict"}], + [ + { + "buildername": "html", + "srcdir": "doc_test/doc_needextend_unknown_id", + "no_plantuml": True, + } + ], indirect=True, ) -def test_doc_needextend_strict(test_app): - import os - import subprocess - +def test_doc_needextend_unknown_id(test_app: Sphinx): app = test_app + app.build() - srcdir = Path(app.srcdir) - out_dir = os.path.join(srcdir, "_build") - - out = subprocess.run( - ["sphinx-build", "-b", "html", srcdir, out_dir], capture_output=True - ) - - # Strict option is set to false on needextend. Log info-level message - assert ( - "Provided id strict_disable_extend_test for needextend does not exist." - in out.stdout.decode("utf-8") - ) - # Strict option is set to true on needextend. Raise Exception - if sys.platform == "win32": - assert ( - "Sphinx error:\r\nProvided id strict_enable_extend_test for needextend does not exist." - in out.stderr.decode("utf-8") - ) - else: - assert ( - "Sphinx error:\nProvided id strict_enable_extend_test for needextend does not exist." - in out.stderr.decode("utf-8") - ) + warnings = strip_colors(app._warning.getvalue()).splitlines() + assert warnings == [ + f"{Path(str(app.srcdir)) / 'index.rst'}:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.extend]" + ] @pytest.mark.parametrize( diff --git a/tests/test_needs_builder.py b/tests/test_needs_builder.py index a53b568c8..7560dd4ef 100644 --- a/tests/test_needs_builder.py +++ b/tests/test_needs_builder.py @@ -79,7 +79,7 @@ def test_needs_html_and_json(test_app): build_dir = os.path.join(app.outdir, "../needs") print(build_dir) output = subprocess.run( - ["python", "-m", "sphinx.cmd.build", "-b", "needs", srcdir, build_dir], + ["sphinx-build", "-b", "needs", srcdir, build_dir], capture_output=True, ) print(output)