From a3a3cf6cf9d8f228e49424bc12e8a9fcbb77affa Mon Sep 17 00:00:00 2001 From: rtuck99 Date: Fri, 25 Oct 2024 16:46:42 +0100 Subject: [PATCH 1/2] Fix unit test borkage due to loading from /dls_sw (#867) --- tests/conftest.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index f410e4cbdb..10d6ea14a2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,7 +17,7 @@ PathProvider, ) -from dodal.common.beamlines import beamline_utils +from dodal.common.beamlines import beamline_parameters, beamline_utils from dodal.common.visit import ( DirectoryServiceClient, LocalDirectoryServiceClient, @@ -37,8 +37,29 @@ "s03": mock_paths, "i04": mock_paths, "s04": mock_paths, + "i24": mock_paths, } +BANNED_PATHS = [Path("/dls"), Path("/dls_sw")] + + +@pytest.fixture(autouse=True) +def patch_open_to_prevent_dls_reads_in_tests(): + unpatched_open = open + + def patched_open(*args, **kwargs): + requested_path = Path(args[0]) + if requested_path.is_absolute(): + for p in BANNED_PATHS: + assert not requested_path.is_relative_to( + p + ), f"Attempt to open {requested_path} from inside a unit test" + return unpatched_open(*args, **kwargs) + + with patch("builtins.open", side_effect=patched_open): + yield [] + + # Prevent pytest from catching exceptions when debugging in vscode so that break on # exception works correctly (see: https://github.com/pytest-dev/pytest/issues/7409) if os.getenv("PYTEST_RAISE", "0") == "1": @@ -55,6 +76,9 @@ def pytest_internalerror(excinfo): 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] + beamline_parameters.BEAMLINE_PARAMETER_PATHS[bl_name] = ( + "tests/test_data/i04_beamlineParameters" + ) @pytest.fixture(scope="function") From ff126104835d099dfe395fdf7ea538d233cda090 Mon Sep 17 00:00:00 2001 From: rtuck99 Date: Fri, 25 Oct 2024 17:05:45 +0100 Subject: [PATCH 2/2] 850 fix for exceptions being ignored in beamline unit tests (#868) * Fix exceptions in factory methods not caught in tests * Fix broken device initialisation * Fix i10 beamline tests * Change undulator default lookup table path to os.devnull --- src/dodal/beamlines/i22.py | 3 +-- src/dodal/beamlines/p38.py | 3 +-- src/dodal/devices/i22/dcm.py | 15 +++++++-------- src/dodal/devices/undulator.py | 3 ++- tests/beamlines/unit_tests/test_i24.py | 3 ++- .../common/beamlines/test_device_instantiation.py | 5 +++-- tests/conftest.py | 6 ++++-- tests/devices/i22/test_dcm.py | 4 ++-- 8 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/dodal/beamlines/i22.py b/src/dodal/beamlines/i22.py index 4b42694a98..80797c64ef 100644 --- a/src/dodal/beamlines/i22.py +++ b/src/dodal/beamlines/i22.py @@ -165,11 +165,10 @@ def dcm( return device_instantiation( DoubleCrystalMonochromator, "dcm", - "", + f"{BeamlinePrefix(BL).beamline_prefix}-MO-DCM-01:", wait_for_connection, fake_with_ophyd_sim, bl_prefix=False, - motion_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-MO-DCM-01:", temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", crystal_1_metadata=CrystalMetadata( usage="Bragg", diff --git a/src/dodal/beamlines/p38.py b/src/dodal/beamlines/p38.py index 11da31b0da..07bf0d7e80 100644 --- a/src/dodal/beamlines/p38.py +++ b/src/dodal/beamlines/p38.py @@ -222,11 +222,10 @@ def dcm( return device_instantiation( DoubleCrystalMonochromator, "dcm", - "", + f"{BeamlinePrefix(BL).beamline_prefix}-MO-DCM-01:", wait_for_connection, fake_with_ophyd_sim, bl_prefix=False, - motion_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-MO-DCM-01:", temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", crystal_1_metadata=CrystalMetadata( usage="Bragg", diff --git a/src/dodal/devices/i22/dcm.py b/src/dodal/devices/i22/dcm.py index 6424dfc604..352ed4fe74 100644 --- a/src/dodal/devices/i22/dcm.py +++ b/src/dodal/devices/i22/dcm.py @@ -39,7 +39,6 @@ class DoubleCrystalMonochromator(StandardReadable): def __init__( self, - motion_prefix: str, temperature_prefix: str, crystal_1_metadata: CrystalMetadata | None = None, crystal_2_metadata: CrystalMetadata | None = None, @@ -48,13 +47,13 @@ def __init__( ) -> None: with self.add_children_as_readables(): # Positionable Parameters - self.bragg = Motor(motion_prefix + "BRAGG") - self.offset = Motor(motion_prefix + "OFFSET") - self.perp = Motor(motion_prefix + "PERP") - self.energy = Motor(motion_prefix + "ENERGY") - self.crystal_1_roll = Motor(motion_prefix + "XTAL1:ROLL") - self.crystal_2_roll = Motor(motion_prefix + "XTAL2:ROLL") - self.crystal_2_pitch = Motor(motion_prefix + "XTAL2:PITCH") + self.bragg = Motor(prefix + "BRAGG") + self.offset = Motor(prefix + "OFFSET") + self.perp = Motor(prefix + "PERP") + self.energy = Motor(prefix + "ENERGY") + self.crystal_1_roll = Motor(prefix + "XTAL1:ROLL") + self.crystal_2_roll = Motor(prefix + "XTAL2:ROLL") + self.crystal_2_pitch = Motor(prefix + "XTAL2:PITCH") # Temperatures self.backplate_temp = epics_signal_r(float, temperature_prefix + "PT100-7") diff --git a/src/dodal/devices/undulator.py b/src/dodal/devices/undulator.py index 5e61564073..6309fb4f04 100644 --- a/src/dodal/devices/undulator.py +++ b/src/dodal/devices/undulator.py @@ -1,3 +1,4 @@ +import os from enum import Enum import numpy as np @@ -54,7 +55,7 @@ class Undulator(StandardReadable, Movable): def __init__( self, prefix: str, - id_gap_lookup_table_path: str, + id_gap_lookup_table_path: str = os.devnull, name: str = "", poles: int | None = None, length: float | None = None, diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index 6a9256ae27..4736e3d69f 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -5,7 +5,8 @@ @pytest.mark.parametrize("module_and_devices_for_beamline", ["i24"], indirect=True) def test_device_creation(RE, module_and_devices_for_beamline): - _, devices = module_and_devices_for_beamline + _, devices, exceptions = module_and_devices_for_beamline + assert not exceptions vgonio: VGonio = devices["vgonio"] # type: ignore assert vgonio.prefix == "BL24I-MO-VGON-01:" assert vgonio.kappa.prefix == "BL24I-MO-VGON-01:KAPPA" diff --git a/tests/common/beamlines/test_device_instantiation.py b/tests/common/beamlines/test_device_instantiation.py index 704bad1343..eac84157f5 100644 --- a/tests/common/beamlines/test_device_instantiation.py +++ b/tests/common/beamlines/test_device_instantiation.py @@ -21,7 +21,8 @@ def test_device_creation(RE, module_and_devices_for_beamline): Ensures that for every beamline all device factories are using valid args and creating types that conform to Bluesky protocols. """ - module, devices = module_and_devices_for_beamline + module, devices, exceptions = module_and_devices_for_beamline + assert not exceptions for device_name, device in devices.items(): assert device_name in beamline_utils.ACTIVE_DEVICES, ( f"No device named {device_name} was created for {module}, " @@ -40,7 +41,7 @@ def test_devices_are_identical(RE, module_and_devices_for_beamline): """ Ensures that for every beamline all device functions prevent duplicate instantiation. """ - bl_mod, devices_a = module_and_devices_for_beamline + bl_mod, devices_a, _ = module_and_devices_for_beamline devices_b, _ = make_all_devices( bl_mod, include_skipped=True, diff --git a/tests/conftest.py b/tests/conftest.py index 10d6ea14a2..c757681669 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -31,9 +31,11 @@ ("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"), + ("LOOK_UPTABLE_DIR", "tests/devices/i10/lookupTables/"), ] mock_attributes_table = { "i03": mock_paths, + "i10": mock_paths, "s03": mock_paths, "i04": mock_paths, "s04": mock_paths, @@ -88,12 +90,12 @@ def module_and_devices_for_beamline(request): bl_mod = importlib.import_module("dodal.beamlines." + beamline) importlib.reload(bl_mod) mock_beamline_module_filepaths(beamline, bl_mod) - devices, _ = make_all_devices( + devices, exceptions = make_all_devices( bl_mod, include_skipped=True, fake_with_ophyd_sim=True, ) - yield (bl_mod, devices) + yield (bl_mod, devices, exceptions) beamline_utils.clear_devices() del bl_mod diff --git a/tests/devices/i22/test_dcm.py b/tests/devices/i22/test_dcm.py index fab25936f5..6a7039cb6f 100644 --- a/tests/devices/i22/test_dcm.py +++ b/tests/devices/i22/test_dcm.py @@ -19,7 +19,7 @@ async def dcm() -> DoubleCrystalMonochromator: async with DeviceCollector(mock=True): dcm = DoubleCrystalMonochromator( - motion_prefix="FOO-MO", + prefix="FOO-MO", temperature_prefix="FOO-DI", crystal_1_metadata=CrystalMetadata( usage="Bragg", @@ -56,7 +56,7 @@ def test_count_dcm( async def test_crystal_metadata_not_propagated_when_not_supplied(): async with DeviceCollector(mock=True): dcm = DoubleCrystalMonochromator( - motion_prefix="FOO-MO", + prefix="FOO-MO", temperature_prefix="FOO-DI", crystal_1_metadata=None, crystal_2_metadata=None,