kindred/create
1
1
Fork 0
Code Issues 53 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

Addon Manager: Linter cleanup

Browse Source
This commit is contained in:
Chris Hennes
2025-02-06 17:55:06 -06:00
parent 2dd2381c70
commit 5dbc5c95ca
3 changed files with 133 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
src/Mod/AddonManager/AddonManagerTest/gui/test_change_branch.py
View File

@@ -21,6 +21,10 @@
# * *
# ***************************************************************************
"""Test the Change Branch GUI code"""
# pylint: disable=wrong-import-position, deprecated-module, too-many-return-statements
import sys
import unittest
from unittest.mock import patch, Mock, MagicMock
@@ -28,7 +32,7 @@ from unittest.mock import patch, Mock, MagicMock
# So that when run standalone, the Addon Manager classes imported below are available
sys.path.append("../..")
from AddonManagerTest.gui.gui_mocks import DialogWatcher, DialogInteractor, AsynchronousMonitor
from AddonManagerTest.gui.gui_mocks import DialogWatcher, AsynchronousMonitor
from change_branch import ChangeBranchDialog
@@ -45,11 +49,14 @@ except ImportError:
class MockFilter(QtCore.QSortFilterProxyModel):
"""Replaces a filter with a non-filter that simply always returns whatever it's given"""
def mapToSource(self, something):
return something
class MockChangeBranchDialogModel(QtCore.QAbstractTableModel):
"""Replace a data-connected model with a static one for testing"""
branches = [
{"ref_name": "ref1", "upstream": "us1"},
@@ -64,16 +71,20 @@ class MockChangeBranchDialogModel(QtCore.QAbstractTableModel):
super().__init__(parent)
def rowCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
"""Number of rows: should always return 3"""
if parent.isValid():
return 0
return len(self.branches)
def columnCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
"""Number of columns (identical to non-mocked version)"""
if parent.isValid():
return 0
return 3 # Local name, remote name, date
def data(self, index: QtCore.QModelIndex, role: int = QtCore.Qt.DisplayRole):
"""Mock returns static untranslated strings for DisplayRole, no tooltips at all, and
otherwise matches the non-mock version"""
if not index.isValid():
return None
row = index.row()
@@ -81,16 +92,16 @@ class MockChangeBranchDialogModel(QtCore.QAbstractTableModel):
if role == QtCore.Qt.DisplayRole:
if column == 2:
return "date"
elif column == 0:
if column == 0:
return "ref_name"
elif column == 1:
if column == 1:
return "upstream"
else:
return None
elif role == MockChangeBranchDialogModel.DataSortRole:
return None
elif role == MockChangeBranchDialogModel.RefAccessRole:
if role == MockChangeBranchDialogModel.DataSortRole:
return None
if role == MockChangeBranchDialogModel.RefAccessRole:
return self.branches[row]
return None
def headerData(
self,
@@ -98,6 +109,7 @@ class MockChangeBranchDialogModel(QtCore.QAbstractTableModel):
orientation: QtCore.Qt.Orientation,
role: int = QtCore.Qt.DisplayRole,
):
"""Mock returns untranslated strings for DisplayRole, and no tooltips at all"""
if orientation == QtCore.Qt.Vertical:
return None
if role != QtCore.Qt.DisplayRole:
@@ -106,16 +118,18 @@ class MockChangeBranchDialogModel(QtCore.QAbstractTableModel):
return "Local"
if section == 1:
return "Remote tracking"
elif section == 2:
if section == 2:
return "Last Updated"
else:
return None
return None
def currentBranch(self) -> str:
"""Mock returns a static string stored in the class: that string could be modified to
return something else by tests that require it."""
return self.current_branch
class TestChangeBranchGui(unittest.TestCase):
"""Tests for the ChangeBranch GUI code"""
MODULE = "test_change_branch" # file name without extension
@@ -128,6 +142,7 @@ class TestChangeBranchGui(unittest.TestCase):
@patch("change_branch.ChangeBranchDialogModel", new=MockChangeBranchDialogModel)
@patch("change_branch.initialize_git", new=Mock(return_value=None))
def test_no_git(self):
"""If git is not present, a dialog saying so is presented"""
# Arrange
gui = ChangeBranchDialog("/some/path")
ref = {"ref_name": "foo/bar", "upstream": "us1"}
@@ -137,7 +152,7 @@ class TestChangeBranchGui(unittest.TestCase):
)
# Act
gui._change_branch("/foo/bar/baz", ref)
gui.change_branch("/foo/bar/baz", ref)
# Assert
self.assertTrue(dialog_watcher.dialog_found, "Failed to find the expected dialog box")
@@ -145,6 +160,7 @@ class TestChangeBranchGui(unittest.TestCase):
@patch("change_branch.ChangeBranchDialogModel", new=MockChangeBranchDialogModel)
@patch("change_branch.initialize_git")
def test_git_failed(self, init_git: MagicMock):
"""If git fails when attempting to change branches, a dialog saying so is presented"""
# Arrange
git_manager = MagicMock()
git_manager.checkout = MagicMock()
@@ -158,7 +174,7 @@ class TestChangeBranchGui(unittest.TestCase):
)
# Act
gui._change_branch("/foo/bar/baz", ref)
gui.change_branch("/foo/bar/baz", ref)
# Assert
self.assertTrue(dialog_watcher.dialog_found, "Failed to find the expected dialog box")
@@ -166,8 +182,8 @@ class TestChangeBranchGui(unittest.TestCase):
@patch("change_branch.ChangeBranchDialogModel", new=MockChangeBranchDialogModel)
@patch("change_branch.initialize_git", new=MagicMock)
def test_branch_change_succeeded(self):
# If nothing gets thrown, then the process is assumed to have worked, and the appropriate
# signal is emitted.
"""If nothing gets thrown, then the process is assumed to have worked, and the appropriate
signal is emitted."""
# Arrange
gui = ChangeBranchDialog("/some/path")
@@ -175,7 +191,7 @@ class TestChangeBranchGui(unittest.TestCase):
monitor = AsynchronousMonitor(gui.branch_changed)
# Act
gui._change_branch("/foo/bar/baz", ref)
gui.change_branch("/foo/bar/baz", ref)
# Assert
monitor.wait_for_at_most(10) # Should be effectively instantaneous
@@ -185,6 +201,8 @@ class TestChangeBranchGui(unittest.TestCase):
@patch("change_branch.ChangeBranchDialogModel", new=MockChangeBranchDialogModel)
@patch("change_branch.initialize_git", new=MagicMock)
def test_warning_is_shown_when_dialog_is_accepted(self):
"""If the dialog is accepted (e.g. a branch change is requested) then a warning dialog is
displayed, and gives the opportunity to cancel. If cancelled, no signal is emitted."""
# Arrange
gui = ChangeBranchDialog("/some/path")
gui.ui.exec = MagicMock()
@@ -197,12 +215,14 @@ class TestChangeBranchGui(unittest.TestCase):
translate("AddonsInstaller", "DANGER: Developer feature"),
QtWidgets.QDialogButtonBox.Cancel,
)
monitor = AsynchronousMonitor(gui.branch_changed)
# Act
gui.exec()
# Assert
self.assertTrue(dialog_watcher.dialog_found, "Failed to find the expected dialog box")
self.assertFalse(monitor.good()) # The watcher cancelled the op, so no signal is emitted
if __name__ == "__main__":

51
src/Mod/AddonManager/addonmanager_utilities.py
View File

@@ -24,6 +24,8 @@
"""Utilities to work across different platforms, providers and python versions"""
# pylint: disable=deprecated-module, ungrouped-imports
from datetime import datetime
from typing import Optional, Any, List
import os
@@ -52,6 +54,7 @@ try:
except ImportError:
def get_python_exe():
"""Use shutil.which to find python executable"""
return shutil.which("python")
@@ -61,9 +64,17 @@ if fci.FreeCADGui:
# loop running this is not possible, so fall back to requests (if available), or the native
# Python urllib.request (if requests is not available).
import NetworkManager # Requires an event loop, so is only available with the GUI
requests = None
ssl = None
urllib = None
else:
NetworkManager = None
try:
import requests
ssl = None
urllib = None
except ImportError:
requests = None
import urllib.request
@@ -85,11 +96,14 @@ else:
except ImportError:
def loadUi(ui_file: str):
"""If there are no available versions of QtUiTools, then raise an error if this
method is used."""
raise RuntimeError("Cannot use QUiLoader without PySide or FreeCAD")
if has_loader:
def loadUi(ui_file: str) -> QtWidgets.QWidget:
"""Load a Qt UI from an on-disk file."""
q_ui_file = QtCore.QFile(ui_file)
q_ui_file.open(QtCore.QFile.OpenModeFlag.ReadOnly)
loader = QUiLoader()
@@ -135,10 +149,13 @@ def symlink(source, link_name):
def rmdir(path: str) -> bool:
"""Remove a directory or symlink, even if it is read-only."""
try:
if os.path.islink(path):
os.unlink(path) # Remove symlink
else:
# NOTE: the onerror argument was deprecated in Python 3.12, replaced by onexc -- replace
# when earlier versions are no longer supported.
shutil.rmtree(path, onerror=remove_readonly)
except (WindowsError, PermissionError, OSError):
return False
@@ -213,7 +230,7 @@ def get_zip_url(repo):
def recognized_git_location(repo) -> bool:
"""Returns whether this repo is based at a known git repo location: works with github, gitlab,
"""Returns whether this repo is based at a known git repo location: works with GitHub, gitlab,
framagit, and salsa.debian.org"""
parsed_url = urlparse(repo.url)
@@ -395,7 +412,7 @@ def is_float(element: Any) -> bool:
def get_pip_target_directory():
# Get the default location to install new pip packages
"""Get the default location to install new pip packages"""
major, minor, _ = platform.python_version_tuple()
vendor_path = os.path.join(
fci.DataPaths().mod_dir, "..", "AdditionalPythonPackages", f"py{major}{minor}"
@@ -417,7 +434,12 @@ def blocking_get(url: str, method=None) -> bytes:
succeeded, or an empty string if it failed, or returned no data. The method argument is
provided mainly for testing purposes."""
p = b""
if fci.FreeCADGui and method is None or method == "networkmanager":
if (
fci.FreeCADGui
and method is None
or method == "networkmanager"
and NetworkManager is not None
):
NetworkManager.InitializeNetworkManager()
p = NetworkManager.AM_NETWORK_MANAGER.blocking_get(url, 10000) # 10 second timeout
if p:
@@ -462,13 +484,13 @@ def run_interruptable_subprocess(args, timeout_secs: int = 10) -> subprocess.Com
# one second timeout allows interrupting the run once per second
stdout, stderr = p.communicate(timeout=1)
return_code = p.returncode
except subprocess.TimeoutExpired:
except subprocess.TimeoutExpired as timeout_exception:
if (
hasattr(QtCore, "QThread")
and QtCore.QThread.currentThread().isInterruptionRequested()
):
p.kill()
raise ProcessInterrupted()
raise ProcessInterrupted() from timeout_exception
if time.time() - start_time >= timeout_secs: # The real timeout
p.kill()
stdout, stderr = p.communicate()
@@ -481,9 +503,10 @@ def run_interruptable_subprocess(args, timeout_secs: int = 10) -> subprocess.Com
def process_date_string_to_python_datetime(date_string: str) -> datetime:
"""For modern macros the expected date format is ISO 8601, YYYY-MM-DD. For older macros this standard was not always
used, and various orderings and separators were used. This function tries to match the majority of those older
macros. Commonly-used separators are periods, slashes, and dashes."""
"""For modern macros the expected date format is ISO 8601, YYYY-MM-DD. For older macros this
standard was not always used, and various orderings and separators were used. This function
tries to match the majority of those older macros. Commonly-used separators are periods,
slashes, and dashes."""
def raise_error(bad_string: str, root_cause: Exception = None):
raise ValueError(
@@ -499,19 +522,19 @@ def process_date_string_to_python_datetime(date_string: str) -> datetime:
# The earliest possible year an addon can be created or edited is 2001:
if split_result[0] > 2000:
return datetime(split_result[0], split_result[1], split_result[2])
elif split_result[2] > 2000:
# Generally speaking it's not possible to distinguish between DD-MM and MM-DD, so try the first, and
# only if that fails try the second
if split_result[2] > 2000:
# Generally speaking it's not possible to distinguish between DD-MM and MM-DD, so try
# the first, and only if that fails try the second
if split_result[1] <= 12:
return datetime(split_result[2], split_result[1], split_result[0])
return datetime(split_result[2], split_result[0], split_result[1])
else:
raise ValueError(f"Invalid year in date string '{date_string}'")
raise ValueError(f"Invalid year in date string '{date_string}'")
except ValueError as exception:
raise_error(date_string, exception)
def get_main_am_window():
"""Find the Addon Manager's main window in the Qt widget hierarchy."""
windows = QtWidgets.QApplication.topLevelWidgets()
for widget in windows:
if widget.objectName() == "AddonManager_Main_Window":
@@ -540,7 +563,7 @@ def create_pip_call(args: List[str]) -> List[str]:
else:
python_exe = get_python_exe()
if not python_exe:
raise (RuntimeError("Could not locate Python executable on this system"))
raise RuntimeError("Could not locate Python executable on this system")
call_args = [python_exe, "-m", "pip", "--disable-pip-version-check"]
call_args.extend(args)
return call_args

103
src/Mod/AddonManager/change_branch.py
View File

@@ -21,6 +21,8 @@
# * *
# ***************************************************************************
"""The Change Branch dialog and utility classes and methods"""
import os
from typing import Dict
@@ -35,12 +37,15 @@ except ImportError:
try:
from PySide6 import QtWidgets, QtCore
except ImportError:
from PySide2 import QtWidgets, QtCore
from PySide2 import QtWidgets, QtCore # pylint: disable=deprecated-module
translate = fci.translate
class ChangeBranchDialog(QtWidgets.QWidget):
"""A dialog that displays available git branches and allows the user to select one to change
to. Includes code that does that change, as well as some modal dialogs to warn them of the
possible consequences and display various error messages."""
branch_changed = QtCore.Signal(str, str)
@@ -80,6 +85,9 @@ class ChangeBranchDialog(QtWidgets.QWidget):
header.setSectionResizeMode(QtWidgets.QHeaderView.ResizeToContents)
def exec(self):
"""Run the Change Branch dialog and its various sub-dialogs. May result in the branch
being changed. Code that cares if that happens should connect to the branch_changed
signal."""
if self.ui.exec() == QtWidgets.QDialog.Accepted:
selection = self.ui.tableView.selectedIndexes()
@@ -120,9 +128,9 @@ class ChangeBranchDialog(QtWidgets.QWidget):
if result == QtWidgets.QMessageBox.Cancel:
return
self._change_branch(self.item_model.path, ref)
self.change_branch(self.item_model.path, ref)
def _change_branch(self, path: str, ref: Dict[str, str]) -> None:
def change_branch(self, path: str, ref: Dict[str, str]) -> None:
"""Change the git clone in `path` to git ref `ref`. Emits the branch_changed signal
on success."""
remote_name = ref["ref_name"]
@@ -168,6 +176,10 @@ class ChangeBranchDialog(QtWidgets.QWidget):
class ChangeBranchDialogModel(QtCore.QAbstractTableModel):
"""The data for the dialog comes from git: this model handles the git interactions and
returns branch information as its rows. Use user data in the RefAccessRole to get information
about the git refs. RefAccessRole data is a dictionary defined by the GitManager class as the
results of a `get_branches_with_info()` call."""
branches = []
DataSortRole = QtCore.Qt.UserRole
@@ -184,54 +196,61 @@ class ChangeBranchDialogModel(QtCore.QAbstractTableModel):
self._remove_tracking_duplicates()
def rowCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
"""Returns the number of rows in the model, e.g. the number of branches."""
if parent.isValid():
return 0
return len(self.branches)
def columnCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
"""Returns the number of columns in the model, e.g. the number of entries in the git ref
structure (currently 3, 'ref_name', 'upstream', and 'date')."""
if parent.isValid():
return 0
return 3 # Local name, remote name, date
def data(self, index: QtCore.QModelIndex, role: int = QtCore.Qt.DisplayRole):
"""The data access method for this model. Supports four roles: ToolTipRole, DisplayRole,
DataSortRole, and RefAccessRole."""
if not index.isValid():
return None
row = index.row()
column = index.column()
if role == QtCore.Qt.ToolTipRole:
tooltip = self.branches[row]["author"] + ": " + self.branches[row]["subject"]
return tooltip
elif role == QtCore.Qt.DisplayRole:
dd = self.branches[row]
if column == 2:
if dd["date"] is not None:
q_date = QtCore.QDateTime.fromString(
dd["date"], QtCore.Qt.DateFormat.RFC2822Date
)
return QtCore.QLocale().toString(q_date, QtCore.QLocale.ShortFormat)
return None
elif column == 0:
return dd["ref_name"]
elif column == 1:
return dd["upstream"]
else:
return None
elif role == ChangeBranchDialogModel.DataSortRole:
if column == 2:
if self.branches[row]["date"] is not None:
q_date = QtCore.QDateTime.fromString(
self.branches[row]["date"], QtCore.Qt.DateFormat.RFC2822Date
)
return q_date
return None
elif column == 0:
return self.branches[row]["ref_name"]
elif column == 1:
return self.branches[row]["upstream"]
else:
return None
elif role == ChangeBranchDialogModel.RefAccessRole:
return self.branches[row]["author"] + ": " + self.branches[row]["subject"]
if role == QtCore.Qt.DisplayRole:
return self._data_display_role(column, row)
if role == ChangeBranchDialogModel.DataSortRole:
return self._data_sort_role(column, row)
if role == ChangeBranchDialogModel.RefAccessRole:
return self.branches[row]
return None
def _data_display_role(self, column, row):
dd = self.branches[row]
if column == 2:
if dd["date"] is not None:
q_date = QtCore.QDateTime.fromString(dd["date"], QtCore.Qt.DateFormat.RFC2822Date)
return QtCore.QLocale().toString(q_date, QtCore.QLocale.ShortFormat)
return None
if column == 0:
return dd["ref_name"]
if column == 1:
return dd["upstream"]
return None
def _data_sort_role(self, column, row):
if column == 2:
if self.branches[row]["date"] is not None:
q_date = QtCore.QDateTime.fromString(
self.branches[row]["date"], QtCore.Qt.DateFormat.RFC2822Date
)
return q_date
return None
if column == 0:
return self.branches[row]["ref_name"]
if column == 1:
return self.branches[row]["upstream"]
return None
def headerData(
self,
@@ -239,6 +258,7 @@ class ChangeBranchDialogModel(QtCore.QAbstractTableModel):
orientation: QtCore.Qt.Orientation,
role: int = QtCore.Qt.DisplayRole,
):
"""Returns the header information for the data in this model."""
if orientation == QtCore.Qt.Vertical:
return None
if role != QtCore.Qt.DisplayRole:
@@ -255,14 +275,13 @@ class ChangeBranchDialogModel(QtCore.QAbstractTableModel):
"Remote tracking",
"Table header for git remote tracking branch name",
)
elif section == 2:
if section == 2:
return translate(
"AddonsInstaller",
"Last Updated",
"Table header for git update date",
)
else:
return None
return None
def _remove_tracking_duplicates(self):
remote_tracking_branches = []
@@ -280,12 +299,12 @@ class ChangeBranchDialogModel(QtCore.QAbstractTableModel):
class ChangeBranchDialogFilter(QtCore.QSortFilterProxyModel):
"""Uses the DataSortRole in the model to provide a comparison method to sort the data."""
def lessThan(self, left: QtCore.QModelIndex, right: QtCore.QModelIndex):
"""Compare two git refs according to the DataSortRole in the model."""
left_data = self.sourceModel().data(left, ChangeBranchDialogModel.DataSortRole)
right_data = self.sourceModel().data(right, ChangeBranchDialogModel.DataSortRole)
if left_data is None or right_data is None:
if right_data is not None:
return True
else:
return False
return right_data is not None
return left_data < right_data