Repository: zyskarch/pytestarch (language: python)

Issue:
When base module == module to scan and both are a submodule, no edges are calculated



Discussion / hints:

Fix patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 0d6f066..d70e114 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -4,6 +4,8 @@ This project uses semantic versioning and follows [keep a changelog](https://kee
 
 
 ## [Unreleased]
+### Fixed
+- All internal modules filtered out if module and root path are not identical and are a sub module of the actual root module.
 
 ## [1.0.1] -- 2022-09-27
 ### Fixed
diff --git a/docs/parsing.md b/docs/parsing.md
index d73b912..2b2c7bc 100644
--- a/docs/parsing.md
+++ b/docs/parsing.md
@@ -6,6 +6,8 @@
 
 ## ::: src.pytestarch.importer.import_filter
 
+## ::: src.pytestarch.importer.import_path_padder
+
 ## ::: src.pytestarch.importer.import_types
 
 ## ::: src.pytestarch.importer.importee_module_calculator


Test patch:
diff --git a/src/pytestarch/importer/import_filter.py b/src/pytestarch/importer/import_filter.py
index e2bc5e7..a8a9353 100644
--- a/src/pytestarch/importer/import_filter.py
+++ b/src/pytestarch/importer/import_filter.py
@@ -1,4 +1,3 @@
-from pathlib import Path
 from typing import List
 
 from pytestarch.importer.import_types import Import
@@ -9,17 +8,15 @@ class ImportFilter:
     External modules are all modules that are not submodules of the configured module to search for imports.
     """
 
-    def __init__(
-        self, exclude_external_libraries: bool, root_module_path: Path
-    ) -> None:
+    def __init__(self, exclude_external_libraries: bool, root_module_name: str) -> None:
         """
         Args:
             exclude_external_libraries: If True, external modules will be filtered out, otherwise this class implements a no-op.
-            root_module_path: path to the module that determines which modules are considered external. If a module
+            root_module_name: name of the module that determines which modules are considered external. If a module
                 is a submodule of this module, it is considered internal.
         """
         self._exclude_external_libraries = exclude_external_libraries
-        self._root_module_path = root_module_path
+        self._root_module_name = root_module_name
 
     def filter(self, imports: List[Import]) -> List[Import]:
         """According to the configuration, imports will be filtered.
@@ -37,6 +34,4 @@ def filter(self, imports: List[Import]) -> List[Import]:
     def _is_internal_import(self, i: Import) -> bool:
         importee = i.importee()
 
-        top_level_module = self._root_module_path.name
-
-        return importee.startswith(top_level_module)
+        return importee.startswith(self._root_module_name)
diff --git a/src/pytestarch/importer/import_path_padder.py b/src/pytestarch/importer/import_path_padder.py
new file mode 100644
index 0000000..877a99c
--- /dev/null
+++ b/src/pytestarch/importer/import_path_padder.py
@@ -0,0 +1,33 @@
+import os
+from pathlib import Path
+from typing import List
+
+from pytestarch.importer.import_types import Import
+
+
+class ImportPathPadder:
+    """Ensures that paths to imported internal modules match path structure of importing internal modules."""
+
+    @classmethod
+    def pad(cls, imports: List[Import], root_path: Path, module_path: Path) -> str:
+        """
+        Appends a prefix to all internal import module paths to ensure that importing and imported modules have the
+        same module prefix. This becomes necessary if the root and module path are not identical.
+
+        Args:
+            imports: list of Imports to update
+            root_path: Path to the root module
+            module_path: Path to the top level module to scan
+
+        Returns:
+            appended prefix based on the difference between root and module path
+        """
+        short_prefix = str(module_path.relative_to(root_path)).replace(os.sep, ".")
+        # drop name of module_path module itself, as this is already part of the import paths
+        full_prefix = ".".join(short_prefix.split(".")[:-1])
+
+        for imp in imports:
+            if not imp.importer().startswith(full_prefix):
+                imp.update_import_prefixes(full_prefix, module_path.name)
+
+        return full_prefix
diff --git a/src/pytestarch/importer/import_types.py b/src/pytestarch/importer/import_types.py
index ceb64cf..57f0c1a 100644
--- a/src/pytestarch/importer/import_types.py
+++ b/src/pytestarch/importer/import_types.py
@@ -87,6 +87,23 @@ def _get_parent_modules(self, module: str) -> List[str]:
 
         return parent_modules
 
+    def update_import_prefixes(self, prefix: str, top_level_module_name: str) -> None:
+        """Adds the given prefix to the import modules' path.
+        This ensures that importers and importees have the same module path structure. For example, if the importees
+        have a structure like src.A.x, but the module path has been restricted to src.A, the importers will all have
+        a structure like A.x. This causes no edges in the resulting graph, as no matching nodes are found for the edges.
+        The prefix will only be appended to modules that are submodules of the top level module.
+
+        Args:
+            prefix: to append to all internal module names
+            top_level_module_name: name of the top level module. Used to differentiate between internal and external
+                dependencies.
+        """
+        pass
+
+    def _append_prefix(self, value: str, prefix: str) -> str:
+        return f"{prefix}.{value}"
+
 
 class AbsoluteImport(Import):
     """Represents an absolute import."""
@@ -102,6 +119,14 @@ def importee(self) -> str:
     def importee_parent_modules(self) -> List[str]:
         return self._importee_module_hierarchy
 
+    def update_import_prefixes(self, prefix: str, top_level_module_name: str) -> None:
+        if self._module_name.startswith(top_level_module_name):
+            self._module_name = self._append_prefix(self._module_name, prefix)
+            self._importee_module_hierarchy = [
+                self._append_prefix(name, prefix)
+                for name in self._importer_module_hierarchy
+            ]
+
 
 class RelativeImport(Import):
     """Represents a relative import."""
@@ -139,3 +164,11 @@ def _calculate_importee(self) -> str:
         module_name = self._module_name or self._import_name
 
         return self._importer_module_hierarchy[-self._level] + "." + module_name
+
+    def update_import_prefixes(self, prefix: str, top_level_module_name: str) -> None:
+        if self._module_name.startswith(top_level_module_name):
+            self._module_name = self._append_prefix(self._module_name, prefix)
+            self._importee_module_hierarchy = [
+                self._append_prefix(name, prefix)
+                for name in self._importer_module_hierarchy
+            ]
diff --git a/src/pytestarch/pytestarch.py b/src/pytestarch/pytestarch.py
index 5a1c7ee..998beed 100755
--- a/src/pytestarch/pytestarch.py
+++ b/src/pytestarch/pytestarch.py
@@ -11,6 +11,7 @@
 from pytestarch.eval_structure.eval_structure_types import EvaluableArchitecture
 from pytestarch.eval_structure.evaluable_graph import EvaluableArchitectureGraph
 from pytestarch.eval_structure.graph import Graph
+from pytestarch.importer.import_path_padder import ImportPathPadder
 from pytestarch.importer.converter import ImportConverter
 from pytestarch.importer.file_filter import FileFilter
 from pytestarch.importer.import_filter import ImportFilter
@@ -31,7 +32,7 @@ def get_evaluable_architecture(
     """Constructs an evaluable object based on the given module.
 
     Args:
-        root_path: root directory of the source code
+        root_path: root directory of the source code. Should not be set to a submodule of the top level module.
         module_path: path of module to generate the evaluable for. Must be a submodule of the root_path module.
         exclusions: pseudo-regex to exclude files and directories from being integrated into the evaluable, e.g. *Test.py
         exclude_external_libraries: if True, external dependencies (defined as all dependencies to module outside the
@@ -60,7 +61,11 @@ def get_evaluable_architecture(
 
     imports = converter.convert(ast)
 
-    import_filter = ImportFilter(exclude_external_libraries, module_path)
+    full_prefix = ImportPathPadder.pad(imports, root_path, module_path)
+
+    import_filter = ImportFilter(
+        exclude_external_libraries, full_prefix + module_path.name
+    )
     imports = import_filter.filter(imports)
 
     if not exclude_external_libraries:
diff --git a/tests/importer/test_import_filter.py b/tests/importer/test_import_filter.py
index 97a2398..d50b74f 100644
--- a/tests/importer/test_import_filter.py
+++ b/tests/importer/test_import_filter.py
@@ -12,7 +12,7 @@
 
 
 def test_import_filter_excludes_external_dependencies() -> None:
-    filter = ImportFilter(True, ROOT_PATH)
+    filter = ImportFilter(True, str(ROOT_PATH))
     filtered_imports = filter.filter(imports)
 
     assert len(filtered_imports) == 1
@@ -20,7 +20,7 @@ def test_import_filter_excludes_external_dependencies() -> None:
 
 
 def test_import_filter_includes_internal_dependencies() -> None:
-    filter = ImportFilter(False, ROOT_PATH)
+    filter = ImportFilter(False, str(ROOT_PATH))
     filtered_imports = filter.filter(imports)
 
     assert len(filtered_imports) == 2
diff --git a/tests/resources/test_project/src/moduleA/submoduleA1/submoduleA11/fileA11.py b/tests/resources/test_project/src/moduleA/submoduleA1/submoduleA11/fileA11.py
index a16d5d1..a364dd8 100644
--- a/tests/resources/test_project/src/moduleA/submoduleA1/submoduleA11/fileA11.py
+++ b/tests/resources/test_project/src/moduleA/submoduleA1/submoduleA11/fileA11.py
@@ -1,4 +1,5 @@
 from src.moduleB.submoduleB1.fileB1 import b1  # type: ignore
+import os  # noqa: F401 E401
 
 
 def a11():
diff --git a/tests/test_integration.py b/tests/test_integration.py
index 6e9974e..1d46d23 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -1,10 +1,16 @@
+import os
+
 import pytest
 
 # from matplotlib import pyplot as plt
 # from matplotlib.pyplot import figure
 
 from pytestarch.eval_structure.eval_structure_types import EvaluableArchitecture
+from pytestarch.pytestarch import get_evaluable_architecture
 from pytestarch.query_language.base_language import Rule
+from resources import test_project
+from resources.test_project.src import moduleA
+from resources.test_project.src.moduleA import submoduleA1
 
 rules_to_test = [
     (
@@ -236,3 +242,91 @@ def test_level_limit_flattens_dependencies_correctly(
 #         },
 #     )
 #     plt.savefig("test.png")
+
+
+def test_edges_correctly_calculated_for_level_2_module_path() -> None:
+    level_2_graph = get_evaluable_architecture(
+        os.path.dirname(test_project.__file__),
+        os.path.dirname(moduleA.__file__),
+        ("*__pycache__", "*__init__.py", "*Test.py"),
+    )
+
+    assert len(level_2_graph._graph._graph.edges) == 0  # no internal dependencies left
+
+
+def test_edges_correctly_calculated_for_level_2_module_path_no_external_dependencies_modified() -> None:
+    level_2_graph = get_evaluable_architecture(
+        os.path.dirname(test_project.__file__),
+        os.path.dirname(moduleA.__file__),
+        ("*__pycache__", "*__init__.py", "*Test.py"),
+        exclude_external_libraries=False,
+    )
+
+    assert len(level_2_graph._graph._graph.edges) == 16
+
+    for edge in [
+        ("src", "src.moduleA"),
+        ("src", "src.moduleB"),  # must not be src.moduleA.src.moduleB
+        ("src", "src.moduleC"),
+        ("src.moduleA", "src.moduleA.fileA"),
+        ("src.moduleA", "src.moduleA.submoduleA1"),
+        ("src.moduleA", "src.moduleA.submoduleA2"),
+        ("src.moduleA.fileA", "src.moduleC.fileC"),
+        ("src.moduleA.submoduleA1", "src.moduleA.submoduleA1.submoduleA11"),
+        (
+            "src.moduleA.submoduleA1.submoduleA11",
+            "src.moduleA.submoduleA1.submoduleA11.fileA11",
+        ),
+        (
+            "src.moduleA.submoduleA1.submoduleA11.fileA11",
+            "os",
+        ),  # must not be src.moduleA.os!
+        (
+            "src.moduleA.submoduleA1.submoduleA11.fileA11",
+            "src.moduleB.submoduleB1.fileB1",
+        ),
+        ("src.moduleA.submoduleA2", "src.moduleA.submoduleA2.fileA2"),
+        ("src.moduleA.submoduleA2.fileA2", "src.moduleC.fileC"),
+        ("src.moduleB", "src.moduleB.submoduleB1"),
+        ("src.moduleB.submoduleB1", "src.moduleB.submoduleB1.fileB1"),
+        ("src.moduleC", "src.moduleC.fileC"),
+    ]:
+        assert edge in level_2_graph._graph
+
+
+def test_edges_correctly_calculated_for_level_3_module_path() -> None:
+    level_3_graph = get_evaluable_architecture(
+        os.path.dirname(test_project.__file__),
+        os.path.dirname(submoduleA1.__file__),
+        ("*__pycache__", "*__init__.py", "*Test.py"),
+    )
+
+    assert len(level_3_graph._graph._graph.edges) == 0  # no internal dependencies left
+
+
+def test_edges_correctly_calculated_for_level_3_module_path_no_external_dependencies_modified() -> None:
+    level_3_graph = get_evaluable_architecture(
+        os.path.dirname(test_project.__file__),
+        os.path.dirname(submoduleA1.__file__),
+        ("*__pycache__", "*__init__.py", "*Test.py"),
+        exclude_external_libraries=False,
+    )
+
+    assert len(level_3_graph._graph._graph.edges) == 7
+
+    for edge in [
+        ("src", "src.moduleB"),
+        ("src.moduleA.submoduleA1", "src.moduleA.submoduleA1.submoduleA11"),
+        (
+            "src.moduleA.submoduleA1.submoduleA11",
+            "src.moduleA.submoduleA1.submoduleA11.fileA11",
+        ),
+        ("src.moduleA.submoduleA1.submoduleA11.fileA11", "os"),
+        (
+            "src.moduleA.submoduleA1.submoduleA11.fileA11",
+            "src.moduleB.submoduleB1.fileB1",
+        ),
+        ("src.moduleB", "src.moduleB.submoduleB1"),
+        ("src.moduleB.submoduleB1", "src.moduleB.submoduleB1.fileB1"),
+    ]:
+        assert edge in level_3_graph._graph