Merge pull request #333 from DiamondLightSource/331_fix_tests

331 fix and improve tests

src/dodal/beamlines/beamline_parameters.py

@@ -1,4 +1,4 @@
-from typing import Any, Tuple, cast
+from typing import Any, Optional, Tuple, cast
 
 from dodal.log import LOGGER
 from dodal.utils import get_beamline_name
@@ -87,11 +87,14 @@ def parse_list(cls, value: str):
         return list_output
 
 
-def get_beamline_parameters():
-    beamline_name = get_beamline_name("s03")
-    beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline_name)
-    if beamline_param_path is None:
-        raise KeyError(
-            "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!"
-        )
+def get_beamline_parameters(beamline_param_path: Optional[str] = None):
+    """Loads the beamline parameters from the specified path, or according to the
+    environment variable if none is given"""
+    if not beamline_param_path:
+        beamline_name = get_beamline_name("s03")
+        beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline_name)
+        if beamline_param_path is None:
+            raise KeyError(
+                "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!"
+            )
     return GDABeamlineParameters.from_file(beamline_param_path)

src/dodal/beamlines/i24.py

@@ -10,14 +10,14 @@
 from dodal.devices.i24.pmac import PMAC
 from dodal.devices.oav.oav_detector import OAV, OAVConfigParams
 from dodal.devices.zebra import Zebra
-from dodal.log import set_beamline
+from dodal.log import set_beamline as set_log_beamline
 from dodal.utils import get_beamline_name, skip_device
 
 ZOOM_PARAMS_FILE = "/dls_sw/i24/software/gda/config/xml/jCameraManZoomLevels.xml"
 DISPLAY_CONFIG = "/dls_sw/i24/software/gda_versions/var/display.configuration"
 
 BL = get_beamline_name("s24")
-set_beamline(BL)
+set_log_beamline(BL)
 set_utils_beamline(BL)
 
 
@@ -108,7 +108,9 @@ def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) ->
 
 
 @skip_device(lambda: BL == "s24")
-def vgonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> OAV:
+def vgonio(
+    wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
+) -> VGonio:
     """Get the i24 vgonio device, instantiate it if it hasn't already been.
     If this is called when already instantiated, it will return the existing object.
     """

src/dodal/devices/DCM.py

@@ -15,7 +15,9 @@ def __init__(self, *args, daq_configuration_path: str, **kwargs):
         )
         # I03 configures the DCM Perp as a side effect of applying this fixed value to the DCM Offset after an energy change
         # Nb this parameter is misleadingly named to confuse you
-        self.fixed_offset_mm = get_beamline_parameters()["DCM_Perp_Offset_FIXED"]
+        self.fixed_offset_mm = get_beamline_parameters(
+            daq_configuration_path + "/domain/beamlineParameters"
+        )["DCM_Perp_Offset_FIXED"]
 
     """
     A double crystal monochromator (DCM), used to select the energy of the beam.

src/dodal/utils.py

@@ -110,7 +110,7 @@ def wrapper(*args, **kwds) -> T:
 
 
 def make_all_devices(
-    module: Union[str, ModuleType, None] = None, **kwargs
+    module: Union[str, ModuleType, None] = None, include_skipped: bool = False, **kwargs
 ) -> Dict[str, AnyDevice]:
     """Makes all devices in the given beamline module.
 
@@ -126,7 +126,7 @@ def make_all_devices(
     """
     if isinstance(module, str) or module is None:
         module = import_module(module or __name__)
-    factories = collect_factories(module)
+    factories = collect_factories(module, include_skipped)
     devices: dict[str, AnyDevice] = invoke_factories(factories, **kwargs)
 
     return devices
@@ -167,11 +167,17 @@ def extract_dependencies(
             yield name
 
 
-def collect_factories(module: ModuleType) -> dict[str, AnyDeviceFactory]:
+def collect_factories(
+    module: ModuleType, include_skipped: bool = False
+) -> dict[str, AnyDeviceFactory]:
     factories: dict[str, AnyDeviceFactory] = {}
 
     for var in module.__dict__.values():
-        if callable(var) and is_any_device_factory(var) and not _is_device_skipped(var):
+        if (
+            callable(var)
+            and is_any_device_factory(var)
+            and (include_skipped or not _is_device_skipped(var))
+        ):
             factories[var.__name__] = var
 
     return factories

tests/beamlines/unit_tests/test_beamline_utils.py

@@ -13,6 +13,14 @@
 from dodal.devices.zebra import Zebra
 from dodal.utils import make_all_devices
 
+from ...conftest import mock_beamline_module_filepaths
+
+
+@pytest.fixture
+def reset_i03():
+    beamline_utils.clear_devices()
+    mock_beamline_module_filepaths("i03", i03)
+
 
 def test_instantiate_function_makes_supplied_device():
     device_types = [Zebra, ApertureScatterguard, Smargon]
@@ -24,8 +32,7 @@ def test_instantiate_function_makes_supplied_device():
         assert isinstance(dev, device)
 
 
-def test_instantiating_different_device_with_same_name():
-    beamline_utils.clear_devices()
+def test_instantiating_different_device_with_same_name(reset_i03):
     dev1 = beamline_utils.device_instantiation(  # noqa
         Zebra, "device", "", False, False, None
     )
@@ -43,26 +50,23 @@ def test_instantiating_different_device_with_same_name():
     assert dev2 in beamline_utils.ACTIVE_DEVICES.values()
 
 
-def test_instantiate_function_fake_makes_fake():
-    beamline_utils.clear_devices()
+def test_instantiate_function_fake_makes_fake(reset_i03):
     fake_zeb: Zebra = beamline_utils.device_instantiation(
         i03.Zebra, "zebra", "", True, True, None
     )
     assert isinstance(fake_zeb, Device)
     assert isinstance(fake_zeb.pc.arm_source, FakeEpicsSignal)
 
 
-def test_clear_devices(RE):
-    beamline_utils.clear_devices()
+def test_clear_devices(RE, reset_i03):
+    mock_beamline_module_filepaths("i03", i03)
     devices = make_all_devices(i03, fake_with_ophyd_sim=True)
     assert len(beamline_utils.ACTIVE_DEVICES) == len(devices.keys())
     beamline_utils.clear_devices()
     assert beamline_utils.ACTIVE_DEVICES == {}
 
 
 def test_device_is_new_after_clearing(RE):
-    beamline_utils.clear_devices()
-
     def _make_devices_and_get_id():
         return [
             id(device)

tests/beamlines/unit_tests/test_device_instantiation.py

@@ -1,36 +1,61 @@
+import importlib
+import os
 from typing import Any
+from unittest.mock import patch
 
-from dodal.beamlines import beamline_utils, i03, i04, i04_1, i23, i24, p38, p45
+import pytest
+
+from dodal.beamlines import beamline_utils
 from dodal.utils import BLUESKY_PROTOCOLS, make_all_devices
 
-ALL_BEAMLINES = {i03, i04, i04_1, i23, i24, p38, p45}
+from ...conftest import mock_beamline_module_filepaths
+
+ALL_BEAMLINES = {"i03", "i04", "i04_1", "i23", "i24", "p38", "p45"}
 
 
 def follows_bluesky_protocols(obj: Any) -> bool:
     return any((isinstance(obj, protocol) for protocol in BLUESKY_PROTOCOLS))
 
 
-def test_device_creation(RE):
+def mock_bl(beamline):
+    bl_mod = importlib.import_module("dodal.beamlines." + beamline)
+    mock_beamline_module_filepaths(beamline, bl_mod)
+    return bl_mod
+
+
+@pytest.mark.parametrize("beamline", ALL_BEAMLINES)
+def test_device_creation(RE, beamline):
     """
     Ensures that for every beamline all device factories are using valid args
     and creating types that conform to Bluesky protocols.
     """
-    for beamline in ALL_BEAMLINES:
-        devices = make_all_devices(beamline, fake_with_ophyd_sim=True)
+    with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True):
+        bl_mod = mock_bl(beamline)
+        devices = make_all_devices(
+            bl_mod, include_skipped=True, fake_with_ophyd_sim=True
+        )
         for device_name, device in devices.items():
             assert device_name in beamline_utils.ACTIVE_DEVICES
             assert follows_bluesky_protocols(device)
         assert len(beamline_utils.ACTIVE_DEVICES) == len(devices)
         beamline_utils.clear_devices()
+        del bl_mod
 
 
-def test_devices_are_identical(RE):
+@pytest.mark.parametrize("beamline", ALL_BEAMLINES)
+def test_devices_are_identical(RE, beamline):
     """
     Ensures that for every beamline all device functions prevent duplicate instantiation.
     """
-    for beamline in ALL_BEAMLINES:
-        devices_a = make_all_devices(beamline, fake_with_ophyd_sim=True)
-        devices_b = make_all_devices(beamline, fake_with_ophyd_sim=True)
+    with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True):
+        bl_mod = mock_bl(beamline)
+        devices_a = make_all_devices(
+            bl_mod, include_skipped=True, fake_with_ophyd_sim=True
+        )
+        devices_b = make_all_devices(
+            bl_mod, include_skipped=True, fake_with_ophyd_sim=True
+        )
         for device_name in devices_a.keys():
             assert devices_a[device_name] is devices_b[device_name]
         beamline_utils.clear_devices()
+        del bl_mod

tests/beamlines/unit_tests/test_i24.py

@@ -13,13 +13,14 @@ def setup_module():
 
 
 def test_device_creation():
+    i24.BL = "i24"
     devices = make_all_devices(i24, fake_with_ophyd_sim=True)
     assert len(devices) > 0
     for device_name in devices.keys():
         assert device_name in beamline_utils.ACTIVE_DEVICES
     assert len(beamline_utils.ACTIVE_DEVICES) == len(devices)
 
-    vgonio: VGonio = beamline_utils.ACTIVE_DEVICES["vgonio"]
+    vgonio: VGonio = beamline_utils.ACTIVE_DEVICES["vgonio"]  # type: ignore
     assert vgonio.prefix == "BL24I-MO-VGON-01:"
     assert vgonio.kappa.prefix == "BL24I-MO-VGON-01:KAPPA"
 

tests/conftest.py

@@ -12,6 +12,24 @@
 from dodal.devices.focusing_mirror import VFMMirrorVoltages
 from dodal.log import LOGGER, GELFTCPHandler, set_up_logging_handlers
 
+MOCK_DAQ_CONFIG_PATH = "tests/devices/unit_tests/test_daq_configuration"
+mock_paths = [
+    ("DAQ_CONFIGURATION_PATH", MOCK_DAQ_CONFIG_PATH),
+    ("ZOOM_PARAMS_FILE", "tests/devices/unit_tests/test_jCameraManZoomLevels.xml"),
+    ("DISPLAY_CONFIG", "tests/devices/unit_tests/test_display.configuration"),
+]
+mock_attributes_table = {
+    "i03": mock_paths,
+    "s03": mock_paths,
+    "i04": mock_paths,
+    "s04": mock_paths,
+}
+
+
+def mock_beamline_module_filepaths(bl_name, bl_module):
+    if mock_attributes := mock_attributes_table.get(bl_name):
+        [bl_module.__setattr__(attr[0], attr[1]) for attr in mock_attributes]
+
 
 def pytest_runtest_setup(item):
     beamline_utils.clear_devices()