fix: CheckLogger bug + increase test coverage (#587)

Author: MaximilianSoerenPollak

src/extensions/score_metamodel/checks/graph_checks.py

@@ -107,37 +107,48 @@ def filter_needs_by_criteria(
     needs_selection_criteria: dict[str, str],
     log: CheckLogger,
 ) -> list[NeedItem]:
-    """Create a list of needs that match the selection criteria.:
-    - If it is an include selection add the include to the pattern
-    - If it is an exclude selection add a "^" to the pattern
     """
+    Filter needs by include/exclude type patterns and an additional condition.
 
+    The function:
+    - accepts exactly one selector key: "include" or "exclude"
+    - validates that "condition" exists
+    - logs warnings for unknown need types in selector patterns
+    - returns needs matching selector + condition
+    """
     selected_needs: list[NeedItem] = []
-    pattern: list[str] = []
-    need_pattern: str = list(needs_selection_criteria.keys())[0]
-    # Verify Inputs
-    if need_pattern in ["include", "exclude"]:
-        for pat in list(needs_selection_criteria.values())[0].split(","):
-            pattern.append(pat.lstrip())
+
+    if "include" in needs_selection_criteria and "exclude" in needs_selection_criteria:
+        raise ValueError(
+            f"Invalid need selection: both include and exclude are set: {needs_selection_criteria}"
+        )
+
+    if "include" in needs_selection_criteria:
+        need_pattern = "include"
+        raw_patterns = needs_selection_criteria["include"]
+    elif "exclude" in needs_selection_criteria:
+        need_pattern = "exclude"
+        raw_patterns = needs_selection_criteria["exclude"]
     else:
         raise ValueError(f"Invalid need selection: {needs_selection_criteria}")
 
-    if "condition" in needs_selection_criteria:
-        condition = needs_selection_criteria["condition"]
-    else:
+    if "condition" not in needs_selection_criteria:
         raise ValueError(f"Invalid selection: {needs_selection_criteria}")
 
+    condition = needs_selection_criteria["condition"]
+    pattern = [pat.strip() for pat in raw_patterns.split(",") if pat.strip()]
+
     for pat in pattern:
         if not any(need_type["directive"] == pat for need_type in needs_types):
             log.warning(f"Unknown need type `{pat}` in graph check.", location="")
 
     for need in needs:
-        if need_pattern == "include":
-            sel = need["type"] in pattern
-        else:
-            sel = need["type"] not in pattern
-
-        if sel and (eval_need_condition(need, condition, log)):
+        sel = (
+            need["type"] in pattern
+            if need_pattern == "include"
+            else need["type"] not in pattern
+        )
+        if sel and eval_need_condition(need, condition, log):
             selected_needs.append(need)
 
     return selected_needs

src/extensions/score_metamodel/log.py

@@ -91,7 +91,6 @@ def _log_message(
             self._info_count += 1
         else:
             self.warning(msg, location)
-            self._warning_count += 1
 
     def info(
         self,
@@ -106,6 +105,7 @@ def warning(
         location: Location,
     ):
         self._log.warning(msg, type="score_metamodel", location=location)
+        self._warning_count += 1
 
     @property
     def warnings(self):

src/extensions/score_metamodel/tests/test_graph_checks.py

@@ -0,0 +1,196 @@
+# *******************************************************************************
+# Copyright (c) 2026 Contributors to the Eclipse Foundation
+#
+# See the NOTICE file(s) distributed with this work for additional
+# information regarding copyright ownership.
+#
+# This program and the accompanying materials are made available under the
+# terms of the Apache License Version 2.0 which is available at
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+
+from __future__ import annotations
+
+from typing import Any
+
+import pytest
+
+# Adjust this import path if your project layout differs.
+import score_metamodel.checks.graph_checks as graph_checks
+from score_metamodel.tests import fake_check_logger, need as test_need
+from sphinx_needs.config import NeedType
+
+
+class DummyNeedsView:
+    """Minimal NeedsView-like test double."""
+
+    def __init__(self, needs: list[dict[str, Any]]) -> None:
+        """Create a view over needs represented as dict-like structures."""
+        self._needs = needs
+
+    def values(self) -> list[dict[str, Any]]:
+        """Return all needs."""
+        return self._needs
+
+    def filter_is_external(self, is_external: bool) -> DummyNeedsView:
+        """Filter needs by their is_external flag."""
+        return DummyNeedsView(
+            [n for n in self._needs if n.get("is_external", False) == is_external]
+        )
+
+
+class NeedObject(dict):
+    """Need object that supports both mapping and attribute style access used by code."""
+
+    @property
+    def id(self) -> str:
+        """Return need identifier."""
+        return self["id"]
+
+    @property
+    def _links(self) -> dict[str, list[NeedObject]]:
+        """Return links map."""
+        return self["links"]
+
+
+def test_eval_need_check_invalid_check_parts_raises_value_error() -> None:
+    """Raise error when a check does not contain exactly three parts."""
+    log = fake_check_logger()
+    need = test_need(status="valid")
+
+    with pytest.raises(ValueError, match="Invalid check defined"):
+        graph_checks.eval_need_check(need, "status==valid", log)
+
+
+def test_eval_need_check_unknown_operator_raises_value_error() -> None:
+    """Raise error when an unsupported binary operator is used."""
+    log = fake_check_logger()
+    need = test_need(status="valid")
+
+    with pytest.raises(ValueError, match="Binary Operator not defined"):
+        graph_checks.eval_need_check(need, "status <> valid", log)
+
+
+def test_eval_need_check_missing_attribute_logs_and_returns_false() -> None:
+    """Return False and log warning when referenced attribute is not present."""
+    log = fake_check_logger()
+    need = test_need(status="valid")
+
+    result = graph_checks.eval_need_check(need, "priority == high", log)
+
+    assert result is False
+    log.assert_warning("Attribute not defined: priority")
+
+    result2 = graph_checks.eval_need_condition(need, {"not": ["status == valid"]}, log)
+
+    assert result2 is False
+    # assert "Attribute not defined: priority" in msg
+
+
+def test_eval_need_condition_invalid_type_raises_value_error() -> None:
+    """Raise error for condition values that are neither string nor dict."""
+    log = fake_check_logger()
+    need = test_need(status="valid")
+
+    with pytest.raises(ValueError, match="Invalid condition type"):
+        graph_checks.eval_need_condition(need, 123, log)  # type: ignore[arg-type]
+
+
+def test_eval_need_condition_not_with_wrong_operand_count_raises_value_error() -> None:
+    """Raise error when 'not' does not receive exactly one operand."""
+    log = fake_check_logger()
+    need = test_need(status="valid")
+
+    with pytest.raises(ValueError, match="requires exactly one operand"):
+        graph_checks.eval_need_condition(
+            need,
+            {"not": ["status == valid", "status != invalid"]},
+            log,
+        )
+
+
+def test_eval_need_condition_and_or_xor_branches() -> None:
+    """Evaluate logical combination branches and, or and xor."""
+    log = fake_check_logger()
+    need = test_need(status="valid", kind="req")
+
+    and_result = graph_checks.eval_need_condition(
+        need,
+        {"and": ["status == valid", "kind == req"]},
+        log,
+    )
+    or_result = graph_checks.eval_need_condition(
+        need,
+        {"or": ["status == invalid", "kind == req"]},
+        log,
+    )
+    xor_result = graph_checks.eval_need_condition(
+        need,
+        {"xor": ["status == valid", "kind == req"]},
+        log,
+    )
+
+    assert and_result is True
+    assert or_result is True
+    assert xor_result is False
+
+    with pytest.raises(ValueError, match="Unsupported condition operator: blah"):
+        graph_checks.eval_need_condition(
+            need,
+            {"blah": ["status == valid", "kind == req"]},
+            log,
+        )
+
+
+def test_filter_needs_by_criteria_invalid() -> None:
+    """Raise error when include/exclude selector key is invalid."""
+    log = fake_check_logger()
+    test_need()
+    needs_types = [NeedType({"title": "testtype", "prefix": "t", "directive": "req"})]
+    needs = [test_need(id="N1", type="testtype", status="valid")]
+
+    with pytest.raises(ValueError, match="Invalid need selection"):
+        graph_checks.filter_needs_by_criteria(
+            needs_types,
+            needs,
+            {"invalid": "req", "condition": "status == valid"},
+            log,
+        )
+
+    with pytest.raises(ValueError, match="Invalid selection"):
+        graph_checks.filter_needs_by_criteria(
+            needs_types,
+            needs,
+            {"exclude": "req"},
+            log,
+        )
+    with pytest.raises(
+        ValueError, match="Invalid need selection: both include and exclude are set"
+    ):
+        graph_checks.filter_needs_by_criteria(
+            needs_types,
+            needs,
+            {"include": "req", "exclude": "req"},
+            log,
+        )
+
+
+def test_filter_needs_by_criteria_unknown_type_logs_warning() -> None:
+    """Log warning when selected pattern contains unknown need type."""
+    log = fake_check_logger()
+    needs_types = [NeedType({"title": "testtype", "prefix": "t", "directive": "req"})]
+    needs = [test_need(id="N1", type="whasxd", status="valid")]
+
+    selected = graph_checks.filter_needs_by_criteria(
+        needs_types,
+        needs,
+        {"include": "unknown", "condition": "status == valid"},
+        log,
+    )
+    assert selected == []
+    assert log.warnings == 1
+    log.assert_warning(
+        "Unknown need type `unknown` in graph check.", expect_location=False
+    )