Merge pull request #511 from DiamondLightSource/510_readd_zocalo_time…

…_to_comment

Restore writing the analysis time to comments

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"

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]

src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py

@@ -81,7 +81,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
@@ -103,6 +103,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"
  )

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)

src/artemis/external_interaction/system_tests/test_ispyb_dev_connection.py

@@ -1,66 +1,15 @@
-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,
  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):
- 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 = (

src/artemis/external_interaction/system_tests/test_zocalo_system.py

@@ -60,3 +60,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

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")

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(

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
@@ -19,9 +17,9 @@
  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,
- get_current_datacollection_comment,
 )
 from artemis.parameters import (
  SIM_BEAMLINE,
@@ -177,6 +175,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 +193,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()