From 6659bd879ae7a365cfa70c91f0573c61d97dd0b1 Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Tue, 11 Mar 2025 16:43:28 +0100 Subject: [PATCH 1/3] Add xfailing test case to capture the issue --- .../unit/adaptors/graph/test_manipulation.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unit/adaptors/graph/test_manipulation.py b/tests/unit/adaptors/graph/test_manipulation.py index 82a882d8..39616c6e 100644 --- a/tests/unit/adaptors/graph/test_manipulation.py +++ b/tests/unit/adaptors/graph/test_manipulation.py @@ -447,3 +447,23 @@ def test_raises_module_not_present_if_no_module(self): with pytest.raises(ModuleNotPresent): graph.squash_module("foo") + + @pytest.mark.xfail(strict=True, reason="raises PanicException") + def test_correctly_handles_imports_within_descendants(self): + graph = ImportGraph() + + graph.add_module("animals") + graph.add_module("food") + graph.add_import(importer="animals.dog", imported="food.chicken") + graph.add_import(importer="app.cli", imported="animals.dog") + # We want to check that this import within the descendants of `animals` does + # not cause problems. If this import is not properly removed then the imports map can + # become corrupted, since it will contain an import to modules that no longer exist. + # See https://github.com/seddonym/grimp/issues/195 for more details. + graph.add_import(importer="animals.dog", imported="animals.base") + + graph.squash_module("animals") + + # If the `animals` module was squashed correctly then the following calls should not panic. + assert graph.find_modules_directly_imported_by("animals") == {"food.chicken"} + assert graph.find_modules_that_directly_import("animals") == {"app.cli"} From 7a22b94535e18889d7fa29637db700950d2963d3 Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Tue, 11 Mar 2025 16:44:27 +0100 Subject: [PATCH 2/3] Fix handling of within-descendant-imports when squashing modules --- rust/src/graph/graph_manipulation.rs | 10 +++++----- tests/unit/adaptors/graph/test_manipulation.py | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/rust/src/graph/graph_manipulation.rs b/rust/src/graph/graph_manipulation.rs index caed21c1..e73e021b 100644 --- a/rust/src/graph/graph_manipulation.rs +++ b/rust/src/graph/graph_manipulation.rs @@ -178,11 +178,6 @@ impl Graph { }) .collect(); - // Remove any descendants. - for descendant in descendants { - self.remove_module(descendant); - } - // Add descendants and imports to parent module. for imported in modules_imported_by_descendants { self.add_import(module, imported); @@ -192,6 +187,11 @@ impl Graph { self.add_import(importer, module); } + // Remove any descendants. + for descendant in descendants { + self.remove_module(descendant); + } + self.mark_module_squashed(module); } } diff --git a/tests/unit/adaptors/graph/test_manipulation.py b/tests/unit/adaptors/graph/test_manipulation.py index 39616c6e..47341585 100644 --- a/tests/unit/adaptors/graph/test_manipulation.py +++ b/tests/unit/adaptors/graph/test_manipulation.py @@ -448,7 +448,6 @@ def test_raises_module_not_present_if_no_module(self): with pytest.raises(ModuleNotPresent): graph.squash_module("foo") - @pytest.mark.xfail(strict=True, reason="raises PanicException") def test_correctly_handles_imports_within_descendants(self): graph = ImportGraph() From 650eb7e016c32b36abb224d23a7eda60059721aa Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Tue, 11 Mar 2025 16:46:44 +0100 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a6cd2f25..10e9a83d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog ========= +Unreleased +---------- + +* Fixed handling of within-descendant-imports when squashing modules (see `Issue 195 `_). + 3.7 (2025-03-07) ----------------