Validate jira ticket reason for manifest declarations in the parser to improve error clarity

tests/test_the_test/manifests/manifests_agent_edit/agent.yml

@@ -1,5 +1,5 @@
 ---
 manifest:
   tests/otel_tracing_e2e/test_e2e.py::Test_OTelLogE2E: "v7.48.0"
-  tests/test_sampling_rates.py::Test_SamplingRates: bug
+  tests/test_sampling_rates.py::Test_SamplingRates: bug (TEST-123)
   tests/test_telemetry.py::Test_APMOnboardingInstallID: "v7.50.0"

tests/test_the_test/test_decorators.py

@@ -4,6 +4,8 @@
 import pytest
 
 from utils import irrelevant, missing_feature, flaky, rfc, logger
+from utils._decorators import add_pytest_marker
+from utils.manifest import TestDeclaration
 
 
 pytestmark = pytest.mark.scenario("TEST_THE_TEST")
@@ -74,6 +76,13 @@ def test_regular(self):
         assert is_not_skipped(Test_Class)
         assert is_not_skipped(Test_Class.test_good_method)
 
+    def test_invalid_bug_reason(self) -> None:
+        def test_invalid() -> None:
+            pass
+
+        with pytest.raises(pytest.exit.Exception, match="Please set a jira ticket"):
+            add_pytest_marker(test_invalid, TestDeclaration.BUG, "APPSEC-123 & APPSEC-456")
+
 
 if __name__ == "__main__":
     sys.exit("Usage: pytest utils/test_the_test.py")

tests/test_the_test/test_easy_win.py

@@ -623,7 +623,7 @@ def test_manifest_editor_add_condition_to_function_level_inline_declaration():
         manifest_content = """---
 manifest:
   tests/appsec/test_feature.py::Test_Feature: missing_feature
-  tests/appsec/test_feature.py::Test_Feature::test_method2: bug (XXXX)
+  tests/appsec/test_feature.py::Test_Feature::test_method2: bug (TEST-123)
 """
         (manifest_dir / "python.yml").write_text(manifest_content)
 
@@ -637,7 +637,7 @@ def test_manifest_editor_add_condition_to_function_level_inline_declaration():
             result = yaml.safe_load(f)
 
         rule = result["manifest"].get("tests/appsec/test_feature.py::Test_Feature::test_method2")
-        assert rule == [{"declaration": "bug (XXXX)"}, {"weblog_declaration": {"flask": "missing_feature"}}]
+        assert rule == [{"declaration": "bug (TEST-123)"}, {"weblog_declaration": {"flask": "missing_feature"}}]
 
 
 # =============================================================================

tests/test_the_test/test_manifest.py

@@ -6,6 +6,7 @@
 from utils import scenarios
 from utils._context.component_version import Version
 from utils.manifest import Manifest, SkipDeclaration, TestDeclaration
+from utils.manifest._internal.declaration import Declaration
 from utils.manifest._internal.types import ManifestData, SemverRange as CustomSpec, Condition
 from utils.manifest._internal.validate import assert_nodeids_exist
 from utils.scripts.activate_easy_wins._internal.manifest_editor import ManifestEditor
@@ -32,6 +33,33 @@ class TestManifest:
     def test_formats(self):
         Manifest.validate()
 
+    @pytest.mark.parametrize(
+        "raw_declaration",
+        [
+            "bug (APPSEC-123)",
+            "bug (APPSEC-123, APPSEC-456)",
+            "flaky (APPSEC-123)",
+            "flaky (APPSEC-123, APPSEC-456)",
+        ],
+    )
+    def test_bug_and_flaky_declarations_with_jira_reasons(self, raw_declaration: str) -> None:
+        Declaration(raw_declaration, "python", is_inline=True)
+
+    @pytest.mark.parametrize(
+        "raw_declaration",
+        [
+            "bug",
+            "bug (APPSEC-123 & APPSEC-456)",
+            "bug (arbitrary reason)",
+            "flaky",
+            "flaky (APPSEC-123 & APPSEC-456)",
+            "flaky (arbitrary reason)",
+        ],
+    )
+    def test_bug_and_flaky_declarations_without_jira_reasons(self, raw_declaration: str) -> None:
+        with pytest.raises(ValueError, match="Please set a jira ticket"):
+            Declaration(raw_declaration, "python", is_inline=True)
+
     def test_parser(self):
         manifest = Manifest.parse(Path("tests/test_the_test/manifests/manifests_parser_test/"))
         assert manifest == {

utils/_decorators.py

@@ -1,34 +1,33 @@
 import inspect
 import os
-import re
 from functools import partial
 from types import FunctionType, MethodType
 from typing import Any
 
 import pytest
 
-from utils.manifest import TestDeclaration
+from utils.manifest import TestDeclaration, validate_declaration_reason
 from utils._context.core import context
 
 
-_jira_ticket_pattern = re.compile(r"([A-Z]{3,}-\d+)(, [A-Z]{3,}-\d+)*")
-
-
 def configure(config: pytest.Config):
     pass  # nothing to do right now
 
 
 _MANIFEST_ERROR_MESSAGE = "Please use manifest file, See docs/edit/manifest.md"
 
 
-def _is_jira_ticket(declaration_details: str | None) -> bool:
-    return declaration_details is not None and _jira_ticket_pattern.fullmatch(declaration_details) is not None
-
-
-def _ensure_jira_ticket_as_reason(item: type[Any] | FunctionType | MethodType, declaration_details: str | None) -> None:
+def _ensure_valid_declaration_reason(
+    item: type[Any] | FunctionType | MethodType,
+    declaration: TestDeclaration,
+    declaration_details: str | None,
+) -> None:
     if isinstance(item, pytest.Function):
         item = item.function
-    if not _is_jira_ticket(declaration_details):
+
+    try:
+        validate_declaration_reason(declaration, declaration_details)
+    except ValueError:
         path = inspect.getfile(item)
         rel_path = os.path.relpath(path)
         nodeid = f"{rel_path}::{item.__name__ if inspect.isclass(item) else item.__qualname__}"
@@ -49,8 +48,7 @@ def add_pytest_marker(
     ):
         raise ValueError(f"Unexpected skipped object: {item}")
 
-    if declaration in (TestDeclaration.BUG, TestDeclaration.FLAKY):
-        _ensure_jira_ticket_as_reason(item, declaration_details)
+    _ensure_valid_declaration_reason(item, declaration, declaration_details)
 
     if declaration in (TestDeclaration.IRRELEVANT, TestDeclaration.FLAKY):
         marker = pytest.mark.skip

utils/manifest/__init__.py

@@ -1,10 +1,11 @@
 from ._internal import Manifest
-from ._internal import SkipDeclaration, Condition, ManifestData, TestDeclaration
+from ._internal import SkipDeclaration, Condition, ManifestData, TestDeclaration, validate_declaration_reason
 
 __all__ = [
     "Condition",
     "Manifest",
     "ManifestData",
     "SkipDeclaration",
     "TestDeclaration",
+    "validate_declaration_reason",
 ]

utils/manifest/_internal/__init__.py

@@ -1,5 +1,6 @@
 from .const import TestDeclaration
 from .core import Manifest
+from .declaration import validate_declaration_reason
 from .types import SkipDeclaration, Condition, ManifestData
 
 __all__ = [
@@ -8,4 +9,5 @@
     "ManifestData",
     "SkipDeclaration",
     "TestDeclaration",
+    "validate_declaration_reason",
 ]

utils/manifest/_internal/declaration.py

@@ -1,7 +1,20 @@
+import re
+
 from utils._context.component_version import ComponentVersion
+
 from .const import skip_declaration_regex, full_regex, TestDeclaration
 from .types import SemverRange
-import re
+
+
+_jira_ticket_pattern = re.compile(r"([A-Z]{3,}-\d+)(, [A-Z]{3,}-\d+)*")
+
+
+def validate_declaration_reason(declaration: TestDeclaration, reason: str | None) -> None:
+    if declaration not in (TestDeclaration.BUG, TestDeclaration.FLAKY):
+        return
+
+    if reason is None or _jira_ticket_pattern.fullmatch(reason) is None:
+        raise ValueError(f"Please set a jira ticket instead of reason: {reason}")
 
 
 class Declaration:
@@ -41,10 +54,8 @@ def parse_declaration(self) -> None:
         elements = re.fullmatch(skip_declaration_regex, self.raw, re.ASCII)
         if elements:
             self.is_skip = True
-            skip_declaration = _parse_skip_declaration(self.raw)
-            self.value = skip_declaration[0]
-            if elements[1]:
-                self.reason = skip_declaration[1]
+            self.value, self.reason = _parse_skip_declaration(self.raw)
+            validate_declaration_reason(self.value, self.reason)
             return
         if not self.is_inline:
             raise ValueError(f"Wrong declaration format: {self.raw} (is inline: {self.is_inline})")