System tests on k8s

docs/developer/hyperion/index.rst

@@ -15,6 +15,7 @@ Documentation specific for the Hyperion module within MX-Bluesky
             reference/param-hierarchy
             reference/readme
             deploying-hyperion
+            system-tests
 
         +++
 

docs/developer/hyperion/system-tests.rst

@@ -0,0 +1,20 @@
+System Tests
+============
+
+The system tests are run by a project hosted in a GitLab private repository, `Hyperion System Testing`_.
+
+This contains a ``.gitlab-ci`` pipeline that defines a job that runs the tests.
+
+System Test Images
+------------------
+
+The system tests require a number of container images to run:
+
+   * The test runner image
+   * An ISPyB MariaDB database server image
+   * An ExpEye image
+
+It is expected that these images should need to change only infrequently and builds are triggered
+manually as needed - see the Hyperion System Testing project for how to rebuild them and how to trigger the tests. 
+
+.. _`Hyperion System Testing`: https://gitlab.diamond.ac.uk/MX-GDA/hyperion-system-testing

pyproject.toml

@@ -119,6 +119,7 @@ markers = [
     "dlstbx: marks tests as requiring dlstbx (deselect with '-m \"not dlstbx\"')",
     "skip_log_setup: marks tests so that loggers are not setup before the test.",
     "skip_in_pycharm: marks test as not working in pycharm testrunner",
+    "system_test: marks tests as a system test"
 ]
 addopts = """
     --tb=native -vv --doctest-modules --doctest-glob="*.rst" --durations=10
@@ -165,7 +166,7 @@ legacy_tox_ini = """
 [tox]
 skipsdist=True
 
-[testenv:{pre-commit,type-checking,tests,docs}]
+[testenv:{pre-commit,type-checking,tests,docs,systemtests}]
 # Don't create a virtualenv for the command, requires tox-direct plugin
 direct = True
 passenv = *
@@ -180,6 +181,7 @@ commands =
     type-checking: pyright src tests {posargs}
     tests: pytest --cov=mx_bluesky --cov-report term --cov-report xml:cov.xml {posargs}
     docs: sphinx-{posargs:build -EW --keep-going} -T docs build/html
+    systemtests: pytest --junit-xml=systemtests_output.xml -m system_test tests/system_tests
 commands_pre =
     docs: /usr/bin/bash -c "{toxinidir}/utility_scripts/generate_plantuml.py > \
         docs/developer/hyperion/reference/param_hierarchy.puml"

src/mx_bluesky/common/external_interaction/ispyb/exp_eye_store.py

@@ -2,7 +2,7 @@
 from dataclasses import dataclass
 from enum import StrEnum
 
-from requests import patch, post
+from requests import JSONDecodeError, patch, post
 from requests.auth import AuthBase
 
 from mx_bluesky.common.external_interaction.ispyb.ispyb_utils import (
@@ -34,7 +34,11 @@ def _get_base_url_and_token() -> tuple[str, str]:
 def _send_and_get_response(auth, url, data, send_func) -> dict:
     response = send_func(url, auth=auth, json=data)
     if not response.ok:
-        raise ISPyBDepositionNotMade(f"Could not write {data} to {url}: {response}")
+        try:
+            resp_txt = str(response.json())
+        except JSONDecodeError:
+            resp_txt = str(response)
+        raise ISPyBDepositionNotMade(f"Could not write {data} to {url}: {resp_txt}")
     return response.json()
 
 

tests/conftest.py

@@ -1509,3 +1509,10 @@ def mock_ispyb_conn_multiscan(base_ispyb_conn):
         list(range(12, 24)),
         list(range(56, 68)),
     )
+
+
+@pytest.fixture(scope="function", autouse=True)
+def clear_device_factory_caches_after_every_test(active_device_factories):
+    yield None
+    for f in active_device_factories:
+        f.cache_clear()  # type: ignore

tests/system_tests/conftest.py

@@ -1,3 +1,4 @@
+import os
 import re
 from decimal import Decimal
 from unittest.mock import AsyncMock, patch
@@ -8,6 +9,8 @@
 from dodal.devices.oav.oav_parameters import OAVConfig
 from ophyd_async.testing import set_mock_value
 
+from mx_bluesky.hyperion.parameters.constants import CONST
+
 # Map all the case-sensitive column names from their normalised versions
 DATA_COLLECTION_COLUMN_MAP = {
     s.lower(): s
@@ -119,6 +122,15 @@
 }
 
 
+@pytest.fixture(autouse=True)
+def use_dev_ispyb_unless_overridden_by_environment():
+    ispyb_config_path = os.environ.get(
+        "ISPYB_CONFIG_PATH", CONST.SIM.DEV_ISPYB_DATABASE_CFG
+    )
+    with patch.dict(os.environ, {"ISPYB_CONFIG_PATH": ispyb_config_path}):
+        yield
+
+
 @pytest.fixture
 def undulator_for_system_test(undulator):
     set_mock_value(undulator.current_gap, 1.11)

tests/system_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py

@@ -80,10 +80,8 @@ def event_monitor(monitor: zmq.Socket, connection_active_lock: threading.Lock) -
 @pytest.fixture
 def RE_with_external_callbacks():
     RE = RunEngine()
-    old_ispyb_config = os.environ.get("ISPYB_CONFIG_PATH")
 
     process_env = os.environ.copy()
-    process_env["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG
 
     external_callbacks_process = subprocess.Popen(
         [
@@ -119,8 +117,6 @@ def RE_with_external_callbacks():
     external_callbacks_process.kill()
     external_callbacks_process.wait(10)
     t.join()
-    if old_ispyb_config:
-        os.environ["ISPYB_CONFIG_PATH"] = old_ispyb_config
 
 
 @pytest.mark.s03

tests/system_tests/hyperion/external_interaction/test_exp_eye_dev.py

@@ -1,6 +1,4 @@
-import os
 from time import sleep
-from unittest.mock import patch
 
 import pytest
 from requests import get
@@ -9,20 +7,11 @@
     BLSampleStatus,
     ExpeyeInteraction,
 )
-from mx_bluesky.hyperion.parameters.constants import CONST
 
 CONTAINER_ID = 288588
 SAMPLE_ID = 5289780
 
 
-@pytest.fixture(autouse=True)
-def use_dev_ispyb():
-    with patch.dict(
-        os.environ, {"ISPYB_CONFIG_PATH": CONST.SIM.DEV_ISPYB_DATABASE_CFG}
-    ):
-        yield
-
-
 @pytest.mark.s03
 @pytest.mark.parametrize(
     "message, expected_message",

tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import os
 from collections.abc import Callable, Sequence
 from copy import deepcopy
 from typing import Any, Literal
@@ -332,7 +331,6 @@ def test_ispyb_deposition_in_gridscan(
     fetch_datacollection_grid_attribute: Callable[..., Any],
     fetch_datacollection_position_attribute: Callable[..., Any],
 ):
-    os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG
     set_mock_value(
         grid_detect_then_xray_centre_composite.s4_slit_gaps.xgap.user_readback, 0.1
     )
@@ -474,7 +472,6 @@ def test_ispyb_deposition_in_rotation_plan(
     fetch_datacollection_position_attribute: Callable[..., Any],
     feature_flags_update_with_omega_flip,
 ):
-    os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG
     ispyb_cb = RotationISPyBCallback()
     RE.subscribe(ispyb_cb)
 

tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py

@@ -15,6 +15,9 @@
 from ophyd_async.core import AsyncStatus
 from ophyd_async.testing import set_mock_value
 
+from mx_bluesky.common.external_interaction.callbacks.common.ispyb_mapping import (
+    get_proposal_and_session_from_visit_string,
+)
 from mx_bluesky.common.external_interaction.callbacks.xray_centre.ispyb_callback import (
     GridscanISPyBCallback,
 )
@@ -47,12 +50,16 @@
 )
 from .conftest import raw_params_from_file
 
+SAMPLE_ID = int(os.environ.get("ST_SAMPLE_ID", 5461074))
+
 
 @pytest.fixture
 def load_centre_collect_params():
     json_dict = raw_params_from_file(
         "tests/test_data/parameter_json_files/example_load_centre_collect_params.json"
     )
+    json_dict["visit"] = os.environ.get("ST_VISIT", "cm37235-4")
+    json_dict["sample_id"] = SAMPLE_ID
     return LoadCentreCollect(**json_dict)
 
 
@@ -105,7 +112,7 @@ def load_centre_collect_composite(
 
 
 GRID_DC_1_EXPECTED_VALUES = {
-    "BLSAMPLEID": 5461074,
+    "BLSAMPLEID": SAMPLE_ID,
     "detectorid": 78,
     "axisstart": 0.0,
     "axisrange": 0,
@@ -186,8 +193,6 @@ def load_centre_collect_composite(
     "6}_oav_snapshot_270\\.png",
 }
 
-SAMPLE_ID = 5461074
-
 
 @pytest.fixture
 def composite_with_no_diffraction(
@@ -203,15 +208,7 @@ async def mock_zocalo_complete():
         yield load_centre_collect_composite
 
 
-@pytest.fixture(autouse=True)
-def use_dev_ispyb():
-    with patch.dict(
-        os.environ, values={"ISPYB_CONFIG_PATH": CONST.SIM.DEV_ISPYB_DATABASE_CFG}
-    ):
-        yield
-
-
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_execute_load_centre_collect_full(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -245,7 +242,13 @@ def test_execute_load_centre_collect_full(
         )
     )
 
-    robot_load_cb.expeye.start_load.assert_called_once_with("cm37235", 4, 5461074, 2, 6)
+    expected_proposal, expected_visit = get_proposal_and_session_from_visit_string(
+        load_centre_collect_params.visit
+    )
+    expected_sample_id = load_centre_collect_params.sample_id
+    robot_load_cb.expeye.start_load.assert_called_once_with(
+        expected_proposal, expected_visit, expected_sample_id, 2, 6
+    )
     # TODO re-enable this https://github.com/DiamondLightSource/mx-bluesky/issues/690
     # robot_load_cb.expeye.update_barcode_and_snapshots.assert_called_once_with(
     #     1234,
@@ -258,7 +261,7 @@ def test_execute_load_centre_collect_full(
     # Compare gridscan collection
     compare_actual_and_expected(
         ispyb_gridscan_cb.ispyb_ids.data_collection_group_id,
-        {"experimentType": "Mesh3D", "blSampleId": 5461074},
+        {"experimentType": "Mesh3D", "blSampleId": expected_sample_id},
         fetch_datacollectiongroup_attribute,
     )
     compare_actual_and_expected(
@@ -293,7 +296,7 @@ def test_execute_load_centre_collect_full(
     rotation_dc_ids = fetch_datacollection_ids_for_group_id(rotation_dcg_id)
     compare_actual_and_expected(
         rotation_dcg_id,
-        {"experimentType": "SAD", "blSampleId": 5461074},
+        {"experimentType": "SAD", "blSampleId": expected_sample_id},
         fetch_datacollectiongroup_attribute,
     )
     compare_actual_and_expected(
@@ -312,10 +315,10 @@ def test_execute_load_centre_collect_full(
         ispyb_rotation_cb.ispyb_ids.data_collection_ids[0],
         "Sample position (µm): (-2309, -591, 341) Hyperion Rotation Scan -   Aperture: Small. ",
     )
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "LOADED"  # type: ignore
+    assert fetch_blsample(expected_sample_id).blSampleStatus == "LOADED"  # type: ignore
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_robot_load_fail(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -343,10 +346,13 @@ def test_load_centre_collect_updates_bl_sample_status_robot_load_fail(
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - beamline"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - beamline"
+    )
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_pin_tip_detection_fail(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -375,10 +381,13 @@ def test_load_centre_collect_updates_bl_sample_status_pin_tip_detection_fail(
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - sample"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - sample"
+    )
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_no_beamstop(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -399,10 +408,13 @@ def test_load_centre_collect_updates_bl_sample_status_no_beamstop(
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - beamline"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - beamline"
+    )
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_grid_detection_fail_tip_not_found(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -449,10 +461,13 @@ def wait_for_first_oav_grid(name: str, doc: dict):
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - sample"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - sample"
+    )
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_gridscan_no_diffraction(
     composite_with_no_diffraction: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -478,10 +493,13 @@ def test_load_centre_collect_updates_bl_sample_status_gridscan_no_diffraction(
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - sample"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - sample"
+    )
 
 
-@pytest.mark.s03
+@pytest.mark.system_test
 def test_load_centre_collect_updates_bl_sample_status_rotation_failure(
     load_centre_collect_composite: LoadCentreCollectComposite,
     load_centre_collect_params: LoadCentreCollect,
@@ -513,4 +531,7 @@ def test_load_centre_collect_updates_bl_sample_status_rotation_failure(
             )
         )
 
-    assert fetch_blsample(SAMPLE_ID).blSampleStatus == "ERROR - beamline"
+    assert (
+        fetch_blsample(load_centre_collect_params.sample_id).blSampleStatus
+        == "ERROR - beamline"
+    )

tests/system_tests/hyperion/external_interaction/test_zocalo_system.py

@@ -68,7 +68,6 @@ def run_zocalo_with_dev_ispyb(
     async def inner(sample_name="", fallback=np.array([0, 0, 0])):
         dummy_params.file_name = sample_name
         _, ispyb_callback = create_gridscan_callbacks()
-        ispyb_callback.ispyb_config = dummy_ispyb_3d.ISPYB_CONFIG_PATH
         RE.subscribe(ispyb_callback)
 
         @bpp.set_run_key_decorator("testing123")