Repository: zyskarch/pytestarch (language: python)

Issue:
One violation if multiple rule objects: improve error message
If a user uses for example a should only rule with multiple rule objects and one of them violates the rule, all rule objects will be included in the error message. This can be confusing to the user. Instead, only the violating module should be included.


Discussion / hints:

Fix patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index cc9863a..96a35eb 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -4,6 +4,11 @@ This project uses semantic versioning and follows [keep a changelog](https://kee
 
 
 ## [Unreleased]
+
+### Changed
+- Error message only lists violated rule objects when multiple rule objects are defined.
+
+## [1.3.1] -- 2023-01-27
 ### Changed
 - Internal module structure to improve coupling, cohesion.
 


Test patch:
diff --git a/src/pytestarch/rule_assessment/error_message/message_generator.py b/src/pytestarch/rule_assessment/error_message/message_generator.py
index c5d36ac..20cb73f 100644
--- a/src/pytestarch/rule_assessment/error_message/message_generator.py
+++ b/src/pytestarch/rule_assessment/error_message/message_generator.py
@@ -143,10 +143,12 @@ def _create_should_import_violated_messages(
         if not rule_violations.should_violated:
             return []
 
-        return self._create_no_import_between_original_subject_and_objects_message()
+        return self._create_no_import_between_original_subject_and_objects_message(
+            rule_violations
+        )
 
     def _create_no_import_between_original_subject_and_objects_message(
-        self,
+        self, rule_violations: RuleViolations
     ) -> List[RuleViolatedMessage]:
         messages = []
 
@@ -162,7 +164,8 @@ def _create_no_import_between_original_subject_and_objects_message(
         for rule_object, is_singular in zip(
             self._original_rule_object_names, self._original_rule_object_singular
         ):
-            rule_objects.append(self._get_rule_object(rule_object, is_singular))
+            if self._rule_object_not_imported(rule_object, rule_violations):
+                rule_objects.append(self._get_rule_object(rule_object, is_singular))
 
         combined_rule_object = ", ".join(rule_objects)
         messages.append(
@@ -231,7 +234,9 @@ def _create_should_only_import_no_import_violated_messages(
         if not rule_violations.should_only_violated_by_no_import:
             return []
 
-        return self._create_no_import_between_original_subject_and_objects_message()
+        return self._create_no_import_between_original_subject_and_objects_message(
+            rule_violations
+        )
 
     def _create_should_not_import_violated_messages(
         self, rule_violations: RuleViolations
@@ -390,3 +395,23 @@ def _get_verb_suffix(self, subject_singular: bool) -> str:
 
         # "is imported by" does not need an additional 's'
         return ""
+
+    @classmethod
+    def _rule_object_not_imported(
+        cls, rule_object: str, rule_violations: RuleViolations
+    ) -> bool:
+        found_dependencies = rule_violations.dependencies
+
+        if not found_dependencies:
+            return True
+
+        dependencies_for_rule_object = [
+            len(concrete_dependency_realisations)
+            for abstract_dependency, concrete_dependency_realisations in found_dependencies.items()
+            if abstract_dependency[1].name == rule_object
+        ]
+
+        if not dependencies_for_rule_object:
+            return True
+
+        return sum(dependencies_for_rule_object) == 0
diff --git a/tests/rule_assessment/test_RuleViolationMessageGenerator.py b/tests/rule_assessment/test_RuleViolationMessageGenerator.py
index 288c33d..e48df9c 100644
--- a/tests/rule_assessment/test_RuleViolationMessageGenerator.py
+++ b/tests/rule_assessment/test_RuleViolationMessageGenerator.py
@@ -4,8 +4,10 @@
 
 import pytest
 
+from pytestarch import Rule
 from pytestarch.eval_structure.evaluable_architecture import (
     DependenciesByBaseModules,
+    EvaluableArchitecture,
     Module,
     UnexpectedDependenciesByBaseModule,
 )
@@ -683,3 +685,21 @@ def test_multiple_messages_if_forbidden_and_no_import_both_present(
 
     assert len(messages) == 2
     assert messages == expected_messages
+
+
+def test_only_offending_rule_object_listed(
+    graph_based_on_string_module_names: EvaluableArchitecture,
+) -> None:
+    rule = (
+        Rule()
+        .modules_that()
+        .are_named("src.moduleA.submoduleA1.submoduleA11.fileA11")
+        .should_only()
+        .import_modules_that()
+        .are_named(["src.moduleB", "src.moduleC"])
+    )
+    with pytest.raises(
+        AssertionError,
+        match='"src.moduleA.submoduleA1.submoduleA11.fileA11" does not import "src.moduleC".',
+    ):
+        rule.assert_applies(graph_based_on_string_module_names)