#1192 fix last tests

src/hyperion/__main__.py

@@ -155,7 +155,11 @@ def wait_on_queue(self):
                 if command.experiment is None:
                     raise ValueError("No experiment provided for START")
                 try:
-                    if command.callbacks and (cbs := list(command.callbacks())):
+                    if (
+                        not self.use_external_callbacks
+                        and command.callbacks
+                        and (cbs := list(command.callbacks()))
+                    ):
                         LOGGER.info(
                             f"Using callbacks for this plan: {not self.use_external_callbacks} - {cbs}"
                         )

src/hyperion/external_interaction/callbacks/rotation/callback_collection.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 
 from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import (
     AbstractPlanCallbackCollection,
@@ -16,10 +16,16 @@
 )
 
 
+def new_ispyb_with_zocalo():
+    return RotationISPyBCallback(emit=ZocaloCallback())
+
+
 @dataclass(frozen=True, order=True)
 class RotationCallbackCollection(AbstractPlanCallbackCollection):
     """Groups the callbacks for external interactions for a rotation scan.
     Cast to a list to pass it to Bluesky.preprocessors.subs_decorator()."""
 
-    nexus_handler: RotationNexusFileCallback = RotationNexusFileCallback()
-    ispyb_handler: RotationISPyBCallback = RotationISPyBCallback(emit=ZocaloCallback())
+    nexus_handler: RotationNexusFileCallback = field(
+        default_factory=RotationNexusFileCallback
+    )
+    ispyb_handler: RotationISPyBCallback = field(default_factory=new_ispyb_with_zocalo)

src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 
 from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import (
     AbstractPlanCallbackCollection,
@@ -16,11 +16,17 @@
 )
 
 
+def new_ispyb_with_zocalo():
+    return GridscanISPyBCallback(emit=ZocaloCallback())
+
+
 @dataclass(frozen=True, order=True)
 class XrayCentreCallbackCollection(AbstractPlanCallbackCollection):
     """Groups the callbacks for external interactions in the fast grid scan, and
     connects the Zocalo and ISPyB handlers. Cast to a list to pass it to
     Bluesky.preprocessors.subs_decorator()."""
 
-    nexus_handler: GridscanNexusFileCallback = GridscanNexusFileCallback()
-    ispyb_handler: GridscanISPyBCallback = GridscanISPyBCallback(emit=ZocaloCallback())
+    nexus_handler: GridscanNexusFileCallback = field(
+        default_factory=GridscanNexusFileCallback
+    )
+    ispyb_handler: GridscanISPyBCallback = field(default_factory=new_ispyb_with_zocalo)

tests/unit_tests/external_interaction/callbacks/test_external_callbacks.py

@@ -37,7 +37,7 @@ def test_main_function(
 
 
 def test_setup_callbacks():
-    current_number_of_callbacks = 6
+    current_number_of_callbacks = 4
     cbs = setup_callbacks()
     assert len(cbs) == current_number_of_callbacks
     assert len(set(cbs)) == current_number_of_callbacks

tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py

@@ -280,6 +280,7 @@ def test_ispyb_handler_grabs_uid_from_main_plan_and_not_first_start_doc(
     zocalo, RE: RunEngine, params: RotationInternalParameters, test_outer_start_doc
 ):
     cb = RotationCallbackCollection()
+    cb.ispyb_handler.emit_cb = None
     activate_callbacks(cb)
     cb.nexus_handler.activity_gated_event = MagicMock(autospec=True)
     cb.nexus_handler.activity_gated_start = MagicMock(autospec=True)

tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py

@@ -1,21 +1,15 @@
 from unittest.mock import MagicMock, call, patch
 
 import pytest
-from dodal.devices.zocalo import ZocaloTrigger
 
 from hyperion.external_interaction.callbacks.xray_centre.callback_collection import (
     XrayCentreCallbackCollection,
 )
 from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback
 from hyperion.external_interaction.exceptions import ISPyBDepositionNotMade
-from hyperion.external_interaction.ispyb.ispyb_store import IspybIds
+from hyperion.external_interaction.ispyb.ispyb_store import IspybIds, StoreInIspyb
 from hyperion.parameters.constants import TRIGGER_ZOCALO_ON
-from hyperion.parameters.external_parameters import from_file as default_raw_params
-from hyperion.parameters.plan_specific.gridscan_internal_params import (
-    GridscanInternalParameters,
-)
 
-from ...experiment_plans.conftest import modified_store_grid_scan_mock
 from .xray_centre.conftest import TestData
 
 EXPECTED_DCID = 100
@@ -29,54 +23,42 @@
 td = TestData()
 
 
-@pytest.fixture
-def dummy_params():
-    return GridscanInternalParameters(**default_raw_params())
-
-
-def init_cbs_with_docs_and_mock_zocalo_and_ispyb(
-    callbacks: XrayCentreCallbackCollection, dcids=(0, 0), dcgid=4
-):
-    with patch(
-        "hyperion.external_interaction.callbacks.xray_centre.ispyb_callback.Store3DGridscanInIspyb",
-        lambda _, __: modified_store_grid_scan_mock(dcids=dcids, dcgid=dcgid),
-    ):
-        callbacks.ispyb_handler.activity_gated_start(td.test_start_document)
-
-
-@patch(
-    "hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger",
-    new=MagicMock(spec=ZocaloTrigger),
-)
 class TestZocaloHandler:
-    def test_handler_gets_plan_name_from_start_doc(self):
+    def _setup_handler(self):
         zocalo_handler = ZocaloCallback()
         assert zocalo_handler.triggering_plan is None
         zocalo_handler.start({TRIGGER_ZOCALO_ON: "test_plan_name"})  # type: ignore
         assert zocalo_handler.triggering_plan == "test_plan_name"
         assert zocalo_handler.zocalo_interactor is None
         return zocalo_handler
 
+    def test_handler_gets_plan_name_from_start_doc(self):
+        self._setup_handler()
+
     def test_handler_doesnt_trigger_on_wrong_plan(self):
-        zocalo_handler = self.test_handler_gets_plan_name_from_start_doc()
+        zocalo_handler = self._setup_handler()
         zocalo_handler.start({TRIGGER_ZOCALO_ON: "_not_test_plan_name"})  # type: ignore
 
     def test_handler_raises_on_right_plan_with_wrong_metadata(self):
-        zocalo_handler = self.test_handler_gets_plan_name_from_start_doc()
+        zocalo_handler = self._setup_handler()
         assert zocalo_handler.zocalo_interactor is None
         with pytest.raises(AssertionError):
             zocalo_handler.start({"subplan_name": "test_plan_name"})  # type: ignore
 
     def test_handler_raises_on_right_plan_with_no_ispyb_ids(self):
-        zocalo_handler = self.test_handler_gets_plan_name_from_start_doc()
+        zocalo_handler = self._setup_handler()
         assert zocalo_handler.zocalo_interactor is None
         with pytest.raises(ISPyBDepositionNotMade):
             zocalo_handler.start(
                 {"subplan_name": "test_plan_name", "zocalo_environment": "test_env"}  # type: ignore
             )
 
-    def test_handler_inits_zocalo_trigger_on_right_plan(self):
-        zocalo_handler = self.test_handler_gets_plan_name_from_start_doc()
+    @patch(
+        "hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger",
+        autospec=True,
+    )
+    def test_handler_inits_zocalo_trigger_on_right_plan(self, zocalo_trigger):
+        zocalo_handler = self._setup_handler()
         assert zocalo_handler.zocalo_interactor is None
         zocalo_handler.start(
             {
@@ -87,55 +69,59 @@ def test_handler_inits_zocalo_trigger_on_right_plan(self):
         )
         assert zocalo_handler.zocalo_interactor is not None
 
-    def test_execution_of_run_gridscan_triggers_zocalo_calls(
-        self,
-        mock_ispyb_update_time_and_status: MagicMock,
-        mock_ispyb_get_time: MagicMock,
-        mock_ispyb_store_grid_scan: MagicMock,
-        nexus_writer: MagicMock,
-        dummy_params,
+    @patch(
+        "hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger",
+        autospec=True,
+    )
+    @patch(
+        "hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter",
+    )
+    @patch(
+        "hyperion.external_interaction.callbacks.xray_centre.ispyb_callback.Store3DGridscanInIspyb",
+    )
+    def test_execution_of_do_fgs_triggers_zocalo_calls(
+        self, ispyb_store: MagicMock, nexus_writer: MagicMock, zocalo_trigger
     ):
         dc_ids = (1, 2)
         dcg_id = 4
 
-        mock_ispyb_store_grid_scan.return_value = IspybIds(
+        mock_ids = IspybIds(
             data_collection_ids=dc_ids, grid_ids=None, data_collection_group_id=dcg_id
         )
-        mock_ispyb_get_time.return_value = td.DUMMY_TIME_STRING
-        mock_ispyb_update_time_and_status.return_value = None
+        ispyb_store.return_value.mock_add_spec(StoreInIspyb)
 
         callbacks = XrayCentreCallbackCollection()
-        assert isinstance(
-            zocalo_handler := callbacks.ispyb_handler.emit_cb, ZocaloCallback
-        )
-        init_cbs_with_docs_and_mock_zocalo_and_ispyb(callbacks, dc_ids, dcg_id)
-        callbacks.ispyb_handler.activity_gated_start(
-            td.test_run_gridscan_start_document
-        )  # type: ignore
-        callbacks.ispyb_handler.activity_gated_descriptor(
+        ispyb_cb = callbacks.ispyb_handler
+        ispyb_cb.active = True
+        assert isinstance(zocalo_handler := ispyb_cb.emit_cb, ZocaloCallback)
+        zocalo_handler._reset_state()
+        zocalo_handler._reset_state = MagicMock()
+
+        ispyb_store.return_value.begin_deposition.return_value = mock_ids
+        ispyb_store.return_value.update_deposition.return_value = mock_ids
+
+        ispyb_cb.start(td.test_start_document)  # type: ignore
+        ispyb_cb.start(td.test_do_fgs_start_document)  # type: ignore
+        ispyb_cb.descriptor(
             td.test_descriptor_document_pre_data_collection
         )  # type: ignore
-        callbacks.ispyb_handler.activity_gated_event(
-            td.test_event_document_pre_data_collection
-        )
-        callbacks.ispyb_handler.activity_gated_descriptor(
+        ispyb_cb.event(td.test_event_document_pre_data_collection)
+        ispyb_cb.descriptor(
             td.test_descriptor_document_during_data_collection  # type: ignore
         )
-        callbacks.ispyb_handler.activity_gated_event(
-            td.test_event_document_during_data_collection
-        )
-        zocalo_handler.start(td.test_do_fgs_start_document)
-        callbacks.ispyb_handler.activity_gated_stop(td.test_stop_document)
-        zocalo_handler.stop(td.test_stop_document)
-
+        ispyb_cb.event(td.test_event_document_during_data_collection)
         assert zocalo_handler.zocalo_interactor is not None
 
         zocalo_handler.zocalo_interactor.run_start.assert_has_calls(
             [call(x) for x in dc_ids]
         )
         assert zocalo_handler.zocalo_interactor.run_start.call_count == len(dc_ids)
 
+        ispyb_cb.stop(td.test_stop_document)
+
         zocalo_handler.zocalo_interactor.run_end.assert_has_calls(
             [call(x) for x in dc_ids]
         )
         assert zocalo_handler.zocalo_interactor.run_end.call_count == len(dc_ids)
+
+        zocalo_handler._reset_state.assert_called()

tests/unit_tests/external_interaction/callbacks/xray_centre/conftest.py

@@ -8,11 +8,13 @@
     GridscanISPyBCallback,
 )
 from hyperion.parameters.constants import (
+    DO_FGS,
     GRIDSCAN_AND_MOVE,
     GRIDSCAN_MAIN_PLAN,
     GRIDSCAN_OUTER_PLAN,
     ISPYB_HARDWARE_READ_PLAN,
     ISPYB_TRANSMISSION_FLUX_READ_PLAN,
+    TRIGGER_ZOCALO_ON,
 )
 from hyperion.parameters.external_parameters import from_file as default_raw_params
 from hyperion.parameters.plan_specific.gridscan_internal_params import (
@@ -90,6 +92,7 @@ class TestData:
         "plan_type": "generator",
         "plan_name": GRIDSCAN_OUTER_PLAN,
         "subplan_name": GRIDSCAN_OUTER_PLAN,
+        TRIGGER_ZOCALO_ON: DO_FGS,
         "hyperion_internal_parameters": dummy_params().json(),
     }
     test_run_gridscan_start_document: RunStart = {  # type: ignore
@@ -107,8 +110,8 @@ class TestData:
         "versions": {"ophyd": "1.6.4.post76+g0895f9f", "bluesky": "1.8.3"},
         "scan_id": 1,
         "plan_type": "generator",
-        "plan_name": GRIDSCAN_AND_MOVE,
-        "subplan_name": "do_fgs",
+        "plan_name": DO_FGS,
+        "subplan_name": DO_FGS,
         "zocalo_environment": "dev_artemis",
     }
     test_descriptor_document_pre_data_collection: EventDescriptor = {

tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py

@@ -11,25 +11,22 @@
 
 def test_callback_collection_init():
     callbacks = XrayCentreCallbackCollection()
-    assert len(list(callbacks)) == 3
+    assert len(list(callbacks)) == 2
 
 
 def test_callback_collection_list():
     callbacks = XrayCentreCallbackCollection()
     callback_list = list(callbacks)
-    assert len(callback_list) == 3
+    assert len(callback_list) == 2
     assert callbacks.ispyb_handler in callback_list
     assert callbacks.nexus_handler in callback_list
-    assert callbacks.zocalo_handler in callback_list
 
 
 def test_subscribe_in_plan():
     callbacks = XrayCentreCallbackCollection()
     document_event_mock = MagicMock()
     callbacks.ispyb_handler.start = document_event_mock
     callbacks.ispyb_handler.activity_gated_stop = document_event_mock
-    callbacks.zocalo_handler.activity_gated_start = document_event_mock
-    callbacks.zocalo_handler.activity_gated_stop = document_event_mock
     callbacks.nexus_handler.activity_gated_start = document_event_mock
     callbacks.nexus_handler.stop = document_event_mock
 

tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py

@@ -118,7 +118,7 @@ def test_begin_deposition_with_alternate_experiment_type(
     dummy_rotation_params,
 ):
     assert dummy_rotation_ispyb_with_experiment_type.begin_deposition() == IspybIds(
-        data_collection_ids=TEST_DATA_COLLECTION_IDS[0],
+        data_collection_ids=(TEST_DATA_COLLECTION_IDS[0],),
         data_collection_group_id=TEST_DATA_COLLECTION_GROUP_ID,
     )
     mx_acq = mx_acquisition_from_conn(ispyb_conn_with_2x2_collections_and_grid_info)

tests/unit_tests/hyperion/test_main_system.py

@@ -355,7 +355,7 @@ def test_blueskyrunner_uses_cli_args_correctly_for_callbacks(
     callback_class_mock = MagicMock(
         spec=AbstractPlanCallbackCollection,
         name="mock_callback_class",
-        return_value=[1, 2],
+        return_value=["test_cb_1", "test_cb_2"],
     )
 
     TEST_REGISTRY = {