From 7eed8ac5303c8e46f89cc61edc1b77c3e17ccee5 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 29 Feb 2024 15:47:46 +0000 Subject: [PATCH] #1192 fix last tests --- src/hyperion/__main__.py | 6 +- .../callbacks/rotation/callback_collection.py | 12 +- .../xray_centre/callback_collection.py | 12 +- .../callbacks/test_external_callbacks.py | 2 +- .../callbacks/test_rotation_callbacks.py | 1 + .../callbacks/test_zocalo_handler.py | 108 ++++++++---------- .../callbacks/xray_centre/conftest.py | 7 +- .../test_xraycentre_callback_collection.py | 7 +- .../ispyb/test_rotation_ispyb_store.py | 2 +- tests/unit_tests/hyperion/test_main_system.py | 2 +- 10 files changed, 81 insertions(+), 78 deletions(-) diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index a53354284..d633bd6d7 100755 --- a/src/hyperion/__main__.py +++ b/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}" ) diff --git a/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py b/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py index 370093eb9..a226e3071 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py +++ b/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) diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py b/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py index 4ab0cf7b5..7f1454c3f 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py +++ b/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) diff --git a/tests/unit_tests/external_interaction/callbacks/test_external_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_external_callbacks.py index adada7d37..697532742 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_external_callbacks.py +++ b/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 diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index e79fe02d9..137b2802e 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/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) diff --git a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py b/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py index 42df597c2..0bf566ff0 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py +++ b/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,27 +23,8 @@ 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 @@ -57,26 +32,33 @@ def test_handler_gets_plan_name_from_start_doc(self): 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,47 +69,47 @@ 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( @@ -135,7 +117,11 @@ def test_execution_of_run_gridscan_triggers_zocalo_calls( ) 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() diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/conftest.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/conftest.py index 0270a02ad..a8c41b8d1 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/conftest.py +++ b/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 = { diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py index c9b056710..9131e9dbe 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py +++ b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py @@ -11,16 +11,15 @@ 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(): @@ -28,8 +27,6 @@ def test_subscribe_in_plan(): 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 diff --git a/tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py b/tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py index 018890704..61c295c6c 100644 --- a/tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py +++ b/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) diff --git a/tests/unit_tests/hyperion/test_main_system.py b/tests/unit_tests/hyperion/test_main_system.py index 1dc7477bb..1ac5dea59 100644 --- a/tests/unit_tests/hyperion/test_main_system.py +++ b/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 = {