From 5adaab13b147cb8893c0b3f73e902d5052465644 Mon Sep 17 00:00:00 2001 From: Paul Larson Date: Fri, 26 Jan 2024 19:50:45 -0600 Subject: [PATCH 1/2] Use importlib instead of imp --- .../src/testflinger_device_connectors/cmd.py | 59 ++++++++++++------- .../devices/__init__.py | 49 ++++++++------- device-connectors/src/tests/test_cmd.py | 47 +++++++++++++++ device-connectors/src/tests/test_devices.py | 38 ++++++++++++ 4 files changed, 152 insertions(+), 41 deletions(-) create mode 100644 device-connectors/src/tests/test_cmd.py create mode 100644 device-connectors/src/tests/test_devices.py diff --git a/device-connectors/src/testflinger_device_connectors/cmd.py b/device-connectors/src/testflinger_device_connectors/cmd.py index 228ed4d7..b3dffdea 100755 --- a/device-connectors/src/testflinger_device_connectors/cmd.py +++ b/device-connectors/src/testflinger_device_connectors/cmd.py @@ -1,5 +1,5 @@ #!/usr/bin/env python -# Copyright (C) 2023 Canonical +# Copyright (C) 2023-2024 Canonical # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -19,33 +19,42 @@ import argparse import logging +import sys -from testflinger_device_connectors.devices import load_devices +from testflinger_device_connectors.devices import ( + DEVICE_CONNECTORS, + get_device_stage_func, +) logger = logging.getLogger() -def main(): +STAGES = ( + "provision", + "firmware_update", + "runtest", + "allocate", + "reserve", + "cleanup", +) + + +def get_args(argv=None): """main command function for testflinger-device-connectors""" - devices = load_devices() parser = argparse.ArgumentParser() # First add a subcommand for each supported device type - dev_parser = parser.add_subparsers() - for dev_name, dev_class in devices: + dev_parser = parser.add_subparsers(dest="device", required=True) + for dev_name in DEVICE_CONNECTORS: dev_subparser = dev_parser.add_parser(dev_name) - dev_module = dev_class() - # Next add the subcommands that can be used and the methods they run - cmd_subparser = dev_subparser.add_subparsers() - for cmd, func in ( - ("provision", dev_module.provision), - ("firmware_update", dev_module.firmware_update), - ("runtest", dev_module.runtest), - ("allocate", dev_module.allocate), - ("reserve", dev_module.reserve), - ("cleanup", dev_module.cleanup), - ): - cmd_parser = cmd_subparser.add_parser(cmd) + + # Next add the subcommands that can be used + cmd_subparser = dev_subparser.add_subparsers( + dest="stage", required=True + ) + + for stage in STAGES: + cmd_parser = cmd_subparser.add_parser(stage) cmd_parser.add_argument( "-c", "--config", @@ -55,6 +64,14 @@ def main(): cmd_parser.add_argument( "job_data", help="Testflinger json data file" ) - cmd_parser.set_defaults(func=func) - args = parser.parse_args() - raise SystemExit(args.func(args)) + + return parser.parse_args(argv) + + +def main(): + """ + Dynamically load the selected module and call the selected method + """ + args = get_args() + func = get_device_stage_func(args.device, args.stage) + sys.exit(func(args)) diff --git a/device-connectors/src/testflinger_device_connectors/devices/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/__init__.py index b6cf6fde..ffd95473 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/__init__.py @@ -1,4 +1,4 @@ -# Copyright (C) 2015 Canonical +# Copyright (C) 2015-2024 Canonical # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -12,7 +12,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see -import imp import json import logging import multiprocessing @@ -22,6 +21,8 @@ import subprocess import time from datetime import datetime, timedelta +from importlib import import_module +from typing import Callable import yaml @@ -31,6 +32,23 @@ ) +DEVICE_CONNECTORS = ( + "cm3", + "dell_oemscript", + "dragonboard", + "hp_oemscript", + "iotscript", + "lenovo_oemscript", + "maas2", + "multi", + "muxpi", + "netboot", + "noprovision", + "oemrecovery", + "oemscript", +) + + class ProvisioningError(Exception): pass @@ -329,21 +347,12 @@ def wrapper(*args, **kwargs): return _wrapper -def load_devices(): - devices = [] - device_path = os.path.dirname(os.path.realpath(__file__)) - devs = [ - os.path.join(device_path, device) - for device in os.listdir(device_path) - if os.path.isdir(os.path.join(device_path, device)) - ] - for device in devs: - if "__pycache__" in device: - continue - module = imp.load_source("module", os.path.join(device, "__init__.py")) - devices.append((module.device_name, module.DeviceConnector)) - return tuple(devices) - - -if __name__ == "__main__": - load_devices() +def get_device_stage_func(device: str, stage: str) -> Callable: + """ + Load the selected device connector and return the selected stage method + """ + module = import_module(f".{device}", package=__package__) + device_class = getattr(module, "DeviceConnector") + device_instance = device_class() + stage_method = getattr(device_instance, stage) + return stage_method diff --git a/device-connectors/src/tests/test_cmd.py b/device-connectors/src/tests/test_cmd.py new file mode 100644 index 00000000..acb545a2 --- /dev/null +++ b/device-connectors/src/tests/test_cmd.py @@ -0,0 +1,47 @@ +# Copyright (C) 2024 Canonical +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see +"""Tests for the cmd argument parser""" + +import pytest +from testflinger_device_connectors.cmd import get_args + + +def test_good_args(): + """Test that we can parse good arguments""" + argv = ["noprovision", "reserve", "-c", "config.cfg", "job_data.json"] + + args = get_args(argv) + + assert args.device == "noprovision" + assert args.stage == "reserve" + assert args.config == "config.cfg" + assert args.job_data == "job_data.json" + + +def test_invalid_device(): + """Test that an invalid device raises an exception""" + argv = ["INVALID", "provision", "-c", "config.cfg", "job_data.json"] + + with pytest.raises(SystemExit) as err: + get_args(argv) + assert err.value.code == 2 + + +def test_invalid_stage(): + """Test that an invalid stage raises an exception""" + argv = ["INVALID", "provision", "-c", "config.cfg", "job_data.json"] + + with pytest.raises(SystemExit) as err: + get_args(argv) + assert err.value.code == 2 diff --git a/device-connectors/src/tests/test_devices.py b/device-connectors/src/tests/test_devices.py new file mode 100644 index 00000000..98c8d2e5 --- /dev/null +++ b/device-connectors/src/tests/test_devices.py @@ -0,0 +1,38 @@ +# Copyright (C) 2024 Canonical +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see +"""Tests for the devices module""" + +from importlib import import_module +from itertools import product + +import pytest + +from testflinger_device_connectors.cmd import STAGES +from testflinger_device_connectors.devices import ( + DEVICE_CONNECTORS, + get_device_stage_func, +) + +STAGES_CONNECTORS_PRODUCT = tuple(product(STAGES, DEVICE_CONNECTORS)) + + +@pytest.mark.parametrize("stage,device", STAGES_CONNECTORS_PRODUCT) +def test_get_device_stage_func(stage, device): + """Check that we can load all stages from all device connectors""" + connector_instance = import_module( + f"testflinger_device_connectors.devices.{device}" + ).DeviceConnector() + orig_func = getattr(connector_instance, stage) + func = get_device_stage_func(device, stage) + assert func.__func__ is orig_func.__func__ From dc0b0ef766e6905479d68cee97cd6867e60998b7 Mon Sep 17 00:00:00 2001 From: Paul Larson Date: Thu, 1 Feb 2024 08:39:31 -0600 Subject: [PATCH 2/2] black 24.1.1 formatting updates --- .../testflinger_device_connectors/devices/cm3/__init__.py | 1 - .../src/testflinger_device_connectors/devices/cm3/cm3.py | 1 - .../devices/dell_oemscript/__init__.py | 1 - .../devices/dragonboard/__init__.py | 1 - .../devices/dragonboard/dragonboard.py | 1 - .../devices/hp_oemscript/__init__.py | 1 - .../devices/lenovo_oemscript/__init__.py | 1 - .../devices/maas2/__init__.py | 1 - .../testflinger_device_connectors/devices/maas2/maas2.py | 1 - .../devices/multi/__init__.py | 1 - .../testflinger_device_connectors/devices/multi/multi.py | 1 - .../devices/muxpi/__init__.py | 1 - .../testflinger_device_connectors/devices/muxpi/muxpi.py | 1 - .../devices/netboot/__init__.py | 1 - .../devices/netboot/netboot.py | 1 - .../devices/noprovision/noprovision.py | 1 - .../devices/oemrecovery/__init__.py | 1 - .../devices/oemrecovery/oemrecovery.py | 1 - .../devices/oemscript/__init__.py | 1 - .../testflinger_device_connectors/fw_devices/LVFS/LVFS.py | 1 - .../fw_devices/LVFS/tests/test_LVFS.py | 7 +++---- .../fw_devices/tests/test_firmware_update.py | 1 - 22 files changed, 3 insertions(+), 25 deletions(-) diff --git a/device-connectors/src/testflinger_device_connectors/devices/cm3/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/cm3/__init__.py index d8bad8f1..92d020b7 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/cm3/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/cm3/__init__.py @@ -32,7 +32,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/cm3/cm3.py b/device-connectors/src/testflinger_device_connectors/devices/cm3/cm3.py index d24fcab6..732af86e 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/cm3/cm3.py +++ b/device-connectors/src/testflinger_device_connectors/devices/cm3/cm3.py @@ -32,7 +32,6 @@ class CM3: - """Device Connector for CM3.""" IMAGE_PATH_IDS = { diff --git a/device-connectors/src/testflinger_device_connectors/devices/dell_oemscript/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/dell_oemscript/__init__.py index e44874ad..715f34f6 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/dell_oemscript/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/dell_oemscript/__init__.py @@ -36,7 +36,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning Dell OEM devices with an oem image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/dragonboard/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/dragonboard/__init__.py index 05a24d1a..206046f3 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/dragonboard/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/dragonboard/__init__.py @@ -34,7 +34,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/dragonboard/dragonboard.py b/device-connectors/src/testflinger_device_connectors/devices/dragonboard/dragonboard.py index 899630d2..6d8fdd52 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/dragonboard/dragonboard.py +++ b/device-connectors/src/testflinger_device_connectors/devices/dragonboard/dragonboard.py @@ -33,7 +33,6 @@ class Dragonboard: - """Testflinger Device Connector for Dragonboard.""" def __init__(self, config, job_data): diff --git a/device-connectors/src/testflinger_device_connectors/devices/hp_oemscript/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/hp_oemscript/__init__.py index df23ed08..a813ed0e 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/hp_oemscript/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/hp_oemscript/__init__.py @@ -36,7 +36,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning HP OEM devices with an oem image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/lenovo_oemscript/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/lenovo_oemscript/__init__.py index 2716962c..048f0632 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/lenovo_oemscript/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/lenovo_oemscript/__init__.py @@ -36,7 +36,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning Lenovo OEM devices with an oem image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/maas2/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/maas2/__init__.py index 4cf6bd17..e3150497 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/maas2/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/maas2/__init__.py @@ -33,7 +33,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/maas2/maas2.py b/device-connectors/src/testflinger_device_connectors/devices/maas2/maas2.py index 06944c1c..7b0eb8a4 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/maas2/maas2.py +++ b/device-connectors/src/testflinger_device_connectors/devices/maas2/maas2.py @@ -37,7 +37,6 @@ class Maas2: - """Device Connector for Maas2.""" def __init__(self, config, job_data): diff --git a/device-connectors/src/testflinger_device_connectors/devices/multi/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/multi/__init__.py index 3ed76d70..4f4d8c6b 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/multi/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/multi/__init__.py @@ -32,7 +32,6 @@ class DeviceConnector(DefaultDevice): - """Device Connector for provisioning multiple devices at the same time""" def init_device(self, args): diff --git a/device-connectors/src/testflinger_device_connectors/devices/multi/multi.py b/device-connectors/src/testflinger_device_connectors/devices/multi/multi.py index 6d65012f..5e305377 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/multi/multi.py +++ b/device-connectors/src/testflinger_device_connectors/devices/multi/multi.py @@ -25,7 +25,6 @@ class Multi: - """Device Connector for multi-device""" def __init__(self, config, job_data, client): diff --git a/device-connectors/src/testflinger_device_connectors/devices/muxpi/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/muxpi/__init__.py index fff51a48..c0ddddf6 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/muxpi/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/muxpi/__init__.py @@ -32,7 +32,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/muxpi/muxpi.py b/device-connectors/src/testflinger_device_connectors/devices/muxpi/muxpi.py index 0380b0aa..a9c9bd4b 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/muxpi/muxpi.py +++ b/device-connectors/src/testflinger_device_connectors/devices/muxpi/muxpi.py @@ -32,7 +32,6 @@ class MuxPi: - """Device Connector for MuxPi.""" IMAGE_PATH_IDS = { diff --git a/device-connectors/src/testflinger_device_connectors/devices/netboot/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/netboot/__init__.py index df52a3c9..b0ca4b3b 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/netboot/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/netboot/__init__.py @@ -34,7 +34,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/netboot/netboot.py b/device-connectors/src/testflinger_device_connectors/devices/netboot/netboot.py index ddb62b76..34f881f1 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/netboot/netboot.py +++ b/device-connectors/src/testflinger_device_connectors/devices/netboot/netboot.py @@ -31,7 +31,6 @@ class Netboot: - """Testflinger Device Connector for Netboot.""" def __init__(self, config): diff --git a/device-connectors/src/testflinger_device_connectors/devices/noprovision/noprovision.py b/device-connectors/src/testflinger_device_connectors/devices/noprovision/noprovision.py index 463cd1d6..93102aaf 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/noprovision/noprovision.py +++ b/device-connectors/src/testflinger_device_connectors/devices/noprovision/noprovision.py @@ -29,7 +29,6 @@ class Noprovision: - """Testflinger Device Connector for Noprovision.""" def __init__(self, config): diff --git a/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/__init__.py index 331106a5..d2dbee2a 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/__init__.py @@ -33,7 +33,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/oemrecovery.py b/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/oemrecovery.py index fe377492..8975c05d 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/oemrecovery.py +++ b/device-connectors/src/testflinger_device_connectors/devices/oemrecovery/oemrecovery.py @@ -30,7 +30,6 @@ class OemRecovery: - """Device Connector for OEM Recovery.""" def __init__(self, config, job_data): diff --git a/device-connectors/src/testflinger_device_connectors/devices/oemscript/__init__.py b/device-connectors/src/testflinger_device_connectors/devices/oemscript/__init__.py index 766141e5..4cef65f3 100644 --- a/device-connectors/src/testflinger_device_connectors/devices/oemscript/__init__.py +++ b/device-connectors/src/testflinger_device_connectors/devices/oemscript/__init__.py @@ -31,7 +31,6 @@ class DeviceConnector(DefaultDevice): - """Tool for provisioning baremetal with a given image.""" @catch(RecoveryError, 46) diff --git a/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/LVFS.py b/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/LVFS.py index 54e79f9f..178a104e 100644 --- a/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/LVFS.py +++ b/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/LVFS.py @@ -1,6 +1,5 @@ """Device class for flashing firmware on device supported by LVFS-fwupd""" - import subprocess import json import time diff --git a/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/tests/test_LVFS.py b/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/tests/test_LVFS.py index 8596195f..f340d1c5 100644 --- a/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/tests/test_LVFS.py +++ b/device-connectors/src/testflinger_device_connectors/fw_devices/LVFS/tests/test_LVFS.py @@ -1,6 +1,5 @@ """Test LVFSDevice""" - import unittest import json from unittest.mock import patch @@ -93,9 +92,9 @@ def test_check_results_good(self): device = LVFSDevice("", "", "") device._parse_fwupd_raw(fwupd_data.GET_DEVICES_RESPONSE_DATA) device_results = json.loads(fwupd_data.GET_RESULTS_RESPONSE_DATA) - device_results[ - "UpdateState" - ] = FwupdUpdateState.FWUPD_UPDATE_STATE_SUCCESS.value + device_results["UpdateState"] = ( + FwupdUpdateState.FWUPD_UPDATE_STATE_SUCCESS.value + ) device_results["Releases"][0]["Version"] = "2.90" device.fw_info[2]["targetVersion"] = "2.90" self.assertTrue(device.check_results()) diff --git a/device-connectors/src/testflinger_device_connectors/fw_devices/tests/test_firmware_update.py b/device-connectors/src/testflinger_device_connectors/fw_devices/tests/test_firmware_update.py index 9bbcf8c3..bf224458 100644 --- a/device-connectors/src/testflinger_device_connectors/fw_devices/tests/test_firmware_update.py +++ b/device-connectors/src/testflinger_device_connectors/fw_devices/tests/test_firmware_update.py @@ -1,6 +1,5 @@ """Test detect_device in firmware_update.py""" - import unittest import pytest from unittest.mock import patch