From 094d0142b628685c6a20a5d9e6c563fc3026cb55 Mon Sep 17 00:00:00 2001 From: Caelean Date: Fri, 9 Feb 2024 13:51:34 -0800 Subject: [PATCH 1/6] hit more branches --- tests/example/domain_four/subsystem.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/example/domain_four/subsystem.py b/tests/example/domain_four/subsystem.py index e3832dc6..8d89ff33 100644 --- a/tests/example/domain_four/subsystem.py +++ b/tests/example/domain_four/subsystem.py @@ -1,7 +1,8 @@ -from modguard import Boundary +import modguard +import math -Boundary() +modguard.Boundary() def private_subsystem_call(): - ... + print(math.random()) From 8135cf90dfd8434cff76a6c76aa900fc1cf7505e Mon Sep 17 00:00:00 2001 From: Caelean Date: Fri, 9 Feb 2024 17:22:32 -0800 Subject: [PATCH 2/6] updated test coverage cases --- modguard/parsing/public.py | 1 - tests/example/__init__.py | 4 ++-- tests/example/domain_one/interface.py | 5 +++++ tests/example/domain_three/api.py | 2 +- tests/example/domain_two/__init__.py | 7 ++++++- tests/test_check.py | 7 ++++++- tests/test_init.py | 12 +++++++++--- tests/test_parsing.py | 3 ++- 8 files changed, 31 insertions(+), 10 deletions(-) diff --git a/modguard/parsing/public.py b/modguard/parsing/public.py index 53ab20b0..f2aba2e3 100644 --- a/modguard/parsing/public.py +++ b/modguard/parsing/public.py @@ -72,7 +72,6 @@ def _extract_allowlist(self, public_call: ast.Call) -> Optional[list[str]]: for elt in allowlist_value.elts if isinstance(elt, ast.Constant) and isinstance(elt.value, str) ] or None - return None def _add_public_member_from_decorator( self, node: Union[ast.FunctionDef, ast.ClassDef], decorator: ast.expr diff --git a/tests/example/__init__.py b/tests/example/__init__.py index 32333adc..8dbdec90 100644 --- a/tests/example/__init__.py +++ b/tests/example/__init__.py @@ -1,8 +1,8 @@ from modguard import Boundary # intentional import violations -from .domain_one.interface import domain_one_interface -from .domain_three.api import public_for_domain_two +from .domain_one.interface import domain_one_interface, domain_one_var +from .domain_three.api import PublicForDomainTwo from .domain_four.subsystem import private_subsystem_call # OK import diff --git a/tests/example/domain_one/interface.py b/tests/example/domain_one/interface.py index 5a793010..7ae93a44 100644 --- a/tests/example/domain_one/interface.py +++ b/tests/example/domain_one/interface.py @@ -4,3 +4,8 @@ @public(allowlist=["example.domain_two"]) def domain_one_interface(): ... + + +domain_one_var = "hello domain two" + +public(domain_one_var, allowlist=["example.domain_two"]) diff --git a/tests/example/domain_three/api.py b/tests/example/domain_three/api.py index 360146f1..cc0a9ef0 100644 --- a/tests/example/domain_three/api.py +++ b/tests/example/domain_three/api.py @@ -3,5 +3,5 @@ public(allowlist=["example.domain_two"]) -def public_for_domain_two(): +class PublicForDomainTwo: ... diff --git a/tests/example/domain_two/__init__.py b/tests/example/domain_two/__init__.py index 8a55c428..9fd26566 100644 --- a/tests/example/domain_two/__init__.py +++ b/tests/example/domain_two/__init__.py @@ -1,3 +1,8 @@ import example.domain_three.api -from ..domain_one.interface import domain_one_interface +from ..domain_one.interface import domain_one_interface, domain_one_var from ..domain_three import api + +api.PublicForDomainTwo() +example.domain_three.api.PublicForDomainTwo() +domain_one_interface() +domain_two_var = domain_one_var diff --git a/tests/test_check.py b/tests/test_check.py index b4b6a221..244471e9 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -79,7 +79,7 @@ def test_check_example_dir_end_to_end(): boundary_path="example.domain_one", ), ErrorInfo( - import_mod_path="example.domain_three.api.public_for_domain_two", + import_mod_path="example.domain_three.api.PublicForDomainTwo", location="example/__init__.py", boundary_path="example.domain_three", ), @@ -93,6 +93,11 @@ def test_check_example_dir_end_to_end(): location="example/__init__.py", boundary_path="example.domain_four.subsystem", ), + ErrorInfo( + location="example/__init__.py", + import_mod_path="example.domain_one.interface.domain_one_var", + boundary_path="example.domain_one", + ), ] check_results = check("example") diff --git a/tests/test_init.py b/tests/test_init.py index 692a25cc..a41634df 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -18,6 +18,7 @@ def init_project_from_root(root) -> None: # Change back to the original directory os.chdir(saved_directory) + @pytest.fixture(scope="module") def test_root(): # Create a temporary directory to use as the root for testing @@ -56,7 +57,6 @@ def test_init_project_with_valid_root(test_root): with open(os.path.join(test_root, file_path), "w") as f: f.write(content) - # Call init_project with the test root init_project_from_root(test_root) @@ -68,8 +68,14 @@ def test_init_project_with_valid_root(test_root): # Check if public members have been marked as expected expected_public_files = [ - ("package1/module1.py", 'import modguard\n@modguard.public\nclass Package1Class:\n pass\n'), - ("package2/module2.py", 'import modguard\n@modguard.public\nclass Package2Class:\n pass\n'), + ( + "package1/module1.py", + "import modguard\n@modguard.public\nclass Package1Class:\n pass\n", + ), + ( + "package2/module2.py", + "import modguard\n@modguard.public\nclass Package2Class:\n pass\n", + ), ] for file_path, expected_state in expected_public_files: with open(os.path.join(test_root, file_path)) as f: diff --git a/tests/test_parsing.py b/tests/test_parsing.py index bababbe9..aa6c8bec 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -23,7 +23,8 @@ def test_get_imports(): assert set(get_imports("example/__init__.py")) == { "modguard.Boundary", "example.domain_one.interface.domain_one_interface", - "example.domain_three.api.public_for_domain_two", + "example.domain_three.api.PublicForDomainTwo", "example.domain_four", "example.domain_four.subsystem.private_subsystem_call", + "example.domain_one.interface.domain_one_var", } From b876a686a198c5471b3b2a40ef28a3383c8f2903 Mon Sep 17 00:00:00 2001 From: Caelean Date: Fri, 9 Feb 2024 17:26:41 -0800 Subject: [PATCH 3/6] ruff check passing without ignore rule --- .github/workflows/ci.yml | 3 +-- tests/example/__init__.py | 10 ++++++++++ tests/example/domain_one/__init__.py | 6 +++++- tests/example/domain_three/__init__.py | 8 +++++++- tests/test_cli.py | 2 -- 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76c7d680..7c8470b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,8 +30,7 @@ jobs: python -m pip install ruff pytest pytest-mock coverage pyright - name: Check ruff run: | - # Ignore unused imports as they are needed to test modguard - ruff check --ignore F401 + ruff check . - name: Test with pytest and report coverage run: | cd tests diff --git a/tests/example/__init__.py b/tests/example/__init__.py index 8dbdec90..67a686d4 100644 --- a/tests/example/__init__.py +++ b/tests/example/__init__.py @@ -11,4 +11,14 @@ # modguard-ignore from .domain_two.other import internal_api + Boundary() + + +# Usages +domain_one_interface() +example = domain_one_var +PublicForDomainTwo() +private_subsystem_call() +example.domain_four +internal_api() \ No newline at end of file diff --git a/tests/example/domain_one/__init__.py b/tests/example/domain_one/__init__.py index dea55c34..c10090b8 100644 --- a/tests/example/domain_one/__init__.py +++ b/tests/example/domain_one/__init__.py @@ -1,4 +1,8 @@ from modguard import Boundary -from .interface import domain_one_interface +from .interface import domain_one_interface, domain_one_var Boundary() + +# Usages +domain_one_interface() +local_var = domain_one_var \ No newline at end of file diff --git a/tests/example/domain_three/__init__.py b/tests/example/domain_three/__init__.py index 5a34dcd6..808bde21 100644 --- a/tests/example/domain_three/__init__.py +++ b/tests/example/domain_three/__init__.py @@ -1,5 +1,11 @@ from modguard import Boundary from .api import public_for_domain_two -from ..domain_one.interface import domain_one_interface +from ..domain_one.interface import domain_one_interface, domain_one_var Boundary() + + +# Usages +public_for_domain_two() +domain_one_interface() +local_var =domain_one_var \ No newline at end of file diff --git a/tests/test_cli.py b/tests/test_cli.py index 45f3fba2..4996589c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,3 @@ -import io -import subprocess from unittest.mock import Mock import pytest From 088745aeee779596b03524f606472c506fee242e Mon Sep 17 00:00:00 2001 From: Caelean Date: Fri, 9 Feb 2024 17:28:37 -0800 Subject: [PATCH 4/6] more usages --- tests/example/__init__.py | 2 +- tests/example/domain_one/__init__.py | 2 +- tests/example/domain_three/__init__.py | 2 +- tests/test_check.py | 5 +++++ tests/test_parsing.py | 1 + 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/example/__init__.py b/tests/example/__init__.py index 67a686d4..e1122cbd 100644 --- a/tests/example/__init__.py +++ b/tests/example/__init__.py @@ -21,4 +21,4 @@ PublicForDomainTwo() private_subsystem_call() example.domain_four -internal_api() \ No newline at end of file +internal_api() diff --git a/tests/example/domain_one/__init__.py b/tests/example/domain_one/__init__.py index c10090b8..de94670a 100644 --- a/tests/example/domain_one/__init__.py +++ b/tests/example/domain_one/__init__.py @@ -5,4 +5,4 @@ # Usages domain_one_interface() -local_var = domain_one_var \ No newline at end of file +local_var = domain_one_var diff --git a/tests/example/domain_three/__init__.py b/tests/example/domain_three/__init__.py index 808bde21..83742da1 100644 --- a/tests/example/domain_three/__init__.py +++ b/tests/example/domain_three/__init__.py @@ -8,4 +8,4 @@ # Usages public_for_domain_two() domain_one_interface() -local_var =domain_one_var \ No newline at end of file +local_var = domain_one_var diff --git a/tests/test_check.py b/tests/test_check.py index 244471e9..05a67e33 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -98,6 +98,11 @@ def test_check_example_dir_end_to_end(): import_mod_path="example.domain_one.interface.domain_one_var", boundary_path="example.domain_one", ), + ErrorInfo( + location="example/domain_three/__init__.py", + import_mod_path="example.domain_one.interface.domain_one_var", + boundary_path="example.domain_one", + ), ] check_results = check("example") diff --git a/tests/test_parsing.py b/tests/test_parsing.py index aa6c8bec..64bdc79e 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -19,6 +19,7 @@ def test_get_imports(): assert set(get_imports("example/domain_one/__init__.py")) == { "modguard.Boundary", "example.domain_one.interface.domain_one_interface", + "example.domain_one.interface.domain_one_var", } assert set(get_imports("example/__init__.py")) == { "modguard.Boundary", From 9228f6596b0db4bdc90aae40120d93fd2e5226ce Mon Sep 17 00:00:00 2001 From: Caelean Date: Fri, 9 Feb 2024 18:00:52 -0800 Subject: [PATCH 5/6] more test cases Please enter the commit message for your changes. Lines starting --- modguard/parsing/public.py | 6 +++--- tests/test_init.py | 20 +++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/modguard/parsing/public.py b/modguard/parsing/public.py index f2aba2e3..f5d86854 100644 --- a/modguard/parsing/public.py +++ b/modguard/parsing/public.py @@ -238,7 +238,7 @@ def visit_ClassDef(self, node: ast.ClassDef): self.depth -= 1 -def _public_module_prelude(should_import: bool = True) -> str: +def _public_module_end(should_import: bool = True) -> str: if should_import: return "import modguard\nmodguard.public()\n" return "modguard.public()\n" @@ -261,8 +261,8 @@ def mark_as_public(file_path: str, member_name: str = ""): modguard_public_is_imported = is_modguard_imported(parsed_ast, "public") if not member_name: file.write( - _public_module_prelude(should_import=not modguard_public_is_imported) - + file_content + file_content + + _public_module_end(should_import=not modguard_public_is_imported) ) return diff --git a/tests/test_init.py b/tests/test_init.py index a41634df..27b65642 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -36,6 +36,7 @@ def test_init_project_with_valid_root(test_root): "package3", "package4/subpackage", "package5/subpackage", + "package6/subpackage" ] for d in test_dirs: os.makedirs(os.path.join(test_root, d)) @@ -44,13 +45,16 @@ def test_init_project_with_valid_root(test_root): # Create some mock Python files with imports and member names file_contents = { - "package1/__init__.py": "from package4.subpackage import SubPackageClass\n", - "package2/__init__.py": "from package5.subpackage import SubPackageClass\n", - "package3/__init__.py": "from package1.module1 import Package1Class\nfrom package2.module2 import Package2Class\n", - "package4/subpackage/__init__.py": "", - "package5/subpackage/__init__.py": "", + "package1/__init__.py": "from package6.subpackage.module6 import x", + "package2/__init__.py": "", "package1/module1.py": "class Package1Class:\n pass\n", - "package2/module2.py": "class Package2Class:\n pass\n", + "package2/module2.py": "def package_2_func():\n pass\n", + "package3/__init__.py": "from package1.module1 import Package1Class\nfrom package2.module2 import package_2_func\n", + "package3/module3.py": "", + "package4/subpackage/__init__.py": "", + "package5/subpackage/__init__.py": "import package3.module3", + "package6/subpackage/__init__.py": "", + "package6/subpackage/module6.py": "x = 3\n", } for file_path, content in file_contents.items(): @@ -74,8 +78,10 @@ def test_init_project_with_valid_root(test_root): ), ( "package2/module2.py", - "import modguard\n@modguard.public\nclass Package2Class:\n pass\n", + "import modguard\n@modguard.public\ndef package_2_func():\n pass\n", ), + ("package6/subpackage/module6.py","import modguard\nx = 3\nmodguard.public(x)\n",), + ('package3/module3.py', 'import modguard\nmodguard.public()\n') ] for file_path, expected_state in expected_public_files: with open(os.path.join(test_root, file_path)) as f: From fc0cc5360ba8f82cf79ae18872bf586db69fe29e Mon Sep 17 00:00:00 2001 From: Caelean Date: Mon, 12 Feb 2024 09:09:46 -0800 Subject: [PATCH 6/6] remove math.random usage, update types --- tests/example/__init__.py | 2 +- tests/example/domain_four/subsystem.py | 3 +-- tests/example/domain_three/__init__.py | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/example/__init__.py b/tests/example/__init__.py index e1122cbd..9cea5708 100644 --- a/tests/example/__init__.py +++ b/tests/example/__init__.py @@ -17,7 +17,7 @@ # Usages domain_one_interface() -example = domain_one_var +example_usage = domain_one_var PublicForDomainTwo() private_subsystem_call() example.domain_four diff --git a/tests/example/domain_four/subsystem.py b/tests/example/domain_four/subsystem.py index 8d89ff33..805c1ca8 100644 --- a/tests/example/domain_four/subsystem.py +++ b/tests/example/domain_four/subsystem.py @@ -1,8 +1,7 @@ import modguard -import math modguard.Boundary() def private_subsystem_call(): - print(math.random()) + ... diff --git a/tests/example/domain_three/__init__.py b/tests/example/domain_three/__init__.py index 83742da1..039022cd 100644 --- a/tests/example/domain_three/__init__.py +++ b/tests/example/domain_three/__init__.py @@ -1,11 +1,9 @@ from modguard import Boundary -from .api import public_for_domain_two from ..domain_one.interface import domain_one_interface, domain_one_var Boundary() # Usages -public_for_domain_two() domain_one_interface() local_var = domain_one_var