From 55048e038812a2e8aab033bac4285fbb2aec0014 Mon Sep 17 00:00:00 2001 From: Evan Doyle Date: Thu, 8 Feb 2024 17:10:52 -0800 Subject: [PATCH 1/3] Write tests for BoundaryTrie functionality --- modguard/core/boundary.py | 4 +- tests/mocks/__init__.py | 0 tests/mocks/boundary_trie.py | 12 --- tests/test_boundary_trie.py | 166 +++++++++++++++++++++++++++++++++++ tests/test_check.py | 11 ++- 5 files changed, 177 insertions(+), 16 deletions(-) delete mode 100644 tests/mocks/__init__.py delete mode 100644 tests/mocks/boundary_trie.py diff --git a/modguard/core/boundary.py b/modguard/core/boundary.py index 15d53cb0..672c4835 100644 --- a/modguard/core/boundary.py +++ b/modguard/core/boundary.py @@ -27,13 +27,14 @@ def __iter__(self): def get(self, path: str) -> Optional[BoundaryNode]: node = self.root parts = path.split(".") + parts = [part for part in parts if part] for part in parts: if part not in node.children: return None node = node.children[part] - return node + return node if node.is_end_of_path else None def insert(self, path: str, public_members: Optional[list[PublicMember]] = None): node = self.root @@ -67,6 +68,7 @@ def register_public_member(self, path: str, member: PublicMember): def find_nearest(self, path: str) -> Optional[BoundaryNode]: node = self.root parts = path.split(".") + parts = [part for part in parts if part] nearest_parent = node if node.is_end_of_path else None for part in parts: diff --git a/tests/mocks/__init__.py b/tests/mocks/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/mocks/boundary_trie.py b/tests/mocks/boundary_trie.py deleted file mode 100644 index df74ce5c..00000000 --- a/tests/mocks/boundary_trie.py +++ /dev/null @@ -1,12 +0,0 @@ -from modguard.core.boundary import BoundaryTrie -from modguard.core.public import PublicMember - - -def build_example_boundary_trie() -> BoundaryTrie: - trie = BoundaryTrie() - trie.insert("") - trie.insert("domain_one") - trie.insert("domain_two") - trie.insert("domain_three") - trie.insert("domain_four", [PublicMember(name="domain_four.public_api")]) - return trie diff --git a/tests/test_boundary_trie.py b/tests/test_boundary_trie.py index e69de29b..e16dc038 100644 --- a/tests/test_boundary_trie.py +++ b/tests/test_boundary_trie.py @@ -0,0 +1,166 @@ +import pytest + +from modguard.core import BoundaryTrie, PublicMember, BoundaryNode + + +@pytest.fixture +def boundary_trie(): + return BoundaryTrie( + root=BoundaryNode( + public_members={}, + children={ + "domain_one": BoundaryNode( + public_members={}, + children={}, + is_end_of_path=True, + full_path="domain_one", + ), + "domain_two": BoundaryNode( + public_members={}, + children={ + "subdomain": BoundaryNode( + public_members={}, + children={}, + is_end_of_path=True, + full_path="domain_two.subdomain", + ) + }, + is_end_of_path=True, + full_path="domain_two", + ), + "domain_three": BoundaryNode( + public_members={}, + children={}, + is_end_of_path=True, + full_path="domain_three", + ), + "domain_four": BoundaryNode( + public_members={ + "domain_four.public_api": PublicMember( + name="domain_four.public_api", allowlist=None + ) + }, + children={}, + is_end_of_path=True, + full_path="domain_four", + ), + }, + is_end_of_path=True, + full_path="", + ) + ) + + +def test_iterate_over_empty_trie(): + assert list(BoundaryTrie()) == [] + + +def test_iterate_over_populated_trie(boundary_trie): + assert set((node.full_path for node in boundary_trie)) == { + "", + "domain_one", + "domain_two", + "domain_two.subdomain", + "domain_three", + "domain_four", + } + + +def test_get_nonexistent_path(boundary_trie): + assert boundary_trie.get("fakepath") is None + + +def test_get_nonexistent_empty_path(): + trie = BoundaryTrie() + assert trie.get("") is None + + +def test_get_actual_path(boundary_trie): + assert boundary_trie.get("domain_one") is not None + + +def test_get_actual_empty_path(boundary_trie): + assert boundary_trie.get("") is not None + + +def test_insert_empty_path(): + trie = BoundaryTrie() + trie.insert("") + assert set((node.full_path for node in trie)) == {""} + + +def test_insert_single_level_path(): + trie = BoundaryTrie() + trie.insert("domain") + assert set((node.full_path for node in trie)) == {"domain"} + + +def test_insert_multi_level_path(): + trie = BoundaryTrie() + trie.insert("domain.subdomain") + assert set((node.full_path for node in trie)) == {"domain.subdomain"} + + +def test_register_unnamed_public_member_at_root(boundary_trie): + pub_member = PublicMember(name="") + boundary_trie.register_public_member("member_path", pub_member) + assert boundary_trie.get("").public_members == {"member_path": pub_member} + + +def test_register_named_public_member_at_root(boundary_trie): + pub_member = PublicMember(name="member_variable") + boundary_trie.register_public_member("member_path", pub_member) + assert boundary_trie.get("").public_members == { + "member_path.member_variable": pub_member + } + + +def test_register_unnamed_public_member_at_root(boundary_trie): + pub_member = PublicMember(name="") + boundary_trie.register_public_member("member_path", pub_member) + assert boundary_trie.get("").public_members == {"member_path": pub_member} + + +def test_register_named_public_member_at_single_level_domain(boundary_trie): + pub_member = PublicMember(name="member_variable") + boundary_trie.register_public_member("domain_one", pub_member) + assert boundary_trie.get("domain_one").public_members == { + "domain_one.member_variable": pub_member + } + + +def test_register_unnamed_public_member_at_single_level_domain(boundary_trie): + pub_member = PublicMember(name="") + boundary_trie.register_public_member("domain_one", pub_member) + assert boundary_trie.get("domain_one").public_members == {"domain_one": pub_member} + + +def test_register_named_public_member_at_nested_domain(boundary_trie): + pub_member = PublicMember(name="member_variable") + boundary_trie.register_public_member("domain_two.subdomain", pub_member) + assert boundary_trie.get("domain_two.subdomain").public_members == { + "domain_two.subdomain.member_variable": pub_member + } + + +def test_register_unnamed_public_member_at_nested_domain(boundary_trie): + pub_member = PublicMember(name="") + boundary_trie.register_public_member("domain_two.subdomain", pub_member) + assert boundary_trie.get("domain_two.subdomain").public_members == { + "domain_two.subdomain": pub_member + } + + +def test_find_nearest_at_root(boundary_trie): + boundary = boundary_trie.find_nearest("other_domain") + assert boundary.full_path == "" + + +def test_find_nearest_in_single_domain(boundary_trie): + boundary = boundary_trie.find_nearest("domain_one.thing") + assert boundary.full_path == "domain_one" + + +def test_find_nearest_in_nested_domain(boundary_trie): + boundary = boundary_trie.find_nearest("domain_two.subdomain.thing") + assert boundary.full_path == "domain_two.subdomain" diff --git a/tests/test_check.py b/tests/test_check.py index 091e5e60..b4b6a221 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -1,12 +1,17 @@ import pytest from modguard.check import check, ErrorInfo, check_import -from modguard.core import BoundaryTrie -from .mocks.boundary_trie import build_example_boundary_trie +from modguard.core import BoundaryTrie, PublicMember @pytest.fixture def boundary_trie() -> BoundaryTrie: - return build_example_boundary_trie() + trie = BoundaryTrie() + trie.insert("") + trie.insert("domain_one") + trie.insert("domain_two") + trie.insert("domain_three") + trie.insert("domain_four", [PublicMember(name="domain_four.public_api")]) + return trie def _test_check_import( From 6645302b655b5e867eb7a14f452a44610785d0d1 Mon Sep 17 00:00:00 2001 From: Evan Doyle Date: Thu, 8 Feb 2024 17:12:09 -0800 Subject: [PATCH 2/3] Remove redefined test method --- tests/test_boundary_trie.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/test_boundary_trie.py b/tests/test_boundary_trie.py index e16dc038..0765c635 100644 --- a/tests/test_boundary_trie.py +++ b/tests/test_boundary_trie.py @@ -115,12 +115,6 @@ def test_register_named_public_member_at_root(boundary_trie): } -def test_register_unnamed_public_member_at_root(boundary_trie): - pub_member = PublicMember(name="") - boundary_trie.register_public_member("member_path", pub_member) - assert boundary_trie.get("").public_members == {"member_path": pub_member} - - def test_register_named_public_member_at_single_level_domain(boundary_trie): pub_member = PublicMember(name="member_variable") boundary_trie.register_public_member("domain_one", pub_member) From e786bb77aceb7dee7aa951373f716c80baf41d2a Mon Sep 17 00:00:00 2001 From: Evan Doyle Date: Thu, 8 Feb 2024 17:18:01 -0800 Subject: [PATCH 3/3] Some cleanup of path splitting and null checks in BoundaryTrie methods --- modguard/core/boundary.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/modguard/core/boundary.py b/modguard/core/boundary.py index 672c4835..13738ee8 100644 --- a/modguard/core/boundary.py +++ b/modguard/core/boundary.py @@ -24,10 +24,15 @@ class BoundaryTrie: def __iter__(self): return boundary_trie_iterator(self) + @staticmethod + def _split_mod_path(path: str) -> list[str]: + # By default "".split(".") -> [''] + # so we want to remove any whitespace path components + return [part for part in path.split(".") if part] + def get(self, path: str) -> Optional[BoundaryNode]: node = self.root - parts = path.split(".") - parts = [part for part in parts if part] + parts = self._split_mod_path(path) for part in parts: if part not in node.children: @@ -38,8 +43,7 @@ def get(self, path: str) -> Optional[BoundaryNode]: def insert(self, path: str, public_members: Optional[list[PublicMember]] = None): node = self.root - parts = path.split(".") - parts = [part for part in parts if part] + parts = self._split_mod_path(path) for part in parts: if part not in node.children: @@ -67,9 +71,8 @@ def register_public_member(self, path: str, member: PublicMember): def find_nearest(self, path: str) -> Optional[BoundaryNode]: node = self.root - parts = path.split(".") - parts = [part for part in parts if part] - nearest_parent = node if node.is_end_of_path else None + parts = self._split_mod_path(path) + nearest_parent = node for part in parts: if part in node.children: @@ -79,7 +82,7 @@ def find_nearest(self, path: str) -> Optional[BoundaryNode]: else: break - return nearest_parent + return nearest_parent if nearest_parent.is_end_of_path else None def boundary_trie_iterator(trie: BoundaryTrie) -> Generator[BoundaryNode, None, None]: