From 6dbe59f7ffd2857ae66196cbc966fd1f03f38b0e Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 31 Mar 2023 09:40:12 -0500 Subject: [PATCH 1/4] Addon Manager: Black and pylint cleanup --- src/Mod/AddonManager/Addon.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Mod/AddonManager/Addon.py b/src/Mod/AddonManager/Addon.py index 24cba65223..e631f717d9 100644 --- a/src/Mod/AddonManager/Addon.py +++ b/src/Mod/AddonManager/Addon.py @@ -593,7 +593,7 @@ class Addon: "The existence of this file prevents FreeCAD from loading this Addon. To re-enable, delete the file." ) - if (self.contains_workbench()): + if self.contains_workbench(): self.disable_workbench() def enable(self): @@ -605,7 +605,7 @@ class Addon: except FileNotFoundError: pass - if (self.contains_workbench()): + if self.contains_workbench(): self.enable_workbench() def enable_workbench(self): @@ -614,7 +614,6 @@ class Addon: # Remove from the list of disabled. self.remove_from_disabled_wbs(wbName) - def disable_workbench(self): pref = fci.ParamGet("User parameter:BaseApp/Preferences/Workbenches") wbName = self.get_workbench_name() @@ -628,7 +627,6 @@ class Addon: pref.SetString("Disabled", disabled_wbs) # print(f"done disabling : {disabled_wbs} \n") - def desinstall_workbench(self): pref = fci.ParamGet("User parameter:BaseApp/Preferences/Workbenches") wbName = self.get_workbench_name() @@ -649,7 +647,6 @@ class Addon: # Remove from the list of disabled. self.remove_from_disabled_wbs(wbName) - def remove_from_disabled_wbs(self, wbName: str): pref = fci.ParamGet("User parameter:BaseApp/Preferences/Workbenches") @@ -665,13 +662,15 @@ class Addon: pref.SetString("Disabled", disabled_wbs) # print(f"Done enabling {disabled_wbs} \n") - def get_workbench_name(self) -> str: - """Find the name of the workbench class (ie the name under which it's registered in freecad core)')""" + """Find the name of the workbench class (ie the name under which it's + registered in freecad core)'""" wb_name = "" if self.repo_type == Addon.Kind.PACKAGE: - for wb in self.metadata.content["workbench"]: # we may have more than one wb. + for wb in self.metadata.content[ + "workbench" + ]: # we may have more than one wb. if wb_name != "": wb_name += "," wb_name += wb.classname @@ -681,7 +680,6 @@ class Addon: wb_name = self.name return wb_name - def try_find_wbname_in_files(self) -> str: wb_name = "" filesInDir = [] @@ -704,11 +702,12 @@ class Addon: if m: wb_name = m.group(1) break - except Exception as e: + except OSError as e: continue # print(f"Found name {wb_name} \n") return wb_name + # @dataclass(frozen) class MissingDependencies: """Encapsulates a group of four types of dependencies: @@ -771,7 +770,9 @@ class MissingDependencies: translate( "AddonsInstaller", "Got an error when trying to import {}", - ).format(py_dep) + ":\n" + str(e) + ).format(py_dep) + + ":\n" + + str(e) ) self.python_optional = [] @@ -785,7 +786,9 @@ class MissingDependencies: translate( "AddonsInstaller", "Got an error when trying to import {}", - ).format(py_dep) + ":\n" + str(e) + ).format(py_dep) + + ":\n" + + str(e) ) self.wbs.sort() From 5e9b2ac91a5fe2edbe0f6d7fa55e88848c71376f Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 31 Mar 2023 09:40:39 -0500 Subject: [PATCH 2/4] Addon Manager: Tests for classname finder --- .../AddonManagerTest/app/test_addon.py | 98 ++++++++++++++++++- 1 file changed, 93 insertions(+), 5 deletions(-) diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py b/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py index 8ba6e8e447..dd5956bcf0 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py @@ -20,7 +20,7 @@ # * . * # * * # *************************************************************************** - +import tempfile import unittest import os import sys @@ -32,7 +32,6 @@ from addonmanager_macro import Macro class TestAddon(unittest.TestCase): - MODULE = "test_addon" # file name without extension def setUp(self): @@ -41,7 +40,6 @@ class TestAddon(unittest.TestCase): ) def test_display_name(self): - # Case 1: No display name set elsewhere: name == display_name addon = Addon( "FreeCAD", @@ -84,7 +82,6 @@ class TestAddon(unittest.TestCase): self.assertEqual(expected_tags, tags) def test_contains_functions(self): - # Test package.xml combinations: # Workbenches @@ -201,7 +198,6 @@ class TestAddon(unittest.TestCase): self.assertTrue(addon.__dict__, second_addon.__dict__) def test_dependency_resolution(self): - addonA = Addon( "AddonA", "https://github.com/FreeCAD/FakeAddonA", @@ -303,3 +299,95 @@ class TestAddon(unittest.TestCase): "TagC" in addon.tags, "Found 'TagA' in tags, it should have been excluded by version requirement", ) + + def test_try_find_wbname_in_files_empty_dir(self): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + os.mkdir(os.path.join(mod_dir, test_addon.name)) + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "") + + def test_try_find_wbname_in_files_non_python_ignored(self): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + base_path = os.path.join(mod_dir, test_addon.name) + os.mkdir(base_path) + file_path = os.path.join(base_path, "test.txt") + with open(file_path, "w", encoding="utf-8") as f: + f.write("Gui.addWorkbench(TestWorkbench())") + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "") + + def test_try_find_wbname_in_files_simple(self): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + base_path = os.path.join(mod_dir, test_addon.name) + os.mkdir(base_path) + file_path = os.path.join(base_path, "test.py") + with open(file_path, "w", encoding="utf-8") as f: + f.write("Gui.addWorkbench(TestWorkbench())") + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "TestWorkbench") + + def test_try_find_wbname_in_files_variable_used(self): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + base_path = os.path.join(mod_dir, test_addon.name) + os.mkdir(base_path) + file_path = os.path.join(base_path, "test.py") + with open(file_path, "w", encoding="utf-8") as f: + f.write("Gui.addWorkbench(wb)") + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "") + + def test_try_find_wbname_in_files_variants(self): + variants = [ + "Gui.addWorkbench(TestWorkbench())", + "Gui.addWorkbench (TestWorkbench())", + "Gui.addWorkbench( TestWorkbench() )", + "Gui.addWorkbench(TestWorkbench( ))", + "Gui.addWorkbench( TestWorkbench( ) )", + "Gui.addWorkbench( TestWorkbench ( ) )", + "Gui.addWorkbench ( TestWorkbench ( ) )", + ] + for variant in variants: + with self.subTest(variant=variant): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + base_path = os.path.join(mod_dir, test_addon.name) + os.mkdir(base_path) + file_path = os.path.join(base_path, "test.py") + with open(file_path, "w", encoding="utf-8") as f: + f.write(variant) + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "TestWorkbench") From e604c3742f32c25937be40e98d851b433b22aa67 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 31 Mar 2023 10:13:47 -0500 Subject: [PATCH 3/4] Addon Manager: Modify regex for whitespace --- src/Mod/AddonManager/Addon.py | 49 +++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/Mod/AddonManager/Addon.py b/src/Mod/AddonManager/Addon.py index e631f717d9..adb896cd27 100644 --- a/src/Mod/AddonManager/Addon.py +++ b/src/Mod/AddonManager/Addon.py @@ -681,31 +681,36 @@ class Addon: return wb_name def try_find_wbname_in_files(self) -> str: - wb_name = "" - filesInDir = [] - + """Attempt to locate a line with an addWorkbench command in the workbench's + Python files. If it is directly instantiating a workbench, then we can use + the line to determine classname for this workbench. If it uses a variable, + or if the line doesn't exist at all, an empty string is returned.""" mod_dir = os.path.join(self.mod_directory, self.name) - for root, subdirs, files in os.walk(mod_dir): + for root, _, files in os.walk(mod_dir): for f in files: - if not os.path.isdir(os.path.join(root, f).replace("\\", "/")): - filesInDir.append(os.path.join(root, f).replace("\\", "/")) - if filesInDir: - for currentfile in filesInDir: - filename, extension = os.path.splitext(currentfile) - if extension == ".py": - # print(f"Current file: {currentfile} ") - try: - with open(os.path.join(root, currentfile), "r") as f: - content = f.read() - m = re.search("Gui.addWorkbench\((\w+)\(\)\)", content) - if m: - wb_name = m.group(1) - break - except OSError as e: - continue - # print(f"Found name {wb_name} \n") - return wb_name + current_file = os.path.join(root, f) + if not os.path.isdir(current_file): + filename, extension = os.path.splitext(current_file) + if extension == ".py": + wb_classname = self._find_classname_in_file(current_file) + print(f"Current file: {current_file} ") + if wb_classname: + print(f"Found name {wb_classname} \n") + return wb_classname + return "" + + @staticmethod + def _find_classname_in_file(current_file) -> str: + try: + with open(current_file, "r", encoding="utf-8") as python_file: + content = python_file.read() + search_result = re.search(r"Gui.addWorkbench\s*\(\s*(\w+)\s*\(\s*\)\s*\)", content) + if search_result: + return search_result.group(1) + except OSError: + pass + return "" # @dataclass(frozen) From fb0bcc4968fdb7c2cf5344aadbe71f927f175e34 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 31 Mar 2023 10:21:12 -0500 Subject: [PATCH 4/4] Addon Manager: Add test for subdirectory --- .../AddonManagerTest/app/test_addon.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py b/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py index dd5956bcf0..b7e0ff4149 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_addon.py @@ -347,6 +347,25 @@ class TestAddon(unittest.TestCase): # Assert self.assertEqual(wb_name, "TestWorkbench") + def test_try_find_wbname_in_files_subdir(self): + with tempfile.TemporaryDirectory() as mod_dir: + # Arrange + test_addon = Addon("test") + test_addon.mod_directory = mod_dir + base_path = os.path.join(mod_dir, test_addon.name) + os.mkdir(base_path) + subdir = os.path.join(base_path, "subdirectory") + os.mkdir(subdir) + file_path = os.path.join(subdir, "test.py") + with open(file_path, "w", encoding="utf-8") as f: + f.write("Gui.addWorkbench(TestWorkbench())") + + # Act + wb_name = test_addon.try_find_wbname_in_files() + + # Assert + self.assertEqual(wb_name, "TestWorkbench") + def test_try_find_wbname_in_files_variable_used(self): with tempfile.TemporaryDirectory() as mod_dir: # Arrange