From 0d09dc58da8990ab92915ba102fafb6d2eefc1ad Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 18:57:44 +0000 Subject: [PATCH 01/11] Add way to get thing from test environment by type --- tests/utilities/lt_test_utils.py | 56 ++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/tests/utilities/lt_test_utils.py b/tests/utilities/lt_test_utils.py index 86565ed6..c88ae309 100644 --- a/tests/utilities/lt_test_utils.py +++ b/tests/utilities/lt_test_utils.py @@ -3,13 +3,15 @@ import tempfile import time from types import TracebackType -from typing import Any, Optional, Self +from typing import Any, Mapping, Optional, Self, TypeVar import requests from fastapi.testclient import TestClient import labthings_fastapi as lt +ThingSubclass = TypeVar("ThingSubclass", bound=lt.Thing) + ACTION_RUNNING_KEYWORDS = ["idle", "pending", "running"] @@ -29,7 +31,9 @@ class LabThingsTestEnv: """ def __init__( - self, things: dict[str, lt.Thing | str], settings_folder: Optional[str] = None + self, + things: Mapping[str, lt.Thing | str], + settings_folder: Optional[str] = None, ) -> None: """Initialise the test environment. @@ -87,11 +91,49 @@ class LabThingsTestEnv: if thing_name not in self.server.things: raise ValueError(f"No Thing named {thing_name}") - def get_thing(self, thing_name: str) -> lt.Thing: + def get_thing_by_name(self, thing_name: str) -> lt.Thing: """Get a Thing from the server by name.""" self.check_thing_exists(thing_name) return self.server.things[thing_name] + def get_thing_by_type(self, thing_class: type[ThingSubclass]) -> ThingSubclass: + """Get a thing by type. + + :param thing_class: The subclass of thing to match. + + :return: The Thing that matches the subclass. + + :raises RuntimeError: If there are multiple things of the same type, or no + matching thing. If there are multiple things of this type use + ``get_thing_by_name`` or ``get_all_things_by_type``. + """ + matching = self.get_all_things_by_type(thing_class) + n_things = len(matching) + if n_things == 0: + raise RuntimeError(f"No Thing of type {thing_class} on this server.") + if n_things > 1: + raise RuntimeError( + f"Cannot get Thing by type as there are {n_things} of type " + f"{thing_class} on this server." + ) + return next(iter(matching.values())) + + def get_all_things_by_type( + self, thing_class: type[ThingSubclass] + ) -> Mapping[str, ThingSubclass]: + """Get a dictionary of all things by matching a type. + + :param thing_class: The subclass of thing to match. + + :return: A dictionary of Things that match the subclass. If none match this + will be and empty dictionary. + """ + matching = {} + for thing_name, thing in self.server.things.items(): + if isinstance(thing, thing_class): + matching[thing_name] = thing + return matching + def get_thing_client(self, thing_name: str) -> lt.ThingClient: """Get a ThingClient for a Thing by name.""" self.check_thing_exists(thing_name) @@ -102,7 +144,7 @@ class LabThingsTestEnv: self, thing_name: str, action_name: str, - action_kwargs: Optional[dict[str, Any]] = None, + action_kwargs: Optional[Mapping[str, Any]] = None, ) -> requests.Response: """Start an action and return the server response. @@ -117,7 +159,7 @@ class LabThingsTestEnv: def poll_action( self, response: requests.Response, interval: float = 0.01 - ) -> dict[str, Any]: + ) -> Mapping[str, Any]: """Poll an action until it completes and return the final response data.""" invocation_data = response.json() @@ -141,11 +183,11 @@ class LabThingsTestEnv: response.raise_for_status() -def _get_link(obj: dict[str, Any], rel: str) -> dict[str, Any]: +def _get_link(obj: Mapping[str, Any], rel: str) -> Mapping[str, Any]: """Retrieve a link from an object's `links` list, by its `rel` attribute.""" return next(link for link in obj["links"] if link["rel"] == rel) -def _invocation_href(invocation_data: dict[str, Any]) -> str: +def _invocation_href(invocation_data: Mapping[str, Any]) -> str: """Get the invocation href from the invocation response data.""" return _get_link(invocation_data, "self")["href"] From b70d36c629466b0d71cfcea50ea2c7633a8f83fa Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 18:58:35 +0000 Subject: [PATCH 02/11] Move main test suite to new use new test environment. --- tests/test_camera.py | 70 ++++++++++++++++---------------------- tests/test_dummy_server.py | 65 +++++++++++------------------------ tests/test_stage.py | 26 +++++--------- 3 files changed, 57 insertions(+), 104 deletions(-) diff --git a/tests/test_camera.py b/tests/test_camera.py index 07fb1181..9275bdf2 100644 --- a/tests/test_camera.py +++ b/tests/test_camera.py @@ -1,38 +1,32 @@ """Use the Simulation camera to test base camera functionality.""" -import tempfile - import numpy as np import pytest -from fastapi.testclient import TestClient from PIL import Image import labthings_fastapi as lt +from openflexure_microscope_server.things.camera.simulation import SimulatedCamera +from openflexure_microscope_server.things.stage.dummy import DummyStage + +from .utilities.lt_test_utils import LabThingsTestEnv + @pytest.fixture -def camera_server() -> lt.ThingClient: - """Add the camera to a ThingServer and start a TestClient application. - - The test client will be needed for the camera to run async frame generation code. - """ - with tempfile.TemporaryDirectory() as tmpdir: - thing_conf = { - "camera": "openflexure_microscope_server.things.camera.simulation:SimulatedCamera", - "stage": "openflexure_microscope_server.things.stage.dummy:DummyStage", - } - - server = lt.ThingServer(things=thing_conf, settings_folder=tmpdir) - yield server +def test_env() -> lt.ThingClient: + """Yield a test environment with the Simulated Camera and Dummy Stage.""" + thing_conf = {"camera": SimulatedCamera, "stage": DummyStage} + with LabThingsTestEnv(things=thing_conf) as env: + yield env -def test_handle_broken_frame(camera_server): +def test_handle_broken_frame(test_env): """Monkey patch the the mjpeg steam so 1 in 5 frames are broken, then test operation. This simulates the very occasional broken frames that can occur when grabbing directly from the MJPEG stream. """ - camera = camera_server.things["camera"] + camera = test_env.get_thing_by_type(SimulatedCamera) # Money patch the mjpeg_stream grab_frame to break 1 in 5 frames. frame_number = 0 @@ -51,29 +45,25 @@ def test_handle_broken_frame(camera_server): camera.mjpeg_stream.grab_frame = flaky_grabber - with TestClient(camera_server.app): - # Check that this does cause broken frames. - # The noqa is because we don't know exactly when the error is thrown so we - # can't have a single simple statement in the pytest raises. - with pytest.raises( # noqa PT012 - OSError, match="broken data stream when reading image file" - ): - for _i in range(15): - jpeg = camera.grab_jpeg() - np.asarray(Image.open(jpeg.open())) - - # Check that grab_as_array handles the broken frames and completes without - # the same error. + # Check that this does cause broken frames. + # The noqa is because we don't know exactly when the error is thrown so we + # can't have a single simple statement in the pytest raises. + with pytest.raises(OSError, match="broken data stream when reading image file"): # noqa PT012 for _i in range(15): - array = camera.grab_as_array() - assert isinstance(array, np.ndarray) + jpeg = camera.grab_jpeg() + np.asarray(Image.open(jpeg.open())) + + # Check that grab_as_array handles the broken frames and completes without + # the same error. + for _i in range(15): + array = camera.grab_as_array() + assert isinstance(array, np.ndarray) -def test_simulation_cam_calibration(camera_server): +def test_simulation_cam_calibration(test_env): """Test that the simulated camera can be calibrated and reports calibration correctly.""" - camera = camera_server.things["camera"] - with TestClient(camera_server.app): - assert camera.calibration_required - camera.full_auto_calibrate() - assert not camera.calibration_required - assert camera.background_detector_status.ready + camera = test_env.get_thing_by_type(SimulatedCamera) + assert camera.calibration_required + camera.full_auto_calibrate() + assert not camera.calibration_required + assert camera.background_detector_status.ready diff --git a/tests/test_dummy_server.py b/tests/test_dummy_server.py index 38808df3..e26e3d62 100644 --- a/tests/test_dummy_server.py +++ b/tests/test_dummy_server.py @@ -9,22 +9,18 @@ directory in the root of the repository. """ import json -import os -import tempfile import numpy as np import piexif import pytest -from fastapi.testclient import TestClient from PIL import Image -import labthings_fastapi as lt +from .utilities.lt_test_utils import LabThingsTestEnv @pytest.fixture -def thing_server(): +def test_env(): """Yield a server with a very basic configuration.""" - temp_folder = tempfile.TemporaryDirectory() thing_conf = { "camera": { "class": "openflexure_microscope_server.things.camera.simulation:SimulatedCamera", @@ -41,51 +37,28 @@ def thing_server(): "autofocus": "openflexure_microscope_server.things.autofocus:AutofocusThing", "camera_stage_mapping": "openflexure_microscope_server.things.camera_stage_mapping:CameraStageMapper", } - - server = lt.ThingServer(things=thing_conf, settings_folder=temp_folder.name) - - assert os.path.exists(os.path.join(temp_folder.name, "camera")) - # Note: yield is important. If return is used the temp folder gets deleted - # before the test runs. Silence PT022 as ruff doesn't think yield is needed. - yield server # noqa: PT022 + with LabThingsTestEnv(things=thing_conf) as env: + yield env -@pytest.fixture -def client(thing_server): - """Yield a FastAPI TestClient for the server.""" - with TestClient(thing_server.app) as client: - yield client - - -@pytest.fixture -def slower_client(thing_server): - """Yield a FastAPI TestClient for the server with a slower moving stage. - - The step time for the stage is 100 microseconds rather than - 1 microsecond. - """ - thing_server.things["stage"].step_time = 0.0001 - with TestClient(thing_server.app) as client: - yield client - - -def test_autofocus(slower_client): +def test_autofocus(test_env): """Test Fast Autofocus can run doesn't raise an exception.""" - client = slower_client - autofocus = lt.ThingClient.from_url("/autofocus/", client) + # Adjust the time for stage is 100 microseconds rather than 1 microsecond. + test_env.get_thing_by_name["stage"].step_time = 0.0001 + autofocus = test_env.get_thing_client("autofocus") _ = autofocus.fast_autofocus() -def test_grab_jpeg(client): +def test_grab_jpeg(test_env): """Check that grab_jpeg returns a blob that can be opened.""" - camera = lt.ThingClient.from_url("/camera/", client) + camera = test_env.get_thing_client("camera") blob = camera.grab_jpeg() _image = Image.open(blob.open()) -def test_capture_jpeg_metadata(client): +def test_capture_jpeg_metadata(test_env): """Check that the position is encoded into the image metadata.""" - camera = lt.ThingClient.from_url("/camera/", client) + camera = test_env.get_thing_client("camera") blob = camera.capture_jpeg() image = Image.open(blob.open()) exif_dict = piexif.load(image.info["exif"]) @@ -94,9 +67,9 @@ def test_capture_jpeg_metadata(client): assert "position" in metadata["stage"] -def test_stage(client): +def test_stage(test_env): """Test moving th stage forwards and backwards.""" - stage = lt.ThingClient.from_url("/stage/", client) + stage = test_env.get_thing_client("stage") start = stage.position move = {"x": 1, "y": 2, "z": 3} stage.move_relative(**move) @@ -109,16 +82,16 @@ def test_stage(client): assert s == p -def test_capture_array(client): +def test_capture_array(test_env): """Capture array from simulation and check the size is as expected.""" - camera = lt.ThingClient.from_url("/camera/", client) + camera = test_env.get_thing_client("camera") array = np.asarray(camera.capture_array()) assert array.shape == (240, 320, 3) -def test_camera_stage_mapping_calibration(client): +def test_camera_stage_mapping_calibration(test_env): """Check that camera stage mapping can run without an exception.""" - camera = lt.ThingClient.from_url("/camera/", client) + camera = test_env.get_thing_client("camera") camera.settling_time = 0 - camera_stage_mapping = lt.ThingClient.from_url("/camera_stage_mapping/", client) + camera_stage_mapping = test_env.get_thing_client("camera_stage_mapping") camera_stage_mapping.calibrate_xy() diff --git a/tests/test_stage.py b/tests/test_stage.py index 22ec73dc..36acba7b 100644 --- a/tests/test_stage.py +++ b/tests/test_stage.py @@ -1,10 +1,8 @@ """Test the stage without creating a full HTTP server and socket connection.""" import itertools -import tempfile import pytest -from fastapi.testclient import TestClient from httpx import HTTPStatusError from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st @@ -12,12 +10,15 @@ from hypothesis import strategies as st import labthings_fastapi as lt from labthings_fastapi.testing import create_thing_without_server +from openflexure_microscope_server.things.camera.simulation import SimulatedCamera from openflexure_microscope_server.things.stage import ( BaseStage, RedefinedBaseMovementError, ) from openflexure_microscope_server.things.stage.dummy import DummyStage +from .utilities.lt_test_utils import LabThingsTestEnv + # Keep the size and number of moves fairly small or the tests can take forever point3d = st.tuples( st.integers(min_value=-100, max_value=100), @@ -34,18 +35,6 @@ def dummy_stage(): return create_thing_without_server(DummyStage, step_time=0.000001) -@pytest.fixture -def stage_server(): - """Yield a server with a very basic configuration.""" - thing_conf = { - "camera": "openflexure_microscope_server.things.camera.simulation:SimulatedCamera", - "stage": "openflexure_microscope_server.things.stage.dummy:DummyStage", - } - with tempfile.TemporaryDirectory() as tmpdir: - server = lt.ThingServer(things=thing_conf, settings_folder=tmpdir) - yield server - - def test_override_base_movement(): """Child classes of stage should implement functions in the hardware reference frame. @@ -157,11 +146,12 @@ def test_direction_inversion(dummy_stage): assert dummy_stage.axis_inverted == {"x": True, "y": False, "z": False} -def test_direction_errors_local_and_http(stage_server): +def test_direction_errors_local_and_http(): """Check for expected errors both locally and over http.""" - dummy_stage = stage_server.things["stage"] - with TestClient(stage_server.app) as test_client: - stage_client = lt.ThingClient.from_url("/stage/", client=test_client) + thing_conf = {"camera": SimulatedCamera, "stage": DummyStage} + with LabThingsTestEnv(things=thing_conf) as test_env: + dummy_stage = test_env.get_thing_by_type(DummyStage) + stage_client = test_env.get_thing_client("stage") assert stage_client.axis_inverted == {"x": True, "y": False, "z": False} # Can't set an arbitrary value via a client as read only: From 17b1f0e72a3f0a429b312aff7b43e5117771eb79 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 19:18:57 +0000 Subject: [PATCH 03/11] Update picamera tests to use new test environment --- .../picamera2/cam_test_utils.py | 37 +++++++++++++ .../picamera2/cam_test_utils/__init__.py | 53 ------------------- hardware-specific-tests/picamera2/conftest.py | 30 +++-------- .../picamera2/test_calibration.py | 8 +-- 4 files changed, 50 insertions(+), 78 deletions(-) create mode 100644 hardware-specific-tests/picamera2/cam_test_utils.py delete mode 100644 hardware-specific-tests/picamera2/cam_test_utils/__init__.py diff --git a/hardware-specific-tests/picamera2/cam_test_utils.py b/hardware-specific-tests/picamera2/cam_test_utils.py new file mode 100644 index 00000000..25e5f020 --- /dev/null +++ b/hardware-specific-tests/picamera2/cam_test_utils.py @@ -0,0 +1,37 @@ +"""Utilities to help with testing the camera.""" + +from contextlib import contextmanager +from typing import Optional + +from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 + +from ..tests.utilities.lt_test_utils import LabThingsTestEnv + + +@contextmanager +def camera_test_env(settings_folder: Optional[str] = None): + """Yield a test environment with a server that contains just the camera. + + This is a context manager, not a pytest fixture, as it needs to be created + multiple times in some tests. + + :param settings_folder: The settings folder for the camera, if none is supplied, new + temporary directory will be used as the settings folder. + """ + thing_conf = {"camera": StreamingPiCamera2} + with LabThingsTestEnv(things=thing_conf, settings_folder=settings_folder) as env: + yield env + + +@contextmanager +def camera_test_client(settings_folder: Optional[str] = None): + """Yield a camera ThingClient on a camera server. + + This is a context manager, not a pytest fixture, as it needs to be created + multiple times in some tests. + + :param settings_folder: The settings folder for the camera, if none is supplied, new + temporary directory will be used as the settings folder. + """ + with camera_test_env(settings_folder=settings_folder) as env: + return env.get_thing_client("camera") diff --git a/hardware-specific-tests/picamera2/cam_test_utils/__init__.py b/hardware-specific-tests/picamera2/cam_test_utils/__init__.py deleted file mode 100644 index 3893d583..00000000 --- a/hardware-specific-tests/picamera2/cam_test_utils/__init__.py +++ /dev/null @@ -1,53 +0,0 @@ -"""Utilities to help with testing the camera.""" - -import tempfile -from contextlib import contextmanager -from typing import Optional - -from fastapi.testclient import TestClient - -import labthings_fastapi as lt - - -@contextmanager -def camera_test_client_and_server(settings_folder: Optional[str] = None): - """Yield a camera ThingClient and the associated camera server. - - This is a context manager, not a pytest fixture, as it needs to be created - multiple times in some tests. - - :param cam: The camera Thing to be used. If not supplied a new one will be created. - :param settings_folder: The settings folder for the camera, if none is supplied, new - temporary directory will be used as the settings folder. - """ - # Create a temp dir, if the setting folder is set it isn't really needed - # but doesn't add much overhead. - with tempfile.TemporaryDirectory() as tmpdir: - if settings_folder is None: - settings_folder = tmpdir - thing_conf = { - "camera": "openflexure_microscope_server.things.camera.picamera:StreamingPiCamera2", - } - server = lt.ThingServer(things=thing_conf, settings_folder=settings_folder) - - with TestClient(server.app) as test_client: - client = lt.ThingClient.from_url("/camera/", client=test_client) - yield client, server - del server - - -@contextmanager -def camera_test_client(settings_folder: Optional[str] = None): - """Yield a camera ThingClient on a camera server. - - This is a context manager, not a pytest fixture, as it needs to be created - multiple times in some tests. - - :param cam: The camera Thing to be used. If not supplied a new one will be created. - :param settings_folder: The settings folder for the camera, if none is supplied, new - temporary directory will be used as the settings folder. - """ - with camera_test_client_and_server( - settings_folder=settings_folder - ) as client_and_server: - yield client_and_server[0] diff --git a/hardware-specific-tests/picamera2/conftest.py b/hardware-specific-tests/picamera2/conftest.py index 68ec8dc1..63b3500f 100644 --- a/hardware-specific-tests/picamera2/conftest.py +++ b/hardware-specific-tests/picamera2/conftest.py @@ -8,28 +8,14 @@ from .cam_test_utils import camera_test_client_and_server @pytest.fixture -def picamera_client_and_server() -> lt.ThingClient: - """Initialise a test picamera_client and server for the StreamingPiCamera2 Thing. - - This fixture: - - * Sets up a ThingServer, - * Registers a StreamingPiCamera2 instance at the "camera" endpoint - * Yields a ThingClient and the server for interacting with it during tests. - * The picamera thing can be found at server.things["camera"] - """ - with camera_test_client_and_server() as client_and_server: - yield client_and_server +def picamera_test_env() -> lt.ThingClient: + """Initialise a test environment with only a StreamingPiCamera2 Thing.""" + with camera_test_client_and_server() as env: + yield env @pytest.fixture -def picamera_client(picamera_client_and_server) -> lt.ThingClient: - """Initialise a test picamera_client for the StreamingPiCamera2 Thing. - - This fixture: - - * Sets up a ThingServer, - * Registers a StreamingPiCamera2 instance at the "camera" endpoint - * return a ThingClient for interacting with it during tests. - """ - return picamera_client_and_server[0] +def picamera_client() -> lt.ThingClient: + """Initialise a test picamera_client (in a LabThings test env).""" + with camera_test_client_and_server() as env: + return env.get_thing_client["camera"] diff --git a/hardware-specific-tests/picamera2/test_calibration.py b/hardware-specific-tests/picamera2/test_calibration.py index 1e239344..f02e363a 100644 --- a/hardware-specific-tests/picamera2/test_calibration.py +++ b/hardware-specific-tests/picamera2/test_calibration.py @@ -3,13 +3,15 @@ import tempfile from copy import deepcopy +from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 + from .cam_test_utils import camera_test_client -def test_calibration(picamera_client_and_server): +def test_calibration(picamera_test_env): """Check that full auto calibrate completes and set the expected values.""" - picamera_client, server = picamera_client_and_server - picamera_thing = server.things["camera"] + picamera_client, server = picamera_test_env.get_thing_client["camera"] + picamera_thing = picamera_test_env.get_thing_by_type(StreamingPiCamera2) # Check the calibration_required property used by the calibration wizard assert picamera_thing.calibration_required # Save copy of default tuning file for end of test From 62b414af3cc3c86f85d12ac04e1fb314c34692b3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 16:43:38 +0000 Subject: [PATCH 04/11] Update docs and internal names for LabThingsTestEnvironment --- tests/utilities/lt_test_utils.py | 46 +++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/tests/utilities/lt_test_utils.py b/tests/utilities/lt_test_utils.py index c88ae309..44793b0e 100644 --- a/tests/utilities/lt_test_utils.py +++ b/tests/utilities/lt_test_utils.py @@ -45,7 +45,7 @@ class LabThingsTestEnv: """ self._server: Optional[lt.ThingServer] self._test_client: Optional[TestClient] - self._thing_config = things + self._things_config = things self._settings_folder = settings_folder self._tmp_dir_obj: Optional[tempfile.TemporaryDirectory] = None @@ -55,7 +55,7 @@ class LabThingsTestEnv: self._tmp_dir_obj = tempfile.TemporaryDirectory() self._settings_folder = self._tmp_dir_obj.name self._server = lt.ThingServer( - things=self._thing_config, settings_folder=self._settings_folder + things=self._things_config, settings_folder=self._settings_folder ) self._test_client = TestClient(self._server.app) self._test_client.__enter__() @@ -92,7 +92,12 @@ class LabThingsTestEnv: raise ValueError(f"No Thing named {thing_name}") def get_thing_by_name(self, thing_name: str) -> lt.Thing: - """Get a Thing from the server by name.""" + """Get a Thing from the server by name. + + :param thing_name: The name of the thing to on the server. + + :return: The Thing with the specified name. + """ self.check_thing_exists(thing_name) return self.server.things[thing_name] @@ -135,7 +140,12 @@ class LabThingsTestEnv: return matching def get_thing_client(self, thing_name: str) -> lt.ThingClient: - """Get a ThingClient for a Thing by name.""" + """Get a ThingClient for a Thing by name. + + :param thing_name: The name of the thing to on the server. + + :return: A LabThings ThingClient for the Thing with the specified name. + """ self.check_thing_exists(thing_name) thing = self.server.things[thing_name] return lt.ThingClient.from_url(thing.path, self.client) @@ -148,7 +158,23 @@ class LabThingsTestEnv: ) -> requests.Response: """Start an action and return the server response. - This response can be used to poll or cancel the action. + For most purposes the best way to run an action is to use ``get_thing_client`` + to create a ThingClient. At this point any actions can be run with a similar + Python API to calling directly. However, using ThingClient blocks the test + thread. + + This function provides an alternative way to start actions without blocking the + test thread. It will return the HTTP response, this response can be used to + poll or cancel the action. Use this method if you: + + * Want to test cancelling an action. + * Want to inspect the actions logs exactly as they would come to a web client + * Direcltly interact with the HTTP API + + :param thing_name: The name of the Thing on the server. + :param action_name: The name of the action to start. + :action_kwargs: The keyword inputs to the action. + :return: A Response object with the HTTP response. """ self.check_thing_exists(thing_name) url = f"/{thing_name}/{action_name}" @@ -160,7 +186,10 @@ class LabThingsTestEnv: def poll_action( self, response: requests.Response, interval: float = 0.01 ) -> Mapping[str, Any]: - """Poll an action until it completes and return the final response data.""" + """Poll an action until it completes and return the final response data. + + :param response: The response from starting this action with ``start_action``. + """ invocation_data = response.json() if "status" not in invocation_data: @@ -177,7 +206,10 @@ class LabThingsTestEnv: return invocation_data def cancel_action(self, response: requests.Response) -> None: - """Cancel an ongoing action.""" + """Cancel an ongoing action. + + :param response: The response from starting this action with ``start_action``. + """ invocation_data = response.json() response = self.client.delete(_invocation_href(invocation_data)) response.raise_for_status() From d08d4cd325f700c47d1b4366314877bbff861231 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 17:08:02 +0000 Subject: [PATCH 05/11] Move unit tests to their own sub-dir in tests --- .gitignore | 5 ++--- pyproject.toml | 2 +- tests/{ => unit_tests}/__init__.py | 0 .../{ => unit_tests}/mock_stitching/mock-stitch.py | 0 tests/{ => unit_tests}/test_autofocus.py | 0 tests/{ => unit_tests}/test_background_detectors.py | 0 tests/{ => unit_tests}/test_camera.py | 0 tests/{ => unit_tests}/test_camera_buffer.py | 0 tests/{ => unit_tests}/test_cameras.py | 0 tests/{ => unit_tests}/test_config_utilities.py | 0 tests/{ => unit_tests}/test_dummy_server.py | 2 +- tests/{ => unit_tests}/test_legacy_api.py | 0 tests/{ => unit_tests}/test_lock_decorator.py | 0 tests/{ => unit_tests}/test_logging.py | 0 .../{ => unit_tests}/test_picamera_tuning_files.py | 0 tests/{ => unit_tests}/test_sangaboard.py | 0 tests/{ => unit_tests}/test_scan_data.py | 0 tests/{ => unit_tests}/test_scan_directories.py | 0 tests/{ => unit_tests}/test_scan_planners.py | 0 tests/{ => unit_tests}/test_serve_static_files.py | 0 tests/{ => unit_tests}/test_server_cli.py | 0 tests/{ => unit_tests}/test_server_config.py | 2 +- tests/{ => unit_tests}/test_smart_scan.py | 0 tests/{ => unit_tests}/test_stack.py | 0 tests/{ => unit_tests}/test_stage.py | 0 tests/{ => unit_tests}/test_stage_measure.py | 0 tests/{ => unit_tests}/test_stitching.py | 0 tests/{ => unit_tests}/test_system_thing.py | 0 tests/{ => unit_tests}/test_version_strings.py | 2 +- tests/{ => unit_tests}/utilities/__init__.py | 0 .../utilities/example_smart_spiral_core.pkl | Bin .../utilities/example_smart_spiral_lobed.pkl | Bin .../utilities/example_smart_spiral_regular.pkl | Bin tests/{ => unit_tests}/utilities/lt_test_utils.py | 0 .../{ => unit_tests}/utilities/scan_test_helpers.py | 0 35 files changed, 6 insertions(+), 7 deletions(-) rename tests/{ => unit_tests}/__init__.py (100%) rename tests/{ => unit_tests}/mock_stitching/mock-stitch.py (100%) rename tests/{ => unit_tests}/test_autofocus.py (100%) rename tests/{ => unit_tests}/test_background_detectors.py (100%) rename tests/{ => unit_tests}/test_camera.py (100%) rename tests/{ => unit_tests}/test_camera_buffer.py (100%) rename tests/{ => unit_tests}/test_cameras.py (100%) rename tests/{ => unit_tests}/test_config_utilities.py (100%) rename tests/{ => unit_tests}/test_dummy_server.py (98%) rename tests/{ => unit_tests}/test_legacy_api.py (100%) rename tests/{ => unit_tests}/test_lock_decorator.py (100%) rename tests/{ => unit_tests}/test_logging.py (100%) rename tests/{ => unit_tests}/test_picamera_tuning_files.py (100%) rename tests/{ => unit_tests}/test_sangaboard.py (100%) rename tests/{ => unit_tests}/test_scan_data.py (100%) rename tests/{ => unit_tests}/test_scan_directories.py (100%) rename tests/{ => unit_tests}/test_scan_planners.py (100%) rename tests/{ => unit_tests}/test_serve_static_files.py (100%) rename tests/{ => unit_tests}/test_server_cli.py (100%) rename tests/{ => unit_tests}/test_server_config.py (99%) rename tests/{ => unit_tests}/test_smart_scan.py (100%) rename tests/{ => unit_tests}/test_stack.py (100%) rename tests/{ => unit_tests}/test_stage.py (100%) rename tests/{ => unit_tests}/test_stage_measure.py (100%) rename tests/{ => unit_tests}/test_stitching.py (100%) rename tests/{ => unit_tests}/test_system_thing.py (100%) rename tests/{ => unit_tests}/test_version_strings.py (99%) rename tests/{ => unit_tests}/utilities/__init__.py (100%) rename tests/{ => unit_tests}/utilities/example_smart_spiral_core.pkl (100%) rename tests/{ => unit_tests}/utilities/example_smart_spiral_lobed.pkl (100%) rename tests/{ => unit_tests}/utilities/example_smart_spiral_regular.pkl (100%) rename tests/{ => unit_tests}/utilities/lt_test_utils.py (100%) rename tests/{ => unit_tests}/utilities/scan_test_helpers.py (100%) diff --git a/.gitignore b/.gitignore index acc08e3c..3054abce 100644 --- a/.gitignore +++ b/.gitignore @@ -69,7 +69,6 @@ target/ #Big-o files *.data -tests/images/out/ #IDE files .vscode/ @@ -89,8 +88,8 @@ openflexure_settings/ /src/openflexure_microscope_server/static/ # Files created by test utilities -/tests/utilities/*.pstats -/tests/utilities/*.png +/tests/unit_tests/utilities/*.pstats +/tests/unit_tests/utilities/*.png # Files created by simulator openflexure/ diff --git a/pyproject.toml b/pyproject.toml index b7728ed5..8f065ad5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,7 +94,7 @@ addopts = [ norecursedirs = [".git", "build", "node_modules"] pythonpath = ["."] -testpaths = ["tests"] +testpaths = ["tests/unit_tests"] [tool.ruff.format] # Use native line endings for all files diff --git a/tests/__init__.py b/tests/unit_tests/__init__.py similarity index 100% rename from tests/__init__.py rename to tests/unit_tests/__init__.py diff --git a/tests/mock_stitching/mock-stitch.py b/tests/unit_tests/mock_stitching/mock-stitch.py similarity index 100% rename from tests/mock_stitching/mock-stitch.py rename to tests/unit_tests/mock_stitching/mock-stitch.py diff --git a/tests/test_autofocus.py b/tests/unit_tests/test_autofocus.py similarity index 100% rename from tests/test_autofocus.py rename to tests/unit_tests/test_autofocus.py diff --git a/tests/test_background_detectors.py b/tests/unit_tests/test_background_detectors.py similarity index 100% rename from tests/test_background_detectors.py rename to tests/unit_tests/test_background_detectors.py diff --git a/tests/test_camera.py b/tests/unit_tests/test_camera.py similarity index 100% rename from tests/test_camera.py rename to tests/unit_tests/test_camera.py diff --git a/tests/test_camera_buffer.py b/tests/unit_tests/test_camera_buffer.py similarity index 100% rename from tests/test_camera_buffer.py rename to tests/unit_tests/test_camera_buffer.py diff --git a/tests/test_cameras.py b/tests/unit_tests/test_cameras.py similarity index 100% rename from tests/test_cameras.py rename to tests/unit_tests/test_cameras.py diff --git a/tests/test_config_utilities.py b/tests/unit_tests/test_config_utilities.py similarity index 100% rename from tests/test_config_utilities.py rename to tests/unit_tests/test_config_utilities.py diff --git a/tests/test_dummy_server.py b/tests/unit_tests/test_dummy_server.py similarity index 98% rename from tests/test_dummy_server.py rename to tests/unit_tests/test_dummy_server.py index e26e3d62..217bc9d2 100644 --- a/tests/test_dummy_server.py +++ b/tests/unit_tests/test_dummy_server.py @@ -44,7 +44,7 @@ def test_env(): def test_autofocus(test_env): """Test Fast Autofocus can run doesn't raise an exception.""" # Adjust the time for stage is 100 microseconds rather than 1 microsecond. - test_env.get_thing_by_name["stage"].step_time = 0.0001 + test_env.get_thing_by_name("stage").step_time = 0.0001 autofocus = test_env.get_thing_client("autofocus") _ = autofocus.fast_autofocus() diff --git a/tests/test_legacy_api.py b/tests/unit_tests/test_legacy_api.py similarity index 100% rename from tests/test_legacy_api.py rename to tests/unit_tests/test_legacy_api.py diff --git a/tests/test_lock_decorator.py b/tests/unit_tests/test_lock_decorator.py similarity index 100% rename from tests/test_lock_decorator.py rename to tests/unit_tests/test_lock_decorator.py diff --git a/tests/test_logging.py b/tests/unit_tests/test_logging.py similarity index 100% rename from tests/test_logging.py rename to tests/unit_tests/test_logging.py diff --git a/tests/test_picamera_tuning_files.py b/tests/unit_tests/test_picamera_tuning_files.py similarity index 100% rename from tests/test_picamera_tuning_files.py rename to tests/unit_tests/test_picamera_tuning_files.py diff --git a/tests/test_sangaboard.py b/tests/unit_tests/test_sangaboard.py similarity index 100% rename from tests/test_sangaboard.py rename to tests/unit_tests/test_sangaboard.py diff --git a/tests/test_scan_data.py b/tests/unit_tests/test_scan_data.py similarity index 100% rename from tests/test_scan_data.py rename to tests/unit_tests/test_scan_data.py diff --git a/tests/test_scan_directories.py b/tests/unit_tests/test_scan_directories.py similarity index 100% rename from tests/test_scan_directories.py rename to tests/unit_tests/test_scan_directories.py diff --git a/tests/test_scan_planners.py b/tests/unit_tests/test_scan_planners.py similarity index 100% rename from tests/test_scan_planners.py rename to tests/unit_tests/test_scan_planners.py diff --git a/tests/test_serve_static_files.py b/tests/unit_tests/test_serve_static_files.py similarity index 100% rename from tests/test_serve_static_files.py rename to tests/unit_tests/test_serve_static_files.py diff --git a/tests/test_server_cli.py b/tests/unit_tests/test_server_cli.py similarity index 100% rename from tests/test_server_cli.py rename to tests/unit_tests/test_server_cli.py diff --git a/tests/test_server_config.py b/tests/unit_tests/test_server_config.py similarity index 99% rename from tests/test_server_config.py rename to tests/unit_tests/test_server_config.py index 82b22929..d7ee4e86 100644 --- a/tests/test_server_config.py +++ b/tests/unit_tests/test_server_config.py @@ -11,7 +11,7 @@ import pytest from openflexure_microscope_server import server as ofm_server THIS_DIR = os.path.dirname(os.path.abspath(__file__)) -REPO_ROOT = os.path.dirname(THIS_DIR) +REPO_ROOT = os.path.dirname(os.path.dirname(THIS_DIR)) FULL_CONFIG = os.path.join(REPO_ROOT, "ofm_config_full.json") SIM_CONFIG = os.path.join(REPO_ROOT, "ofm_config_simulation.json") diff --git a/tests/test_smart_scan.py b/tests/unit_tests/test_smart_scan.py similarity index 100% rename from tests/test_smart_scan.py rename to tests/unit_tests/test_smart_scan.py diff --git a/tests/test_stack.py b/tests/unit_tests/test_stack.py similarity index 100% rename from tests/test_stack.py rename to tests/unit_tests/test_stack.py diff --git a/tests/test_stage.py b/tests/unit_tests/test_stage.py similarity index 100% rename from tests/test_stage.py rename to tests/unit_tests/test_stage.py diff --git a/tests/test_stage_measure.py b/tests/unit_tests/test_stage_measure.py similarity index 100% rename from tests/test_stage_measure.py rename to tests/unit_tests/test_stage_measure.py diff --git a/tests/test_stitching.py b/tests/unit_tests/test_stitching.py similarity index 100% rename from tests/test_stitching.py rename to tests/unit_tests/test_stitching.py diff --git a/tests/test_system_thing.py b/tests/unit_tests/test_system_thing.py similarity index 100% rename from tests/test_system_thing.py rename to tests/unit_tests/test_system_thing.py diff --git a/tests/test_version_strings.py b/tests/unit_tests/test_version_strings.py similarity index 99% rename from tests/test_version_strings.py rename to tests/unit_tests/test_version_strings.py index 3fa6a8fd..fc8fd817 100644 --- a/tests/test_version_strings.py +++ b/tests/unit_tests/test_version_strings.py @@ -16,7 +16,7 @@ from openflexure_microscope_server import utilities # Useful for really finding the repo dir when we mock where it is. THIS_DIR = os.path.dirname(__file__) -TRUE_REPO_DIR = os.path.dirname(THIS_DIR) +TRUE_REPO_DIR = os.path.dirname(os.path.dirname(THIS_DIR)) # For explicit version checking. VER_STRING = "3.0.0-alpha4" diff --git a/tests/utilities/__init__.py b/tests/unit_tests/utilities/__init__.py similarity index 100% rename from tests/utilities/__init__.py rename to tests/unit_tests/utilities/__init__.py diff --git a/tests/utilities/example_smart_spiral_core.pkl b/tests/unit_tests/utilities/example_smart_spiral_core.pkl similarity index 100% rename from tests/utilities/example_smart_spiral_core.pkl rename to tests/unit_tests/utilities/example_smart_spiral_core.pkl diff --git a/tests/utilities/example_smart_spiral_lobed.pkl b/tests/unit_tests/utilities/example_smart_spiral_lobed.pkl similarity index 100% rename from tests/utilities/example_smart_spiral_lobed.pkl rename to tests/unit_tests/utilities/example_smart_spiral_lobed.pkl diff --git a/tests/utilities/example_smart_spiral_regular.pkl b/tests/unit_tests/utilities/example_smart_spiral_regular.pkl similarity index 100% rename from tests/utilities/example_smart_spiral_regular.pkl rename to tests/unit_tests/utilities/example_smart_spiral_regular.pkl diff --git a/tests/utilities/lt_test_utils.py b/tests/unit_tests/utilities/lt_test_utils.py similarity index 100% rename from tests/utilities/lt_test_utils.py rename to tests/unit_tests/utilities/lt_test_utils.py diff --git a/tests/utilities/scan_test_helpers.py b/tests/unit_tests/utilities/scan_test_helpers.py similarity index 100% rename from tests/utilities/scan_test_helpers.py rename to tests/unit_tests/utilities/scan_test_helpers.py From 4e043f3df230a8d00857b81a02cadcbde1ac086e Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 17:13:15 +0000 Subject: [PATCH 06/11] Move lt_test_utils into a shared testing directory --- tests/__init__.py | 4 ++++ tests/shared_utils/__init__.py | 1 + tests/{unit_tests/utilities => shared_utils}/lt_test_utils.py | 0 tests/unit_tests/test_camera.py | 2 +- tests/unit_tests/test_dummy_server.py | 2 +- tests/unit_tests/test_stage.py | 2 +- tests/unit_tests/test_stitching.py | 2 +- 7 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 tests/__init__.py create mode 100644 tests/shared_utils/__init__.py rename tests/{unit_tests/utilities => shared_utils}/lt_test_utils.py (100%) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..98500514 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,4 @@ +"""Tests for the OpenFlexure Server. + +Each test suite should be run separately. +""" diff --git a/tests/shared_utils/__init__.py b/tests/shared_utils/__init__.py new file mode 100644 index 00000000..f0a5ab00 --- /dev/null +++ b/tests/shared_utils/__init__.py @@ -0,0 +1 @@ +"""Shared utilities for all test suites.""" diff --git a/tests/unit_tests/utilities/lt_test_utils.py b/tests/shared_utils/lt_test_utils.py similarity index 100% rename from tests/unit_tests/utilities/lt_test_utils.py rename to tests/shared_utils/lt_test_utils.py diff --git a/tests/unit_tests/test_camera.py b/tests/unit_tests/test_camera.py index 9275bdf2..6f49b6f3 100644 --- a/tests/unit_tests/test_camera.py +++ b/tests/unit_tests/test_camera.py @@ -9,7 +9,7 @@ import labthings_fastapi as lt from openflexure_microscope_server.things.camera.simulation import SimulatedCamera from openflexure_microscope_server.things.stage.dummy import DummyStage -from .utilities.lt_test_utils import LabThingsTestEnv +from ..shared_utils.lt_test_utils import LabThingsTestEnv @pytest.fixture diff --git a/tests/unit_tests/test_dummy_server.py b/tests/unit_tests/test_dummy_server.py index 217bc9d2..212cd814 100644 --- a/tests/unit_tests/test_dummy_server.py +++ b/tests/unit_tests/test_dummy_server.py @@ -15,7 +15,7 @@ import piexif import pytest from PIL import Image -from .utilities.lt_test_utils import LabThingsTestEnv +from ..shared_utils.lt_test_utils import LabThingsTestEnv @pytest.fixture diff --git a/tests/unit_tests/test_stage.py b/tests/unit_tests/test_stage.py index 36acba7b..65b6ab63 100644 --- a/tests/unit_tests/test_stage.py +++ b/tests/unit_tests/test_stage.py @@ -17,7 +17,7 @@ from openflexure_microscope_server.things.stage import ( ) from openflexure_microscope_server.things.stage.dummy import DummyStage -from .utilities.lt_test_utils import LabThingsTestEnv +from ..shared_utils.lt_test_utils import LabThingsTestEnv # Keep the size and number of moves fairly small or the tests can take forever point3d = st.tuples( diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 0c9b52e7..31918f41 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -22,7 +22,7 @@ from openflexure_microscope_server.stitching import ( StitcherValidationError, ) -from .utilities.lt_test_utils import LabThingsTestEnv +from ..shared_utils.lt_test_utils import LabThingsTestEnv # A global logger pretending to the logger from a thing LOGGER = logging.getLogger("mock-thing_logger") From ef9a88104bbf3ca0184caa2cb1c01d88005fa43c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 17:21:17 +0000 Subject: [PATCH 07/11] Move hardware-specific-tests to tests dir --- integration-tests/testfile.py | 214 ------------------ picamera_tests.py | 2 +- pyproject.toml | 2 +- .../hardware_specific_tests}/README.md | 4 +- .../picamera2/cam_test_utils.py | 2 +- .../picamera2/conftest.py | 0 .../picamera2/test_acquisition.py | 0 .../picamera2/test_calibration.py | 0 .../picamera2/test_exposure_time_drift.py | 0 .../picamera2/test_sensor_mode.py | 0 .../picamera2/test_tuning.py | 0 tests/unit_tests/__init__.py | 2 +- tests/unit_tests/test_cameras.py | 2 +- 13 files changed, 7 insertions(+), 221 deletions(-) delete mode 100755 integration-tests/testfile.py rename {hardware-specific-tests => tests/hardware_specific_tests}/README.md (97%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/cam_test_utils.py (95%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/conftest.py (100%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/test_acquisition.py (100%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/test_calibration.py (100%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/test_exposure_time_drift.py (100%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/test_sensor_mode.py (100%) rename {hardware-specific-tests => tests/hardware_specific_tests}/picamera2/test_tuning.py (100%) diff --git a/integration-tests/testfile.py b/integration-tests/testfile.py deleted file mode 100755 index a9b37264..00000000 --- a/integration-tests/testfile.py +++ /dev/null @@ -1,214 +0,0 @@ -#! /usr/bin/env python3 -"""Start a server subprocess for integration tests. - -These tests are separated from unit tests to avoid inflating test coverage. -For now, this file should be run directly rather than through a test framework. - -They are designed to run on CI and should work on Linux or WSL for local debugging. -""" - -import os -import shutil -import subprocess -from time import sleep, time -from typing import Optional - -from PIL import Image - -import labthings_fastapi as lt - -THIS_DIR: str = os.path.dirname(os.path.realpath(__file__)) -WORKING_DIR: str = os.path.join(THIS_DIR, "working_dir") -REPO_DIR: str = os.path.dirname(THIS_DIR) -CONFIG_FILE: str = os.path.join(REPO_DIR, "ofm_config_simulation.json") -SERVER_CMD: list[str] = [ - "openflexure-microscope-server", - "--fallback", - "-c", - CONFIG_FILE, -] - - -def main() -> None: - """Set up the server, run basic checks, shutdown, check for graceful exit. - - The basic checks include checks that: - - The server boots - - The server reports the expected IP and port - - A ThingClient can connect to the camera - - The client grab a frame from the stream, and it is the expected size - - A background process can subscribe to the stream - """ - set_up_working_dir() - os.chdir(WORKING_DIR) - - server_process: Optional[subprocess.Popen] = None - subscriber_process: Optional[subprocess.Popen] = None - try: - print("Starting server") - server_process = start_server() - error_if_server_not_started(server_process, timeout=15) - - test_client_connection() - - # This also sleeps for 2s and checks it is still connected - subscriber_process = subscribe_to_mjpeg_stream() - - server_process.terminate() - sleep(3) - - check_for_graceful_shutdown(server_process) - - finally: - # Ensure the server and subscriber processes are really dead - if subscriber_process is not None and subscriber_process.poll() is None: - subscriber_process.kill() - if server_process is not None and server_process.poll() is None: - server_process.kill() - raise RuntimeError("Server needed killing at end of test.") - - -def test_client_connection() -> None: - """Check a ThingClient can interact with the simulation microscope camera.""" - print("Connecting Python client to microscope, and capturing image") - cam_client = lt.ThingClient.from_url("http://localhost:5000/camera/") - img = Image.open(cam_client.grab_jpeg().open()) - print(f"Successfully grabbed image of size {img.size}") - assert img.size == (820, 616) - print("Successfully grabbed image from camera mjpeg stream") - - -def subscribe_to_mjpeg_stream() -> subprocess.Popen: - """Start a background process subscribed to the mjpeg stream. - - :returns: The Popen object for the ongoing process. - - :raises: RuntimeError if the stream is not still connected after 2s - """ - # Use nohup to stop process hanging up unexpectedly. Explicitly forward stream - # to /dev/null to keep curl connected. - - curl_command = [ - "nohup", - "curl", - "-s", - "http://localhost:5000/camera/mjpeg_stream", - ">", - "/dev/null", - "2>&1", - ] - - process = subprocess.Popen( - curl_command, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - universal_newlines=True, - ) - os.set_blocking(process.stdout.fileno(), False) - - sleep(2) - if process.poll() is not None: - raise RuntimeError( - "MJPEG Subscriber process is not running. This likely means it could not" - " stay connected to the stream.\n" - ) - return process - - -def set_up_working_dir() -> None: - """If working dir exists, delete it and make a new one.""" - if os.path.exists(WORKING_DIR): - shutil.rmtree(WORKING_DIR) - os.makedirs(WORKING_DIR) - - -def start_server() -> subprocess.Popen: - """Start the server in a subprocess. - - The server is started in a subprocess and all outputs are buffered. - - :returns: Popen object for the ongoing process - """ - process = subprocess.Popen( - SERVER_CMD, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - universal_newlines=True, - ) - os.set_blocking(process.stdout.fileno(), False) - return process - - -def error_if_server_not_started( - server_process: subprocess.Popen, timeout: float = 15.0 -) -> None: - """Check the server started up as expected. - - Check the subprocess is running - Check the logs have no errors - Read the Logs to check uvicorn is running on http://127.0.0.1:5000 - - :param server_process: The Popen object for the the server process. - - :raises RuntimeError: If the server is not running as expected. - """ - confirmed_uvicorn_is_running = False - - t_start = time() - while not confirmed_uvicorn_is_running and time() - t_start < timeout: - sleep(0.2) - if stdout := read_process_buffers(server_process): - print("".join(stdout), end="") - if server_process.poll() is not None: - raise RuntimeError("Server process is not running!") - - for line in stdout: - if line.startswith("ERROR:"): - raise RuntimeError(f"Server started up with error\n{line}") - if "Uvicorn running on http://127.0.0.1:5000" in line: - confirmed_uvicorn_is_running = True - - if not confirmed_uvicorn_is_running: - raise RuntimeError("Cannot confirm Uvicorn is running on http://127.0.0.1:5000") - - # If we reached here Everything is fine! - print("Server is running as expected\n\n") - - -def check_for_graceful_shutdown(server_process: subprocess.Popen) -> None: - """Check the server shutdown gracefully. - - Check the subprocess is not running - Check the logs have no errors - Read the Logs to check the shutdown was graceful, rather than killed on Uvicorn - timeout - - :param server_process: The Popen object for the the server process. - - :raises RuntimeError: If the server didn't shutdown gracefully. - """ - stdout = read_process_buffers(server_process) - print("".join(stdout)) - print("\n\n") - for line in stdout: - if line.startswith("ERROR:"): - raise RuntimeError(f"Server encountered error\n{line}") - if "graceful shutdown exceeded" in line: - # This should be logged as an ERROR. This check is belts and braces. - raise RuntimeError("Server failed to shutdown gracefully.") - - -def read_process_buffers(process: subprocess.Popen) -> list[str]: - """Return STDOUT from a process.""" - stdout = [] - - while line := process.stdout.readline(): - stdout.append(line) - - return stdout - - -if __name__ == "__main__": - main() diff --git a/picamera_tests.py b/picamera_tests.py index 29097b60..8f4c4cdd 100755 --- a/picamera_tests.py +++ b/picamera_tests.py @@ -52,7 +52,7 @@ def _get_hashes(include_coverage: bool = False) -> dict[str, str]: def run_tests() -> None: """Run the picamera tests, zip the coverage database, hash relevant source files.""" - subprocess.run(["pytest", "hardware-specific-tests"], check=True) + subprocess.run(["pytest", "tests/hardware_specific_tests"], check=True) shutil.copyfile(".coverage", COVERAGE_FILE) hash_dict = _get_hashes(include_coverage=True) with open(HASH_FILE, "w", encoding="utf-8") as json_file: diff --git a/pyproject.toml b/pyproject.toml index 8f065ad5..f2b3f8c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -144,7 +144,7 @@ ignore = [ [tool.ruff.lint.per-file-ignores] # All testing dirs -"{tests,integration-tests,hardware-specific-tests}/**" = [ +"{tests,integration-tests}/**" = [ "B018", # Complaining about useless attribute access in tests, but we need them to check errors are raised "ANN", # Tests are not typehinted for fixtures etc "S101", # Allow asserts in tests diff --git a/hardware-specific-tests/README.md b/tests/hardware_specific_tests/README.md similarity index 97% rename from hardware-specific-tests/README.md rename to tests/hardware_specific_tests/README.md index c55ecc20..ec86b9d2 100644 --- a/hardware-specific-tests/README.md +++ b/tests/hardware_specific_tests/README.md @@ -34,13 +34,13 @@ As before, the server must be stopped before running these tests. The camera test are very slow as they run on hardware. They can be run with: - pytest hardware-specific-tests + pytest tests/hardware_specific_tests However, this will not archive the tests in the Git repository for reporting the coverage. For this, see the section above on reporting the coverage. When writing and debugging these unit tests, it is often best to run a specific test and to use the `-s` flag to see the print statements. It is also often useful to use `--pdb` to drop you into a python debug session on any failure. For example, you might run: - pytest hardware-specific-tests/picamera2/test_exposure_time_drift.py::test_exposure_time_saves_and_loads -s --pdb + pytest tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py::test_exposure_time_saves_and_loads -s --pdb ### CI explanation diff --git a/hardware-specific-tests/picamera2/cam_test_utils.py b/tests/hardware_specific_tests/picamera2/cam_test_utils.py similarity index 95% rename from hardware-specific-tests/picamera2/cam_test_utils.py rename to tests/hardware_specific_tests/picamera2/cam_test_utils.py index 25e5f020..ebe86980 100644 --- a/hardware-specific-tests/picamera2/cam_test_utils.py +++ b/tests/hardware_specific_tests/picamera2/cam_test_utils.py @@ -5,7 +5,7 @@ from typing import Optional from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 -from ..tests.utilities.lt_test_utils import LabThingsTestEnv +from ...shared_utils.lt_test_utils import LabThingsTestEnv @contextmanager diff --git a/hardware-specific-tests/picamera2/conftest.py b/tests/hardware_specific_tests/picamera2/conftest.py similarity index 100% rename from hardware-specific-tests/picamera2/conftest.py rename to tests/hardware_specific_tests/picamera2/conftest.py diff --git a/hardware-specific-tests/picamera2/test_acquisition.py b/tests/hardware_specific_tests/picamera2/test_acquisition.py similarity index 100% rename from hardware-specific-tests/picamera2/test_acquisition.py rename to tests/hardware_specific_tests/picamera2/test_acquisition.py diff --git a/hardware-specific-tests/picamera2/test_calibration.py b/tests/hardware_specific_tests/picamera2/test_calibration.py similarity index 100% rename from hardware-specific-tests/picamera2/test_calibration.py rename to tests/hardware_specific_tests/picamera2/test_calibration.py diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py similarity index 100% rename from hardware-specific-tests/picamera2/test_exposure_time_drift.py rename to tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py diff --git a/hardware-specific-tests/picamera2/test_sensor_mode.py b/tests/hardware_specific_tests/picamera2/test_sensor_mode.py similarity index 100% rename from hardware-specific-tests/picamera2/test_sensor_mode.py rename to tests/hardware_specific_tests/picamera2/test_sensor_mode.py diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/tests/hardware_specific_tests/picamera2/test_tuning.py similarity index 100% rename from hardware-specific-tests/picamera2/test_tuning.py rename to tests/hardware_specific_tests/picamera2/test_tuning.py diff --git a/tests/unit_tests/__init__.py b/tests/unit_tests/__init__.py index b58ffa86..6383f17d 100644 --- a/tests/unit_tests/__init__.py +++ b/tests/unit_tests/__init__.py @@ -1,6 +1,6 @@ """The unit-test suite for the OpenFlexure Microscope Server. This package contains all of the unit tests that can be run without specific -hardware. See also the `hardware-specific-tests` directory and the +hardware. See also the `hardware_specific_tests` directory and the `integration-tests` directory for more testing!. """ diff --git a/tests/unit_tests/test_cameras.py b/tests/unit_tests/test_cameras.py index 3f90689f..15d42dc0 100644 --- a/tests/unit_tests/test_cameras.py +++ b/tests/unit_tests/test_cameras.py @@ -1,6 +1,6 @@ """Tests for camera classes. These tests focus on checking the camera APIs are equivalent. -Tests for specific camera hardware are in the hardware-specific-tests directory. Tests +Tests for specific camera hardware are in the hardware_specific_tests directory. Tests on camera functionality using the simulation camera are in "test_camera". """ From 782977e01c725c81718d890658ef2b9359f79094 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 17:26:38 +0000 Subject: [PATCH 08/11] Move integration tests to tests dir --- .gitlab-ci.yml | 2 +- pyproject.toml | 2 +- tests/integration_tests/testfile.py | 214 ++++++++++++++++++++++++++ tests/unit_tests/__init__.py | 2 +- tests/unit_tests/test_dummy_server.py | 4 +- 5 files changed, 219 insertions(+), 5 deletions(-) create mode 100755 tests/integration_tests/testfile.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 10620d0e..b1ab0055 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -213,7 +213,7 @@ server_integration_tests: - job: build artifacts: true script: - - integration-tests/testfile.py + - tests/integration_tests/testfile.py pages: needs: diff --git a/pyproject.toml b/pyproject.toml index f2b3f8c1..5573e309 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -144,7 +144,7 @@ ignore = [ [tool.ruff.lint.per-file-ignores] # All testing dirs -"{tests,integration-tests}/**" = [ +"{tests}/**" = [ "B018", # Complaining about useless attribute access in tests, but we need them to check errors are raised "ANN", # Tests are not typehinted for fixtures etc "S101", # Allow asserts in tests diff --git a/tests/integration_tests/testfile.py b/tests/integration_tests/testfile.py new file mode 100755 index 00000000..a9b37264 --- /dev/null +++ b/tests/integration_tests/testfile.py @@ -0,0 +1,214 @@ +#! /usr/bin/env python3 +"""Start a server subprocess for integration tests. + +These tests are separated from unit tests to avoid inflating test coverage. +For now, this file should be run directly rather than through a test framework. + +They are designed to run on CI and should work on Linux or WSL for local debugging. +""" + +import os +import shutil +import subprocess +from time import sleep, time +from typing import Optional + +from PIL import Image + +import labthings_fastapi as lt + +THIS_DIR: str = os.path.dirname(os.path.realpath(__file__)) +WORKING_DIR: str = os.path.join(THIS_DIR, "working_dir") +REPO_DIR: str = os.path.dirname(THIS_DIR) +CONFIG_FILE: str = os.path.join(REPO_DIR, "ofm_config_simulation.json") +SERVER_CMD: list[str] = [ + "openflexure-microscope-server", + "--fallback", + "-c", + CONFIG_FILE, +] + + +def main() -> None: + """Set up the server, run basic checks, shutdown, check for graceful exit. + + The basic checks include checks that: + - The server boots + - The server reports the expected IP and port + - A ThingClient can connect to the camera + - The client grab a frame from the stream, and it is the expected size + - A background process can subscribe to the stream + """ + set_up_working_dir() + os.chdir(WORKING_DIR) + + server_process: Optional[subprocess.Popen] = None + subscriber_process: Optional[subprocess.Popen] = None + try: + print("Starting server") + server_process = start_server() + error_if_server_not_started(server_process, timeout=15) + + test_client_connection() + + # This also sleeps for 2s and checks it is still connected + subscriber_process = subscribe_to_mjpeg_stream() + + server_process.terminate() + sleep(3) + + check_for_graceful_shutdown(server_process) + + finally: + # Ensure the server and subscriber processes are really dead + if subscriber_process is not None and subscriber_process.poll() is None: + subscriber_process.kill() + if server_process is not None and server_process.poll() is None: + server_process.kill() + raise RuntimeError("Server needed killing at end of test.") + + +def test_client_connection() -> None: + """Check a ThingClient can interact with the simulation microscope camera.""" + print("Connecting Python client to microscope, and capturing image") + cam_client = lt.ThingClient.from_url("http://localhost:5000/camera/") + img = Image.open(cam_client.grab_jpeg().open()) + print(f"Successfully grabbed image of size {img.size}") + assert img.size == (820, 616) + print("Successfully grabbed image from camera mjpeg stream") + + +def subscribe_to_mjpeg_stream() -> subprocess.Popen: + """Start a background process subscribed to the mjpeg stream. + + :returns: The Popen object for the ongoing process. + + :raises: RuntimeError if the stream is not still connected after 2s + """ + # Use nohup to stop process hanging up unexpectedly. Explicitly forward stream + # to /dev/null to keep curl connected. + + curl_command = [ + "nohup", + "curl", + "-s", + "http://localhost:5000/camera/mjpeg_stream", + ">", + "/dev/null", + "2>&1", + ] + + process = subprocess.Popen( + curl_command, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + bufsize=1, + universal_newlines=True, + ) + os.set_blocking(process.stdout.fileno(), False) + + sleep(2) + if process.poll() is not None: + raise RuntimeError( + "MJPEG Subscriber process is not running. This likely means it could not" + " stay connected to the stream.\n" + ) + return process + + +def set_up_working_dir() -> None: + """If working dir exists, delete it and make a new one.""" + if os.path.exists(WORKING_DIR): + shutil.rmtree(WORKING_DIR) + os.makedirs(WORKING_DIR) + + +def start_server() -> subprocess.Popen: + """Start the server in a subprocess. + + The server is started in a subprocess and all outputs are buffered. + + :returns: Popen object for the ongoing process + """ + process = subprocess.Popen( + SERVER_CMD, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + bufsize=1, + universal_newlines=True, + ) + os.set_blocking(process.stdout.fileno(), False) + return process + + +def error_if_server_not_started( + server_process: subprocess.Popen, timeout: float = 15.0 +) -> None: + """Check the server started up as expected. + + Check the subprocess is running + Check the logs have no errors + Read the Logs to check uvicorn is running on http://127.0.0.1:5000 + + :param server_process: The Popen object for the the server process. + + :raises RuntimeError: If the server is not running as expected. + """ + confirmed_uvicorn_is_running = False + + t_start = time() + while not confirmed_uvicorn_is_running and time() - t_start < timeout: + sleep(0.2) + if stdout := read_process_buffers(server_process): + print("".join(stdout), end="") + if server_process.poll() is not None: + raise RuntimeError("Server process is not running!") + + for line in stdout: + if line.startswith("ERROR:"): + raise RuntimeError(f"Server started up with error\n{line}") + if "Uvicorn running on http://127.0.0.1:5000" in line: + confirmed_uvicorn_is_running = True + + if not confirmed_uvicorn_is_running: + raise RuntimeError("Cannot confirm Uvicorn is running on http://127.0.0.1:5000") + + # If we reached here Everything is fine! + print("Server is running as expected\n\n") + + +def check_for_graceful_shutdown(server_process: subprocess.Popen) -> None: + """Check the server shutdown gracefully. + + Check the subprocess is not running + Check the logs have no errors + Read the Logs to check the shutdown was graceful, rather than killed on Uvicorn + timeout + + :param server_process: The Popen object for the the server process. + + :raises RuntimeError: If the server didn't shutdown gracefully. + """ + stdout = read_process_buffers(server_process) + print("".join(stdout)) + print("\n\n") + for line in stdout: + if line.startswith("ERROR:"): + raise RuntimeError(f"Server encountered error\n{line}") + if "graceful shutdown exceeded" in line: + # This should be logged as an ERROR. This check is belts and braces. + raise RuntimeError("Server failed to shutdown gracefully.") + + +def read_process_buffers(process: subprocess.Popen) -> list[str]: + """Return STDOUT from a process.""" + stdout = [] + + while line := process.stdout.readline(): + stdout.append(line) + + return stdout + + +if __name__ == "__main__": + main() diff --git a/tests/unit_tests/__init__.py b/tests/unit_tests/__init__.py index 6383f17d..c5a29f85 100644 --- a/tests/unit_tests/__init__.py +++ b/tests/unit_tests/__init__.py @@ -2,5 +2,5 @@ This package contains all of the unit tests that can be run without specific hardware. See also the `hardware_specific_tests` directory and the -`integration-tests` directory for more testing!. +`integration_tests` directory for more testing!. """ diff --git a/tests/unit_tests/test_dummy_server.py b/tests/unit_tests/test_dummy_server.py index 212cd814..ed7206c0 100644 --- a/tests/unit_tests/test_dummy_server.py +++ b/tests/unit_tests/test_dummy_server.py @@ -4,8 +4,8 @@ Rather than spinning up a full uvicorn webserver for each test these tests use the FastAPI ``TestClient`` or directly communicate with the underlying LabThings-FastAPI code. This increases speed of testing significantly. -For tests that require a full running server see the ``integration-tests`` -directory in the root of the repository. +For tests that require a full running server see the ``integration_tests`` +directory in the tests directory. """ import json From 765404656aca50aebf0c9caa7e67c5aae91ba09f Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 17:48:33 +0000 Subject: [PATCH 09/11] Fix typos when moving hardware test suit to new test env structure --- tests/hardware_specific_tests/picamera2/cam_test_utils.py | 2 +- tests/hardware_specific_tests/picamera2/conftest.py | 8 ++++---- .../hardware_specific_tests/picamera2/test_calibration.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/hardware_specific_tests/picamera2/cam_test_utils.py b/tests/hardware_specific_tests/picamera2/cam_test_utils.py index ebe86980..73033dc7 100644 --- a/tests/hardware_specific_tests/picamera2/cam_test_utils.py +++ b/tests/hardware_specific_tests/picamera2/cam_test_utils.py @@ -34,4 +34,4 @@ def camera_test_client(settings_folder: Optional[str] = None): temporary directory will be used as the settings folder. """ with camera_test_env(settings_folder=settings_folder) as env: - return env.get_thing_client("camera") + yield env.get_thing_client("camera") diff --git a/tests/hardware_specific_tests/picamera2/conftest.py b/tests/hardware_specific_tests/picamera2/conftest.py index 63b3500f..cc40566f 100644 --- a/tests/hardware_specific_tests/picamera2/conftest.py +++ b/tests/hardware_specific_tests/picamera2/conftest.py @@ -4,18 +4,18 @@ import pytest import labthings_fastapi as lt -from .cam_test_utils import camera_test_client_and_server +from .cam_test_utils import camera_test_env @pytest.fixture def picamera_test_env() -> lt.ThingClient: """Initialise a test environment with only a StreamingPiCamera2 Thing.""" - with camera_test_client_and_server() as env: + with camera_test_env() as env: yield env @pytest.fixture def picamera_client() -> lt.ThingClient: """Initialise a test picamera_client (in a LabThings test env).""" - with camera_test_client_and_server() as env: - return env.get_thing_client["camera"] + with camera_test_env() as env: + yield env.get_thing_client("camera") diff --git a/tests/hardware_specific_tests/picamera2/test_calibration.py b/tests/hardware_specific_tests/picamera2/test_calibration.py index f02e363a..768461f9 100644 --- a/tests/hardware_specific_tests/picamera2/test_calibration.py +++ b/tests/hardware_specific_tests/picamera2/test_calibration.py @@ -10,7 +10,7 @@ from .cam_test_utils import camera_test_client def test_calibration(picamera_test_env): """Check that full auto calibrate completes and set the expected values.""" - picamera_client, server = picamera_test_env.get_thing_client["camera"] + picamera_client = picamera_test_env.get_thing_client("camera") picamera_thing = picamera_test_env.get_thing_by_type(StreamingPiCamera2) # Check the calibration_required property used by the calibration wizard assert picamera_thing.calibration_required From fc7cda01653fd325211af40a62d27810154492dd Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Dec 2025 23:02:43 +0000 Subject: [PATCH 10/11] Update picamera coverage zip --- picamera_coverage.zip | Bin 54038 -> 54038 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index cca9a9682719197518e6577a47aa5b6d598b0147..a227c5ac7832e6d3e68c8d97d47c4a59d2be513b 100644 GIT binary patch delta 293 zcmbQXjCtBJW}yIYW)=|!5cs}rQnY9H$*CKK3>}!)@I`KRbNIl^6v#38jGyh~ZGNJR z?32&<1+v+*u`o0;Prl%1v02LBl96>SUlbqv=7@m%yevU1jp~yXw!V`QIvyU&J^ZL>tbHY4kO{u}%kH#-V! z;-CC$J`Xm}1*3xDPcyMd zHcm=4G)pluOG!;JHZ?FxOiW8owJg<3=V#9r$j-vh zs6KgtuhQfMAKuCP{J18U`iZi#p^3_Emi0GdWM$`@&F8T>BH%vn_%HI`-|Q%`iGT91 z`8@pW>})KIoQ!O2V2We2=mJ#(4v>p87fTv!cDlgJWM^b%WSW|kWSVATl4_D{lxSv@ vnr5DslA2;08`Ud&v_3suNf) From 4a8b88b34b1128db0c555075189e0ef4e2256ae3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 19 Dec 2025 09:30:33 +0000 Subject: [PATCH 11/11] Fix config path in integration test. Don't use fallback in integration test as it masks failure. --- tests/integration_tests/testfile.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration_tests/testfile.py b/tests/integration_tests/testfile.py index a9b37264..02dd30a0 100755 --- a/tests/integration_tests/testfile.py +++ b/tests/integration_tests/testfile.py @@ -19,11 +19,10 @@ import labthings_fastapi as lt THIS_DIR: str = os.path.dirname(os.path.realpath(__file__)) WORKING_DIR: str = os.path.join(THIS_DIR, "working_dir") -REPO_DIR: str = os.path.dirname(THIS_DIR) +REPO_DIR: str = os.path.dirname(os.path.dirname(THIS_DIR)) CONFIG_FILE: str = os.path.join(REPO_DIR, "ofm_config_simulation.json") SERVER_CMD: list[str] = [ "openflexure-microscope-server", - "--fallback", "-c", CONFIG_FILE, ]