diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e1f1e6e..5218940 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -26,17 +26,15 @@ jobs: include: # MANDATORY CHECKS USING CURRENT DEVELOPMENT INTERPRETER - dependencies: > - pylint python3-dbus python3-dbus-python-client-gen python3-justbytes python3-gobject python3-psutil + ruff image: fedora:43 # CURRENT DEVELOPMENT ENVIRONMENT - task: PYTHONPATH=./src make -f Makefile lint - - dependencies: > - black - python3-isort + task: make -f Makefile lint + - dependencies: ruff image: fedora:43 # CURRENT DEVELOPMENT ENVIRONMENT task: make -f Makefile fmt-travis # VERIFICATION OF TEST INFRASTRUCTURE diff --git a/.isort.cfg b/.isort.cfg deleted file mode 100644 index 3516e26..0000000 --- a/.isort.cfg +++ /dev/null @@ -1,16 +0,0 @@ -[settings] -profile = black - -sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCAL,LOCALFOLDER - -import_heading_future=isort: FUTURE -import_heading_stdlib=isort: STDLIB -import_heading_thirdparty=isort: THIRDPARTY -import_heading_firstparty=isort: FIRSTPARTY -import_heading_local=isort: LOCAL - -# All items above should be the same for every -# Stratis project. The items below vary with -# each project. -known_local=testlib -known_first_party=dbus_python_client_gen diff --git a/Makefile b/Makefile index 52cdd31..d6c3a60 100644 --- a/Makefile +++ b/Makefile @@ -1,28 +1,16 @@ .PHONY: lint lint: - pylint test_harness.py - pylint stratis_cli_cert.py - pylint stratisd_cert.py - pylint testlib - pylint scripts --disable=duplicate-code + ruff check .PHONY: fmt fmt: - isort \ - test_harness.py \ - stratis_cli_cert.py \ - stratisd_cert.py \ - testlib scripts - black . + ruff check --fix --select I + ruff format .PHONY: fmt-travis fmt-travis: - isort --diff --check-only \ - test_harness.py \ - stratis_cli_cert.py \ - stratisd_cert.py \ - testlib scripts - black . --check + ruff check --select I + ruff format --check .PHONY: yamllint yamllint: diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..ee8be0c --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,10 @@ +[tool.ruff] +target-version = "py312" +line-length = 88 + +[tool.ruff.lint] +select = ["PL"] + +[tool.ruff.lint.isort] +known-first-party = ["dbus_python_client_gen"] +split-on-trailing-comma = false diff --git a/scripts/monitor_dbus_signals.py b/scripts/monitor_dbus_signals.py index 582d957..fe6ac4b 100755 --- a/scripts/monitor_dbus_signals.py +++ b/scripts/monitor_dbus_signals.py @@ -47,13 +47,13 @@ _EMITS_CHANGED_PROP = "org.freedesktop.DBus.Property.EmitsChangedSignal" -class Diff: # pylint: disable=too-few-public-methods +class Diff: """ Diff between two different managed object results. """ -class AddedProperty(Diff): # pylint: disable=too-few-public-methods +class AddedProperty(Diff): """ Property appears in new result but not in recorded result. """ @@ -71,7 +71,7 @@ def __repr__(self): ) -class RemovedProperty(Diff): # pylint: disable=too-few-public-methods +class RemovedProperty(Diff): """ Property appears in recorded result but not in new result. """ @@ -89,14 +89,12 @@ def __repr__(self): ) -class DifferentProperty(Diff): # pylint: disable=too-few-public-methods +class DifferentProperty(Diff): """ Difference between two properties. """ - def __init__( - self, object_path, interface_name, key, old_value, new_value - ): # pylint: disable=too-many-positional-arguments,too-many-arguments + def __init__(self, object_path, interface_name, key, old_value, new_value): self.object_path = object_path self.interface_name = interface_name self.key = key @@ -110,7 +108,7 @@ def __repr__(self): ) -class DifferentVariantLevel(Diff): # pylint: disable=too-few-public-methods +class DifferentVariantLevel(Diff): """ Represents a case where the property value is correct but the variant level does not match. The variant levels among the GetManagedObjects @@ -120,9 +118,7 @@ class DifferentVariantLevel(Diff): # pylint: disable=too-few-public-methods they must be heterogeneous. """ - def __init__( - self, object_path, interface_name, key, old_value, new_value - ): # pylint: disable=too-many-positional-arguments,too-many-arguments + def __init__(self, object_path, interface_name, key, old_value, new_value): self.object_path = object_path self.interface_name = interface_name self.key = key @@ -137,15 +133,13 @@ def __repr__(self): ) -class NotInvalidatedProperty(Diff): # pylint: disable=too-few-public-methods +class NotInvalidatedProperty(Diff): """ Represents a case where the property should have been invalidated but was updated instead. """ - def __init__( - self, object_path, interface_name, key, old_value, new_value - ): # pylint: disable=too-many-positional-arguments,too-many-arguments + def __init__(self, object_path, interface_name, key, old_value, new_value): self.object_path = object_path self.interface_name = interface_name self.key = key @@ -160,15 +154,13 @@ def __repr__(self): ) -class ChangedProperty(Diff): # pylint: disable=too-few-public-methods +class ChangedProperty(Diff): """ Represents a case where the property should have been constant but seems to have changed. """ - def __init__( - self, object_path, interface_name, key, old_value, new_value - ): # pylint: disable=too-many-positional-arguments,too-many-arguments + def __init__(self, object_path, interface_name, key, old_value, new_value): self.object_path = object_path self.interface_name = interface_name self.key = key @@ -183,7 +175,7 @@ def __repr__(self): ) -class RemovedObjectPath(Diff): # pylint: disable=too-few-public-methods +class RemovedObjectPath(Diff): """ Object path appears in recorded result but not in new result. """ @@ -196,7 +188,7 @@ def __repr__(self): return f"RemovedObjectPath({self.object_path!r}, {self.old_value!r})" -class AddedInterface(Diff): # pylint: disable=too-few-public-methods +class AddedInterface(Diff): """ Interface appears in new result but not in recorded result. """ @@ -213,7 +205,7 @@ def __repr__(self): ) -class AddedObjectPath(Diff): # pylint: disable=too-few-public-methods +class AddedObjectPath(Diff): """ Object path appears in new result but not in recorded result. """ @@ -226,7 +218,7 @@ def __repr__(self): return f"AddedObjectPath({self.object_path!r}, {self.new_value!r})" -class RemovedInterface(Diff): # pylint: disable=too-few-public-methods +class RemovedInterface(Diff): """ Interface appears in recorded result but not in new result. """ @@ -243,7 +235,7 @@ def __repr__(self): ) -class MissingInterface(Diff): # pylint: disable=too-few-public-methods +class MissingInterface(Diff): """ Attempted to update a property on this interface, but the interface itself was missing when that happened. @@ -295,7 +287,7 @@ def from_str(code_str): return item return None - class Invalidated: # pylint: disable=too-few-public-methods + class Invalidated: """ Used to record in the updated GetManagedObjects value that a value has been invalidated. @@ -306,7 +298,7 @@ def __repr__(self): INVALIDATED = Invalidated() - class InterfaceMissing: # pylint: disable=too-few-public-methods + class InterfaceMissing: """ Used to record in the updated GetManagedObjects value that when a property changed signal was received, the interface for that property @@ -424,7 +416,7 @@ def _interfaces_added(object_path, interfaces_added): _MO[object_path][interface] = props else: _MO[object_path] = interfaces_added - except Exception as exc: # pylint: disable=broad-except + except Exception as exc: _CALLBACK_ERRORS.append(exc) def _interfaces_removed(object_path, interfaces): @@ -457,7 +449,7 @@ def _interfaces_removed(object_path, interfaces): # that the object itself has been removed. if _MO[object_path] == {}: del _MO[object_path] - except Exception as exc: # pylint: disable=broad-except + except Exception as exc: _CALLBACK_ERRORS.append(exc) def _properties_changed(*props_changed, object_path=None): @@ -524,7 +516,7 @@ def _properties_changed(*props_changed, object_path=None): data[interface_name][prop] = value for prop in properties_invalidated: data[interface_name][prop] = INVALIDATED - except Exception as exc: # pylint: disable=broad-except + except Exception as exc: _CALLBACK_ERRORS.append(exc) def _monitor( @@ -544,7 +536,7 @@ def _monitor( :type interface_re: re.Pattern """ - global _TOP_OBJECT, _TOP_OBJECT_PATH, _TOP_OBJECT_INTERFACES, _SERVICE, _MO, _INTERFACE_RE # pylint: disable=global-statement + global _TOP_OBJECT, _TOP_OBJECT_PATH, _TOP_OBJECT_INTERFACES, _SERVICE, _MO, _INTERFACE_RE # noqa: PLW0603 dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) bus = dbus.SystemBus() @@ -556,7 +548,7 @@ def _monitor( while True: try: _TOP_OBJECT = bus.get_object(service, _TOP_OBJECT_PATH) - except Exception as err: # pylint: disable=broad-exception-caught + except Exception as err: print( f'Failed to get top object "{_TOP_OBJECT_PATH}" for ' f'service "{_SERVICE}". Error: {err}. Retrying.' @@ -588,7 +580,7 @@ def _monitor( while True: try: _MO = _MAKE_MO() - except Exception as err: # pylint: disable=broad-exception-caught + except Exception as err: print( "Failed to get initial GetManagedObjects result for " f'service "{_SERVICE}" and top object ' @@ -747,7 +739,7 @@ def _check(): if _MO is None: return [] - mos = _MAKE_MO() # pylint: disable=not-callable + mos = _MAKE_MO() diffs = [] @@ -755,16 +747,13 @@ def _check(): new_object_paths = frozenset(mos.keys()) for object_path in old_object_paths - new_object_paths: - diffs.append( - # pylint: disable=unsubscriptable-object - RemovedObjectPath(object_path, _MO[object_path]) - ) + diffs.append(RemovedObjectPath(object_path, _MO[object_path])) for object_path in new_object_paths - old_object_paths: diffs.append(AddedObjectPath(object_path, mos[object_path])) for object_path in new_object_paths & old_object_paths: - old_data = _MO[object_path] # pylint: disable=unsubscriptable-object + old_data = _MO[object_path] new_data = mos[object_path] old_ifns = frozenset(old_data.keys()) @@ -792,7 +781,7 @@ def _check(): try: result = _check() - except Exception as exco: # pylint: disable=broad-except + except Exception as exco: print(f"{exco}") sys.exit(4) diff --git a/scripts/monitor_metadata.py b/scripts/monitor_metadata.py index ca4bfa2..7d43531 100755 --- a/scripts/monitor_metadata.py +++ b/scripts/monitor_metadata.py @@ -89,7 +89,7 @@ def run(namespace): with open( os.path.join( namespace.output_dir, - f'{datetime.datetime.now().strftime("%Y_%m_%d-%I_%M_%S_%p")}.json', + f"{datetime.datetime.now().strftime('%Y_%m_%d-%I_%M_%S_%p')}.json", ), mode="w", encoding="utf-8", diff --git a/stratis_cli_cert.py b/stratis_cli_cert.py index 6699834..5908539 100644 --- a/stratis_cli_cert.py +++ b/stratis_cli_cert.py @@ -14,7 +14,6 @@ """ Tests of the stratis CLI. """ -# pylint: disable=too-many-lines # isort: STDLIB import argparse @@ -169,9 +168,7 @@ def test_access_stratis_man_page(self): self._unittest_command(["man", "--where", "stratis"], 0, True, False) -class StratisCliCertify( - StratisdSystemdStart, StratisCertify -): # pylint: disable=too-many-public-methods +class StratisCliCertify(StratisdSystemdStart, StratisCertify): """ Unit tests for the stratis-cli package. """ @@ -1516,7 +1513,7 @@ def test_filesystem_mount_and_write(self): [ "dd", "if=/dev/urandom", - f'of={os.path.join(mountpoints[0], "file1")}', + f"of={os.path.join(mountpoints[0], 'file1')}", "bs=4096", "count=256", "conv=fsync", diff --git a/stratisd_cert.py b/stratisd_cert.py index 42e9f3f..3645d85 100644 --- a/stratisd_cert.py +++ b/stratisd_cert.py @@ -14,7 +14,6 @@ """ Tests of stratisd. """ -# pylint: disable=too-many-lines # isort: STDLIB import argparse @@ -151,7 +150,7 @@ def _unittest_command(self, result, expected_return_code): :raises: AssertionError if the actual return code is not equal to the expected return code """ - if len(result) == 3: + if len(result) == 3: # noqa: PLR2004 (_, return_code, msg) = result else: (return_code, msg) = result @@ -165,9 +164,7 @@ def _unittest_command(self, result, expected_return_code): ) -class StratisdCertify( - StratisdSystemdStart, StratisCertify -): # pylint: disable=too-many-public-methods +class StratisdCertify(StratisdSystemdStart, StratisCertify): """ Tests on stratisd, the principal daemon. """ @@ -194,7 +191,7 @@ def tearDown(self): def _unittest_set_property( self, object_path, param_iface, dbus_param, dbus_value, exception_name - ): # pylint: disable=too-many-positional-arguments,too-many-arguments + ): """ :param object_path: path to the object :param param_iface: D-Bus interface to use for parameter @@ -1202,7 +1199,7 @@ def test_filesystem_mount_and_write(self): [ "dd", "if=/dev/urandom", - f'of={os.path.join(mountpoints[0], "file1")}', + f"of={os.path.join(mountpoints[0], 'file1')}", "bs=4096", "count=256", "conv=fsync", diff --git a/test_harness.py b/test_harness.py index 2bd4cec..67bcd88 100644 --- a/test_harness.py +++ b/test_harness.py @@ -30,7 +30,7 @@ _SIZE_OF_DEVICE = 8 * (1024**3) # 8 GiB -class _LogBlockdev: # pylint: disable=too-few-public-methods +class _LogBlockdev: """ Allows only running blockdev commands if the result will be logged. """ @@ -42,7 +42,7 @@ def __str__(self): try: with subprocess.Popen(self.cmd, stdout=subprocess.PIPE) as proc: output = proc.stdout.readline().strip().decode("utf-8") - except: # pylint: disable=bare-except + except: return f"could not gather output of {self.cmd}" return f"output of {self.cmd}: {output}" diff --git a/testlib/dbus.py b/testlib/dbus.py index 7f725e4..95d6bc6 100644 --- a/testlib/dbus.py +++ b/testlib/dbus.py @@ -14,6 +14,7 @@ """ DBus methods for blackbox testing. """ + # isort: STDLIB import os @@ -98,7 +99,6 @@ def _get_timeout(value): return timeout_int / 1000 -# pylint: disable=too-many-public-methods class StratisDbus: "Wrappers around stratisd DBus calls" diff --git a/testlib/infra.py b/testlib/infra.py index c1785db..b9d14c8 100644 --- a/testlib/infra.py +++ b/testlib/infra.py @@ -14,6 +14,7 @@ """ Methods and classes that do infrastructure tasks. """ + # isort: STDLIB import base64 import fnmatch @@ -47,7 +48,7 @@ MOUNT = "mount" -def clean_up(): # pylint: disable=too-many-branches,too-many-locals +def clean_up(): # noqa: PLR0912 """ Try to clean up after a test failure. @@ -130,13 +131,13 @@ def check_result(result, format_str, format_str_args): if remnant_filesystems != {}: error_strings.append( f"remnant filesystems: " - f'{", ".join(map(lambda x: f"{x[0]} in pool {x[1]}", remnant_filesystems.items(),))}' + f"{', '.join(map(lambda x: f'{x[0]} in pool {x[1]}', remnant_filesystems.items()))}" ) remnant_pools = StratisDbus.pool_list() if remnant_pools != []: error_strings.append( - f'remnant pools: {", ".join(name for _, name, _ in remnant_pools)}' + f"remnant pools: {', '.join(name for _, name, _ in remnant_pools)}" ) (remnant_keys, return_code, message) = StratisDbus.get_keys() @@ -144,14 +145,13 @@ def check_result(result, format_str, format_str_args): error_strings.append( f"failed to obtain information about Stratis keys: {message}" ) - else: - if remnant_keys != []: - error_strings.append(f'remnant keys: {", ".join(remnant_keys)}') + elif remnant_keys != []: + error_strings.append(f"remnant keys: {', '.join(remnant_keys)}") for mountpoint_dir in fnmatch.filter(os.listdir(VAR_TMP), f"*{MOUNT_POINT_SUFFIX}"): try: shutil.rmtree(os.path.join(VAR_TMP, mountpoint_dir)) - except Exception as err: # pylint: disable=broad-exception-caught + except Exception as err: error_strings.append( f"failed to clean up temporary mountpoint dir {mountpoint_dir}: {err}" ) @@ -159,7 +159,7 @@ def check_result(result, format_str, format_str_args): assert isinstance(error_strings, list) if error_strings: raise RuntimeError( - f'clean_up may not have succeeded: {"; ".join(error_strings)}' + f"clean_up may not have succeeded: {'; '.join(error_strings)}" ) @@ -251,7 +251,7 @@ def _check_encryption_information_consistency(self, pool_object_path, metadata): elif features is not None: self.assertNotIn("Encryption", metadata["features"]) - def run_check(self, stop_time): # pylint: disable=too-many-locals + def run_check(self, stop_time): """ Run the check. @@ -259,8 +259,7 @@ def run_check(self, stop_time): # pylint: disable=too-many-locals """ stratisd_tools = "stratisd-tools" - if PoolMetadataMonitor.verify: # pylint: disable=no-member - + if PoolMetadataMonitor.verify: # Wait for D-Bus to settle, so D-Bus and metadata can be compared time.sleep(sleep_time(stop_time, 16)) @@ -335,9 +334,9 @@ def run_check(self, stop_time): # pylint: disable=too-many-locals proc.returncode, 0, ( - f'stdout: {stdoutdata.decode("utf-8")}' + f"stdout: {stdoutdata.decode('utf-8')}" "; " - f'stderr: {stderrdata.decode("utf-8")}' + f"stderr: {stderrdata.decode('utf-8')}" ), ) except FileNotFoundError as err: @@ -370,7 +369,7 @@ def run_check(self): """ Run the check. """ - if SysfsMonitor.verify_sysfs: # pylint: disable=no-member + if SysfsMonitor.verify_sysfs: dm_devices = { os.path.basename( os.path.realpath(os.path.join(DEV_MAPPER, dmdev)) @@ -409,7 +408,7 @@ def run_check(self): """ Run the check. """ - if SymlinkMonitor.verify_devices: # pylint: disable=no-member + if SymlinkMonitor.verify_devices: try: disallowed_symlinks = [] for dev in os.listdir("/dev/disk/by-id"): @@ -429,7 +428,7 @@ class FilesystemSymlinkMonitor(unittest.TestCase): maxDiff = None - def run_check(self, stop_time): # pylint: disable=too-many-locals + def run_check(self, stop_time): """ Check that the filesystem links on the D-Bus and the filesystem links expected from looking at filesystem devicemapper paths match exactly. @@ -437,7 +436,7 @@ def run_check(self, stop_time): # pylint: disable=too-many-locals :param int stop_time: the time the test completed """ - if not FilesystemSymlinkMonitor.verify_devices: # pylint: disable=no-member + if not FilesystemSymlinkMonitor.verify_devices: return decode_dm = "stratis-decode-dm" @@ -565,7 +564,7 @@ def setUp(self): """ Set up the D-Bus monitor for a test run. """ - if DbusMonitor.monitor_dbus: # pylint: disable=no-member + if DbusMonitor.monitor_dbus: command = [ MONITOR_DBUS_SIGNALS, StratisDbus.BUS_NAME, @@ -573,11 +572,7 @@ def setUp(self): ] command.extend( f"--top-interface={intf}" - for intf in manager_interfaces( - # pylint: disable=no-member - DbusMonitor.highest_revision_number - + 1 - ) + for intf in manager_interfaces(DbusMonitor.highest_revision_number + 1) ) only_check = ( @@ -589,7 +584,6 @@ def setUp(self): ) command.append(f"--only-check={only_check}") - # pylint: disable=consider-using-with try: self.trace = subprocess.Popen( command, @@ -616,18 +610,18 @@ def run_check(self, stop_time): self.trace.send_signal(signal.SIGINT) (stdoutdata, stderrdata) = self.trace.communicate() - if self.trace.returncode == 3: + if self.trace.returncode == 3: # noqa: PLR2004 raise RuntimeError( "Failure while processing D-Bus signals: " - f'stderr: {stderrdata.decode("utf-8")}, ' - f'stdout: {stdoutdata.decode("utf-8")}' + f"stderr: {stderrdata.decode('utf-8')}, " + f"stdout: {stdoutdata.decode('utf-8')}" ) - if self.trace.returncode == 4: + if self.trace.returncode == 4: # noqa: PLR2004 raise RuntimeError( "Failure while comparing D-Bus states: " - f'stderr: {stderrdata.decode("utf-8")}, ' - f'stdout: {stdoutdata.decode("utf-8")}' + f"stderr: {stderrdata.decode('utf-8')}, " + f"stdout: {stdoutdata.decode('utf-8')}" ) msg = stdoutdata.decode("utf-8") @@ -647,7 +641,7 @@ def run_check(self, stop_time): ) -class KernelKey: # pylint: disable=attribute-defined-outside-init +class KernelKey: """ A handle for operating on keys in the kernel keyring. The specified key will be available for the lifetime of the test when used with the Python with @@ -767,7 +761,7 @@ def set_from_post_test_check_option(post_test_check): PoolMetadataMonitor.verify = PostTestCheck.POOL_METADATA in post_test_check -class MountPointManager: # pylint: disable=too-few-public-methods +class MountPointManager: """ Handle mounting Stratis filesystems in a temp directory. """ diff --git a/testlib/utils.py b/testlib/utils.py index 30a9b61..464bec4 100644 --- a/testlib/utils.py +++ b/testlib/utils.py @@ -14,6 +14,7 @@ """ Utility functions for blackbox testing. """ + # isort: STDLIB import os import random @@ -148,7 +149,6 @@ class RandomKeyTmpFile: Generate a random passphrase and put it in a temporary file. """ - # pylint: disable=consider-using-with def __init__(self, key_bytes=32): """ Initializer