diff --git a/pyproject.toml b/pyproject.toml index a149d3e9a4..5e8e78d383 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,6 +90,7 @@ ignore_missing_imports = true # Ignore missing stubs in imported modules asyncio_mode = "auto" markers = [ "s03: marks tests as requiring the s03 simulator running (deselect with '-m \"not s03\"')", + "skip_in_pycharm: marks test as not working in pycharm testrunner" ] addopts = """ --cov=dodal --cov-report term --cov-report xml:cov.xml diff --git a/src/dodal/devices/robot.py b/src/dodal/devices/robot.py index 016573731e..3959fee447 100644 --- a/src/dodal/devices/robot.py +++ b/src/dodal/devices/robot.py @@ -1,5 +1,5 @@ import asyncio -from asyncio import FIRST_COMPLETED, Task +from asyncio import FIRST_COMPLETED, CancelledError, Task from dataclasses import dataclass from enum import Enum @@ -73,19 +73,28 @@ async def raise_if_no_pin(): await wait_for_value(self.error_code, self.NO_PIN_ERROR_CODE, None) raise RobotLoadFailed(self.NO_PIN_ERROR_CODE, "Pin was not detected") - finished, unfinished = await asyncio.wait( - [ - Task(raise_if_no_pin()), - Task( - wait_for_value(self.gonio_pin_sensor, PinMounted.PIN_MOUNTED, None) - ), - ], - return_when=FIRST_COMPLETED, - ) - for task in unfinished: - task.cancel() - for task in finished: - await task + async def wfv(): + await wait_for_value(self.gonio_pin_sensor, PinMounted.PIN_MOUNTED, None) + + tasks = [ + (Task(raise_if_no_pin())), + (Task(wfv())), + ] + try: + finished, unfinished = await asyncio.wait( + tasks, + return_when=FIRST_COMPLETED, + ) + for task in unfinished: + task.cancel() + for task in finished: + await task + except CancelledError: + # If the outer enclosing task cancels after LOAD_TIMEOUT, this causes CancelledError to be raised + # in the current task, when it propagates to here we should cancel all pending tasks before bubbling up + for task in tasks: + task.cancel() + raise async def _load_pin_and_puck(self, sample_location: SampleLocation): LOGGER.info(f"Loading pin {sample_location}") diff --git a/src/dodal/devices/webcam.py b/src/dodal/devices/webcam.py index a07fb7a2b5..3adec94e2a 100644 --- a/src/dodal/devices/webcam.py +++ b/src/dodal/devices/webcam.py @@ -4,7 +4,7 @@ import aiofiles from aiohttp import ClientSession from bluesky.protocols import Triggerable -from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw +from ophyd_async.core import AsyncStatus, HintedSignal, StandardReadable, soft_signal_rw from PIL import Image from dodal.devices.oav.utils import save_thumbnail @@ -18,7 +18,7 @@ def __init__(self, name, prefix, url): self.directory = soft_signal_rw(str, name="directory") self.last_saved_path = soft_signal_rw(str, name="last_saved_path") - self.set_readable_signals([self.last_saved_path]) + self.add_readables([self.last_saved_path], wrapper=HintedSignal) super().__init__(name=name) async def _write_image(self, file_path: str): diff --git a/src/dodal/devices/zocalo/zocalo_results.py b/src/dodal/devices/zocalo/zocalo_results.py index f847f67ab1..ddfda701df 100644 --- a/src/dodal/devices/zocalo/zocalo_results.py +++ b/src/dodal/devices/zocalo/zocalo_results.py @@ -10,7 +10,7 @@ import workflows.transport from bluesky.protocols import Descriptor, Triggerable from numpy.typing import NDArray -from ophyd_async.core import StandardReadable, soft_signal_r_and_setter +from ophyd_async.core import HintedSignal, StandardReadable, soft_signal_r_and_setter from ophyd_async.core.async_status import AsyncStatus from workflows.transport.common_transport import CommonTransport @@ -88,14 +88,15 @@ def __init__( ) self.ispyb_dcid, _ = soft_signal_r_and_setter(int, name="ispyb_dcid") self.ispyb_dcgid, _ = soft_signal_r_and_setter(int, name="ispyb_dcgid") - self.set_readable_signals( - read=[ + self.add_readables( + [ self.results, self.centres_of_mass, self.bbox_sizes, self.ispyb_dcid, self.ispyb_dcgid, - ] + ], + wrapper=HintedSignal, ) super().__init__(name) diff --git a/tests/common/beamlines/test_beamline_utils.py b/tests/common/beamlines/test_beamline_utils.py index 71400355a1..2f74ede3e4 100644 --- a/tests/common/beamlines/test_beamline_utils.py +++ b/tests/common/beamlines/test_beamline_utils.py @@ -1,4 +1,5 @@ -from unittest.mock import ANY, MagicMock, patch +import asyncio +from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest from bluesky.run_engine import RunEngine as RE @@ -13,11 +14,22 @@ from dodal.devices.aperturescatterguard import ApertureScatterguard from dodal.devices.smargon import Smargon from dodal.devices.zebra import Zebra +from dodal.log import LOGGER from dodal.utils import make_all_devices from ...conftest import mock_beamline_module_filepaths +@pytest.fixture(autouse=True) +def flush_event_loop_on_finish(event_loop): + # wait for the test function to complete + yield None + + if pending_tasks := asyncio.all_tasks(event_loop): + LOGGER.warning(f"Waiting for pending tasks to complete {pending_tasks}") + event_loop.run_until_complete(asyncio.gather(*pending_tasks)) + + @pytest.fixture(autouse=True) def setup(): beamline_utils.clear_devices() @@ -106,10 +118,11 @@ def test_wait_for_v1_device_connection_passes_through_timeout(kwargs, expected_t @pytest.mark.parametrize( "kwargs,expected_timeout", [({}, 5.0), ({"timeout": 15.0}, 15.0)] ) -@patch.object(beamline_utils, "call_in_bluesky_event_loop", spec=callable) -def test_wait_for_v2_device_connection_passes_through_timeout( - call_in_bluesky_el, kwargs, expected_timeout -): +@patch( + "dodal.common.beamlines.beamline_utils.v2_device_wait_for_connection", + new=AsyncMock(), +) +def test_wait_for_v2_device_connection_passes_through_timeout(kwargs, expected_timeout): RE() device = OphydV2Device() device.connect = MagicMock() diff --git a/tests/devices/unit_tests/test_eiger.py b/tests/devices/unit_tests/test_eiger.py index 6ac5427a73..55d298e5ab 100644 --- a/tests/devices/unit_tests/test_eiger.py +++ b/tests/devices/unit_tests/test_eiger.py @@ -7,7 +7,6 @@ from ophyd.status import Status from ophyd.utils import UnknownStatusFailure -from conftest import failed_status from dodal.devices.detector import DetectorParams, TriggerMode from dodal.devices.detector.det_dim_constants import EIGER2_X_16M_SIZE from dodal.devices.eiger import TEST_1169_FIX, EigerDetector @@ -15,6 +14,8 @@ from dodal.devices.util.epics_util import run_functions_without_blocking from dodal.log import LOGGER +from ...conftest import failed_status + TEST_DETECTOR_SIZE_CONSTANTS = EIGER2_X_16M_SIZE TEST_EXPECTED_ENERGY = 100.0 diff --git a/tests/devices/unit_tests/test_focusing_mirror.py b/tests/devices/unit_tests/test_focusing_mirror.py index 23424f3b2a..21872aaf6b 100644 --- a/tests/devices/unit_tests/test_focusing_mirror.py +++ b/tests/devices/unit_tests/test_focusing_mirror.py @@ -6,7 +6,6 @@ from ophyd.status import StatusBase from ophyd.utils import UnknownStatusFailure -from conftest import failed_status from dodal.devices.focusing_mirror import ( FocusingMirrorWithStripes, MirrorStripe, @@ -15,6 +14,8 @@ VFMMirrorVoltages, ) +from ...conftest import failed_status + @pytest.fixture def vfm_mirror_voltages_not_ok(vfm_mirror_voltages) -> VFMMirrorVoltages: diff --git a/tests/devices/unit_tests/test_webcam.py b/tests/devices/unit_tests/test_webcam.py index 2368052fc2..aeaf76d78b 100644 --- a/tests/devices/unit_tests/test_webcam.py +++ b/tests/devices/unit_tests/test_webcam.py @@ -39,6 +39,8 @@ async def test_given_filename_and_directory_when_trigger_and_read_then_returns_e webcam: Webcam, ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + # raise_for_status should be MagicMock() not AsyncMock() + mock_response.raise_for_status = MagicMock() mock_response.read.return_value = b"TEST" await webcam.filename.set(filename) await webcam.directory.set(directory) @@ -54,6 +56,8 @@ async def test_given_data_returned_from_url_when_trigger_then_data_written( mock_image, mock_get: MagicMock, mock_aiofiles, webcam: Webcam ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + # raise_for_status should be MagicMock() not AsyncMock() + mock_response.raise_for_status = MagicMock() mock_response.read.return_value = (test_web_data := b"TEST") mock_open = mock_aiofiles.open mock_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) diff --git a/tests/test_cli.py b/tests/test_cli.py index 2aa61c7aaa..0653ebd9f0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,9 +1,12 @@ import subprocess import sys +import pytest + from dodal import __version__ +@pytest.mark.skip_in_pycharm(reason="subprocess returns tty escape sequences") def test_cli_version(): cmd = [sys.executable, "-m", "dodal", "--version"] assert subprocess.check_output(cmd).decode().strip() == __version__ diff --git a/tests/unit_tests/test_log.py b/tests/unit_tests/test_log.py index 3eb408536b..30f03f464d 100644 --- a/tests/unit_tests/test_log.py +++ b/tests/unit_tests/test_log.py @@ -66,8 +66,12 @@ def test_dev_mode_sets_correct_graypy_handler( mock_logger: MagicMock, ): mock_GELFTCPHandler.return_value.level = logging.INFO - set_up_all_logging_handlers(mock_logger, Path("tmp/dev"), "dodal.log", True, 10000) + handler_config = set_up_all_logging_handlers( + mock_logger, Path("tmp/dev"), "dodal.log", True, 10000 + ) mock_GELFTCPHandler.assert_called_once_with("localhost", 5555) + for handler in handler_config.values(): + handler.close() @patch("dodal.log.GELFTCPHandler", autospec=True) @@ -76,10 +80,14 @@ def test_prod_mode_sets_correct_graypy_handler( mock_logger: MagicMock, ): mock_GELFTCPHandler.return_value.level = logging.INFO - set_up_all_logging_handlers(mock_logger, Path("tmp/dev"), "dodal.log", False, 10000) + handler_config = set_up_all_logging_handlers( + mock_logger, Path("tmp/dev"), "dodal.log", False, 10000 + ) mock_GELFTCPHandler.assert_called_once_with( "graylog-log-target.diamond.ac.uk", 12231 ) + for handler in handler_config.values(): + handler.close() @patch("dodal.log.GELFTCPHandler", autospec=True) @@ -95,7 +103,7 @@ def test_no_env_variable_sets_correct_file_handler( mock_file_handler.return_value.level = logging.INFO mock_GELFTCPHandler.return_value.level = logging.INFO clear_all_loggers_and_handlers() - _ = set_up_all_logging_handlers( + handler_config = set_up_all_logging_handlers( LOGGER, get_logging_file_path(), "dodal.log", True, ERROR_LOG_BUFFER_LINES ) integrate_bluesky_and_ophyd_logging(LOGGER) @@ -106,6 +114,8 @@ def test_no_env_variable_sets_correct_file_handler( ] mock_file_handler.assert_has_calls(expected_calls, any_order=True) + for handler in handler_config.values(): + handler.close() def test_beamline_filter_adds_dev_if_no_beamline():