From 648e007ab5e6e05db648b832e2a1f10249b33ead Mon Sep 17 00:00:00 2001
From: Chris Sewell <chrisj_sewell@hotmail.com>
Date: Wed, 28 Aug 2024 12:33:44 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C=20Improve=20need=20warnings=20(#12?=
 =?UTF-8?q?50)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- remove "fix" that is now fixed in sphinx (https://github.com/sphinx-doc/sphinx/commit/6065ab676c08fe37b84bfd0a0146ab08754c8b92)
- add subtype to to warnings
- improve test to check entire warning string
---
 sphinx_needs/warnings.py    | 13 ++------
 tests/test_needs_warning.py | 65 +++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/sphinx_needs/warnings.py b/sphinx_needs/warnings.py
index 94346f3b9..7d010d072 100644
--- a/sphinx_needs/warnings.py
+++ b/sphinx_needs/warnings.py
@@ -81,15 +81,6 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
             else:
                 need_ids = [x["id"] for x in result]
 
-                # Set Sphinx statuscode to 1, only if -W is used with sphinx-build
-                # Because Sphinx statuscode got calculated in very early build phase and based on warning_count
-                # Sphinx-needs warnings check hasn't happened yet
-                # see deatils in https://github.com/sphinx-doc/sphinx/blob/81a4fd973d4cfcb25d01a7b0be62cdb28f82406d/sphinx/application.py#L345
-                # To be clear, app.keep_going = -W and --keep-going, and will overrite -W after
-                # see details in https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/application.py#L182
-                if app.statuscode == 0 and (app.keep_going or app.warningiserror):
-                    app.statuscode = 1
-
                 # get the text for used filter, either from filter string or function name
                 if callable(warning_filter):
                     warning_text = warning_filter.__name__
@@ -105,7 +96,7 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
                             ", ".join(need_ids),
                             warning_text,
                         ),
-                        None,
+                        "warnings",
                         None,
                     )
                 else:
@@ -123,6 +114,6 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
             log_warning(
                 logger,
                 "warnings were raised. See console / log output for details.",
-                None,
+                "warnings",
                 None,
             )
diff --git a/tests/test_needs_warning.py b/tests/test_needs_warning.py
index 866d9bab1..b805b57cd 100644
--- a/tests/test_needs_warning.py
+++ b/tests/test_needs_warning.py
@@ -1,11 +1,20 @@
+import os
 from pathlib import Path
 
 import pytest
+from sphinx import version_info
+from sphinx.util.console import strip_colors
 
 
 @pytest.mark.parametrize(
     "test_app",
-    [{"buildername": "html", "srcdir": "doc_test/doc_needs_warnings"}],
+    [
+        {
+            "buildername": "html",
+            "srcdir": "doc_test/doc_needs_warnings",
+            "no_plantuml": True,
+        }
+    ],
     indirect=True,
 )
 def test_needs_warnings(test_app):
@@ -13,33 +22,33 @@ def test_needs_warnings(test_app):
     app.build()
 
     # stdout warnings
-    warning = app._warning
-    warnings = warning.getvalue()
-
-    # check multiple warning registration
-    assert "'invalid_status' in 'needs_warnings' is already registered." in warnings
-
-    # check warnings contents
-    assert "WARNING: invalid_status: failed" in warnings
-    assert "failed needs: 2 (SP_TOO_001, US_63252)" in warnings
-    assert (
-        "used filter: status not in ['open', 'closed', 'done', 'example_2', 'example_3']"
-        in warnings
-    )
-
-    # check needs warning from custom defined filter code
-    assert "failed needs: 1 (TC_001)" in warnings
-    assert "used filter: my_custom_warning_check" in warnings
-
-    # negative test to check needs warning if need passed the warnings-check
-    assert "TC_NEG_001" not in warnings
-
-    # Check for warning registered via config api
-    assert "WARNING: api_warning_filter: failed" in warnings
-    assert "WARNING: api_warning_func: failed" in warnings
-
-    # Check warnings not including external needs
-    assert "EXT_TEST_01" not in warnings
+    warnings = strip_colors(
+        app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/")
+    ).splitlines()
+
+    expected = [
+        "WARNING: 'invalid_status' in 'needs_warnings' is already registered. [needs.config]",
+        "WARNING: api_warning_filter: failed",
+        "\t\tfailed needs: 1 (TC_002)",
+        "\t\tused filter: status == 'example_2' [needs.warnings]",
+        "WARNING: api_warning_func: failed",
+        "\t\tfailed needs: 1 (TC_003)",
+        "\t\tused filter: custom_warning_func [needs.warnings]",
+        "WARNING: invalid_status: failed",
+        "\t\tfailed needs: 2 (SP_TOO_001, US_63252)",
+        "\t\tused filter: status not in ['open', 'closed', 'done', 'example_2', 'example_3'] [needs.warnings]",
+        "WARNING: type_match: failed",
+        "\t\tfailed needs: 1 (TC_001)",
+        "\t\tused filter: my_custom_warning_check [needs.warnings]",
+    ]
+
+    if version_info >= (7, 3):
+        expected.insert(
+            1,
+            "WARNING: cannot cache unpickable configuration value: 'needs_warnings' (because it contains a function, class, or module object)",
+        )
+
+    assert warnings == expected
 
 
 @pytest.mark.parametrize(