From 5f45b0007876cc0cc1abc6482bde1db9b28de79d Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sun, 18 Dec 2022 20:45:09 -0600 Subject: [PATCH] AddonManager: Refactor uninstaller GUI Offload uninstaller GUI into its own class, add tests for that class, and do some additional minor cleanup of AddonManager.py. --- src/Mod/AddonManager/AddonManager.py | 222 ++++++------------ .../AddonManagerTest/app/mocks.py | 16 +- .../app/test_dependency_installer.py | 2 +- .../AddonManagerTest/app/test_installer.py | 7 +- .../AddonManagerTest/app/test_macro.py | 22 +- .../AddonManagerTest/app/test_uninstaller.py | 11 +- .../AddonManagerTest/gui/gui_mocks.py | 80 +++++++ .../gui/test_installer_gui.py | 69 +----- .../gui/test_uninstaller_gui.py | 148 ++++++++++++ src/Mod/AddonManager/CMakeLists.txt | 10 +- src/Mod/AddonManager/TestAddonManagerGui.py | 4 + src/Mod/AddonManager/addonmanager_devmode.py | 4 +- .../AddonManager/addonmanager_installer.py | 9 + .../AddonManager/addonmanager_uninstaller.py | 7 +- .../addonmanager_uninstaller_gui.py | 145 ++++++++++++ 15 files changed, 516 insertions(+), 240 deletions(-) create mode 100644 src/Mod/AddonManager/AddonManagerTest/gui/test_uninstaller_gui.py create mode 100644 src/Mod/AddonManager/addonmanager_uninstaller_gui.py diff --git a/src/Mod/AddonManager/AddonManager.py b/src/Mod/AddonManager/AddonManager.py index 16ebdd6ceb..4347f2a7d7 100644 --- a/src/Mod/AddonManager/AddonManager.py +++ b/src/Mod/AddonManager/AddonManager.py @@ -25,7 +25,6 @@ import os import functools -import stat import tempfile import hashlib import threading @@ -49,9 +48,10 @@ from addonmanager_workers_installation import ( UpdateMetadataCacheWorker, ) from addonmanager_installer_gui import AddonInstallerGUI, MacroInstallerGUI +from addonmanager_uninstaller_gui import AddonUninstallerGUI from addonmanager_update_all_gui import UpdateAllGUI import addonmanager_utilities as utils -import AddonManager_rc +import AddonManager_rc # This is required by Qt, it's not unused from package_list import PackageList, PackageListItemModel from package_details import PackageDetails from Addon import Addon @@ -130,6 +130,10 @@ class CommandAddonManager: self.developer_mode = None self.installer_gui = None + self.update_cache = False + self.dialog = None + self.startup_sequence = [] + # Set up the connection checker self.connection_checker = ConnectionCheckerGUI() self.connection_checker.connection_available.connect(self.launch) @@ -272,14 +276,12 @@ class CommandAddonManager: # set the label text to start with self.show_information(translate("AddonsInstaller", "Loading addon information")) - if hasattr(self, "connection_check_message") and self.connection_check_message: - self.connection_check_message.close() - # rock 'n roll!!! self.dialog.exec() def cleanup_workers(self) -> None: - """Ensure that no workers are running by explicitly asking them to stop and waiting for them until they do""" + """Ensure that no workers are running by explicitly asking them to stop and waiting for + them until they do""" for worker in self.workers: if hasattr(self, worker): thread = getattr(self, worker) @@ -305,16 +307,15 @@ class CommandAddonManager: """Determine whether we need to update the cache, based on user preference, and previous cache update status. Sets self.update_cache to either True or False.""" - # Figure out our cache update frequency: there is a combo box in the preferences dialog with three - # options: never, daily, and weekly. Check that first, but allow it to be overridden by a more specific - # DaysBetweenUpdates selection, if the user has provided it. For that parameter we use: + # Figure out our cache update frequency: there is a combo box in the preferences dialog + # with three options: never, daily, and weekly. Check that first, but allow it to be + # overridden by a more specific DaysBetweenUpdates selection, if the user has provided it. + # For that parameter we use: # -1: Only manual updates (default) # 0: Update every launch # >0: Update every n days pref = FreeCAD.ParamGet("User parameter:BaseApp/Preferences/Addons") self.update_cache = False - if hasattr(self, "trigger_recache") and self.trigger_recache: - self.update_cache = True update_frequency = pref.GetInt("UpdateFrequencyComboEntry", 0) if update_frequency == 0: days_between_updates = -1 @@ -421,7 +422,8 @@ class CommandAddonManager: else: self.write_cache_stopfile() FreeCAD.Console.PrintLog( - "Not writing the cache because a process was forcibly terminated and the state is unknown.\n" + "Not writing the cache because a process was forcibly terminated and the state is " + "unknown.\n" ) if self.restart_required: @@ -450,29 +452,30 @@ class CommandAddonManager: def startup(self) -> None: """Downloads the available packages listings and populates the table - This proceeds in four stages: first, the main GitHub repository is queried for a list of possible - addons. Each addon is specified as a git submodule with name and branch information. The actual specific - commit ID of the submodule (as listed on Github) is ignored. Any extra repositories specified by the - user are appended to this list. + This proceeds in four stages: first, the main GitHub repository is queried for a list of + possible addons. Each addon is specified as a git submodule with name and branch + information. The actual specific commit ID of the submodule (as listed on Github) is + ignored. Any extra repositories specified by the user are appended to this list. - Second, the list of macros is downloaded from the FreeCAD/FreeCAD-macros repository and the wiki + Second, the list of macros is downloaded from the FreeCAD/FreeCAD-macros repository and + the wiki. - Third, each of these items is queried for a package.xml metadata file. If that file exists it is - downloaded, cached, and any icons that it references are also downloaded and cached. + Third, each of these items is queried for a package.xml metadata file. If that file exists + it is downloaded, cached, and any icons that it references are also downloaded and cached. - Finally, for workbenches that are not contained within a package (e.g. they provide no metadata), an - additional git query is made to see if an update is available. Macros are checked for file changes. + Finally, for workbenches that are not contained within a package (e.g. they provide no + metadata), an additional git query is made to see if an update is available. Macros are + checked for file changes. - Each of these stages is launched in a separate thread to ensure that the UI remains responsive, and - the operation can be cancelled. + Each of these stages is launched in a separate thread to ensure that the UI remains + responsive, and the operation can be cancelled. - Each stage is also subject to caching, so may return immediately, if no cache update has been requested. + Each stage is also subject to caching, so may return immediately, if no cache update has + been requested.""" - """ - - # Each function in this list is expected to launch a thread and connect its completion signal - # to self.do_next_startup_phase, or to shortcut to calling self.do_next_startup_phase if it - # is not launching a worker + # Each function in this list is expected to launch a thread and connect its completion + # signal to self.do_next_startup_phase, or to shortcut to calling + # self.do_next_startup_phase if it is not launching a worker self.startup_sequence = [ self.populate_packages_table, self.activate_table_widgets, @@ -518,7 +521,9 @@ class CommandAddonManager: use_cache = not self.update_cache if use_cache: if os.path.isfile(utils.get_cache_file_name("package_cache.json")): - with open(utils.get_cache_file_name("package_cache.json")) as f: + with open( + utils.get_cache_file_name("package_cache.json"), encoding="utf-8" + ) as f: data = f.read() try: from_json = json.loads(data) @@ -530,7 +535,10 @@ class CommandAddonManager: use_cache = False if not use_cache: - self.update_cache = True # Make sure to trigger the other cache updates, if the json file was missing + self.update_cache = ( + True # Make sure to trigger the other cache updates, if the json + ) + # file was missing self.create_addon_list_worker = CreateAddonListWorker() self.create_addon_list_worker.status_message.connect(self.show_information) self.create_addon_list_worker.addon_repo.connect(self.add_addon_repo) @@ -558,7 +566,7 @@ class CommandAddonManager: def write_package_cache(self): if hasattr(self, "package_cache"): package_cache_path = utils.get_cache_file_name("package_cache.json") - with open(package_cache_path, "w") as f: + with open(package_cache_path, "w", encoding="utf-8") as f: f.write(json.dumps(self.package_cache, indent=" ")) def activate_table_widgets(self) -> None: @@ -575,7 +583,10 @@ class CommandAddonManager: cache_is_bad = False if cache_is_bad: if not self.update_cache: - self.update_cache = True # Make sure to trigger the other cache updates, if the json file was missing + self.update_cache = ( + True # Make sure to trigger the other cache updates, if the + ) + # json file was missing self.create_addon_list_worker = CreateAddonListWorker() self.create_addon_list_worker.status_message.connect( self.show_information @@ -587,7 +598,8 @@ class CommandAddonManager: ) # Link to step 2 self.create_addon_list_worker.start() else: - # It's already been done in the previous step (TODO: Refactor to eliminate this step) + # It's already been done in the previous step (TODO: Refactor to eliminate this + # step) self.do_next_startup_phase() else: self.macro_worker = LoadMacrosFromCacheWorker( @@ -611,7 +623,7 @@ class CommandAddonManager: if not hasattr(self, "macro_cache"): return macro_cache_path = utils.get_cache_file_name("macro_cache.json") - with open(macro_cache_path, "w") as f: + with open(macro_cache_path, "w", encoding="utf-8") as f: f.write(json.dumps(self.macro_cache, indent=" ")) self.macro_cache = [] @@ -791,7 +803,8 @@ class CommandAddonManager: addon_repo.icon = self.get_icon(addon_repo) for repo in self.item_model.repos: if repo.name == addon_repo.name: - # self.item_model.reload_item(repo) # If we want to have later additions supersede earlier + # self.item_model.reload_item(repo) # If we want to have later additions superseded + # earlier return self.item_model.append_item(addon_repo) @@ -899,7 +912,7 @@ class CommandAddonManager: self.installer_gui = MacroInstallerGUI(addon) else: self.installer_gui = AddonInstallerGUI(addon, self.item_model.repos) - self.installer_gui.success.connect(self.on_package_installed) + self.installer_gui.success.connect(self.on_package_status_changed) self.installer_gui.finished.connect(self.cleanup_installer) self.installer_gui.run() # Does not block @@ -923,7 +936,7 @@ class CommandAddonManager: return self.installer_gui = UpdateAllGUI(self.item_model.repos) - self.installer_gui.addon_updated.connect(self.on_package_installed) + self.installer_gui.addon_updated.connect(self.on_package_status_changed) self.installer_gui.finished.connect(self.cleanup_installer) self.installer_gui.run() # Does not block @@ -979,12 +992,9 @@ class CommandAddonManager: "AddonManager recaches." ) - def on_package_installed(self, repo: Addon) -> None: - if repo.contains_workbench(): - repo.set_status(Addon.Status.PENDING_RESTART) + def on_package_status_changed(self, repo: Addon) -> None: + if repo.status() == Addon.Status.PENDING_RESTART: self.restart_required = True - else: - repo.set_status(Addon.Status.NO_UPDATE_AVAILABLE) self.item_model.reload_item(repo) self.packageDetails.show_repo(repo) if repo in self.packages_with_updates: @@ -1011,116 +1021,28 @@ class CommandAddonManager: "AddonsInstaller", "Execution of macro failed. See console for failure details.", ) - self.on_installation_failed(repo, message) return - else: - macro_path = os.path.join(dir, macro.filename) - FreeCADGui.open(str(macro_path)) - self.dialog.hide() - FreeCADGui.SendMsgToActiveView("Run") + macro_path = os.path.join(dir, macro.filename) + FreeCADGui.open(str(macro_path)) + self.dialog.hide() + FreeCADGui.SendMsgToActiveView("Run") - def remove(self, repo: Addon) -> None: - """uninstalls a macro or workbench""" - - confirm = QtWidgets.QMessageBox.question( - self.dialog, - translate("AddonsInstaller", "Confirm remove"), - translate( - "AddonsInstaller", "Are you sure you want to uninstall {}?" - ).format(repo.display_name), - QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.Cancel, - ) - if confirm == QtWidgets.QMessageBox.Cancel: + def remove(self, addon: Addon) -> None: + """Remove this addon.""" + if self.installer_gui is not None: + FreeCAD.Console.PrintError( + translate( + "AddonsInstaller", + "Cannot launch a new installer until the previous one has finished.", + ) + ) return - - if ( - repo.repo_type == Addon.Kind.WORKBENCH - or repo.repo_type == Addon.Kind.PACKAGE - ): - basedir = FreeCAD.getUserAppDataDir() - moddir = basedir + os.sep + "Mod" - clonedir = moddir + os.sep + repo.name - - # First remove any macros that were copied or symlinked in, as long as they have not been modified - macro_dir = FreeCAD.getUserMacroDir(True) - if os.path.exists(macro_dir) and os.path.exists(clonedir): - for macro_filename in os.listdir(clonedir): - if macro_filename.lower().endswith(".fcmacro"): - mod_macro_path = os.path.join(clonedir, macro_filename) - macro_path = os.path.join(macro_dir, macro_filename) - - if not os.path.isfile(macro_path): - continue - - # Load both files (one may be a symlink of the other, this will still work in that case) - with open(mod_macro_path) as f1: - f1_contents = f1.read() - with open(macro_path) as f2: - f2_contents = f2.read() - - if f1_contents == f2_contents: - os.remove(macro_path) - else: - FreeCAD.Console.PrintMessage( - translate( - "AddonsInstaller", - "Macro {} has local changes in the macros directory, so is not being removed by this uninstall process.\n", - ).format(macro_filename) - ) - - # Second, run the Addon's "uninstall.py" script, if it exists - uninstall_script = os.path.join(clonedir, "uninstall.py") - if os.path.exists(uninstall_script): - try: - with open(uninstall_script) as f: - exec(f.read()) - except Exception: - FreeCAD.Console.PrintError( - translate( - "AddonsInstaller", - "Execution of Addon's uninstall.py script failed. Proceeding with uninstall...", - ) - + "\n" - ) - - if os.path.exists(clonedir): - utils.rmdir(clonedir) - self.item_model.update_item_status( - repo.name, Addon.Status.NOT_INSTALLED - ) - if repo.contains_workbench(): - self.restart_required = True - self.packageDetails.show_repo(repo) - else: - self.dialog.textBrowserReadMe.setText( - translate( - "AddonsInstaller", - "Unable to remove this addon with the Addon Manager.", - ) - ) - - elif repo.repo_type == Addon.Kind.MACRO: - macro = repo.macro - if macro.remove(): - # TODO: reimplement when refactored... remove_custom_toolbar_button(repo) - FreeCAD.Console.PrintMessage( - translate("AddonsInstaller", "Successfully uninstalled {}").format( - repo.name - ) - + "\n" - ) - self.item_model.update_item_status( - repo.name, Addon.Status.NOT_INSTALLED - ) - self.packageDetails.show_repo(repo) - else: - FreeCAD.Console.PrintMessage( - translate( - "AddonsInstaller", - "Failed to uninstall {}. Please remove manually.", - ).format(repo.name) - + "\n" - ) + self.installer_gui = AddonUninstallerGUI(addon) + self.installer_gui.finished.connect(self.cleanup_installer) + self.installer_gui.finished.connect( + functools.partial(self.on_package_status_changed, addon) + ) + self.installer_gui.run() # Does not block # @} diff --git a/src/Mod/AddonManager/AddonManagerTest/app/mocks.py b/src/Mod/AddonManager/AddonManagerTest/app/mocks.py index 7a878be6b7..c6e4f9609f 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/mocks.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/mocks.py @@ -25,15 +25,25 @@ import os import shutil +import FreeCAD + class MockAddon: """Minimal Addon class""" def __init__(self): - self.name = "TestAddon" - self.url = "https://github.com/FreeCAD/FreeCAD-addons" - self.branch = "master" + test_dir = os.path.join( + FreeCAD.getHomePath(), "Mod", "AddonManager", "AddonManagerTest", "data" + ) + self.name = "MockAddon" + self.display_name = "Mock Addon" + self.url = os.path.join(test_dir, "test_simple_repo.zip") + self.branch = "main" self.macro = None + self.status = None + + def set_status(self, status): + self.status = status class MockMacro: diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py b/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py index 749e581bb3..492702abba 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py @@ -33,7 +33,7 @@ from addonmanager_dependency_installer import DependencyInstaller class CompleteProcessMock(subprocess.CompletedProcess): def __init__(self): super().__init__(["fake_arg"], 0) - self.stdout = "Mock suprocess call stdout result" + self.stdout = "Mock subprocess call stdout result" class SubprocessMock: diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_installer.py b/src/Mod/AddonManager/AddonManagerTest/app/test_installer.py index 30bbda80d3..bfdd89354d 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_installer.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_installer.py @@ -115,7 +115,7 @@ class TestAddonInstaller(unittest.TestCase): with tempfile.TemporaryDirectory() as temp_dir: installer.installation_path = temp_dir installer._update_metadata() - addon_dir = os.path.join(temp_dir, self.mock_addon.name) + addon_dir = os.path.join(temp_dir, self.real_addon.name) os.mkdir(addon_dir) shutil.copy( os.path.join(self.test_data_dir, "good_package.xml"), @@ -125,7 +125,7 @@ class TestAddonInstaller(unittest.TestCase): installer._update_metadata() self.assertEqual(self.real_addon.installed_version, good_metadata.Version) - def test_finalize_zip_installation(self): + def test_finalize_zip_installation_non_github(self): """Ensure that zipfiles are correctly extracted.""" with tempfile.TemporaryDirectory() as temp_dir: test_simple_repo = os.path.join(self.test_data_dir, "test_simple_repo.zip") @@ -140,12 +140,15 @@ class TestAddonInstaller(unittest.TestCase): os.path.isfile(expected_location), "Non-GitHub zip extraction failed" ) + def test_finalize_zip_installation_github(self): with tempfile.TemporaryDirectory() as temp_dir: test_github_style_repo = os.path.join( self.test_data_dir, "test_github_style_repo.zip" ) installer = AddonInstaller(self.mock_addon, []) installer.installation_path = temp_dir + self.mock_addon.url = test_github_style_repo + self.mock_addon.branch = "master" installer._finalize_zip_installation(test_github_style_repo) expected_location = os.path.join(temp_dir, self.mock_addon.name, "README") self.assertTrue( diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_macro.py b/src/Mod/AddonManager/AddonManagerTest/app/test_macro.py index 21fdf2aab3..1a3cf89ae9 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_macro.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_macro.py @@ -184,19 +184,22 @@ static char * blarg_xpm[] = { return m def test_fetch_raw_code_no_data(self): - - class MockNetworkManagerNoData(): + class MockNetworkManagerNoData: def __init__(self): self.fetched_url = None + def blocking_get(self, url): self.fetched_url = url return None + nmNoData = MockNetworkManagerNoData() m = Macro("Unit Test Macro") Macro.network_manager = nmNoData - returned_data = m._fetch_raw_code("rawcodeurl Totally fake") + returned_data = m._fetch_raw_code( + 'rawcodeurl Totally fake' + ) self.assertIsNone(returned_data) - self.assertEqual(nmNoData.fetched_url,"https://fake_url.com") + self.assertEqual(nmNoData.fetched_url, "https://fake_url.com") nmNoData.fetched_url = None returned_data = m._fetch_raw_code("Fake pagedata with no URL at all.") @@ -206,13 +209,14 @@ static char * blarg_xpm[] = { Macro.network_manager = None def test_fetch_raw_code_with_data(self): - - class MockNetworkManagerWithData(): + class MockNetworkManagerWithData: class MockQByteArray: def data(self): return "Data returned to _fetch_raw_code".encode("utf-8") + def __init__(self): self.fetched_url = None + def blocking_get(self, url): self.fetched_url = url return MockNetworkManagerWithData.MockQByteArray() @@ -220,7 +224,9 @@ static char * blarg_xpm[] = { nmWithData = MockNetworkManagerWithData() m = Macro("Unit Test Macro") Macro.network_manager = nmWithData - returned_data = m._fetch_raw_code("rawcodeurl Totally fake") - self.assertEqual(returned_data,"Data returned to _fetch_raw_code") + returned_data = m._fetch_raw_code( + 'rawcodeurl Totally fake' + ) + self.assertEqual(returned_data, "Data returned to _fetch_raw_code") Macro.network_manager = None diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_uninstaller.py b/src/Mod/AddonManager/AddonManagerTest/app/test_uninstaller.py index 6c775136ad..d1305b2658 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_uninstaller.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_uninstaller.py @@ -449,10 +449,15 @@ class TestMacroUninstaller(unittest.TestCase): self.assertTrue(os.path.exists(f)) os.chmod(f, S_IREAD | S_IRGRP | S_IROTH) self.test_object.run() - os.chmod(f, S_IWUSR | S_IREAD) - self.assertIn("failure", self.signals_caught) - self.assertNotIn("success", self.signals_caught) + if os.path.exists(f): + os.chmod(f, S_IWUSR | S_IREAD) + self.assertNotIn("success", self.signals_caught) + self.assertIn("failure", self.signals_caught) + else: + # In some cases we managed to delete it anyway: + self.assertIn("success", self.signals_caught) + self.assertNotIn("failure", self.signals_caught) self.assertIn("finished", self.signals_caught) def test_cleanup_directories_multiple_empty(self): diff --git a/src/Mod/AddonManager/AddonManagerTest/gui/gui_mocks.py b/src/Mod/AddonManager/AddonManagerTest/gui/gui_mocks.py index 7a1d4979ae..4a4f107d19 100644 --- a/src/Mod/AddonManager/AddonManagerTest/gui/gui_mocks.py +++ b/src/Mod/AddonManager/AddonManagerTest/gui/gui_mocks.py @@ -20,3 +20,83 @@ # * * # *************************************************************************** +from PySide import QtCore, QtWidgets + + +class DialogWatcher(QtCore.QObject): + """Examine the running GUI and look for a modal dialog with a given title, containing a button + with a given label. Click that button, which is expected to close the dialog. Generally run on + a one-shot QTimer to allow the dialog time to open up.""" + + def __init__(self, dialog_to_watch_for, button): + super().__init__() + self.dialog_found = False + self.has_run = False + self.dialog_to_watch_for = dialog_to_watch_for + self.button = button + + def run(self): + widget = QtWidgets.QApplication.activeModalWidget() + if widget: + # Is this the widget we are looking for? + if ( + hasattr(widget, "windowTitle") + and callable(widget.windowTitle) + and widget.windowTitle() == self.dialog_to_watch_for + ): + # Found the dialog we are looking for: now try to "click" the appropriate button + self.click_button(widget) + self.dialog_found = True + self.has_run = True + + def click_button(self, widget): + buttons = widget.findChildren(QtWidgets.QPushButton) + for button in buttons: + text = button.text().replace("&", "") + if text == self.button: + button.click() + + +class DialogInteractor(DialogWatcher): + def __init__(self, dialog_to_watch_for, interaction): + """Takes the title of the dialog, a button string, and a callable. The callable is passed + the widget we found and can do whatever it wants to it. Whatever it does should eventually + close the dialog, however.""" + super().__init__(dialog_to_watch_for, None) + self.interaction = interaction + + def run(self): + widget = QtWidgets.QApplication.activeModalWidget() + if widget: + # Is this the widget we are looking for? + if ( + hasattr(widget, "windowTitle") + and callable(widget.windowTitle) + and widget.windowTitle() == self.dialog_to_watch_for + ): + self.dialog_found = True + if self.dialog_found: + self.has_run = True + if self.interaction is not None and callable(self.interaction): + self.interaction(widget) + + +class FakeWorker: + def __init__(self): + self.called = False + self.should_continue = True + + def work(self): + while self.should_continue: + QtCore.QCoreApplication.processEvents(QtCore.QEventLoop.AllEvents, 100) + + def stop(self): + self.should_continue = False + + +class MockThread: + def wait(self): + pass + + def isRunning(self): + return False diff --git a/src/Mod/AddonManager/AddonManagerTest/gui/test_installer_gui.py b/src/Mod/AddonManager/AddonManagerTest/gui/test_installer_gui.py index 38394db6c1..ba7e00fbc8 100644 --- a/src/Mod/AddonManager/AddonManagerTest/gui/test_installer_gui.py +++ b/src/Mod/AddonManager/AddonManagerTest/gui/test_installer_gui.py @@ -19,6 +19,7 @@ # * . * # * * # *************************************************************************** + import os import tempfile import unittest @@ -28,78 +29,18 @@ from PySide import QtCore, QtWidgets from addonmanager_installer_gui import AddonInstallerGUI, MacroInstallerGUI +from AddonManagerTest.gui.gui_mocks import DialogWatcher, DialogInteractor +from AddonManagerTest.app.mocks import MockAddon + translate = FreeCAD.Qt.translate -class DialogWatcher(QtCore.QObject): - def __init__(self, dialog_to_watch_for, button): - super().__init__() - self.dialog_found = False - self.has_run = False - self.dialog_to_watch_for = dialog_to_watch_for - self.button = button - - def run(self): - widget = QtWidgets.QApplication.activeModalWidget() - if widget: - # Is this the widget we are looking for? - if ( - hasattr(widget, "windowTitle") - and callable(widget.windowTitle) - and widget.windowTitle() == self.dialog_to_watch_for - ): - # Found the dialog we are looking for: now try to "click" the appropriate button - self.click_button(widget) - self.dialog_found = True - self.has_run = True - - def click_button(self, widget): - buttons = widget.findChildren(QtWidgets.QPushButton) - for button in buttons: - text = button.text().replace("&", "") - if text == self.button: - button.click() - - -class DialogInteractor(DialogWatcher): - def __init__(self, dialog_to_watch_for, interaction): - """Takes the title of the dialog, a button string, and a callable.""" - super().__init__(dialog_to_watch_for, None) - self.interaction = interaction - - def run(self): - widget = QtWidgets.QApplication.activeModalWidget() - if widget: - # Is this the widget we are looking for? - if ( - hasattr(widget, "windowTitle") - and callable(widget.windowTitle) - and widget.windowTitle() == self.dialog_to_watch_for - ): - # Found the dialog we are looking for: now try to "click" the appropriate button - self.dialog_found = True - if self.dialog_found: - self.has_run = True - if self.interaction is not None and callable(self.interaction): - self.interaction(widget) - - class TestInstallerGui(unittest.TestCase): MODULE = "test_installer_gui" # file name without extension - class MockAddon: - def __init__(self): - test_dir = os.path.join( - FreeCAD.getHomePath(), "Mod", "AddonManager", "AddonManagerTest", "data" - ) - self.name = "MockAddon" - self.display_name = "Mock Addon" - self.url = os.path.join(test_dir, "test_simple_repo.zip") - self.branch = "main" - def setUp(self): - self.addon_to_install = TestInstallerGui.MockAddon() + self.addon_to_install = MockAddon() self.installer_gui = AddonInstallerGUI(self.addon_to_install) self.finalized_thread = False diff --git a/src/Mod/AddonManager/AddonManagerTest/gui/test_uninstaller_gui.py b/src/Mod/AddonManager/AddonManagerTest/gui/test_uninstaller_gui.py new file mode 100644 index 0000000000..8e43e34380 --- /dev/null +++ b/src/Mod/AddonManager/AddonManagerTest/gui/test_uninstaller_gui.py @@ -0,0 +1,148 @@ +# *************************************************************************** +# * * +# * Copyright (c) 2022 FreeCAD Project Association * +# * * +# * This file is part of FreeCAD. * +# * * +# * FreeCAD is free software: you can redistribute it and/or modify it * +# * under the terms of the GNU Lesser General Public License as * +# * published by the Free Software Foundation, either version 2.1 of the * +# * License, or (at your option) any later version. * +# * * +# * FreeCAD is distributed in the hope that it will be useful, but * +# * WITHOUT ANY WARRANTY; without even the implied warranty of * +# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * +# * Lesser General Public License for more details. * +# * * +# * You should have received a copy of the GNU Lesser General Public * +# * License along with FreeCAD. If not, see * +# * . * +# * * +# *************************************************************************** + +import functools +import unittest + +from PySide import QtCore + +import FreeCAD + +from AddonManagerTest.gui.gui_mocks import ( + DialogWatcher, + DialogInteractor, + FakeWorker, + MockThread, +) +from AddonManagerTest.app.mocks import MockAddon + +from addonmanager_uninstaller_gui import AddonUninstallerGUI + +translate = FreeCAD.Qt.translate + + +class TestUninstallerGUI(unittest.TestCase): + + MODULE = "test_uninstaller_gui" # file name without extension + + def setUp(self): + self.addon_to_remove = MockAddon() + self.uninstaller_gui = AddonUninstallerGUI(self.addon_to_remove) + self.finalized_thread = False + self.signals_caught = [] + + def tearDown(self): + pass + + def catch_signal(self, signal_name, *_): + self.signals_caught.append(signal_name) + + def test_confirmation_dialog_yes(self): + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Confirm remove"), + translate("AddonsInstaller", "Yes"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + answer = self.uninstaller_gui._confirm_uninstallation() + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + self.assertTrue(answer, "Expected a 'Yes' click to return True, but got False") + + def test_confirmation_dialog_cancel(self): + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Confirm remove"), + translate("AddonsInstaller", "Cancel"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + answer = self.uninstaller_gui._confirm_uninstallation() + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + self.assertFalse( + answer, "Expected a 'Cancel' click to return False, but got True" + ) + + def test_progress_dialog(self): + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Removing Addon"), + translate("AddonsInstaller", "Cancel"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + self.uninstaller_gui._show_progress_dialog() + # That call isn't modal, so spin our own event loop: + while self.uninstaller_gui.progress_dialog.isVisible(): + QtCore.QCoreApplication.processEvents(QtCore.QEventLoop.AllEvents, 100) + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + + def test_timer_launches_progress_dialog(self): + worker = FakeWorker() + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Removing Addon"), + translate("AddonsInstaller", "Cancel"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + QtCore.QTimer.singleShot(20, worker.stop) + self.uninstaller_gui._confirm_uninstallation = lambda: True + self.uninstaller_gui._run_uninstaller = worker.work + self.uninstaller_gui._finalize = lambda: None + self.uninstaller_gui.dialog_timer.setInterval( + 1 + ) # To speed up the test, only wait 1ms + self.uninstaller_gui.run() # Blocks once it hits the fake worker + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + + def test_success_dialog(self): + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Uninstall complete"), + translate("AddonsInstaller", "OK"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + self.uninstaller_gui._succeeded(self.addon_to_remove) + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + + def test_failure_dialog(self): + dialog_watcher = DialogWatcher( + translate("AddonsInstaller", "Uninstall failed"), + translate("AddonsInstaller", "OK"), + ) + QtCore.QTimer.singleShot(10, dialog_watcher.run) + self.uninstaller_gui._failed( + self.addon_to_remove, "Some failure message\nAnother failure message" + ) + self.assertTrue( + dialog_watcher.dialog_found, "Failed to find the expected dialog box" + ) + + def test_finalize(self): + self.uninstaller_gui.finished.connect( + functools.partial(self.catch_signal, "finished") + ) + self.uninstaller_gui.worker_thread = MockThread() + self.uninstaller_gui._finalize() + self.assertIn("finished", self.signals_caught) diff --git a/src/Mod/AddonManager/CMakeLists.txt b/src/Mod/AddonManager/CMakeLists.txt index 829fb8d4be..6fbab00b5b 100644 --- a/src/Mod/AddonManager/CMakeLists.txt +++ b/src/Mod/AddonManager/CMakeLists.txt @@ -25,6 +25,7 @@ SET(AddonManager_SRCS addonmanager_macro.py addonmanager_update_all_gui.py addonmanager_uninstaller.py + addonmanager_uninstaller_gui.py addonmanager_utilities.py addonmanager_workers_installation.py addonmanager_workers_startup.py @@ -94,6 +95,7 @@ SET(AddonManagerTestsGui_SRCS AddonManagerTest/gui/test_gui.py AddonManagerTest/gui/test_installer_gui.py AddonManagerTest/gui/test_update_all_gui.py + AddonManagerTest/gui/test_uninstaller_gui.py AddonManagerTest/gui/test_workers_startup.py AddonManagerTest/gui/test_workers_utility.py ) @@ -120,13 +122,13 @@ SET(AddonManagerTestsFiles_SRCS ) SET(AddonManagerTests_ALL - ${AddonManagerTests_SRCS} - ${AddonManagerTestsApp_SRCS} - ${AddonManagerTestsFiles_SRCS} + ${AddonManagerTests_SRCS} + ${AddonManagerTestsApp_SRCS} + ${AddonManagerTestsFiles_SRCS} ) IF (BUILD_GUI) - LIST(APPEND AddonManagerTests_ALL ${AddonManagerTestsGui_SRCS}) + LIST(APPEND AddonManagerTests_ALL ${AddonManagerTestsGui_SRCS}) ENDIF (BUILD_GUI) ADD_CUSTOM_TARGET(AddonManager ALL diff --git a/src/Mod/AddonManager/TestAddonManagerGui.py b/src/Mod/AddonManager/TestAddonManagerGui.py index fe9e7ea016..3fa6287545 100644 --- a/src/Mod/AddonManager/TestAddonManagerGui.py +++ b/src/Mod/AddonManager/TestAddonManagerGui.py @@ -38,6 +38,9 @@ from AddonManagerTest.gui.test_installer_gui import ( from AddonManagerTest.gui.test_update_all_gui import ( TestUpdateAllGui as AddonManagerTestUpdateAllGui, ) +from AddonManagerTest.gui.test_uninstaller_gui import ( + TestUninstallerGUI as AddonManagerTestUninstallerGUI, +) # dummy usage to get flake8 and lgtm quiet False if AddonManagerTestGui.__name__ else True @@ -46,3 +49,4 @@ False if AddonManagerTestWorkersStartup.__name__ else True False if AddonManagerTestInstallerGui.__name__ else True False if AddonManagerTestMacroInstallerGui.__name__ else True False if AddonManagerTestUpdateAllGui.__name__ else True +False if AddonManagerTestUninstallerGUI.__name__ else True diff --git a/src/Mod/AddonManager/addonmanager_devmode.py b/src/Mod/AddonManager/addonmanager_devmode.py index 4ccdb4852a..f34a624fd4 100644 --- a/src/Mod/AddonManager/addonmanager_devmode.py +++ b/src/Mod/AddonManager/addonmanager_devmode.py @@ -606,9 +606,7 @@ class DeveloperMode: for filename in filenames: if filename.endswith(".py"): - with open( - os.path.join(dirpath, filename), encoding="utf-8" - ) as f: + with open(os.path.join(dirpath, filename), encoding="utf-8") as f: contents = f.read() version_strings = vermin.version_strings( vermin.detect(contents) diff --git a/src/Mod/AddonManager/addonmanager_installer.py b/src/Mod/AddonManager/addonmanager_installer.py index 6bcd0c63fa..2d338f66b7 100644 --- a/src/Mod/AddonManager/addonmanager_installer.py +++ b/src/Mod/AddonManager/addonmanager_installer.py @@ -154,6 +154,14 @@ class AddonInstaller(QtCore.QObject): success = self._install_by_copy() except utils.ProcessInterrupted: pass + if success: + if ( + hasattr(self.addon_to_install, "contains_workbench") + and self.addon_to_install.contains_workbench() + ): + self.addon_to_install.set_status(Addon.Status.PENDING_RESTART) + else: + self.addon_to_install.set_status(Addon.Status.NO_UPDATE_AVAILABLE) self.finished.emit() return success @@ -491,6 +499,7 @@ class MacroInstaller(QtCore.QObject): dst = os.path.join(self.installation_path, item) shutil.move(src, dst) self.success.emit(self.addon_to_install) + self.addon_to_install.set_status(Addon.Status.NO_UPDATE_AVAILABLE) self.finished.emit() return True diff --git a/src/Mod/AddonManager/addonmanager_uninstaller.py b/src/Mod/AddonManager/addonmanager_uninstaller.py index 3e0b1d10ff..769efab439 100644 --- a/src/Mod/AddonManager/addonmanager_uninstaller.py +++ b/src/Mod/AddonManager/addonmanager_uninstaller.py @@ -32,6 +32,7 @@ import FreeCAD from PySide import QtCore import addonmanager_utilities as utils +from Addon import Addon translate = FreeCAD.Qt.translate @@ -58,7 +59,7 @@ class AddonUninstaller(QtCore.QObject): addon_to_remove = MyAddon() # Some class with 'name' attribute self.worker_thread = QtCore.QThread() - self.uninstaller = AddonInstaller(addon_to_remove) + self.uninstaller = AddonUninstaller(addon_to_remove) self.uninstaller.moveToThread(self.worker_thread) self.uninstaller.success.connect(self.removal_succeeded) self.uninstaller.failure.connect(self.removal_failed) @@ -100,7 +101,7 @@ class AddonUninstaller(QtCore.QObject): """Remove an addon. Returns True if the addon was removed cleanly, or False if not. Emits either success or failure prior to returning.""" success = False - error_message = translate("AddonsInstaller", "An unknown error occured") + error_message = translate("AddonsInstaller", "An unknown error occurred") if hasattr(self.addon_to_remove, "name") and self.addon_to_remove.name: # Make sure we don't accidentally remove the Mod directory path_to_remove = os.path.normpath( @@ -124,6 +125,7 @@ class AddonUninstaller(QtCore.QObject): self.success.emit(self.addon_to_remove) else: self.failure.emit(self.addon_to_remove, error_message) + self.addon_to_remove.set_status(Addon.Status.NOT_INSTALLED) self.finished.emit() def run_uninstall_script(self, path_to_remove): @@ -248,6 +250,7 @@ class MacroUninstaller(QtCore.QObject): self.success.emit(self.addon_to_remove) else: self.failure.emit(self.addon_to_remove, "\n".join(errors)) + self.addon_to_remove.set_status(Addon.Status.NOT_INSTALLED) self.finished.emit() def _get_files_to_remove(self) -> List[os.PathLike]: diff --git a/src/Mod/AddonManager/addonmanager_uninstaller_gui.py b/src/Mod/AddonManager/addonmanager_uninstaller_gui.py new file mode 100644 index 0000000000..d76d2cd0b3 --- /dev/null +++ b/src/Mod/AddonManager/addonmanager_uninstaller_gui.py @@ -0,0 +1,145 @@ +# *************************************************************************** +# * * +# * Copyright (c) 2022 FreeCAD Project Association * +# * * +# * This file is part of FreeCAD. * +# * * +# * FreeCAD is free software: you can redistribute it and/or modify it * +# * under the terms of the GNU Lesser General Public License as * +# * published by the Free Software Foundation, either version 2.1 of the * +# * License, or (at your option) any later version. * +# * * +# * FreeCAD is distributed in the hope that it will be useful, but * +# * WITHOUT ANY WARRANTY; without even the implied warranty of * +# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * +# * Lesser General Public License for more details. * +# * * +# * You should have received a copy of the GNU Lesser General Public * +# * License along with FreeCAD. If not, see * +# * . * +# * * +# *************************************************************************** + +"""GUI functions for uninstalling an Addon or Macro.""" + +import FreeCAD +import FreeCADGui + +from PySide import QtCore, QtWidgets + +from addonmanager_uninstaller import AddonUninstaller, MacroUninstaller + +translate = FreeCAD.Qt.translate + + +class AddonUninstallerGUI(QtCore.QObject): + """User interface for uninstalling an Addon: asks for confirmation, displays a progress dialog, + displays completion and/or error dialogs, and emits the finished() signal when all work is + complete.""" + + finished = QtCore.Signal() + + def __init__(self, addon_to_remove): + super().__init__() + self.addon_to_remove = addon_to_remove + if ( + hasattr(self.addon_to_remove, "macro") + and self.addon_to_remove.macro is not None + ): + self.uninstaller = MacroUninstaller(self.addon_to_remove) + else: + self.uninstaller = AddonUninstaller(self.addon_to_remove) + self.uninstaller.success.connect(self._succeeded) + self.uninstaller.failure.connect(self._failed) + self.worker_thread = QtCore.QThread() + self.uninstaller.moveToThread(self.worker_thread) + self.uninstaller.finished.connect(self.worker_thread.quit) + self.worker_thread.started.connect(self.uninstaller.run) + self.progress_dialog = None + self.dialog_timer = QtCore.QTimer() + self.dialog_timer.timeout.connect(self._show_progress_dialog) + self.dialog_timer.setSingleShot(True) + self.dialog_timer.setInterval( + 1000 + ) # Can override from external (e.g. testing) code + + def run(self): + """Begin the user interaction: asynchronous, only blocks while showing the initial modal + confirmation dialog.""" + ok_to_proceed = self._confirm_uninstallation() + if not ok_to_proceed: + self._finalize() + return + + self.dialog_timer.start() + self._run_uninstaller() + + def _confirm_uninstallation(self) -> bool: + """Present a modal dialog asking the user if they really want to uninstall. Returns True to + continue with the uninstallation, or False to stop the process.""" + confirm = QtWidgets.QMessageBox.question( + None, + translate("AddonsInstaller", "Confirm remove"), + translate( + "AddonsInstaller", "Are you sure you want to uninstall {}?" + ).format(self.addon_to_remove.display_name), + QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.Cancel, + ) + return confirm == QtWidgets.QMessageBox.Yes + + def _show_progress_dialog(self): + self.progress_dialog = QtWidgets.QMessageBox( + QtWidgets.QMessageBox.NoIcon, + translate("AddonsInstaller", "Removing Addon"), + translate("AddonsInstaller", "Removing {}").format( + self.addon_to_remove.display_name + ) + + "...", + QtWidgets.QMessageBox.Cancel, + ) + self.progress_dialog.rejected.connect(self._cancel_removal) + self.progress_dialog.show() + + def _run_uninstaller(self): + self.worker_thread.start() + + def _cancel_removal(self): + """Ask the QThread to interrupt. Probably has no effect, most of the work is in a single OS + call.""" + self.worker_thread.requestInterruption() + + def _succeeded(self, addon): + """Callback for successful removal""" + self.dialog_timer.stop() + if self.progress_dialog: + self.progress_dialog.hide() + QtWidgets.QMessageBox.information( + None, + translate("AddonsInstaller", "Uninstall complete"), + translate("AddonInstaller", "Finished removing {}").format( + addon.display_name + ), + ) + self._finalize() + + def _failed(self, addon, message): + """Callback for failed or partially failed removal""" + self.dialog_timer.stop() + if self.progress_dialog: + self.progress_dialog.hide() + QtWidgets.QMessageBox.critical( + None, + translate("AddonsInstaller", "Uninstall failed"), + translate("AddonInstaller", "Failed to remove some files") + + ":\n" + + message, + ) + self._finalize() + + def _finalize(self): + """Clean up and emit finished signal""" + if self.worker_thread.isRunning(): + self.worker_thread.requestInterruption() + self.worker_thread.quit() + self.worker_thread.wait(500) + self.finished.emit()