(#986) fixes to unit tests

src/dodal/devices/focusing_mirror.py

@@ -45,27 +45,32 @@ def set(self, value, *args, **kwargs) -> StatusBase:
             return Status(success=True, done=True)
 
         LOGGER.debug(f"setting {setpoint_v.name} to {value}")
-        setpoint_status = setpoint_v.set(value)
         demand_accepted_status = Status(self, DEFAULT_SETTLE_TIME_S)
 
-        subscription: dict[str, Any] = {}
-
-        def demand_check_callback(value, **kwargs):
-            if value == DEMAND_ACCEPTED_OK:
+        subscription: dict[str, Any] = {"handle": None, "old_value": None}
+
+        def demand_check_callback(old_value, value, **kwargs):
+            # old_value parameter is unreliable when first called on subscribe, therefore we must
+            # save the current value and compare later to ensure that we definitely trigger on a rising edge.
+            LOGGER.debug(f"Got event old={old_value} new={value}")
+            if (
+                subscription["old_value"] is not None
+                and subscription["old_value"] != DEMAND_ACCEPTED_OK
+                and value == DEMAND_ACCEPTED_OK
+            ):
                 LOGGER.debug(f"Demand accepted for {setpoint_v.name}")
-                demand_accepted_status.set_finished()
-
-                subs_handle = subscription.pop("value", None)
+                subs_handle = subscription.pop("handle", None)
                 if subs_handle is None:
                     raise AssertionError("Demand accepted before set attempted")
                 demand_accepted.unsubscribe(subs_handle)
-            # else timeout handled by parent demand_accepted_status
 
-        def setpoint_callback(status: Status):
-            if status.success:
-                subscription["value"] = demand_accepted.subscribe(demand_check_callback)
+                demand_accepted_status.set_finished()
+            # else timeout handled by parent demand_accepted_status
+            else:
+                subscription["old_value"] = value
 
-        setpoint_status.add_callback(setpoint_callback)
+        subscription["handle"] = demand_accepted.subscribe(demand_check_callback)
+        setpoint_status = setpoint_v.set(value)
         status = setpoint_status & demand_accepted_status
         return status
 

tests/conftest.py

@@ -1,11 +1,15 @@
 import logging
 import sys
 from os import environ, getenv
+from typing import cast
 from unittest.mock import MagicMock, patch
 
 import pytest
 
+from ophyd.sim import make_fake_device
+
 from dodal.beamlines import i03
+from dodal.devices.focusing_mirror import VFMMirrorVoltages
 from dodal.log import LOGGER, GELFTCPHandler, set_up_logging_handlers
 
 
@@ -27,8 +31,10 @@ def pytest_runtest_teardown():
 
 
 @pytest.fixture
-def vfm_mirror_voltages():
-    voltages = i03.vfm_mirror_voltages(fake_with_ophyd_sim=True)
+def vfm_mirror_voltages() -> VFMMirrorVoltages:
+    voltages = cast(VFMMirrorVoltages, make_fake_device(VFMMirrorVoltages)(name="vfm_mirror_voltages",
+                                                   prefix=f"BL-I03-MO-PSU-01:",
+                                                   daq_configuration_path=i03.DAQ_CONFIGURATION_PATH))
     voltages.voltage_lookup_table_path = "tests/test_data/test_mirror_focus.json"
     return voltages
 

tests/devices/unit_tests/test_focusing_mirror.py

@@ -1,4 +1,5 @@
-from unittest.mock import MagicMock, patch
+from threading import Timer
+from unittest.mock import DEFAULT, MagicMock, patch
 
 import pytest
 from ophyd.sim import NullStatus
@@ -13,7 +14,31 @@
 
 @pytest.fixture
 def vfm_mirror_voltages_with_set(vfm_mirror_voltages) -> VFMMirrorVoltages:
-    vfm_mirror_voltages._channel14_voltage_device._setpoint_v.set = MagicMock()
+    def not_ok_then_ok(_):
+        vfm_mirror_voltages._channel14_voltage_device._demand_accepted.sim_put(0)
+        Timer(
+            0.1,
+            lambda: vfm_mirror_voltages._channel14_voltage_device._demand_accepted.sim_put(
+                1
+            ),
+        ).start()
+        return DEFAULT
+
+    vfm_mirror_voltages._channel14_voltage_device._setpoint_v.set = MagicMock(
+        side_effect=not_ok_then_ok
+    )
+    return vfm_mirror_voltages
+
+
+@pytest.fixture
+def vfm_mirror_voltages_with_set_timing_out(vfm_mirror_voltages) -> VFMMirrorVoltages:
+    def not_ok(_):
+        vfm_mirror_voltages._channel14_voltage_device._demand_accepted.sim_put(0)
+        return DEFAULT
+
+    vfm_mirror_voltages._channel14_voltage_device._setpoint_v.set = MagicMock(
+        side_effect=not_ok
+    )
     return vfm_mirror_voltages
 
 
@@ -53,14 +78,15 @@ def test_mirror_set_voltage_sets_and_waits_set_fail(
 
 @patch("dodal.devices.focusing_mirror.DEFAULT_SETTLE_TIME_S", 3)
 def test_mirror_set_voltage_sets_and_waits_settle_timeout_expires(
-    vfm_mirror_voltages_with_set: VFMMirrorVoltages,
+    vfm_mirror_voltages_with_set_timing_out: VFMMirrorVoltages,
 ):
-    vfm_mirror_voltages_with_set._channel14_voltage_device._setpoint_v.set.return_value = (
+    vfm_mirror_voltages_with_set_timing_out._channel14_voltage_device._setpoint_v.set.return_value = (
         NullStatus()
     )
-    vfm_mirror_voltages_with_set._channel14_voltage_device._demand_accepted.sim_put(0)
 
-    status: StatusBase = vfm_mirror_voltages_with_set.voltage_channels[0].set(100)
+    status: StatusBase = vfm_mirror_voltages_with_set_timing_out.voltage_channels[
+        0
+    ].set(100)
 
     actual_exception = None
     try: