Repository: jwbargsten/pytest-archon (language: python)

Issue:
Show transtive path
When I get a violation, I like to know the chain of module imports that lead to this architecture exception.



Discussion / hints:

Fix patch:
diff --git a/pyproject.toml b/pyproject.toml
index 2d0ee04..8f80441 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -23,7 +23,6 @@ classifiers = [
     "Development Status :: 5 - Production/Stable",
 ]
 dependencies = [
-    "pytest-check",
     "pytest>=7.2",
 ]
 dynamic = ["version"]


Test patch:
diff --git a/pytest_archon/failure.py b/pytest_archon/failure.py
new file mode 100644
index 0000000..c9a64d9
--- /dev/null
+++ b/pytest_archon/failure.py
@@ -0,0 +1,32 @@
+"""
+This code is based on pytest-check, by Bryan Okken (MIT License)
+https://github.com/okken/pytest-check.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+
+@dataclass(frozen=True)
+class Failure:
+    rule_name: str
+    rule_comment: str
+    reason: str
+    path: tuple[str, ...]
+
+
+_failures: list[Failure] = []
+
+
+def add_failure(rule_name, rule_comment, reason, path: list[str] | None = None):
+    global _failures
+    _failures.append(Failure(rule_name, rule_comment, reason, tuple(path) if path else ()))
+
+
+def pop_failures():
+    global _failures
+    try:
+        return _failures
+    finally:
+        _failures = []
diff --git a/pytest_archon/plugin.py b/pytest_archon/plugin.py
index be9bf12..62b644e 100644
--- a/pytest_archon/plugin.py
+++ b/pytest_archon/plugin.py
@@ -1,8 +1,56 @@
+from itertools import groupby
+from operator import attrgetter
+
 import pytest
+from _pytest._code.code import ExceptionInfo
 
+from pytest_archon.failure import pop_failures
 from pytest_archon.rule import archrule
 
 
 @pytest.fixture(name="archrule")
 def check_fixture():
     return archrule
+
+
+from _pytest.skipping import xfailed_key
+
+
+@pytest.hookimpl(hookwrapper=True, trylast=True)
+def pytest_runtest_makereport(item, call):
+    outcome = yield
+    report = outcome.get_result()
+
+    failures = pop_failures()
+
+    if failures and item._store[xfailed_key]:
+        report.outcome = "skipped"
+        report.wasxfail = item._store[xfailed_key].reason
+        return
+
+    longrepr = format_failures(failures)
+    if longrepr:
+        report.longrepr = longrepr
+        report.outcome = "failed"
+
+    if failures:
+        try:
+            raise AssertionError(report.longrepr)
+        except AssertionError:
+            excinfo = ExceptionInfo.from_current()
+        call.excinfo = excinfo
+
+
+def format_failures(failures):
+    longrepr = []
+    for rule_name, rule_failures in groupby(failures, attrgetter("rule_name")):
+        longrepr.append(f"FAILED Rule '{rule_name}':")
+        for reason, reason_failures in groupby(rule_failures, attrgetter("reason")):
+            longrepr.append(f"- {reason}")
+            for path in set(f.path for f in reason_failures):
+                longrepr.append(f"    from {' ↣ '.join(path)}")
+    return "\n".join(longrepr)
+
+
+class ModelViolation(AssertionError):
+    pass
diff --git a/pytest_archon/rule.py b/pytest_archon/rule.py
index 83edb65..baa86e0 100644
--- a/pytest_archon/rule.py
+++ b/pytest_archon/rule.py
@@ -4,9 +4,8 @@
 from types import ModuleType
 from typing import Iterable, Sequence
 
-from pytest_check.check_log import log_failure  # type: ignore[import]
-
 from pytest_archon.collect import ImportMap, collect_imports, walk, walk_runtime, walk_toplevel
+from pytest_archon.failure import add_failure  # type: ignore[import]
 
 
 def archrule(name: str, comment: str | None = None) -> Rule:
@@ -162,7 +161,9 @@ def check(
             candidates = [k for k in candidates if not fnmatch(k, ep)]
 
         if not candidates:
-            log_failure(
+            add_failure(
+                rule_name,
+                rule_comment,
                 f"NO CANDIDATES MATCHED. Match criteria: {match_criteria}, "
                 f"exclude_criteria: {exclude_criteria}",
             )
@@ -174,23 +175,18 @@ def check(
             import_map = {candidate: all_imports[candidate]} if only_direct_imports else all_imports
 
             for constraint in self._check_required_constraints(candidate, import_map):
-                log_failure(
-                    _fmt_rule(
-                        rule_name,
-                        rule_comment,
-                        f"module '{candidate}' is missing REQUIRED imports matching pattern /{constraint}/",
-                    ),
+                add_failure(
+                    rule_name,
+                    rule_comment,
+                    f"module '{candidate}' is missing REQUIRED imports matching pattern /{constraint}/",
                 )
 
             for constraint, path in self._check_forbidden_constraints(candidate, import_map):
-                log_failure(
-                    _fmt_rule(
-                        rule_name,
-                        rule_comment,
-                        f"module '{candidate}' has FORBIDDEN imports:\n{path[-1]} "
-                        f"(matched by /{constraint}/), "
-                        f"through modules {' ↣ '.join(path[:-1])}.",
-                    ),
+                add_failure(
+                    rule_name,
+                    rule_comment,
+                    f"module '{candidate}' has FORBIDDEN import {path[-1]} (matched by /{constraint}/) ",
+                    path,
                 )
 
     def _check_required_constraints(self, module: str, all_imports: ImportMap):
@@ -214,13 +210,6 @@ def _check_forbidden_constraints(
             )
 
 
-def _fmt_rule(name, comment, text):
-    res = f"RULE {name}: {text}"
-    if comment:
-        res += f"\n({comment})"
-    return res
-
-
 def recurse_imports(module: str, all_imports: ImportMap) -> Iterable[Sequence[str]]:
     seen = set()
 
diff --git a/tests/test_pytest_fixture.py b/tests/test_pytest_fixture.py
index 491e01e..c77b24c 100644
--- a/tests/test_pytest_fixture.py
+++ b/tests/test_pytest_fixture.py
@@ -2,7 +2,7 @@ def test_rule_basic(archrule):
     (
         archrule("abc", "def")
         .match("*collect")
-        .should_not_import("pytest_archon.import_finder")
+        .should_not_import("pytest_archon.failure")
         .check("pytest_archon")
     )
 
@@ -23,3 +23,4 @@ def test_rule_fail(archrule):
     )
     result = pytester.runpytest()
     result.assert_outcomes(failed=1)
+    result.stdout.fnmatch_lines("FAILED Rule 'abc':")
diff --git a/tests/test_rule.py b/tests/test_rule.py
index f955d1e..620f3ca 100644
--- a/tests/test_rule.py
+++ b/tests/test_rule.py
@@ -1,5 +1,7 @@
 import pytest_archon
 from pytest_archon import archrule
+from pytest_archon.failure import pop_failures
+from pytest_archon.plugin import format_failures
 
 
 def test_rule_basic():
@@ -53,25 +55,6 @@ def test_only_direct():
     )
 
 
-def test_rule_fail(pytester):
-    pytester.makepyfile(
-        """
-        from pytest_archon.plugin import archrule
-        import pytest_archon
-
-        def test_rule_fail():
-            (
-                archrule("abc", "def")
-                .match("*collect")
-                .should_not_import("importl*")
-                .check(pytest_archon)
-            )
-    """
-    )
-    result = pytester.runpytest()
-    result.assert_outcomes(failed=1)
-
-
 def test_transitive_dependency_succeeds(create_testset):
     create_testset(
         ("abcz/__init__.py", ""),
@@ -100,36 +83,25 @@ def test_transitive_dependency_via_may_import_succeeds(create_testset):
     )
 
 
-def test_required_transitive_dependency_fails(create_testset, pytester):
+def test_required_transitive_dependency_fails(create_testset):
     create_testset(
         ("abcz/__init__.py", ""),
         ("abcz/moduleA.py", "import abcz.moduleB"),
         ("abcz/moduleB.py", "import abcz.moduleC"),
         ("abcz/moduleC.py", ""),
     )
-    pytester.makepyfile(
-        """
-        from pytest_archon.plugin import archrule
-        import pytest_archon
-
-        def test_rule_fail():
-            (
-                archrule("rule exclusion")
-                .match("abcz.moduleA")
-                .should_import("abcz.moduleD")
-                .check("abcz")
-            )
-    """
-    )
-    result = pytester.runpytest()
-    result.assert_outcomes(failed=1)
-    result.stdout.fnmatch_lines(
-        "FAILURE: RULE rule exclusion: module 'abcz.moduleA' is missing REQUIRED imports "
-        "matching pattern /abcz.moduleD/"
-    )
+    (archrule("rule exclusion").match("abcz.moduleA").should_import("abcz.moduleD").check("abcz"))
+
+    failures = pop_failures()
+    longrepr = format_failures(failures)
 
+    assert failures
 
-def test_forbidden_transitive_dependency_fails(create_testset, pytester):
+    assert "FAILED Rule 'rule exclusion':" in longrepr
+    assert "- module 'abcz.moduleA' is missing REQUIRED imports matching pattern /abcz.moduleD/" in longrepr
+
+
+def test_forbidden_transitive_dependency_fails(create_testset):
     create_testset(
         ("abcz/__init__.py", ""),
         ("abcz/moduleA.py", "import abcz.moduleB"),
@@ -137,22 +109,14 @@ def test_forbidden_transitive_dependency_fails(create_testset, pytester):
         ("abcz/moduleC.py", "import abcz.moduleD"),
         ("abcz/moduleD.py", ""),
     )
-    pytester.makepyfile(
-        """
-        from pytest_archon.plugin import archrule
-        import pytest_archon
-
-        def test_rule_fail():
-            (
-                archrule("rule exclusion")
-                .match("abcz.moduleA")
-                .should_not_import("abcz.moduleD")
-                .check("abcz")
-            )
-    """
-    )
-    result = pytester.runpytest()
-    result.assert_outcomes(failed=1)
-    result.stdout.fnmatch_lines("FAILURE: RULE rule exclusion: module 'abcz.moduleA' has FORBIDDEN imports*")
-    result.stdout.fnmatch_lines("abcz.moduleD (matched by /abcz.moduleD/)*")
-    result.stdout.fnmatch_lines("*through modules abcz.moduleA ↣ abcz.moduleB ↣ abcz.moduleC.*")
+    (archrule("rule exclusion").match("abcz.moduleA").should_not_import("abcz.moduleD").check("abcz"))
+
+    failures = pop_failures()
+    longrepr = format_failures(failures)
+
+    assert failures
+    assert longrepr
+    assert "FAILED Rule 'rule exclusion':" in longrepr
+    assert "- module 'abcz.moduleA' has FORBIDDEN import" in longrepr
+    assert "abcz.moduleD (matched by /abcz.moduleD/)" in longrepr
+    assert "abcz.moduleA ↣ abcz.moduleB ↣ abcz.moduleC ↣ abcz.moduleD" in longrepr