Repository: useblocks/sphinx-needs
Pull request #1105 (2024-02-09 12:17:56)

🧪 Add test for `needreport` directive
Currently there is no test for this directive, this PR adds one.

This PR also fixes the directive:

- Make the options flags
- Change errors in the directive to emit warnings, rather than excepting the whole build
- Allow for `template` to be specified as a directive option
- Allow the the `dropdown` directive used in the default template, which requires an external sphinx extension, to be overriden using `needs_render_context = {"report_directive": "admonition"}` (I left the default as `dropdown`, so as not to introduce a breaking change)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) Report
Attention: `4 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`73b961e`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/73b961e29583bc0ac8892ef9c4e2bfb75f7f46bb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) 83.19% compared to head [(`6136432`)](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) 83.81%.

| [Files](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | Patch % | Lines |
|---|---|---|
| [sphinx\_needs/directives/needreport.py](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#diff-c3BoaW54X25lZWRzL2RpcmVjdGl2ZXMvbmVlZHJlcG9ydC5weQ==) | 33.33% | [4 Missing :warning: ](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
+ Coverage   83.19%   83.81%   +0.62%     
==========================================
  Files          56       56              
  Lines        6456     6439      -17     
==========================================
+ Hits         5371     5397      +26     
+ Misses       1085     1042      -43     
```

| [Flag](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | Coverage Δ | |
|---|---|---|
| [pytests](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | `83.81% <33.33%> (+0.62%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1105?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).

chrisjsewell: I've also just realised there is a bug in this directive:
The `usage` flag creates a count of user defined need types.
However, it does this during the processing of the directive, rather than in a post-processing step.
Therefore, the count will usually be incorrect.

Patch:
diff --git a/docs/conf.py b/docs/conf.py
index ea524939..19e60147 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -82,43 +82,11 @@ NOTE_TEMPLATE = """
    {% endif %}
 """
 
-EXTRA_CONTENT_TEMPLATE_COLLAPSE = """
-.. _{{id}}:
-
-{% if hide == false -%}
-.. role:: needs_tag
-.. role:: needs_status
-.. role:: needs_type
-.. role:: needs_id
-.. role:: needs_title
-
-.. rst-class:: need
-.. rst-class:: need_{{type_name}}
-
-.. dropdown::
-   :class: need
-
-            :needs_type:`{{type_name}}`: {% if title %}:needs_title:`{{title}}`{% endif %} :needs_id:`{{id}}`
-
-{% if status and  status|upper != "NONE" and not hide_status %}        | status: :needs_status:`{{status}}`{% endif %}
-{% if tags and not hide_tags %}        | tags: :needs_tag:`{{tags|join("` :needs_tag:`")}}`{% endif %}
-{% if my_extra_option != "" %}        | my_extra_option: {{ my_extra_option }}{% endif %}
-{% if another_option != "" %}        | another_option: {{ another_option }}{% endif %}
-        | links incoming: :need_incoming:`{{id}}`
-        | links outgoing: :need_outgoing:`{{id}}`
-
-    {{content|indent(4) }}
-
-{% endif -%}
-"""
-
 DEFAULT_DIAGRAM_TEMPLATE = (
     "<size:12>{{type_name}}</size>\\n**{{title|wordwrap(15, wrapstring='**\\\\n**')}}**\\n<size:10>{{id}}</size>"
 )
 
 # You can uncomment some of the following lines to override the default configuration for Sphinx-Needs.
-
-# needs_template = TITLE_TEMPLATE
 # needs_diagram_template = DEFAULT_DIAGRAM_TEMPLATE
 
 # Absolute path to the needs_report_template_file based on the conf.py directory
@@ -251,7 +219,6 @@ plantuml_output_format = "svg_img"
 needs_table_style = "datatables"
 needs_table_columns = "ID;TITLE;STATUS;OUTGOING"
 
-needs_template_collapse = EXTRA_CONTENT_TEMPLATE_COLLAPSE
 needs_extra_options = [
     "my_extra_option",
     "another_option",
diff --git a/docs/directives/needreport.rst b/docs/directives/needreport.rst
index 97c5c433..a5e02e21 100644
--- a/docs/directives/needreport.rst
+++ b/docs/directives/needreport.rst
@@ -21,6 +21,16 @@ The :ref:`needs_report_template` value is a path to the
 `jinja2 <https://jinja.palletsprojects.com/en/2.11.x/templates/>`_ template file.
 You can use the template file to customise the content generated  by ``needreport``.
 
+.. note::
+
+   The default needs report template is set to use ``dropdown`` directives for containing each configuration type, which requires the ``dropdown`` directive to be available in your Sphinx environment. If you do not have the ``dropdown`` directive available, you can use the following configuration to set the default needs report template to use ``admonition`` directives instead:
+
+   .. code-block:: python
+
+      needs_render_context = {
+         "report_directive": "admonition",
+      }
+
 |ex|
 
 .. code-block:: rst
diff --git a/sphinx_needs/directives/needreport.py b/sphinx_needs/directives/needreport.py
index 1f0532ff..b72f7523 100644
--- a/sphinx_needs/directives/needreport.py
+++ b/sphinx_needs/directives/needreport.py
@@ -1,84 +1,71 @@
-import os
+from pathlib import Path
 from typing import Sequence
 
 from docutils import nodes
 from docutils.parsers.rst import directives
 from jinja2 import Template
-from sphinx.errors import SphinxError
+from sphinx.util import logging
 from sphinx.util.docutils import SphinxDirective
 
 from sphinx_needs.config import NeedsSphinxConfig
 from sphinx_needs.directives.utils import analyse_needs_metrics
 from sphinx_needs.utils import add_doc
 
-
-class NeedsReportException(SphinxError):
-    pass
+LOGGER = logging.getLogger(__name__)
 
 
 class NeedReportDirective(SphinxDirective):
     final_argument_whitespace = True
     option_spec = {
-        "types": directives.unchanged,
-        "links": directives.unchanged,
-        "options": directives.unchanged,
-        "usage": directives.unchanged,
+        "types": directives.flag,
+        "links": directives.flag,
+        "options": directives.flag,
+        "usage": directives.flag,
+        "template": directives.unchanged,
     }
 
     def run(self) -> Sequence[nodes.raw]:
         env = self.env
         needs_config = NeedsSphinxConfig(env.config)
 
-        if len(self.options.keys()) == 0:  # Check if options is empty
-            error_file, error_line = self.state_machine.input_lines.items[0]
-            error_msg = "{}:{}: NeedReportError: No options specified to generate need report.".format(
-                error_file, error_line + self.state_machine.input_lines.data.index(".. needreport::") + 1
+        if not set(self.options).intersection({"types", "links", "options", "usage"}):
+            LOGGER.warning(
+                "No options specified to generate need report [needs.report]",
+                location=self.get_location(),
+                type="needs",
+                subtype="report",
             )
-            raise NeedsReportException(error_msg)
-
-        types = self.options.get("types")
-        extra_links = self.options.get("links")
-        extra_options = self.options.get("options")
-        usage = self.options.get("usage")
-
-        needs_types = []
-        needs_extra_links = []
-        needs_extra_options = []
-        needs_metrics = {}
-
-        if types is not None and isinstance(types, str):
-            needs_types = needs_config.types
-        if extra_links is not None and isinstance(extra_links, str):
-            needs_extra_links = needs_config.extra_links
-        if extra_options is not None and isinstance(extra_options, str):
-            needs_extra_options = needs_config.extra_options
-        if usage is not None and isinstance(usage, str):
-            needs_metrics = analyse_needs_metrics(env)
+            return []
 
         report_info = {
-            "types": needs_types,
-            "options": needs_extra_options,
-            "links": needs_extra_links,
-            "usage": needs_metrics,
+            "types": needs_config.types if "types" in self.options else [],
+            "options": needs_config.extra_options if "options" in self.options else [],
+            "links": needs_config.extra_links if "links" in self.options else [],
+            "usage": analyse_needs_metrics(env) if "usage" in self.options else {},
+            "report_directive": "dropdown",
         }
         report_info.update(**needs_config.render_context)
 
-        need_report_template_path: str = needs_config.report_template
-        # Absolute path starts with /, based on the conf.py directory. The / need to be striped
-        correct_need_report_template_path = os.path.join(env.app.srcdir, need_report_template_path.lstrip("/"))
-
-        if len(need_report_template_path) == 0:
-            default_template_path = "needreport_template.rst"
-            correct_need_report_template_path = os.path.join(os.path.dirname(__file__), default_template_path)
-
-        if not os.path.exists(correct_need_report_template_path):
-            raise ReferenceError(f"Could not load needs report template file {correct_need_report_template_path}")
+        if "template" in self.options:
+            need_report_template_path = Path(self.env.relfn2path(self.options["template"], self.env.docname)[1])
+        elif needs_config.report_template:
+            # Absolute path starts with /, based on the conf.py directory. The / need to be striped
+            need_report_template_path = Path(str(env.app.srcdir)) / needs_config.report_template.lstrip("/")
+        else:
+            need_report_template_path = Path(__file__).parent / "needreport_template.rst"
+
+        if not need_report_template_path.is_file():
+            LOGGER.warning(
+                f"Could not load needs report template file {need_report_template_path} [needs.report]",
+                location=self.get_location(),
+                type="needs",
+                subtype="report",
+            )
+            return []
 
-        with open(correct_need_report_template_path) as needs_report_template_file:
-            needs_report_template_file_content = needs_report_template_file.read()
+        needs_report_template_file_content = need_report_template_path.read_text(encoding="utf8")
 
         template = Template(needs_report_template_file_content, autoescape=True)
-
         text = template.render(**report_info)
         self.state_machine.insert_input(text.split("\n"), self.state_machine.document.attributes["source"])
 
diff --git a/sphinx_needs/directives/needreport_template.rst b/sphinx_needs/directives/needreport_template.rst
index 2b34eccf..38b14140 100644
--- a/sphinx_needs/directives/needreport_template.rst
+++ b/sphinx_needs/directives/needreport_template.rst
@@ -1,7 +1,7 @@
 {# Output for needs_types #}
 {% if types|length != 0 %}
 
-.. dropdown:: Need Types
+.. {{ report_directive }}:: Need Types
 
    .. list-table::
       :widths: 40 20 20 20
@@ -23,7 +23,7 @@
 {# Output for needs_extra_links #}
 {% if links|length != 0 %}
 
-.. dropdown:: Need Extra Links
+.. {{ report_directive }}:: Need Extra Links
 
    .. list-table::
       :widths: 10 30 30 5 20
@@ -47,7 +47,7 @@
 {# Output for needs_options #}
 {% if options|length != 0 %}
 
-.. dropdown:: Need Extra Options
+.. {{ report_directive }}:: Need Extra Options
 
    {% for option in options %}
    * {{ option }}
@@ -58,7 +58,7 @@
 {# Output for needs metrics #}
 {% if usage|length != 0 %}
 
-.. dropdown:: Need Metrics
+.. {{ report_directive }}:: Need Metrics
 
    .. list-table::
       :widths: 40 40


Test patch:
diff --git a/tests/doc_test/doc_needreport/conf.py b/tests/doc_test/doc_needreport/conf.py
new file mode 100644
index 00000000..b5a51194
--- /dev/null
+++ b/tests/doc_test/doc_needreport/conf.py
@@ -0,0 +1,5 @@
+extensions = ["sphinx_needs"]
+needs_extra_options = ["other"]
+needs_render_context = {
+    "report_directive": "admonition",
+}
diff --git a/tests/doc_test/doc_needreport/index.rst b/tests/doc_test/doc_needreport/index.rst
new file mode 100644
index 00000000..6c5cdeab
--- /dev/null
+++ b/tests/doc_test/doc_needreport/index.rst
@@ -0,0 +1,17 @@
+Needs Report
+============
+
+.. req:: Requirement 1
+
+.. needreport::
+
+.. needreport::
+      :types:
+      :template: unknown.rst
+
+.. needreport::
+      :types:
+      :links:
+      :options:
+      :usage:
+
diff --git a/tests/test_needreport.py b/tests/test_needreport.py
new file mode 100644
index 00000000..73c474fe
--- /dev/null
+++ b/tests/test_needreport.py
@@ -0,0 +1,18 @@
+from pathlib import Path
+
+import pytest
+
+
+@pytest.mark.parametrize("test_app", [{"buildername": "html", "srcdir": "doc_test/doc_needreport"}], indirect=True)
+def test_doc_needarch(test_app):
+    app = test_app
+    app.build()
+    # check for warning about missing options
+    warnings = app._warning.getvalue()
+    assert "index.rst:6: WARNING: No options specified to generate need report [needs.report]" in warnings
+    assert "index.rst:8: WARNING: Could not load needs report template file" in warnings
+    html = Path(app.outdir, "index.html").read_text(encoding="utf8")
+    assert "Need Types" in html
+    assert "Need Extra Links" in html
+    assert "Need Extra Options" in html
+    assert "Need Metrics" in html