Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Default to warning for missing needextend ID #1066

Merged
merged 3 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions docs/directives/needextend.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand Down
2 changes: 1 addition & 1 deletion sphinx_needs/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": ()}
Expand Down
52 changes: 37 additions & 15 deletions sphinx_needs/directives/needextend.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,11 @@
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()
Expand Down Expand Up @@ -91,19 +89,43 @@
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(

Check warning on line 119 in sphinx_needs/directives/needextend.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needextend.py#L118-L119

Added lines #L118 - L119 were not covered by tests
f"Invalid filter {need_filter!r}: {e} [needs.extend]",
type="needs",
subtype="extend",
location=(
current_needextend["docname"],
current_needextend["lineno"],
),
)
continue

Check warning on line 128 in sphinx_needs/directives/needextend.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needextend.py#L128

Added line #L128 was not covered by tests

for found_need in found_needs:
# Work in the stored needs, not on the search result
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
needextend strict-mode disabled
===============================
needextend unknown id
=====================

.. story:: needextend Example 3
:id: extend_test_003
Expand All @@ -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
43 changes: 14 additions & 29 deletions tests/test_needextend.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_needs_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading