From 3854a0848575907d9b8dd333fecb703a13f1bec0 Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 27 Jan 2023 15:55:45 +0000 Subject: [PATCH 1/7] move time change line --- .../external_interaction/callbacks/fgs/zocalo_callback.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py b/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py index 7127087b0..53bc7a9d4 100644 --- a/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py +++ b/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py @@ -77,7 +77,7 @@ def wait_for_results(self, fallback_xyz: Point3D) -> Point3D: Point3D: The xray centre position to move to """ datacollection_group_id = self.ispyb.ispyb_ids[2] - self.processing_time = time.time() - self.processing_start_time + try: raw_results = self.zocalo_interactor.wait_for_result( datacollection_group_id @@ -99,6 +99,7 @@ def wait_for_results(self, fallback_xyz: Point3D) -> Point3D: xray_centre = fallback_xyz LOGGER.warn(log_msg) + self.processing_time = time.time() - self.processing_start_time self.ispyb.append_to_comment( f"Zocalo processing took {self.processing_time:.2f} s" ) From aefe998056f2edba6ad9724d64e5ac62036b47d3 Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 27 Jan 2023 16:03:37 +0000 Subject: [PATCH 2/7] delete duplicate test --- .../unit_tests/test_zocalo_interaction.py | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py index d52f558b8..799a49693 100644 --- a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py +++ b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py @@ -178,43 +178,6 @@ def test_when_exception_caused_by_zocalo_message_then_exception_propagated( assert str(actual_exception.value) == str(failure_exception) -@patch("workflows.recipe.wrap_subscribe") -@patch("zocalo.configuration.from_file") -@patch("artemis.external_interaction.zocalo.zocalo_interaction.lookup") -def test_when_exception_caused_by_zocalo_message_then_exception_propagated( - mock_transport_lookup, mock_from_file, mock_wrap_subscribe -): - zc = ZocaloInteractor(environment=SIM_ZOCALO_ENV) - - mock_zc: Configuration = MagicMock() - mock_from_file.return_value = mock_zc - mock_transport = MagicMock() - mock_transport_lookup.return_value = MagicMock() - mock_transport_lookup.return_value.return_value = mock_transport - - with concurrent.futures.ThreadPoolExecutor() as executor: - future = executor.submit(zc.wait_for_result, 0) - - for _ in range(10): - sleep(0.1) - if mock_wrap_subscribe.call_args: - break - - result_func = mock_wrap_subscribe.call_args[0][2] - - failure_exception = Exception("Bad function!") - - mock_recipe_wrapper = MagicMock() - mock_transport.ack.side_effect = failure_exception - with pytest.raises(Exception) as actual_exception: - result_func(mock_recipe_wrapper, {}, []) - assert str(actual_exception.value) == str(failure_exception) - - with pytest.raises(Exception) as actual_exception: - future.result() - assert str(actual_exception.value) == str(failure_exception) - - @patch("workflows.recipe.wrap_subscribe") @patch("zocalo.configuration.from_file") @patch("artemis.external_interaction.zocalo.zocalo_interaction.lookup") From 751981312af349dcb608e823a05fc703dc895fef Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 27 Jan 2023 17:01:15 +0000 Subject: [PATCH 3/7] add test --- .../system_tests/conftest.py | 60 +++++++++++++++++++ .../system_tests/test_ispyb_dev_connection.py | 51 ---------------- .../system_tests/test_zocalo_system.py | 21 +++++++ 3 files changed, 81 insertions(+), 51 deletions(-) create mode 100644 src/artemis/external_interaction/system_tests/conftest.py diff --git a/src/artemis/external_interaction/system_tests/conftest.py b/src/artemis/external_interaction/system_tests/conftest.py new file mode 100644 index 000000000..ced1d2100 --- /dev/null +++ b/src/artemis/external_interaction/system_tests/conftest.py @@ -0,0 +1,60 @@ +from functools import partial +from typing import Callable + +import ispyb.sqlalchemy +import pytest +from ispyb.sqlalchemy import DataCollection +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from artemis.external_interaction.ispyb.store_in_ispyb import ( + StoreInIspyb2D, + StoreInIspyb3D, +) +from artemis.parameters import FullParameters +from artemis.utils import Point3D + +ISPYB_CONFIG = "/dls_sw/dasc/mariadb/credentials/ispyb-dev.cfg" + + +def get_current_datacollection_comment(Session: Callable, dcid: int) -> str: + """Read the 'comments' field from the given datacollection id's ISPyB entry. + Returns an empty string if the comment is not yet initialised. + """ + try: + with Session() as session: + query = session.query(DataCollection).filter( + DataCollection.dataCollectionId == dcid + ) + current_comment: str = query.first().comments + except Exception: + current_comment = "" + return current_comment + + +@pytest.fixture +def fetch_comment() -> Callable: + url = ispyb.sqlalchemy.url(ISPYB_CONFIG) + engine = create_engine(url, connect_args={"use_pure": True}) + Session = sessionmaker(engine) + return partial(get_current_datacollection_comment, Session) + + +@pytest.fixture +def dummy_params(): + dummy_params = FullParameters() + dummy_params.ispyb_params.upper_left = Point3D(100, 100, 50) + dummy_params.ispyb_params.pixels_per_micron_x = 0.8 + dummy_params.ispyb_params.pixels_per_micron_y = 0.8 + dummy_params.ispyb_params.visit_path = "/dls/i03/data/2022/cm31105-5/" + return dummy_params + + +@pytest.fixture +def dummy_ispyb(dummy_params) -> StoreInIspyb2D: + return StoreInIspyb2D(ISPYB_CONFIG, dummy_params) + + +@pytest.fixture +def dummy_ispyb_3d(dummy_params) -> StoreInIspyb3D: + return StoreInIspyb3D(ISPYB_CONFIG, dummy_params) diff --git a/src/artemis/external_interaction/system_tests/test_ispyb_dev_connection.py b/src/artemis/external_interaction/system_tests/test_ispyb_dev_connection.py index 6f117e982..1c9a66453 100644 --- a/src/artemis/external_interaction/system_tests/test_ispyb_dev_connection.py +++ b/src/artemis/external_interaction/system_tests/test_ispyb_dev_connection.py @@ -1,11 +1,4 @@ -from functools import partial -from typing import Callable - -import ispyb.sqlalchemy import pytest -from ispyb.sqlalchemy import DataCollection -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker from artemis.external_interaction.ispyb.store_in_ispyb import ( StoreInIspyb, @@ -13,54 +6,10 @@ StoreInIspyb3D, ) from artemis.parameters import FullParameters -from artemis.utils import Point3D ISPYB_CONFIG = "/dls_sw/dasc/mariadb/credentials/ispyb-dev.cfg" -def get_current_datacollection_comment(Session: Callable, dcid: int) -> str: - """Read the 'comments' field from the given datacollection id's ISPyB entry. - Returns an empty string if the comment is not yet initialised. - """ - try: - with Session() as session: - query = session.query(DataCollection).filter( - DataCollection.dataCollectionId == dcid - ) - current_comment: str = query.first().comments - except Exception: - current_comment = "" - return current_comment - - -@pytest.fixture -def fetch_comment() -> Callable: - url = ispyb.sqlalchemy.url(ISPYB_CONFIG) - engine = create_engine(url, connect_args={"use_pure": True}) - Session = sessionmaker(engine) - return partial(get_current_datacollection_comment, Session) - - -@pytest.fixture -def dummy_params(): - dummy_params = FullParameters() - dummy_params.ispyb_params.upper_left = Point3D(100, 100, 50) - dummy_params.ispyb_params.pixels_per_micron_x = 0.8 - dummy_params.ispyb_params.pixels_per_micron_y = 0.8 - dummy_params.ispyb_params.visit_path = "/dls/i03/data/2022/cm31105-5/" - return dummy_params - - -@pytest.fixture -def dummy_ispyb(dummy_params): - return StoreInIspyb2D(ISPYB_CONFIG, dummy_params) - - -@pytest.fixture -def dummy_ispyb_3d(dummy_params): - return StoreInIspyb3D(ISPYB_CONFIG, dummy_params) - - @pytest.mark.s03 def test_ispyb_get_comment_from_collection_correctly(fetch_comment): expected_comment_contents = ( diff --git a/src/artemis/external_interaction/system_tests/test_zocalo_system.py b/src/artemis/external_interaction/system_tests/test_zocalo_system.py index dcede6410..7a2a8a71e 100644 --- a/src/artemis/external_interaction/system_tests/test_zocalo_system.py +++ b/src/artemis/external_interaction/system_tests/test_zocalo_system.py @@ -57,3 +57,24 @@ def test_given_a_result_with_no_diffraction_when_zocalo_called_then_move_to_fall fallback = Point3D(1, 2, 3) centre = zc.wait_for_results(fallback_xyz=fallback) assert centre == fallback + + +@pytest.mark.s03 +def test_zocalo_adds_nonzero_comment_time(zocalo_env, dummy_ispyb_3d, fetch_comment): + params = FullParameters() + zc: FGSZocaloCallback = FGSCallbackCollection.from_params(params).zocalo_handler + zc.ispyb.ispyb = dummy_ispyb_3d + zc.ispyb.ispyb_ids = zc.ispyb.ispyb.begin_deposition() + ispyb_numbers = zc.ispyb.ispyb_ids + assert ispyb_numbers[0] is not None + dcids = ispyb_numbers[0] + for dcid in dcids: + zc.zocalo_interactor.run_start(dcid) + zc.stop({}) + zc.wait_for_results(fallback_xyz=Point3D(0, 0, 0)) + zc.ispyb.ispyb.end_deposition("success", "") + + comment = fetch_comment(ispyb_numbers[0][0]) + assert comment[-29:-6] == "Zocalo processing took " + assert float(comment[-6:-2]) > 0 + assert float(comment[-6:-2]) < 90 From 8c79e232fbfd33e27649c949b35a2377d0e39691 Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 27 Jan 2023 17:09:52 +0000 Subject: [PATCH 4/7] wait with better time resolution --- src/artemis/external_interaction/zocalo/zocalo_interaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/artemis/external_interaction/zocalo/zocalo_interaction.py b/src/artemis/external_interaction/zocalo/zocalo_interaction.py index bb6f5c6c3..41ce3125d 100644 --- a/src/artemis/external_interaction/zocalo/zocalo_interaction.py +++ b/src/artemis/external_interaction/zocalo/zocalo_interaction.py @@ -136,7 +136,7 @@ def receive_result( if exception is not None: raise exception else: - sleep(1) + sleep(0.1) else: return result_received.get_nowait() raise TimeoutError("Timed out waiting for zocalo results") From c5e16b0a4ba624e2dbdee3491e4ecbc4fac1c114 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 6 Feb 2023 10:51:44 +0000 Subject: [PATCH 5/7] fix tests --- src/artemis/system_tests/test_fgs_plan.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/artemis/system_tests/test_fgs_plan.py b/src/artemis/system_tests/test_fgs_plan.py index 5b23a5f5e..17f529600 100644 --- a/src/artemis/system_tests/test_fgs_plan.py +++ b/src/artemis/system_tests/test_fgs_plan.py @@ -1,13 +1,11 @@ import os import uuid +from typing import Callable from unittest.mock import MagicMock, patch import bluesky.preprocessors as bpp -import ispyb.sqlalchemy import pytest from bluesky.run_engine import RunEngine -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker import artemis.experiment_plans.fast_grid_scan_plan as fgs_plan from artemis.devices.eiger import EigerDetector @@ -21,7 +19,6 @@ from artemis.external_interaction.callbacks import FGSCallbackCollection from artemis.external_interaction.system_tests.test_ispyb_dev_connection import ( ISPYB_CONFIG, - get_current_datacollection_comment, ) from artemis.parameters import ( SIM_BEAMLINE, @@ -177,6 +174,7 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en eiger: EigerDetector, RE: RunEngine, fgs_composite: FGSComposite, + fetch_comment: Callable, ): parameters = FullParameters() parameters.detector_params.directory = "./tmp" @@ -194,13 +192,9 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en with pytest.raises(WarningException): RE(get_plan(parameters, callbacks)) - url = ispyb.sqlalchemy.url(ISPYB_CONFIG) - engine = create_engine(url, connect_args={"use_pure": True}) - Session = sessionmaker(engine) - dcid_used = callbacks.ispyb_handler.ispyb.datacollection_ids[0] - comment = get_current_datacollection_comment(Session, dcid_used) + comment = fetch_comment(dcid_used) assert "too long/short/bent" in comment mock_start_zocalo.assert_not_called() From c95ca07ce9faa86d360e8ecee1349aff58f8d9f9 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 6 Feb 2023 12:41:33 +0000 Subject: [PATCH 6/7] (#510) Don't check coverage on conftest files --- codecov.yml | 3 +++ setup.cfg | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/codecov.yml b/codecov.yml index dbaec6930..23b7e58b7 100644 --- a/codecov.yml +++ b/codecov.yml @@ -4,3 +4,6 @@ coverage: default: target: 85% # the required coverage value threshold: 1% # the leniency in hitting the target + +ignore: + - "**/conftest.py" diff --git a/setup.cfg b/setup.cfg index cbdc96081..d1c2cbdb4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -81,9 +81,6 @@ extend-ignore = E501, [coverage:run] -omit = - # This is covered in the versiongit test suite so exclude it here - */_version_git.py data_file = /tmp/python-artemis.coverage [coverage:paths] From a973b1605422ea21d97552e6609e9ad4721f8ef2 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 6 Feb 2023 12:47:29 +0000 Subject: [PATCH 7/7] (#510) Fix using dev database in system tests --- src/artemis/system_tests/test_fgs_plan.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/artemis/system_tests/test_fgs_plan.py b/src/artemis/system_tests/test_fgs_plan.py index 17f529600..d9abda126 100644 --- a/src/artemis/system_tests/test_fgs_plan.py +++ b/src/artemis/system_tests/test_fgs_plan.py @@ -17,6 +17,7 @@ run_gridscan, ) from artemis.external_interaction.callbacks import FGSCallbackCollection +from artemis.external_interaction.system_tests.conftest import fetch_comment # noqa from artemis.external_interaction.system_tests.test_ispyb_dev_connection import ( ISPYB_CONFIG, )