diff --git a/.vscode/hyperion-dodal-nexgen.code-workspace b/.vscode/hyperion-dodal-nexgen.code-workspace index 97f11cbaf..e74558da2 100644 --- a/.vscode/hyperion-dodal-nexgen.code-workspace +++ b/.vscode/hyperion-dodal-nexgen.code-workspace @@ -16,7 +16,7 @@ "esbonio.sphinx.confDir": "", "search.useIgnoreFiles": false, "[python]": { - "editor.defaultFormatter": "ms-python.black-formatter" + "editor.defaultFormatter": "charliermarsh.ruff" }, "python.formatting.provider": "none" } diff --git a/.vscode/settings.json b/.vscode/settings.json index 9e91dc235..4e927f6bc 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -13,7 +13,7 @@ "source.fixAll.ruff": "explicit", "source.organizeImports.ruff": "explicit" }, - "editor.defaultFormatter": "ms-python.black-formatter" + "editor.defaultFormatter": "charliermarsh.ruff" }, "terminal.integrated.gpuAcceleration": "off", "python.analysis.typeCheckingMode": "basic", diff --git a/conftest.py b/conftest.py index 2a83757de..a68f50264 100644 --- a/conftest.py +++ b/conftest.py @@ -1,8 +1,19 @@ +from os import environ, getenv from typing import Iterator from unittest.mock import patch import pytest +print("Adjusting S03 EPICS environment ...") +s03_epics_server_port = getenv("S03_EPICS_CA_SERVER_PORT") +s03_epics_repeater_port = getenv("S03_EPICS_CA_REPEATER_PORT") +if s03_epics_server_port: + environ["EPICS_CA_SERVER_PORT"] = s03_epics_server_port + print(f"[EPICS_CA_SERVER_PORT] = {s03_epics_server_port}") +if s03_epics_repeater_port: + environ["EPICS_CA_REPEATER_PORT"] = s03_epics_repeater_port + print(f"[EPICS_CA_REPEATER_PORT] = {s03_epics_repeater_port}") + def pytest_addoption(parser): parser.addoption( diff --git a/setup.cfg b/setup.cfg index 5bd19db51..dae2d0553 100644 --- a/setup.cfg +++ b/setup.cfg @@ -35,7 +35,7 @@ install_requires = xarray doct databroker - dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@ca9d6df8f17f88ce0128af9cbdfd40d0cfc44a26 + dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@97e3cdc11b1b5092c7f12ab6bc5ea1d702401b68 pydantic<2.0 # See https://github.com/DiamondLightSource/hyperion/issues/774 scipy pyzmq<25 # See https://github.com/DiamondLightSource/hyperion/issues/1103 diff --git a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py index d3dca083f..a0e8cf938 100755 --- a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -246,7 +246,7 @@ def run_gridscan( LOGGER.info("Setting fgs params") yield from set_flyscan_params(fgs_motors, parameters.experiment_params) - + LOGGER.info("Waiting for gridscan validity check") yield from wait_for_gridscan_valid(fgs_motors) LOGGER.info("Waiting for arming to finish") diff --git a/tests/conftest.py b/tests/conftest.py index b18949305..bbd9d1590 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,6 @@ import sys import threading from functools import partial -from os import environ, getenv from typing import Callable, Generator, Optional, Sequence from unittest.mock import MagicMock, patch @@ -116,23 +115,6 @@ def pytest_runtest_setup(item): print("Skipping log setup for log test - deleting existing handlers") _destroy_loggers([*ALL_LOGGERS, dodal_logger]) - if "s03" in markers: - print("Running s03 test - setting EPICS server ports variables...") - s03_epics_server_port = getenv("S03_EPICS_CA_SERVER_PORT") - s03_epics_repeater_port = getenv("S03_EPICS_CA_REPEATER_PORT") - - assert ( - s03_epics_server_port is not None and s03_epics_repeater_port is not None - ), ( - "Please run the S03 launch script with the '-f' flag to run it on a port " - " which doesn't clash with the real EPICS ports." - ) - - environ["EPICS_CA_SERVER_PORT"] = s03_epics_server_port - print(f"[EPICS_CA_SERVER_PORT] = {s03_epics_server_port}") - environ["EPICS_CA_REPEATER_PORT"] = s03_epics_repeater_port - print(f"[EPICS_CA_REPEATER_PORT] = {s03_epics_repeater_port}") - def pytest_runtest_teardown(): if "dodal.beamlines.beamline_utils" in sys.modules: diff --git a/tests/system_tests/experiment_plans/test_fgs_plan.py b/tests/system_tests/experiment_plans/test_fgs_plan.py index e80b42bad..9d6db4722 100644 --- a/tests/system_tests/experiment_plans/test_fgs_plan.py +++ b/tests/system_tests/experiment_plans/test_fgs_plan.py @@ -2,8 +2,10 @@ from typing import Callable from unittest.mock import MagicMock, patch +import bluesky.plan_stubs as bps import bluesky.preprocessors as bpp import pytest +import pytest_asyncio from bluesky.run_engine import RunEngine from dodal.beamlines import i03 from dodal.beamlines.beamline_parameters import ( @@ -11,18 +13,20 @@ GDABeamlineParameters, ) from dodal.devices.aperturescatterguard import AperturePositions +from dodal.devices.smargon import Smargon from ophyd.status import Status from hyperion.device_setup_plans.read_hardware_for_setup import ( read_hardware_for_ispyb_during_collection, read_hardware_for_ispyb_pre_collection, ) +from hyperion.device_setup_plans.xbpm_feedback import ( + transmission_and_xbpm_feedback_for_collection_decorator, +) from hyperion.exceptions import WarningException from hyperion.experiment_plans.flyscan_xray_centre_plan import ( FlyScanXRayCentreComposite, flyscan_xray_centre, - run_gridscan, - run_gridscan_and_move, ) from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( XrayCentreCallbackCollection, @@ -44,11 +48,34 @@ def params(): params = GridscanInternalParameters(**default_raw_params()) params.hyperion_params.beamline = CONST.SIM.BEAMLINE - return params + params.hyperion_params.ispyb_params.current_energy_ev = 10000 + params.hyperion_params.zocalo_environment = "dev_artemis" + params.hyperion_params.ispyb_params.microns_per_pixel_x = 10 + params.hyperion_params.ispyb_params.microns_per_pixel_y = 10 + yield params -@pytest.fixture -def fgs_composite(): +@pytest.fixture() +def callbacks(params): + with patch( + "hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter" + ): + callbacks = XrayCentreCallbackCollection() + callbacks.ispyb_handler.ispyb_config = CONST.SIM.DEV_ISPYB_DATABASE_CFG + yield callbacks + + +def reset_positions(smargon: Smargon): + yield from bps.mv(smargon.x, -1, smargon.y, -1, smargon.z, -1) + + +@pytest_asyncio.fixture +async def fxc_composite(): + with patch("dodal.devices.zocalo.zocalo_results._get_zocalo_connection"), patch( + "dodal.devices.zocalo.zocalo_results.workflows.recipe" + ), patch("dodal.devices.zocalo.zocalo_results.workflows.recipe"): + zocalo = i03.zocalo() + composite = FlyScanXRayCentreComposite( attenuator=i03.attenuator(), aperture_scatterguard=i03.aperture_scatterguard(), @@ -66,9 +93,12 @@ def fgs_composite(): synchrotron=i03.synchrotron(fake_with_ophyd_sim=True), xbpm_feedback=i03.xbpm_feedback(fake_with_ophyd_sim=True), zebra=i03.zebra(), - zocalo=MagicMock(), + zocalo=zocalo, ) + await composite.robot.barcode.bare_signal._backend.put(["ABCDEFGHIJ"]) # type: ignore + composite.dcm.energy_in_kev.user_readback.sim_put(12.345) # type: ignore + gda_beamline_parameters = GDABeamlineParameters.from_file( BEAMLINE_PARAMETER_PATHS["i03"] ) @@ -77,9 +107,9 @@ def fgs_composite(): gda_beamline_parameters ) composite.aperture_scatterguard.load_aperture_positions(aperture_positions) - composite.aperture_scatterguard.aperture.z.move( - aperture_positions.LARGE.location[2], wait=True - ) + composite.aperture_scatterguard._set_raw_unsafe( + aperture_positions.LARGE.location + ).wait() composite.eiger.cam.manual_trigger.put("Yes") composite.eiger.odin.check_odin_initialised = lambda: (True, "") composite.eiger.stage = MagicMock(return_value=Status(done=True, success=True)) @@ -87,66 +117,43 @@ def fgs_composite(): composite.aperture_scatterguard.scatterguard.x.set_lim(-4.8, 5.7) - return composite - + composite.xbpm_feedback.pos_ok.sim_put(1) # type: ignore + composite.xbpm_feedback.pos_stable.sim_put(1) # type: ignore -@pytest.mark.s03 -@patch("bluesky.plan_stubs.wait", autospec=True) -@patch("bluesky.plan_stubs.kickoff", autospec=True) -@patch("bluesky.plan_stubs.complete", autospec=True) -@patch( - "hyperion.experiment_plans.flyscan_xray_centre_plan.wait_for_gridscan_valid", - autospec=True, -) -def test_run_gridscan( - wait_for_gridscan_valid: MagicMock, - complete: MagicMock, - kickoff: MagicMock, - wait: MagicMock, - params: GridscanInternalParameters, - RE: RunEngine, - fgs_composite: FlyScanXRayCentreComposite, -): - RE(run_gridscan(fgs_composite, params)) + return composite +@pytest.mark.asyncio @pytest.mark.s03 -@patch("bluesky.plan_stubs.wait", autospec=True) -@patch("bluesky.plan_stubs.kickoff", autospec=True) -@patch("bluesky.plan_stubs.complete", autospec=True) -@patch( - "hyperion.experiment_plans.flyscan_xray_centre_plan.wait_for_gridscan_valid", - autospec=True, -) -def test_run_gridscan_and_move( - wait_for_gridscan_valid: MagicMock, - complete: MagicMock, - kickoff: MagicMock, - wait: MagicMock, - params: GridscanInternalParameters, - RE: RunEngine, - fgs_composite: FlyScanXRayCentreComposite, -): - RE(run_gridscan_and_move(fgs_composite, params)) +def test_s03_devices_connect(fxc_composite: FlyScanXRayCentreComposite): + assert fxc_composite.aperture_scatterguard + assert fxc_composite.backlight +@pytest.mark.asyncio @pytest.mark.s03 def test_read_hardware_for_ispyb_pre_collection( RE: RunEngine, - fgs_composite: FlyScanXRayCentreComposite, + fxc_composite: FlyScanXRayCentreComposite, ): - undulator = fgs_composite.undulator - synchrotron = fgs_composite.synchrotron - slit_gaps = fgs_composite.s4_slit_gaps - attenuator = fgs_composite.attenuator - flux = fgs_composite.flux - dcm = fgs_composite.dcm - aperture_scatterguard = fgs_composite.aperture_scatterguard - robot = fgs_composite.robot + undulator = fxc_composite.undulator + synchrotron = fxc_composite.synchrotron + slit_gaps = fxc_composite.s4_slit_gaps + attenuator = fxc_composite.attenuator + flux = fxc_composite.flux + dcm = fxc_composite.dcm + aperture_scatterguard = fxc_composite.aperture_scatterguard + robot = fxc_composite.robot @bpp.run_decorator() def read_run(u, s, g, r, a, f, dcm, ap_sg): - yield from read_hardware_for_ispyb_pre_collection(u, s, g, r, ap_sg) + yield from read_hardware_for_ispyb_pre_collection( + undulator=u, + synchrotron=s, + s4_slit_gaps=g, + aperture_scatterguard=ap_sg, + robot=r, + ) yield from read_hardware_for_ispyb_during_collection(a, f, dcm) RE( @@ -163,6 +170,31 @@ def read_run(u, s, g, r, a, f, dcm, ap_sg): ) +@pytest.mark.asyncio +@pytest.mark.s03 +def test_xbpm_feedback_decorator( + RE: RunEngine, + fxc_composite: FlyScanXRayCentreComposite, + params: GridscanInternalParameters, + callbacks: XrayCentreCallbackCollection, +): + # This test is currently kind of more a unit test since we are faking XBPM feedback + # with ophyd.sim, but it should continue to pass when we replace it with something + # in S03 + + @transmission_and_xbpm_feedback_for_collection_decorator( + fxc_composite.xbpm_feedback, + fxc_composite.attenuator, + params.hyperion_params.ispyb_params.transmission_fraction, + ) + def decorated_plan(): + yield from bps.sleep(0.1) + + RE(decorated_plan()) + assert fxc_composite.xbpm_feedback.pos_stable.get() == 1 + + +@pytest.mark.asyncio @pytest.mark.s03 @patch("bluesky.plan_stubs.wait", autospec=True) @patch("bluesky.plan_stubs.kickoff", autospec=True) @@ -181,24 +213,24 @@ def test_full_plan_tidies_at_end( complete: MagicMock, kickoff: MagicMock, wait: MagicMock, - fgs_composite: FlyScanXRayCentreComposite, + fxc_composite: FlyScanXRayCentreComposite, params: GridscanInternalParameters, RE: RunEngine, + callbacks: XrayCentreCallbackCollection, ): - callbacks = XrayCentreCallbackCollection() + RE(reset_positions(fxc_composite.smargon)) + callbacks.nexus_handler.nexus_writer_1 = MagicMock() callbacks.nexus_handler.nexus_writer_2 = MagicMock() callbacks.ispyb_handler.ispyb_ids = IspybIds( data_collection_ids=(0, 0), data_collection_group_id=0, grid_ids=(0,) ) - with patch( - "hyperion.experiment_plans.flyscan_xray_centre_plan.XrayCentreCallbackCollection.setup", - return_value=callbacks, - ): - RE(flyscan_xray_centre(fgs_composite, params)) + [RE.subscribe(cb) for cb in callbacks] + RE(flyscan_xray_centre(fxc_composite, params)) set_shutter_to_manual.assert_called_once() +@pytest.mark.asyncio @pytest.mark.s03 @patch("bluesky.plan_stubs.wait", autospec=True) @patch("bluesky.plan_stubs.kickoff", autospec=True) @@ -217,22 +249,26 @@ def test_full_plan_tidies_at_end_when_plan_fails( complete: MagicMock, kickoff: MagicMock, wait: MagicMock, - fgs_composite: FlyScanXRayCentreComposite, + fxc_composite: FlyScanXRayCentreComposite, params: GridscanInternalParameters, RE: RunEngine, ): run_gridscan_and_move.side_effect = Exception() with pytest.raises(Exception): - RE(flyscan_xray_centre(fgs_composite, params)) + RE(flyscan_xray_centre(fxc_composite, params)) set_shutter_to_manual.assert_called_once() +@patch("hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger") +@pytest.mark.asyncio @pytest.mark.s03 def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_entry( + zocalo_trigger: MagicMock, RE: RunEngine, - fgs_composite: FlyScanXRayCentreComposite, + fxc_composite: FlyScanXRayCentreComposite, fetch_comment: Callable, params: GridscanInternalParameters, + callbacks: XrayCentreCallbackCollection, ): params.hyperion_params.detector_params.directory = "./tmp" params.hyperion_params.detector_params.prefix = str(uuid.uuid1()) @@ -240,55 +276,88 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en # Currently s03 calls anything with z_steps > 1 invalid params.experiment_params.z_steps = 100 + RE(reset_positions(fxc_composite.smargon)) - callbacks = XrayCentreCallbackCollection() - callbacks.ispyb_handler.ispyb.ISPYB_CONFIG_PATH = CONST.SIM.DEV_ISPYB_DATABASE_CFG - mock_start_zocalo = MagicMock() - callbacks.ispyb_handler.emit_cb.zocalo_interactor.run_start = mock_start_zocalo # type: ignore - + [RE.subscribe(cb) for cb in callbacks] with pytest.raises(WarningException): - RE(flyscan_xray_centre(fgs_composite, params)) + RE(flyscan_xray_centre(fxc_composite, params)) - dcid_used = callbacks.ispyb_handler.ispyb_ids = IspybIds( - data_collection_ids=(0, 0), data_collection_group_id=0, grid_ids=(0,) - ) + ids = callbacks.ispyb_handler.ispyb_ids + assert ids.data_collection_group_id is not None + dcid_used = callbacks.ispyb_handler.ispyb_ids.data_collection_ids[0] comment = fetch_comment(dcid_used) assert "too long/short/bent" in comment - mock_start_zocalo.assert_not_called() + zocalo_trigger.run_start.assert_not_called() +@pytest.mark.asyncio @pytest.mark.s03 -@patch("hyperion.experiment_plans.flyscan_xray_centre_plan.bps.kickoff", autospec=True) -@patch("hyperion.experiment_plans.flyscan_xray_centre_plan.bps.complete", autospec=True) -def test_WHEN_plan_run_THEN_move_to_centre_returned_from_zocalo_expected_centre( - complete: MagicMock, - kickoff: MagicMock, +def test_complete_xray_centre_plan_with_no_callbacks_falls_back_to_centre( RE: RunEngine, - fgs_composite: FlyScanXRayCentreComposite, + fxc_composite: FlyScanXRayCentreComposite, zocalo_env: None, params: GridscanInternalParameters, + callbacks, + done_status, ): - """This test currently avoids hardware interaction and is mostly confirming - interaction with dev_ispyb and fake_zocalo""" + fxc_composite.fast_grid_scan.kickoff = MagicMock(return_value=done_status) + fxc_composite.fast_grid_scan.complete = MagicMock(return_value=done_status) params.hyperion_params.detector_params.directory = "./tmp" params.hyperion_params.detector_params.prefix = str(uuid.uuid1()) params.hyperion_params.ispyb_params.visit_path = "/dls/i03/data/2022/cm31105-5/" + params.experiment_params.set_stub_offsets = False + # Currently s03 calls anything with z_steps > 1 invalid params.experiment_params.z_steps = 1 - fgs_composite.eiger.stage = MagicMock() - fgs_composite.eiger.unstage = MagicMock() + RE(reset_positions(fxc_composite.smargon)) + + def zocalo_trigger(): + fxc_composite.zocalo._raw_results_received.put({"results": []}) + return done_status + + # [RE.subscribe(cb) for cb in callbacks] + fxc_composite.zocalo.trigger = MagicMock(side_effect=zocalo_trigger) + RE(flyscan_xray_centre(fxc_composite, params)) + + # The following numbers are derived from the centre returned in fake_zocalo + assert fxc_composite.sample_motors.x.user_readback.get() == pytest.approx(-1) + assert fxc_composite.sample_motors.y.user_readback.get() == pytest.approx(-1) + assert fxc_composite.sample_motors.z.user_readback.get() == pytest.approx(-1) + + +@pytest.mark.asyncio +@pytest.mark.s03 +def test_complete_xray_centre_plan_with_callbacks_moves_to_centre( + RE: RunEngine, + fxc_composite: FlyScanXRayCentreComposite, + zocalo_env: None, + params: GridscanInternalParameters, + callbacks, + done_status, +): + fxc_composite.fast_grid_scan.kickoff = MagicMock(return_value=done_status) + fxc_composite.fast_grid_scan.complete = MagicMock(return_value=done_status) + + params.hyperion_params.detector_params.directory = "./tmp" + params.hyperion_params.detector_params.prefix = str(uuid.uuid1()) + params.hyperion_params.ispyb_params.visit_path = "/dls/i03/data/2022/cm31105-5/" + + params.experiment_params.set_stub_offsets = False + + # Currently s03 calls anything with z_steps > 1 invalid + params.experiment_params.z_steps = 1 - callbacks = XrayCentreCallbackCollection() - callbacks.ispyb_handler.ispyb.ISPYB_CONFIG_PATH = CONST.SIM.DEV_ISPYB_DATABASE_CFG + RE(reset_positions(fxc_composite.smargon)) - RE(flyscan_xray_centre(fgs_composite, params)) + [RE.subscribe(cb) for cb in callbacks] + RE(flyscan_xray_centre(fxc_composite, params)) # The following numbers are derived from the centre returned in fake_zocalo - assert fgs_composite.sample_motors.x.user_readback.get() == pytest.approx(-0.05) - assert fgs_composite.sample_motors.y.user_readback.get() == pytest.approx(0.05) - assert fgs_composite.sample_motors.z.user_readback.get() == pytest.approx(0.15) + assert fxc_composite.sample_motors.x.user_readback.get() == pytest.approx(0.05) + assert fxc_composite.sample_motors.y.user_readback.get() == pytest.approx(0.15) + assert fxc_composite.sample_motors.z.user_readback.get() == pytest.approx(0.25) diff --git a/tests/unit_tests/experiment_plans/test_grid_detection_plan.py b/tests/unit_tests/experiment_plans/test_grid_detection_plan.py index be6596fb9..9267cd550 100644 --- a/tests/unit_tests/experiment_plans/test_grid_detection_plan.py +++ b/tests/unit_tests/experiment_plans/test_grid_detection_plan.py @@ -85,7 +85,6 @@ def test_grid_detection_plan_runs_and_triggers_snapshots( test_config_files, fake_devices, ): - params = OAVParameters("loopCentring", test_config_files["oav_config_json"]) cb = OavSnapshotCallback() RE.subscribe(cb) @@ -250,15 +249,21 @@ def test_when_grid_detection_plan_run_then_grid_detection_callback_gets_correct_ my_grid_params = cb.get_grid_parameters() test_x_grid_axis = GridAxis( - my_grid_params.x_start, my_grid_params.x_step_size, my_grid_params.x_steps + start=my_grid_params.x_start, + step_size=my_grid_params.x_step_size, + full_steps=my_grid_params.x_steps, ) test_y_grid_axis = GridAxis( - my_grid_params.y1_start, my_grid_params.y_step_size, my_grid_params.y_steps + start=my_grid_params.y1_start, + step_size=my_grid_params.y_step_size, + full_steps=my_grid_params.y_steps, ) test_z_grid_axis = GridAxis( - my_grid_params.z2_start, my_grid_params.z_step_size, my_grid_params.z_steps + start=my_grid_params.z2_start, + step_size=my_grid_params.z_step_size, + full_steps=my_grid_params.z_steps, ) assert my_grid_params.x_start == pytest.approx(-0.7942199999999999)