Merge pull request #109 from ISISComputingGroup/Ticket8378_add_ruff_formatting

Ticket8378 add ruff formatting

Author: Tom-Willemsen

.git-blame-ignore-revs

@@ -0,0 +1,2 @@
+adc8be0517795a04d559236949315f101fde9dc6
+fdec7664460c9b21e33e61ab604410ce1a0165e1

.github/workflows/linter.yml

@@ -0,0 +1,7 @@
+name: Linter
+on: [pull_request]
+jobs:
+  call-workflow:
+    uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main
+    with:
+      compare-branch: origin/master

TODO_test_start_stop_ioc_through_proc_control.py

@@ -1,6 +1,7 @@
 """
 System tests for starting or stopping iocs through Proc server
 """
+
 import os
 import unittest
 import xml.etree.ElementTree as ET
@@ -10,48 +11,56 @@
 from hamcrest import assert_that, less_than
 from six.moves import range
 
-from utilities.utilities import g, as_seconds, start_ioc, stop_ioc, wait_for_ioc_start_stop, \
-    load_config_if_not_already_loaded, bulk_start_ioc, bulk_stop_ioc
+from utilities.utilities import (
+    as_seconds,
+    bulk_start_ioc,
+    bulk_stop_ioc,
+    g,
+    load_config_if_not_already_loaded,
+    start_ioc,
+    stop_ioc,
+    wait_for_ioc_start_stop,
+)
 
 # The following iocs are ignored in the test which starts/stops all iocs
 # This is usually because they don't build by default, or have some complex dependency,
 # or are special in some way (e.g. psctrl).
 # we also ignore ISISDAE, INSTETC and RUNCTRL as testing them here messed up subsequent tests
 # by leaving these IOCs permanently stopped. We could re-enable testing them if this test was either
-# always ran last or could re-enabel autostart on these ioc afterwards    
+# always ran last or could re-enabel autostart on these ioc afterwards
 IOCS_TO_IGNORE_START_STOP = [
-    'ASTRIUM_01',
-    'ASTRIUM_02',
-    'BGRSCRPT_01',  # Won't keep running unless it has a config file
-    'BGRSCRPT_02',
-    'CHOPPERSIM',  # Simulation ioc
-    'CAENMCA',  # currently fails to start, and is not used so skip
-    'DELFTDCMAG_01',  # Delft iocs have a weird build/run process?
-    'DELFTDCMAG_02',
-    'DELFTSHEAR_01',
-    'ECLAB_01',
-    'INSTETC',
-    'ISISDAE',
-    'LSICORR_01',  # Needs vendor library in correct place to keep running
-    'LSICORR_02',
-    'MOTORSIM',  # Simulation ioc
-    'MOXA12XX_01',
-    'MOXA12XX_02',
-    'MOXA12XX_03',
-    'MK3CHOPR_01',
-    'NANODAC_01',
-    'OERCONE_02',
-    'PIXELMAN',
-    'PSCTRL',  # Special, controls other IOCs
-    'REFL_01',  # Won't run correctly without a config
-    'RUNCTRL',
-    'SECI2IBEX',  # requires labview
-    'SEPRTR',  # relies on daqMX
-    'TC_01',  # relies on twincat
-    'ZFMAGFLD'  # relies on daqMX
+    "ASTRIUM_01",
+    "ASTRIUM_02",
+    "BGRSCRPT_01",  # Won't keep running unless it has a config file
+    "BGRSCRPT_02",
+    "CHOPPERSIM",  # Simulation ioc
+    "CAENMCA",  # currently fails to start, and is not used so skip
+    "DELFTDCMAG_01",  # Delft iocs have a weird build/run process?
+    "DELFTDCMAG_02",
+    "DELFTSHEAR_01",
+    "ECLAB_01",
+    "INSTETC",
+    "ISISDAE",
+    "LSICORR_01",  # Needs vendor library in correct place to keep running
+    "LSICORR_02",
+    "MOTORSIM",  # Simulation ioc
+    "MOXA12XX_01",
+    "MOXA12XX_02",
+    "MOXA12XX_03",
+    "MK3CHOPR_01",
+    "NANODAC_01",
+    "OERCONE_02",
+    "PIXELMAN",
+    "PSCTRL",  # Special, controls other IOCs
+    "REFL_01",  # Won't run correctly without a config
+    "RUNCTRL",
+    "SECI2IBEX",  # requires labview
+    "SEPRTR",  # relies on daqMX
+    "TC_01",  # relies on twincat
+    "ZFMAGFLD",  # relies on daqMX
 ]
 
-GLOBALS_FILENAME = os.path.join(os.environ['ICPCONFIGROOT'], "globals.txt")
+GLOBALS_FILENAME = os.path.join(os.environ["ICPCONFIGROOT"], "globals.txt")
 
 
 class TestProcControl(unittest.TestCase):
@@ -104,20 +113,29 @@ def test_GIVEN_ioc_is_stopped_WHEN_call_start_multiple_times_quickly_THEN_ioc_is
         g.waitfor_time(seconds=5)  # wait just in case it is starting
         wait_for_ioc_start_stop(timeout=30, is_start=True, ioc_name="SIMPLE")
 
-    def test_GIVEN_ioc_is_running_WHEN_call_restart_multiple_times_quickly_THEN_ioc_is_restarted(self):
+    def test_GIVEN_ioc_is_running_WHEN_call_restart_multiple_times_quickly_THEN_ioc_is_restarted(
+        self,
+    ):
         time_to_restart_and_read_uptime = 10
         start_ioc(ioc_name="SIMPLE")
-        while as_seconds(g.get_pv("CS:IOC:SIMPLE:DEVIOS:UPTIME", is_local=True)) < time_to_restart_and_read_uptime:
+        while (
+            as_seconds(g.get_pv("CS:IOC:SIMPLE:DEVIOS:UPTIME", is_local=True))
+            < time_to_restart_and_read_uptime
+        ):
             g.waitfor_time(seconds=1)
         for _ in range(20):
             g.set_pv("CS:PS:SIMPLE:RESTART", 1, is_local=True, wait=False)
 
         wait_for_ioc_start_stop(timeout=30, is_start=True, ioc_name="SIMPLE")
-        assert_that(as_seconds(g.get_pv("CS:IOC:SIMPLE:DEVIOS:UPTIME", is_local=True)),
-                    less_than(time_to_restart_and_read_uptime), "Uptime")
-
-    def test_GIVEN_ioc_is_off_WHEN_call_restart_multiple_times_quickly_THEN_ioc_is_still_stopped(self):
+        assert_that(
+            as_seconds(g.get_pv("CS:IOC:SIMPLE:DEVIOS:UPTIME", is_local=True)),
+            less_than(time_to_restart_and_read_uptime),
+            "Uptime",
+        )
 
+    def test_GIVEN_ioc_is_off_WHEN_call_restart_multiple_times_quickly_THEN_ioc_is_still_stopped(
+        self,
+    ):
         stop_ioc(ioc_name="SIMPLE")
 
         for _ in range(20):
@@ -126,7 +144,6 @@ def test_GIVEN_ioc_is_off_WHEN_call_restart_multiple_times_quickly_THEN_ioc_is_s
         wait_for_ioc_start_stop(timeout=30, is_start=False, ioc_name="SIMPLE")
 
     def test_WHEN_start_iocs_THEN_iocs_started_WHEN_stop_iocs_THEN_iocs_stopped(self):
-
         # A test to check all IOCs start and stop correctly
         # Implemented to test for the error we encountered where we met our procserv limit and some iocs didn't start
 
@@ -137,16 +154,17 @@ def test_WHEN_start_iocs_THEN_iocs_started_WHEN_stop_iocs_THEN_iocs_stopped(self
         number_to_run = 40
 
         ## disable for moment
-        #iocs_to_test = self._prepare_ioc_list()
+        # iocs_to_test = self._prepare_ioc_list()
         iocs_to_test = []
 
         # Test handles Channel access exceptions, so set us to handle it to reduce prints.
         g.toggle.exceptions_raised(True)
         for chunk in self._chunk_iocs(iocs_to_test, number_to_run):
             start_time = time()
             failed_to_start, not_in_proc_serv = bulk_start_ioc(chunk)
-            failed_to_stop = bulk_stop_ioc([ioc for ioc in chunk if ioc not in failed_to_start
-                                            and ioc not in not_in_proc_serv])
+            failed_to_stop = bulk_stop_ioc(
+                [ioc for ioc in chunk if ioc not in failed_to_start and ioc not in not_in_proc_serv]
+            )
             for ioc in failed_to_start + failed_to_stop:
                 if not self._retry_in_recsim(ioc):
                     error_iocs.append(ioc)
@@ -167,25 +185,36 @@ def _prepare_ioc_list(self):
         :return: The list of IOCs to test, sorted alphanumerically.
         """
         iocs_to_test = []
-        tree = ET.parse(os.path.join("C:\\", "Instrument", "Apps", "EPICS", "iocstartup", "config.xml"))
+        tree = ET.parse(
+            os.path.join("C:\\", "Instrument", "Apps", "EPICS", "iocstartup", "config.xml")
+        )
         root = tree.getroot()
 
         # IOCs are listed in the above XML file under two different schemas, we need both
         schemas = (
             "{http://epics.isis.rl.ac.uk/schema/ioc_config/1.0}",
-            "{http://epics.isis.rl.ac.uk/schema/ioc_configs/1.0}"
+            "{http://epics.isis.rl.ac.uk/schema/ioc_configs/1.0}",
         )
 
         for schema in schemas:
-            iocs_to_test.extend([ioc_config.attrib["name"] for ioc_config in root.iter(f"{schema}ioc_config")])
+            iocs_to_test.extend(
+                [ioc_config.attrib["name"] for ioc_config in root.iter(f"{schema}ioc_config")]
+            )
 
         # Check parsed IOCs are a sensible length check there's at least one known ioc in the list
         if not len(iocs_to_test) > 100:
-            if not any(item in iocs_to_test for item in ["SIMPLE", "AMINT2L_01", "EUROTHRM_01", "INSTETC_01"]):
+            if not any(
+                item in iocs_to_test
+                for item in ["SIMPLE", "AMINT2L_01", "EUROTHRM_01", "INSTETC_01"]
+            ):
                 # Fairly long test so error out early if IOCs aren't in a sensible state
                 raise ValueError("List of IOCs not in a sensible state. Have you run IOC startups?")
         # Check IOC 1 and IOC2, but not other IOCs as they should follow the same format as IOC 2.
-        iocs_to_test = [ioc for ioc in iocs_to_test if self._skip_high_ioc_nums(ioc) and not self._ignore_ioc(ioc)]
+        iocs_to_test = [
+            ioc
+            for ioc in iocs_to_test
+            if self._skip_high_ioc_nums(ioc) and not self._ignore_ioc(ioc)
+        ]
         iocs_to_test.sort()
         return iocs_to_test
 
@@ -216,7 +245,7 @@ def _chunk_iocs(ioc_list: List[str], chunk_size: int):
         :return: an iterator that gives the next chunk of the list.
         """
         for i in range(0, len(ioc_list), chunk_size):
-            yield ioc_list[i:i + chunk_size]
+            yield ioc_list[i : i + chunk_size]
 
     @staticmethod
     def _retry_in_recsim(ioc: str):

run_tests.py

@@ -17,42 +17,61 @@
 Run system tests for genie_python. Copies across needed configs before running the tests.
 """
 
+import argparse
 import os
+import shutil
 import sys
 import time
 import unittest
 
-import shutil
 import xmlrunner
-import argparse
-from utilities import utilities
 from genie_python import genie as g
 
+from utilities import utilities
 
 SCRIPT_DIRECTORY = os.path.abspath(os.path.join(os.path.dirname(__file__)))
-DEFAULT_DIRECTORY = os.path.join(SCRIPT_DIRECTORY, 'test-reports')
-CONFIGS_DIRECTORY = os.path.join(SCRIPT_DIRECTORY, 'configs')
+DEFAULT_DIRECTORY = os.path.join(SCRIPT_DIRECTORY, "test-reports")
+CONFIGS_DIRECTORY = os.path.join(SCRIPT_DIRECTORY, "configs")
 
 NUM_RETRY_DELETION = 5
 
 # default icp config path
-default_configs_path = os.path.join("C:\\", "Instrument", "Settings", "config",
-                                    os.environ.get("COMPUTERNAME", "NAME"), "configurations")
+default_configs_path = os.path.join(
+    "C:\\",
+    "Instrument",
+    "Settings",
+    "config",
+    os.environ.get("COMPUTERNAME", "NAME"),
+    "configurations",
+)
 # path to ICP CONFIG ROOT
 PATH_TO_ICPCONFIGROOT = os.environ.get("ICPCONFIGROOT", default_configs_path)
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     # get output directory from command line arguments
     parser = argparse.ArgumentParser()
-    parser.add_argument('-o', '--output_dir', default=DEFAULT_DIRECTORY,
-                        help='The directory to save the test reports')
-    parser.add_argument('-t', '--tests', default=None, nargs='+',
-                        help="""Dotted names of tests to run. These are of the form module.class.method.
+    parser.add_argument(
+        "-o",
+        "--output_dir",
+        default=DEFAULT_DIRECTORY,
+        help="The directory to save the test reports",
+    )
+    parser.add_argument(
+        "-t",
+        "--tests",
+        default=None,
+        nargs="+",
+        help="""Dotted names of tests to run. These are of the form module.class.method.
                                     Module just runs the tests in a module. 
                                     Module.class runs the the test class in Module.
-                                    Module.class.method runs a specific test.""")
-    parser.add_argument('-f', '--failfast', action='store_true',
-                        help="""Determines if the rest of tests are skipped after the first failure""")
+                                    Module.class.method runs a specific test.""",
+    )
+    parser.add_argument(
+        "-f",
+        "--failfast",
+        action="store_true",
+        help="""Determines if the rest of tests are skipped after the first failure""",
+    )
 
     arguments = parser.parse_args()
     xml_dir = arguments.output_dir
@@ -64,8 +83,11 @@
     else:
         test_suite = unittest.TestLoader().discover(SCRIPT_DIRECTORY, pattern="test_*.py")
 
-    config_dirs = [name for name in os.listdir(CONFIGS_DIRECTORY)
-                   if os.path.isdir(os.path.join(CONFIGS_DIRECTORY, name))]
+    config_dirs = [
+        name
+        for name in os.listdir(CONFIGS_DIRECTORY)
+        if os.path.isdir(os.path.join(CONFIGS_DIRECTORY, name))
+    ]
 
     for config_dir in config_dirs:
         dest = os.path.join(PATH_TO_ICPCONFIGROOT, config_dir)
@@ -99,10 +121,15 @@
     utilities.load_config_if_not_already_loaded("empty_for_system_tests")
     utilities.wait_for_iocs_to_be_up(["ISISDAE_01"], 300)
 
-
     print("\n\n------ BEGINNING genie_python SYSTEM TESTS ------")
     ret_vals = list()
-    ret_vals.append(xmlrunner.XMLTestRunner(output=xml_dir, stream=sys.stdout, failfast=failfast_switch, verbosity=3).run(test_suite).wasSuccessful())
+    ret_vals.append(
+        xmlrunner.XMLTestRunner(
+            output=xml_dir, stream=sys.stdout, failfast=failfast_switch, verbosity=3
+        )
+        .run(test_suite)
+        .wasSuccessful()
+    )
     print("------ UNIT TESTS COMPLETE ------\n\n")
 
     # Return failure exit code if a test failed