From 510a44dd17b44db11a0d7b99c71a4340d2afee70 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 14:02:14 +0000 Subject: [PATCH 1/7] Remove all cancel hooks from smart scan and stitching --- .../stitching.py | 32 +++++++++---------- .../things/smart_scan.py | 19 +++-------- 2 files changed, 19 insertions(+), 32 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index b1a88863..e87afe1f 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -6,6 +6,7 @@ CPU intensity of stitching causing scanning problems due to the Python Global Interpreter Lock (GIL). May be possible to shift to multiprocessing in the future. """ +import logging import os import shlex import signal @@ -167,14 +168,17 @@ class PreviewStitcher(BaseStitcher): return False return self._popen_obj.poll() is None - def wait(self, cancel: lt.deps.CancelHook) -> None: + def wait(self) -> None: """Wait for this preview stitch to return. :raises InvocationCancelledError: if the action is cancelled. """ while self.running: try: - cancel.sleep(0.1) + # This should act exactly like sleep if not started in a LabThings + # action thread. In an action thread it will raise + # InvocationCancelledError if the action is cancelled. + lt.cancellable_sleep(0.2) except lt.exceptions.InvocationCancelledError as e: with self._popen_lock: if self._popen_obj is not None: @@ -189,7 +193,7 @@ class FinalStitcher(BaseStitcher): self, images_dir: str, *, - logger: lt.deps.InvocationLogger, + logger: logging.Logger, overlap: Optional[float] = None, correlation_resize: Optional[float] = None, stitch_tiff: bool = False, @@ -200,7 +204,7 @@ class FinalStitcher(BaseStitcher): All args except images_dir are positional only. :param images_dir: The images directory of the scan to stitch. - :param logger: The invocation logger for this stitch process. + :param logger: The logger from the Thing that created this stitcher. :param overlap: The scan overlap, if not known enter None. A value will be chosen from the scan_data_dict, or set to a default value if no value is available in the scan_data. @@ -281,10 +285,7 @@ class FinalStitcher(BaseStitcher): ) return overlap, correlation_resize - def run( - self, - cancel: lt.deps.CancelHook, - ) -> None: + def run(self) -> None: """Run the final stitch logging any output. :raises ChildProcessError: if exit code is not zero @@ -305,7 +306,7 @@ class FinalStitcher(BaseStitcher): # Stop opening pipe blocking writing to it os.set_blocking(process.stdout.fileno(), False) - self._log_ongoing(process, cancel=cancel) + self._log_ongoing(process) if process.poll() == 0: self.logger.info("Stitching complete") @@ -319,11 +320,7 @@ class FinalStitcher(BaseStitcher): f"Subprocess {STITCHING_CMD} errored with exit code {process.poll()}." ) - def _log_ongoing( - self, - process: subprocess.Popen, - cancel: lt.deps.CancelHook, - ) -> None: + def _log_ongoing(self, process: subprocess.Popen) -> None: """Log the ongoing process unless it is cancelled.""" # Poll returns None while running, will return the error code when finished while process.poll() is None: @@ -331,9 +328,10 @@ class FinalStitcher(BaseStitcher): # Once buffer is clear sleep for 0.2s before trying again. try: - # Note that using cancel.sleep allows the InvocationCancelledError to - # be thrown - cancel.sleep(0.2) + # This should act exactly like sleep if not started in a LabThings + # action thread. In an action thread it will raise + # InvocationCancelledError if the action is cancelled. + lt.cancellable_sleep(0.2) except lt.exceptions.InvocationCancelledError as e: self.logger.info("Stitching cancelled by user") process.kill() diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 0ee5880b..b4ed6c10 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -126,7 +126,6 @@ class SmartScanThing(lt.Thing): # any method that calls these should be decorated with # @_scan_running self._scan_logger: Optional[lt.deps.InvocationLogger] = None - self._cancel: Optional[lt.deps.CancelHook] = None self._autofocus: Optional[AutofocusDep] = None self._stage: Optional[StageDep] = None self._cam: Optional[CameraClient] = None @@ -139,7 +138,6 @@ class SmartScanThing(lt.Thing): @lt.action def sample_scan( self, - cancel: lt.deps.CancelHook, logger: lt.deps.InvocationLogger, autofocus: AutofocusDep, stage: StageDep, @@ -158,7 +156,6 @@ class SmartScanThing(lt.Thing): raise RuntimeError("Trying to run scan while scan is already running!") # Set private variables for this scan - self._cancel = cancel self._scan_logger = logger self._autofocus = autofocus self._stage = stage @@ -188,7 +185,6 @@ class SmartScanThing(lt.Thing): raise e finally: # However the scan finishes, unset all variables and release lock - self._cancel = None self._scan_logger = None self._autofocus = None self._stage = None @@ -390,8 +386,6 @@ class SmartScanThing(lt.Thing): self._save_final_scan_data(scan_result="success") except lt.exceptions.InvocationCancelledError: - # Reset the cancel event so it can be thrown again - self._cancel.clear() self._scan_logger.info("Stopping scan because it was cancelled.") self._save_final_scan_data(scan_result="cancelled by user") except Exception as e: @@ -513,13 +507,12 @@ class SmartScanThing(lt.Thing): self._scan_logger.info("Waiting for background processes to finish...") if self._preview_stitcher is not None: - self._preview_stitcher.wait(self._cancel) + self._preview_stitcher.wait() if self._scan_data.stitch_automatically: self._scan_logger.info("Stitching final image (may take some time)...") self.stitch_scan( logger=self._scan_logger, - cancel=self._cancel, scan_name=self._ongoing_scan.name, correlation_resize=self._scan_data.correlation_resize, overlap=self._scan_data.overlap, @@ -731,7 +724,6 @@ class SmartScanThing(lt.Thing): def stitch_scan( self, logger: lt.deps.InvocationLogger, - cancel: lt.deps.CancelHook, scan_name: str, correlation_resize: Optional[float] = None, overlap: Optional[float] = None, @@ -753,8 +745,7 @@ class SmartScanThing(lt.Thing): scan_data_dict=scan_data_dict, ) try: - # start the final stitch, providing the cancel hook to allow aborting - final_stitcher.run(cancel) + final_stitcher.run() except lt.exceptions.InvocationCancelledError: # Sleep for 1 second just to allow invocation logs to pass to user. time.sleep(1) @@ -774,9 +765,7 @@ class SmartScanThing(lt.Thing): return ZipBlob.from_file(zip_fname) @lt.action - def stitch_all_scans( - self, logger: lt.deps.InvocationLogger, cancel: lt.deps.CancelHook - ) -> None: + def stitch_all_scans(self, logger: lt.deps.InvocationLogger) -> None: """Check the list of scans, and stitch any that don't have a DZI associated with it. :raises RuntimeError: if the microscope is currently running a scan @@ -786,4 +775,4 @@ class SmartScanThing(lt.Thing): raise RuntimeError("Can't stitch previous scans while a scan is ongoing") for scan in self._get_all_scan_info(): if scan.dzi is None: - self.stitch_scan(logger=logger, cancel=cancel, scan_name=scan.name) + self.stitch_scan(logger=logger, scan_name=scan.name) From 8f805c8e6851450173033d23d8337dc5834d4c94 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 14:02:58 +0000 Subject: [PATCH 2/7] Add a testing environment/harness for easily running actions without blocking --- tests/utilities/lt_test_utils.py | 151 +++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 tests/utilities/lt_test_utils.py diff --git a/tests/utilities/lt_test_utils.py b/tests/utilities/lt_test_utils.py new file mode 100644 index 00000000..86565ed6 --- /dev/null +++ b/tests/utilities/lt_test_utils.py @@ -0,0 +1,151 @@ +"""Test utilities for interacting with the labthings Server.""" + +import tempfile +import time +from types import TracebackType +from typing import Any, Optional, Self + +import requests +from fastapi.testclient import TestClient + +import labthings_fastapi as lt + +ACTION_RUNNING_KEYWORDS = ["idle", "pending", "running"] + + +class LabThingsTestEnv: + """A Labthings Server running in a FastAPI Test Client. + + This can be used to create ThingClients for testing, to to enable direct access to + the server, or things on the server. It also provides high level functions to + start actions without blocking the test thread. + + Use this as a context manager: + + .. code-block:: python + + with LabThingsTestEnv(things={"counter", CountingThing}) as env: + ... + """ + + def __init__( + self, things: dict[str, lt.Thing | str], settings_folder: Optional[str] = None + ) -> None: + """Initialise the test environment. + + The server and FastAPI TestClient are server are not created until this is used + as a context manager: + + :param things: The thing configuration dictionary used to initialise the server. + :param settings_folder: The settings folder to use. + """ + self._server: Optional[lt.ThingServer] + self._test_client: Optional[TestClient] + self._thing_config = things + self._settings_folder = settings_folder + self._tmp_dir_obj: Optional[tempfile.TemporaryDirectory] = None + + def __enter__(self) -> Self: + """Create server and run it in the TestClient.""" + if self._settings_folder is None: + 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 + ) + self._test_client = TestClient(self._server.app) + self._test_client.__enter__() + return self + + def __exit__( + self, + exc_type: type[BaseException], + exc_value: Optional[BaseException], + traceback: Optional[TracebackType], + ) -> None: + """Close the TestClient and cleanup.""" + self._test_client.__exit__(exc_type, exc_value, traceback) + if self._tmp_dir_obj is not None: + self._tmp_dir_obj.cleanup() + + @property + def server(self) -> lt.ThingServer: + """The LabThings Server.""" + if self._server is None: + raise RuntimeError("No server was started.") + return self._server + + @property + def client(self) -> TestClient: + """The FastAPI TestClient running the server.""" + if self._test_client is None: + raise RuntimeError("No server was started.") + return self._test_client + + def check_thing_exists(self, thing_name: str): + """Raise a ValueError if no thing of with a matching name exists.""" + 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: + """Get a Thing from the server by name.""" + self.check_thing_exists(thing_name) + return self.server.things[thing_name] + + def get_thing_client(self, thing_name: str) -> lt.ThingClient: + """Get a ThingClient for a Thing by name.""" + self.check_thing_exists(thing_name) + thing = self.server.things[thing_name] + return lt.ThingClient.from_url(thing.path, self.client) + + def start_action( + self, + thing_name: str, + action_name: str, + action_kwargs: Optional[dict[str, Any]] = None, + ) -> requests.Response: + """Start an action and return the server response. + + This response can be used to poll or cancel the action. + """ + self.check_thing_exists(thing_name) + url = f"/{thing_name}/{action_name}" + json_payload = {} if action_kwargs is None else action_kwargs + response = self.client.post(url, json=json_payload) + response.raise_for_status() + return response + + def poll_action( + self, response: requests.Response, interval: float = 0.01 + ) -> dict[str, Any]: + """Poll an action until it completes and return the final response data.""" + invocation_data = response.json() + + if "status" not in invocation_data: + raise ValueError(f"Response json has no status: {invocation_data}") + first_run = True + while invocation_data["status"] in ACTION_RUNNING_KEYWORDS: + if first_run: + first_run = False + else: + time.sleep(interval) + response = self.client.get(_invocation_href(invocation_data)) + response.raise_for_status() + invocation_data = response.json() + return invocation_data + + def cancel_action(self, response: requests.Response) -> None: + """Cancel an ongoing action.""" + invocation_data = response.json() + response = self.client.delete(_invocation_href(invocation_data)) + response.raise_for_status() + + +def _get_link(obj: dict[str, Any], rel: str) -> dict[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: + """Get the invocation href from the invocation response data.""" + return _get_link(invocation_data, "self")["href"] From f6d4b368d00112f0a982846f6f3c5dc68b751e86 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 14:03:45 +0000 Subject: [PATCH 3/7] Update tests for not having cancel hooks --- tests/test_smart_scan.py | 5 -- tests/test_stitching.py | 111 ++++++++++++++++++++++++--------------- 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index f753250f..4220db4a 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -174,7 +174,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): function """ # cancel handle shouldn't be used. Set to arbitrary value for checking - cancel_mock = 1 # not called af_mock = MockAutoFocusThing() stage_mock = MockStageThing() cam_mock = MockCameraThing() @@ -191,7 +190,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): """Check scan vars are set up as expected""" assert not self._scan_lock.acquire(timeout=0.1) - assert self._cancel is cancel_mock assert self._scan_logger is LOGGER assert self._autofocus is af_mock assert self._stage is stage_mock @@ -209,7 +207,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): exec_info = None try: mock_ss_thing.sample_scan( - cancel=cancel_mock, # Shouldn't be used, can be checked logger=LOGGER, autofocus=af_mock, stage=stage_mock, @@ -222,7 +219,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): assert mock_ss_thing._scan_lock.acquire(timeout=0.1) mock_ss_thing._scan_lock.release() - assert mock_ss_thing._cancel is None assert mock_ss_thing._scan_logger is None assert mock_ss_thing._autofocus is None assert mock_ss_thing._stage is None @@ -342,7 +338,6 @@ def scan_thing_mocked_for_run_scan(scan_thing_mocked_for_scan_data, mocker): """ scan_thing = scan_thing_mocked_for_scan_data scan_thing._cam = MockCameraThing() - mocker.patch.object(scan_thing, "_cancel") mocker.patch.object(scan_thing, "_main_scan_loop") mocker.patch.object(scan_thing, "_return_to_starting_position") mocker.patch.object(scan_thing, "_perform_final_stitch") diff --git a/tests/test_stitching.py b/tests/test_stitching.py index 55d8df67..0c9b52e7 100644 --- a/tests/test_stitching.py +++ b/tests/test_stitching.py @@ -6,9 +6,8 @@ generated, and the subprocess calling works as expected. import logging import os -import threading +import re import time -import uuid from copy import copy import pytest @@ -23,8 +22,10 @@ from openflexure_microscope_server.stitching import ( StitcherValidationError, ) -# A global logger pretending to be an Invocation Logger -LOGGER = logging.getLogger("mock-invocation_logger") +from .utilities.lt_test_utils import LabThingsTestEnv + +# A global logger pretending to the logger from a thing +LOGGER = logging.getLogger("mock-thing_logger") FAKE_DIR: list[str] = os.path.join("a", "dir", "that", "is", "fake") THIS_DIR: str = os.path.dirname(os.path.realpath(__file__)) MOCK_STITCHER: str = os.path.join(THIS_DIR, "mock_stitching", "mock-stitch.py") @@ -200,7 +201,6 @@ def test_preview_stitching_command(caplog, mocker): mocker.patch("openflexure_microscope_server.stitching.STITCHING_CMD", mock_cmd) with caplog.at_level(logging.INFO): - cancel_hook = lt.deps.CancelHook(id=uuid.uuid4()) stitcher = PreviewStitcher(FAKE_DIR, overlap=0.1, correlation_resize=0.5) stitcher.start() # Should take a second or so to run so will still be running @@ -209,38 +209,64 @@ def test_preview_stitching_command(caplog, mocker): with pytest.raises(RuntimeError): stitcher.start() # Wait for it to complete - stitcher.wait(cancel_hook) + stitcher.wait() # It is now not running assert not stitcher.running assert len(caplog.records) == 0 -def test_preview_stitching_cancelled(caplog, mocker): +class StitchingTestThing(lt.Thing): + """A Thing for running stitching in invocation threads. + + This is needed to check cancellation behaviour. + """ + + @lt.action + def run_preview(self): + """Run the preview stitcher.""" + stitcher = PreviewStitcher(FAKE_DIR, overlap=0.1, correlation_resize=0.5) + # Send in the argument HANG to mock-stitch and it just hang for 10s + stitcher._extra_args = ["HANG"] + stitcher.start() + stitcher.wait() + + @lt.action + def run_final(self): + """Run the final stitcher.""" + stitcher = FinalStitcher(FAKE_DIR, logger=self.logger) + # Send in the argument HANG to mock-stitch and it just hang for 10s + stitcher.run() + + +@pytest.fixture +def stitching_test_env(): + """Return a test environment for a server with just StitchingTestThing.""" + with LabThingsTestEnv(things={"stitcher": StitchingTestThing}) as env: + yield env + + +def test_preview_stitching_cancelled(stitching_test_env, mocker): """Check that preview stitch can be cancelled.""" mock_cmd = f"python {MOCK_STITCHER}" mocker.patch("openflexure_microscope_server.stitching.STITCHING_CMD", mock_cmd) - with caplog.at_level(logging.INFO): - stitcher = PreviewStitcher(FAKE_DIR, overlap=0.1, correlation_resize=0.5) - # Send in the argument HANG to mock-stitch and it just hang for 10s - stitcher._extra_args = ["HANG"] - cancel_hook = lt.deps.CancelHook(id=uuid.uuid4()) - t_start = time.time() - # Start stitching - stitcher.start() - # Run wait() in a thread. - thread = threading.Thread(target=stitcher.wait, kwargs={"cancel": cancel_hook}) - thread.start() - # Sleep long enough for at least 1 log. - time.sleep(0.5) - # use set() to cancel! - cancel_hook.set() - thread.join() - # If it wasn't cancelled it would hang for 10 s. Here we check the cancel killed - # it within 2s. - assert t_start - time.time() < 2 - assert len(caplog.records) == 0 + t_start = time.time() + # Start the action + response = stitching_test_env.start_action("stitcher", "run_preview") + # Sleep long enough for at least 1 log. + time.sleep(0.5) + # Cancel using a DELETE request + stitching_test_env.cancel_action(response) + invocation_data = stitching_test_env.poll_action(response) + + # If it wasn't cancelled it would hang for 10 s. Here we check the cancel killed + # it within 2s. + assert time.time() - t_start < 2 + assert invocation_data["status"] == "cancelled" + logs = invocation_data["log"] + assert len(logs) == 1 + assert re.match(r"^Invocation [0-9a-f-]+ was cancelled", logs[0]["message"]) def test_final_stitching_command(caplog, mocker): @@ -255,7 +281,7 @@ def test_final_stitching_command(caplog, mocker): FAKE_DIR, logger=LOGGER, overlap=0.1, correlation_resize=0.5 ) # For the final stitcher it will always complete before returning. - stitcher.run(cancel=lt.deps.CancelHook(id=uuid.uuid4())) + stitcher.run() # The mock command logs the inputs (but not the initial command) and the # stitcher logs # "Stitching complete" when it ends. assert len(caplog.records) == len(FINAL_EXPECTED_COMMAND) @@ -267,26 +293,25 @@ def test_final_stitching_command(caplog, mocker): assert msg == FINAL_EXPECTED_COMMAND[i + 1] -def test_final_stitching_command_cancelled(caplog, mocker): +def test_final_stitching_command_cancelled(stitching_test_env, mocker): """Check that final stitch can be cancelled.""" mock_cmd = f"python {MOCK_STITCHER}" mocker.patch("openflexure_microscope_server.stitching.STITCHING_CMD", mock_cmd) - with caplog.at_level(logging.INFO): - stitcher = FinalStitcher(FAKE_DIR, logger=LOGGER) - cancel_hook = lt.deps.CancelHook(id=uuid.uuid4()) + # Start the action + response = stitching_test_env.start_action("stitcher", "run_final") + # Sleep long enough for at least 1 log. + time.sleep(0.5) + # Cancel using a DELETE request + stitching_test_env.cancel_action(response) + invocation_data = stitching_test_env.poll_action(response) - # Start stitching in a thread. - thread = threading.Thread(target=stitcher.run, kwargs={"cancel": cancel_hook}) - thread.start() - # Sleep long enough for at least 1 log. - time.sleep(0.5) - # use set() to cancel! - cancel_hook.set() - thread.join() - assert len(caplog.records) < len(FINAL_EXPECTED_COMMAND) + 1 - assert caplog.records[-1].message == "Stitching cancelled by user" + assert invocation_data["status"] == "cancelled" + logs = invocation_data["log"] + assert len(logs) < len(FINAL_EXPECTED_COMMAND) + 1 + assert logs[-2]["message"] == "Stitching cancelled by user" + assert re.match(r"^Invocation [0-9a-f-]+ was cancelled", logs[-1]["message"]) def test_final_stitching_command_error(caplog, mocker): @@ -301,4 +326,4 @@ def test_final_stitching_command_error(caplog, mocker): # than echo. stitcher._extra_args = ["ERROR"] with pytest.raises(ChildProcessError): - stitcher.run(cancel=lt.deps.CancelHook(id=uuid.uuid4())) + stitcher.run() From be5f9630cadf69e99aeb298f8e3befb2d4ebda05 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 15:41:22 +0000 Subject: [PATCH 4/7] A fond farewell to the last of the InvocationLoggers --- .../stitching.py | 2 +- .../things/smart_scan.py | 93 ++++++++----------- tests/test_scan_directories.py | 3 - tests/test_smart_scan.py | 52 +++++------ tests/test_stage_measure.py | 2 - 5 files changed, 64 insertions(+), 88 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index e87afe1f..5a984bed 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -244,7 +244,7 @@ class FinalStitcher(BaseStitcher): First the scan_data_dict is inspected for values to allow ``overlap`` and ``correlation_resize`` to be set correctly, if these values are not available - then default values are used, and a warning is logged to the invocation logger. + then default values are used, and a warning is logged to the thing logger. :param overlap: overlap as input to __init__ :param correlation_resize: correlation_resize as input to __init__ diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index b4ed6c10..0a0f88a0 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -80,13 +80,13 @@ def _scan_running( This decorator is used by all methods in SmartScanThing that are using the variables set for the scan. It will throw a runtime error if - self._scan_logger is not set, as all scan variables are set at + self._scan_lock is not locked, as all scan variables are set at the same time and released with the lock """ def scan_running_wrapper(self: Self, *args: P.args, **kwargs: P.kwargs) -> T: - # Only start the method is the scan logger is set - if self._scan_logger is not None: + """Only start the requested method if the scan is running.""" + if self._scan_lock.locked(): return method(self, *args, **kwargs) raise ScanNotRunningError( "Calling a @scan_running method can only be done while a scan is running!" @@ -119,13 +119,6 @@ class SmartScanThing(lt.Thing): # Variables set by the scan self._latest_scan_name: Optional[str] = None - # Scan logger is the invocation logger labthings-fastapi creates - # when the `sample_scan` lt.action is called. It is saved as - # private class variable along with many others here. - # Access to these variables requires a scan to be running, - # any method that calls these should be decorated with - # @_scan_running - self._scan_logger: Optional[lt.deps.InvocationLogger] = None self._autofocus: Optional[AutofocusDep] = None self._stage: Optional[StageDep] = None self._cam: Optional[CameraClient] = None @@ -138,7 +131,6 @@ class SmartScanThing(lt.Thing): @lt.action def sample_scan( self, - logger: lt.deps.InvocationLogger, autofocus: AutofocusDep, stage: StageDep, cam: CameraClient, @@ -156,7 +148,6 @@ class SmartScanThing(lt.Thing): raise RuntimeError("Trying to run scan while scan is already running!") # Set private variables for this scan - self._scan_logger = logger self._autofocus = autofocus self._stage = stage self._cam = cam @@ -177,7 +168,7 @@ class SmartScanThing(lt.Thing): self._return_to_starting_position() if not isinstance(e, scan_directories.NotEnoughFreeSpaceError): # Don't stitch if drive is full (already logged) - self._scan_logger.info( + self.logger.info( "Attempting to stitch and archive the images acquired so far." ) self._perform_final_stitch() @@ -185,7 +176,6 @@ class SmartScanThing(lt.Thing): raise e finally: # However the scan finishes, unset all variables and release lock - self._scan_logger = None self._autofocus = None self._stage = None self._cam = None @@ -198,7 +188,7 @@ class SmartScanThing(lt.Thing): self._stack_params = None # Remove any scan folders containing zero images. - self.purge_empty_scans(logger=logger) + self.purge_empty_scans() @_scan_running def _check_background_and_csm_set(self) -> None: @@ -222,7 +212,7 @@ class SmartScanThing(lt.Thing): "Background is not set: you need to calibrate background detection." ) else: - self._scan_logger.warning( + self.logger.warning( "This scan will run in a spiral from the starting point " f"until you cancel it, or until it has moved by {self.max_range} steps " "in every direction. Make sure you watch it run to stop it leaving " @@ -250,7 +240,7 @@ class SmartScanThing(lt.Thing): if z_estimate is None: z_estimate = self._stage.position["z"] - self._scan_logger.info(f"Moving to {next_point}") + self.logger.info(f"Moving to {next_point}") self._stage.move_absolute( x=next_point[0], y=next_point[1], @@ -305,19 +295,19 @@ class SmartScanThing(lt.Thing): dx, dy = self._calc_displacement_from_test_image(overlap) correlation_resize = stitching.STITCHING_RESOLUTION[0] / self.save_resolution[0] - self._scan_logger.debug( + self.logger.debug( f"Resizing images when correlating by a factor of {correlation_resize}" ) - self._scan_logger.info( + self.logger.info( f"Based on an overlap of {overlap}, we will make steps of {dx}, {dy}" ) autofocus_dz = self.autofocus_dz if autofocus_dz == 0: - self._scan_logger.info("Running scan without autofocus") + self.logger.info("Running scan without autofocus") elif autofocus_dz <= 200: - self._scan_logger.warning( + self.logger.warning( f"Your autofocus range is {autofocus_dz} steps, which is too short to " "attempt to focus. Running without autofocus" ) @@ -386,13 +376,13 @@ class SmartScanThing(lt.Thing): self._save_final_scan_data(scan_result="success") except lt.exceptions.InvocationCancelledError: - self._scan_logger.info("Stopping scan because it was cancelled.") + self.logger.info("Stopping scan because it was cancelled.") self._save_final_scan_data(scan_result="cancelled by user") except Exception as e: err_name = type(e).__name__ if self._scan_data is not None: self._save_final_scan_data(scan_result=f"{err_name}: {e}") - self._scan_logger.error( + self.logger.error( f"The scan stopped because of an error: {e}", exc_info=e, ) @@ -457,7 +447,7 @@ class SmartScanThing(lt.Thing): new_pos_xyz, imaged=False, focused=False ) msg = f"Skipping {new_pos_xyz} as it is {bg_message}." - self._scan_logger.info(msg) + self.logger.info(msg) continue focused, focused_height = self._autofocus.run_smart_stack( @@ -482,7 +472,7 @@ class SmartScanThing(lt.Thing): @_scan_running def _return_to_starting_position(self) -> None: """Return to the initial scan position, if set.""" - self._scan_logger.info("Returning to starting position.") + self.logger.info("Returning to starting position.") if self._scan_data is not None: self._stage.move_absolute( **self._scan_data.starting_position, block_cancellation=True @@ -499,20 +489,19 @@ class SmartScanThing(lt.Thing): def _perform_final_stitch(self) -> None: """Update the scan zip and perform final stitch of the data.""" if self._scan_data.image_count <= 3: - self._scan_logger.info("Not performing a stitch as 3 or fewer images taken") + self.logger.info("Not performing a stitch as 3 or fewer images taken") return self._ongoing_scan.zip_files() - self._scan_logger.info("Waiting for background processes to finish...") + self.logger.info("Waiting for background processes to finish...") if self._preview_stitcher is not None: self._preview_stitcher.wait() if self._scan_data.stitch_automatically: - self._scan_logger.info("Stitching final image (may take some time)...") + self.logger.info("Stitching final image (may take some time)...") self.stitch_scan( - logger=self._scan_logger, scan_name=self._ongoing_scan.name, correlation_resize=self._scan_data.correlation_resize, overlap=self._scan_data.overlap, @@ -621,17 +610,17 @@ class SmartScanThing(lt.Thing): 400: {"description": "An error occurred while trying to delete scan"}, }, ) - def delete_scan(self, scan_name: str, logger: lt.deps.InvocationLogger) -> None: + def delete_scan(self, scan_name: str) -> None: """Delete the folder for the specified scan. This endpoint allows scans to be deleted from disk. - Takes the scan name to delete, and the Invocation Logger + :param scan_name: The name of the scan to delete """ if not self._scan_dir_manager.exists(scan_name): - logger.warning(f"Cannot find a scan of name {scan_name}") + self.logger.warning(f"Cannot find a scan of name {scan_name}") raise HTTPException(400, "Scan not found") - deleted_scan_success = self._delete_scan(scan_name, logger) + deleted_scan_success = self._delete_scan(scan_name) if not deleted_scan_success: raise HTTPException(400, "Couldn't delete scan, check log for details") @@ -639,7 +628,7 @@ class SmartScanThing(lt.Thing): "delete", "scans", ) - def delete_all_scans(self, logger: lt.deps.InvocationLogger) -> None: + def delete_all_scans(self) -> None: """Delete all the scans on the microscope. **This will irreversibly remove all scanned data from the @@ -647,30 +636,30 @@ class SmartScanThing(lt.Thing): Use with extreme caution. """ for scan_name in self._scan_dir_manager.all_scans: - self._delete_scan(scan_name, logger) + self._delete_scan(scan_name) @lt.action - def purge_empty_scans(self, logger: lt.deps.InvocationLogger) -> None: + def purge_empty_scans(self) -> None: """Delete all scan folders containing no images at the top level.""" # JSON is ignored as it's created before any images are captured for scan_info in self._get_all_scan_info(): if scan_info.number_of_images == 0: - self._delete_scan(scan_info.name, logger) + self._delete_scan(scan_info.name) - def _delete_scan(self, scan_name: str, logger: lt.deps.InvocationLogger) -> bool: + def _delete_scan(self, scan_name: str) -> bool: """Delete a scan. This is a wrapper around scan manager's delete_scan that logs to the - invocation logger id there is a problem. + things logger if there is a problem. """ if self._ongoing_scan is not None and scan_name == self._ongoing_scan.name: - logger.error("Attempted to delete ongoing scan.") + self.logger.error("Attempted to delete ongoing scan.") return False try: self._scan_dir_manager.delete_scan(scan_name) return True except Exception as e: - logger.warning( + self.logger.warning( "Attempted to delete scan " + scan_name + ", which failed." " Server sent response" + str(e) ) @@ -723,22 +712,19 @@ class SmartScanThing(lt.Thing): @lt.action def stitch_scan( self, - logger: lt.deps.InvocationLogger, scan_name: str, correlation_resize: Optional[float] = None, overlap: Optional[float] = None, ) -> None: - """Generate a stitched image based on stage position metadata. - - Note that as this is a lt.action it needs the logger passed as - a variable if called from another thing action - """ + """Generate a stitched image based on stage position metadata.""" scan_data_dict = self._scan_dir_manager.get_scan_data_dict(scan_name) if scan_data_dict is None: - logger.warning("Couldn't read scan data - it may be missing or corrupt.") + self.logger.warning( + "Couldn't read scan data - it may be missing or corrupt." + ) final_stitcher = stitching.FinalStitcher( self._scan_dir_manager.img_dir_for(scan_name), - logger=logger, + logger=self.logger, overlap=overlap, correlation_resize=correlation_resize, stitch_tiff=self.stitch_tiff, @@ -750,7 +736,7 @@ class SmartScanThing(lt.Thing): # Sleep for 1 second just to allow invocation logs to pass to user. time.sleep(1) except SubprocessError as e: - self._scan_logger.error(f"Stitching failed: {e}", exc_info=e) + self.logger.error(f"Stitching failed: {e}", exc_info=e) @lt.action def download_zip( @@ -765,14 +751,13 @@ class SmartScanThing(lt.Thing): return ZipBlob.from_file(zip_fname) @lt.action - def stitch_all_scans(self, logger: lt.deps.InvocationLogger) -> None: + def stitch_all_scans(self) -> None: """Check the list of scans, and stitch any that don't have a DZI associated with it. :raises RuntimeError: if the microscope is currently running a scan - """ - if self._scan_logger is not None: + if self._scan_lock.locked(): raise RuntimeError("Can't stitch previous scans while a scan is ongoing") for scan in self._get_all_scan_info(): if scan.dzi is None: - self.stitch_scan(logger=logger, scan_name=scan.name) + self.stitch_scan(scan_name=scan.name) diff --git a/tests/test_scan_directories.py b/tests/test_scan_directories.py index 13336b11..ff7af3eb 100644 --- a/tests/test_scan_directories.py +++ b/tests/test_scan_directories.py @@ -24,9 +24,6 @@ from openflexure_microscope_server.scan_directories import ( from .test_scan_data import _fake_scan_data from .utilities import assert_unique_of_length -# A global logger to pass in as an Invocation Logger -LOGGER = logging.getLogger("mock-invocation_logger") - # Use our own dir in the root temp dir not a dynamically generated one so we # have some control of when it is deleted BASE_SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index 4220db4a..15cd5789 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -40,9 +40,6 @@ from .mock_things.mock_camera import MockCameraThing from .mock_things.mock_csm import MockCSMThing from .mock_things.mock_stage import MockStageThing -# A global logger to pass in as an Invocation Logger -LOGGER = logging.getLogger("mock-invocation_logger") - # Use our own dir in the root temp dir not a dynamically generated one so we # have some control of when it is deleted SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") @@ -92,16 +89,16 @@ def test_private_delete_scan(smart_scan_thing, caplog): # Make the outer scan dir, but not the one to delete os.makedirs(SCAN_DIR) # Attempt to delete the fake scan. Expect it to fail and provide a warning - deleted = smart_scan_thing._delete_scan(fake_scan_name, LOGGER) + deleted = smart_scan_thing._delete_scan(fake_scan_name) assert not deleted assert len(caplog.records) == 1 assert caplog.records[0].levelname == "WARNING" - assert caplog.records[0].name == "mock-invocation_logger" + assert caplog.records[0].name == "labthings_fastapi.things.smartscanthing" # Make a dir for the fake scan and delete it. os.makedirs(fake_scan_path) assert os.path.exists(fake_scan_path) - deleted = smart_scan_thing._delete_scan(fake_scan_name, LOGGER) + deleted = smart_scan_thing._delete_scan(fake_scan_name) assert not os.path.exists(fake_scan_path) assert deleted # Check no extra logs generated @@ -120,18 +117,18 @@ def test_public_delete_scan(smart_scan_thing, caplog): # Attempt to delete the fake scan. Expect it to fail with pytest.raises(HTTPException) as exc_info: - smart_scan_thing.delete_scan(fake_scan_name, LOGGER) + smart_scan_thing.delete_scan(fake_scan_name) # Should raise a 400 error if the scan doesn't exist, not a 404 as the server # was not expecting to receive the scan files assert exc_info.value.status_code == 400 assert len(caplog.records) == 1 assert caplog.records[0].levelname == "WARNING" - assert caplog.records[0].name == "mock-invocation_logger" + assert caplog.records[0].name == "labthings_fastapi.things.smartscanthing" # Make a dir for the fake scan and delete it. os.makedirs(fake_scan_path) assert os.path.exists(fake_scan_path) - smart_scan_thing.delete_scan(fake_scan_name, LOGGER) + smart_scan_thing.delete_scan(fake_scan_name) assert not os.path.exists(fake_scan_path) # Check no extra logs generated assert len(caplog.records) == 1 @@ -151,7 +148,7 @@ def test_delete_all_scans(smart_scan_thing, caplog): fake_scan_path = os.path.join(SCAN_DIR, fake_scan_name) os.makedirs(fake_scan_path) assert os.path.exists(fake_scan_path) - smart_scan_thing.delete_all_scans(LOGGER) + smart_scan_thing.delete_all_scans() for fake_scan_name in fake_scan_names: fake_scan_path = os.path.join(SCAN_DIR, fake_scan_name) assert not os.path.exists(fake_scan_path) @@ -190,7 +187,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): """Check scan vars are set up as expected""" assert not self._scan_lock.acquire(timeout=0.1) - assert self._scan_logger is LOGGER assert self._autofocus is af_mock assert self._stage is stage_mock assert self._cam is cam_mock @@ -207,7 +203,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): exec_info = None try: mock_ss_thing.sample_scan( - logger=LOGGER, autofocus=af_mock, stage=stage_mock, cam=cam_mock, @@ -219,7 +214,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): assert mock_ss_thing._scan_lock.acquire(timeout=0.1) mock_ss_thing._scan_lock.release() - assert mock_ss_thing._scan_logger is None assert mock_ss_thing._autofocus is None assert mock_ss_thing._stage is None assert mock_ss_thing._cam is None @@ -280,23 +274,25 @@ def _expected_scan_data(): @pytest.fixture def scan_thing_mocked_for_scan_data(smart_scan_thing, mocker): """Return a scan thing that is mocked so that _collect_scan_data will run.""" - # Give the scan thing a scan invocation logger so it thinks a scan is running. - smart_scan_thing._scan_logger = LOGGER - mocker.patch.object( - smart_scan_thing, "_calc_displacement_from_test_image", return_value=[100, 100] - ) - mock_ongoing_scan = mocker.Mock() - type(mock_ongoing_scan).name = mocker.PropertyMock(return_value=MOCK_SCAN_NAME) - type(mock_ongoing_scan).images_dir = mocker.PropertyMock(return_value=MOCK_SCAN_DIR) - mock_stage = mocker.Mock() - type(mock_stage).position = mocker.PropertyMock(return_value=MOCK_START_POS) + # Set the lock so it thinks the scan is running + with smart_scan_thing._scan_lock: + mocker.patch.object( + smart_scan_thing, + "_calc_displacement_from_test_image", + return_value=[100, 100], + ) + mock_ongoing_scan = mocker.Mock() + mock_ongoing_scan.name = MOCK_SCAN_NAME + mock_ongoing_scan.images_dir = MOCK_SCAN_DIR + mock_stage = mocker.Mock() + mock_stage.position = MOCK_START_POS - mock_autofocus = mocker.Mock() + mock_autofocus = mocker.Mock() - smart_scan_thing._ongoing_scan = mock_ongoing_scan - smart_scan_thing._stage = mock_stage - smart_scan_thing._autofocus = mock_autofocus - return smart_scan_thing + smart_scan_thing._ongoing_scan = mock_ongoing_scan + smart_scan_thing._stage = mock_stage + smart_scan_thing._autofocus = mock_autofocus + yield smart_scan_thing def test_collect_scan_data(scan_thing_mocked_for_scan_data): diff --git a/tests/test_stage_measure.py b/tests/test_stage_measure.py index 6412771a..5c9e0690 100644 --- a/tests/test_stage_measure.py +++ b/tests/test_stage_measure.py @@ -14,8 +14,6 @@ from openflexure_microscope_server.things.camera_stage_mapping import ( csm_stage_to_img, ) -LOGGER = logging.getLogger("mock-invocation_logger") - # Useful generators def increasing_xy_dict_generator(*_args, **_kwargs): From 4ad1fff65642f1a813242b8926b55ce169453595 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 15:50:03 +0000 Subject: [PATCH 5/7] Remove final dependencies --- .../things/camera/__init__.py | 4 --- .../things/smart_scan.py | 36 +++++-------------- .../things/stage/__init__.py | 3 -- 3 files changed, 8 insertions(+), 35 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 6171f081..18855ec2 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -683,10 +683,6 @@ class BaseCamera(lt.Thing): return {} -CameraDependency = lt.deps.direct_thing_client_dependency(BaseCamera, "camera") -RawCameraDependency = lt.deps.raw_thing_dependency(BaseCamera) - - def downsample(factor: int, image: np.ndarray) -> np.ndarray: """Downsample an image by taking the mean of each nxn region. diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 0a0f88a0..340726d3 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -34,18 +34,13 @@ from openflexure_microscope_server import scan_directories, scan_planners, stitc # Things from .autofocus import AutofocusThing, StackParams -from .camera import CameraDependency as CameraClient +from .camera import BaseCamera from .camera_stage_mapping import CameraStageMapper -from .stage import StageDependency as StageDep +from .stage import BaseStage T = TypeVar("T") P = ParamSpec("P") -CSMDep = lt.deps.direct_thing_client_dependency( - CameraStageMapper, "camera_stage_mapping" -) -AutofocusDep = lt.deps.direct_thing_client_dependency(AutofocusThing, "autofocus") - class ScanListInfo(BaseModel): """The information to be sent to the Scan List tab.""" @@ -103,6 +98,11 @@ class SmartScanThing(lt.Thing): past scans. """ + _autofocus: AutofocusThing = lt.thing_slot() + _cam: BaseCamera = lt.thing_slot() + _csm: CameraStageMapper = lt.thing_slot() + _stage: BaseStage = lt.thing_slot() + def __init__( self, thing_server_interface: lt.ThingServerInterface, scans_folder: str ) -> None: @@ -119,24 +119,13 @@ class SmartScanThing(lt.Thing): # Variables set by the scan self._latest_scan_name: Optional[str] = None - self._autofocus: Optional[AutofocusDep] = None - self._stage: Optional[StageDep] = None - self._cam: Optional[CameraClient] = None - self._csm: Optional[CSMDep] = None self._stack_params: Optional[StackParams] = None self._ongoing_scan: Optional[scan_directories.ScanDirectory] = None self._scan_data: Optional[scan_directories.ScanData] = None self._preview_stitcher: Optional[stitching.PreviewStitcher] = None @lt.action - def sample_scan( - self, - autofocus: AutofocusDep, - stage: StageDep, - cam: CameraClient, - csm: CSMDep, - scan_name: str = "", - ) -> None: + def sample_scan(self, scan_name: str = "") -> None: """Move the stage to cover an area, taking images that can be tiled together. The stage will move in a pattern that grows outwards from the starting point, @@ -147,11 +136,6 @@ class SmartScanThing(lt.Thing): if not got_lock: raise RuntimeError("Trying to run scan while scan is already running!") - # Set private variables for this scan - self._autofocus = autofocus - self._stage = stage - self._cam = cam - self._csm = csm # `scan_data` should already be None. This is added as a precaution as # the presence of `scan_data` is used during error handling to # determine whether the scan started. @@ -176,10 +160,6 @@ class SmartScanThing(lt.Thing): raise e finally: # However the scan finishes, unset all variables and release lock - self._autofocus = None - self._stage = None - self._cam = None - self._csm = None self._ongoing_scan = None self._scan_data = None self._scan_lock.release() diff --git a/src/openflexure_microscope_server/things/stage/__init__.py b/src/openflexure_microscope_server/things/stage/__init__.py index 86733f0a..b8de8e19 100644 --- a/src/openflexure_microscope_server/things/stage/__init__.py +++ b/src/openflexure_microscope_server/things/stage/__init__.py @@ -206,6 +206,3 @@ class BaseStage(lt.Thing): This method provides the interface expected by the camera_stage_mapping. """ self.move_absolute(x=xyz_pos[0], y=xyz_pos[1], z=xyz_pos[2]) - - -StageDependency = lt.deps.direct_thing_client_dependency(BaseStage, "stage") From 3ea7be6531d535ede92c37d59ed0053440fa0b94 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 16:44:54 +0000 Subject: [PATCH 6/7] Update and simplify smart scan tests --- tests/mock_things/__init__.py | 5 -- tests/mock_things/mock_autofocus.py | 24 -------- tests/mock_things/mock_camera.py | 36 ------------ tests/mock_things/mock_csm.py | 23 -------- tests/mock_things/mock_stage.py | 19 ------- tests/test_smart_scan.py | 87 +++++++++-------------------- 6 files changed, 27 insertions(+), 167 deletions(-) delete mode 100644 tests/mock_things/__init__.py delete mode 100644 tests/mock_things/mock_autofocus.py delete mode 100644 tests/mock_things/mock_camera.py delete mode 100644 tests/mock_things/mock_csm.py delete mode 100644 tests/mock_things/mock_stage.py diff --git a/tests/mock_things/__init__.py b/tests/mock_things/__init__.py deleted file mode 100644 index c65a6219..00000000 --- a/tests/mock_things/__init__.py +++ /dev/null @@ -1,5 +0,0 @@ -"""A package of test module providing mock Things for testing. - -The mock things do not subclass the original things to minimise the inflation of -test coverage. -""" diff --git a/tests/mock_things/mock_autofocus.py b/tests/mock_things/mock_autofocus.py deleted file mode 100644 index cf7de4f3..00000000 --- a/tests/mock_things/mock_autofocus.py +++ /dev/null @@ -1,24 +0,0 @@ -"""Testing submodule with mock Autofocus Things. - -These mocks are designed to be inserted as dependencies to provide specific -functionality and return values. - -The mocks do not subclass Things. Instead, they return predefined -answers to functions. -""" - - -class MockAutoFocusThing: - """A mock autofocus Thing that imports no code from AutofocusThing. - - The class needs functionality added to it over time as more complex - mocking is needed. It imports no code from AutofocusThing so that coverage - is not artificially inflated. - """ - - # Counter for checking functions were called - mock_call_count = {"looping_autofocus": 0} - - def looping_autofocus(self, dz: int = 2000, start: str = "centre") -> None: # noqa: ARG002 - """Mock autofocus with no return.""" - self.mock_call_count["looping_autofocus"] += 1 diff --git a/tests/mock_things/mock_camera.py b/tests/mock_things/mock_camera.py deleted file mode 100644 index 2b67ed78..00000000 --- a/tests/mock_things/mock_camera.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Testing submodule with mock Camera Things. - -These mocks are designed to be inserted as dependencies to provide specific -functionality and return values. - -The mocks do not subclass Things. Instead, they return predefined -answers to functions. -""" - -from unittest.mock import Mock, PropertyMock - -from openflexure_microscope_server.background_detect import ( - BackgroundDetectorStatus, - ColourChannelDetectSettings, -) - - -class MockCameraThing(Mock): - """A mock camera Thing that imports no code from ``BaseCamera``. - - The class needs functionality added to it over time as more complex - mocking is needed. It imports no code from ``BaseCamera`` or any other - camera Thing, so that coverage is not artificially inflated. - """ - - def __init__(self, *args, **kwargs): - """Initialise the mock camera, args and kwargs are the mock args and kwargs.""" - super().__init__(*args, **kwargs) - - self.background_detector_status = PropertyMock( - return_value=BackgroundDetectorStatus( - ready=True, - settings=ColourChannelDetectSettings().model_dump(), - settings_schema={"fake": "schema"}, - ) - ) diff --git a/tests/mock_things/mock_csm.py b/tests/mock_things/mock_csm.py deleted file mode 100644 index 4daa0d57..00000000 --- a/tests/mock_things/mock_csm.py +++ /dev/null @@ -1,23 +0,0 @@ -"""Testing submodule with mock Camera Stage Mapping Things. - -These mocks are designed to be inserted as dependencies to provide specific -functionality and return values. - -The mocks do not subclass Things. Instead, they return predefined -answers to functions. -""" - - -class MockCSMThing: - """A mock CSM Thing that imports no code from CameraStageMapper. - - The class needs functionality added to it over time as more complex - mocking is needed. It imports no code from CameraStageMapper so that coverage - is not artificially inflated. - """ - - image_resolution = (123, 456) - image_to_stage_displacement_matrix = [ - [0.03061156624485296, 1.8031242270940833], - [1.773236372778601, 0.006660431608601435], - ] diff --git a/tests/mock_things/mock_stage.py b/tests/mock_things/mock_stage.py deleted file mode 100644 index 6733986a..00000000 --- a/tests/mock_things/mock_stage.py +++ /dev/null @@ -1,19 +0,0 @@ -"""Testing submodule with mocks StageThings. - -These mocks are designed to be inserted as dependencies to provide specific -functionality and return values. - -The mocks do not subclass existing Things. Instead, they return predefined -answers to functions. -""" - - -class MockStageThing: - """A mock Thing for a stage that imports no code from BaseStage. - - The class needs functionality added to it over time as more complex - mocking is needed. It imports no code from BaseStage so that coverage - is not artificially inflated. - """ - - position = {"x": 3635, "y": 10, "z": 617} diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index 15cd5789..4a59e47e 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -35,11 +35,6 @@ from openflexure_microscope_server.things.smart_scan import ( SmartScanThing, ) -from .mock_things.mock_autofocus import MockAutoFocusThing -from .mock_things.mock_camera import MockCameraThing -from .mock_things.mock_csm import MockCSMThing -from .mock_things.mock_stage import MockStageThing - # Use our own dir in the root temp dir not a dynamically generated one so we # have some control of when it is deleted SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") @@ -54,7 +49,9 @@ def _clear_scan_dir() -> None: @pytest.fixture def smart_scan_thing(): """Return a smart scan thing as a fixture.""" - return create_thing_without_server(SmartScanThing, scans_folder=SCAN_DIR) + return create_thing_without_server( + SmartScanThing, scans_folder=SCAN_DIR, mock_all_slots=True + ) def test_initial_properties(smart_scan_thing): @@ -156,7 +153,9 @@ def test_delete_all_scans(smart_scan_thing, caplog): assert len(caplog.records) == 0 -def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): +def _run_only_outer_scan( + smart_scan_thing, mocker, adjust_initial_state: Optional[Callable] = None +): """Create a subclass of SmartScanThing to mock _run_scan and run sample_scan. This should do all the set up for a scan, move into the mocked @@ -170,69 +169,39 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): This seems hard to do with a fixture so it is being done with a private function """ - # cancel handle shouldn't be used. Set to arbitrary value for checking - af_mock = MockAutoFocusThing() - stage_mock = MockStageThing() - cam_mock = MockCameraThing() - csm_mock = MockCSMThing() - class MockedSmartScanThing(SmartScanThing): - """Mocked version of SmartScanThing with a patched _run_scan method.""" + def check_locked(*_args, **_kwargs): + """Check the scan is locked.""" + assert smart_scan_thing._scan_lock.locked() - # Counter for checking functions were called - mock_call_count = {"_run_scan": 0} - - def _run_scan(self): - self.mock_call_count["_run_scan"] += 1 - - """Check scan vars are set up as expected""" - assert not self._scan_lock.acquire(timeout=0.1) - assert self._autofocus is af_mock - assert self._stage is stage_mock - assert self._cam is cam_mock - assert self._csm is csm_mock - - # mock smart scan thing - mock_ss_thing = create_thing_without_server( - MockedSmartScanThing, scans_folder=SCAN_DIR - ) + mocker.patch.object(smart_scan_thing, "_run_scan", side_effect=check_locked) if adjust_initial_state is not None: - adjust_initial_state(mock_ss_thing) + adjust_initial_state(smart_scan_thing) exec_info = None try: - mock_ss_thing.sample_scan( - autofocus=af_mock, - stage=stage_mock, - cam=cam_mock, - csm=csm_mock, - scan_name="FooBar", - ) + smart_scan_thing.sample_scan(scan_name="FooBar") + except Exception as e: exec_info = e - assert mock_ss_thing._scan_lock.acquire(timeout=0.1) - mock_ss_thing._scan_lock.release() - assert mock_ss_thing._autofocus is None - assert mock_ss_thing._stage is None - assert mock_ss_thing._cam is None - assert mock_ss_thing._csm is None + assert not smart_scan_thing._scan_lock.locked() # Return the mock thing for further state testing, and the # exec_info of any uncaught exceptions that were raised - return mock_ss_thing, exec_info + return smart_scan_thing, exec_info -def test_outer_scan(): +def test_outer_scan(smart_scan_thing, mocker): """Test setup and teardown of the scan.""" - mock_ss_thing, exec_info = _run_only_outer_scan() + mock_ss_thing, exec_info = _run_only_outer_scan(smart_scan_thing, mocker) assert exec_info is None # Checked the mocked _run_scan was run exactly once - assert mock_ss_thing.mock_call_count["_run_scan"] == 1 + assert mock_ss_thing._run_scan.call_count == 1 -def test_outer_scan_wo_sample_skip(): +def test_outer_scan_wo_sample_skip(smart_scan_thing, mocker): """Test setup and teardown of the scan.""" def _set_skip_background(mock_ss_thing): @@ -240,11 +209,13 @@ def test_outer_scan_wo_sample_skip(): # __dict__ to avoid triggering property emits that require a server mock_ss_thing.__dict__["skip_background"] = False - mock_ss_thing, exec_info = _run_only_outer_scan(_set_skip_background) + mock_ss_thing, exec_info = _run_only_outer_scan( + smart_scan_thing, mocker, _set_skip_background + ) assert exec_info is None # Checked the mocked _run_scan was run exactly once - assert mock_ss_thing.mock_call_count["_run_scan"] == 1 + assert mock_ss_thing._run_scan.call_count == 1 MOCK_SCAN_NAME = "test_name_0001" @@ -281,17 +252,14 @@ def scan_thing_mocked_for_scan_data(smart_scan_thing, mocker): "_calc_displacement_from_test_image", return_value=[100, 100], ) + + smart_scan_thing._stage.position = MOCK_START_POS + mock_ongoing_scan = mocker.Mock() mock_ongoing_scan.name = MOCK_SCAN_NAME mock_ongoing_scan.images_dir = MOCK_SCAN_DIR - mock_stage = mocker.Mock() - mock_stage.position = MOCK_START_POS - - mock_autofocus = mocker.Mock() - smart_scan_thing._ongoing_scan = mock_ongoing_scan - smart_scan_thing._stage = mock_stage - smart_scan_thing._autofocus = mock_autofocus + yield smart_scan_thing @@ -333,7 +301,6 @@ def scan_thing_mocked_for_run_scan(scan_thing_mocked_for_scan_data, mocker): Mocks and _cam is a MockCameraThing """ scan_thing = scan_thing_mocked_for_scan_data - scan_thing._cam = MockCameraThing() mocker.patch.object(scan_thing, "_main_scan_loop") mocker.patch.object(scan_thing, "_return_to_starting_position") mocker.patch.object(scan_thing, "_perform_final_stitch") From c1a5e351301fb4c7584cf6f6a8e848ba8e5f03f8 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 17 Dec 2025 17:11:23 +0000 Subject: [PATCH 7/7] 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 187414177c106fb662468766500a8624fa55c1cf..cca9a9682719197518e6577a47aa5b6d598b0147 100644 GIT binary patch delta 215 zcmbQXjCtBJW}yIYW)=|!5YX+M82#8T=fFlGl?I_C2L7M?hxvE&$MQYkTf$e$m$cbY zz>9D4#J=C`F)XZ%oQ)QfC-;XjdT)N+@9)3?(wn(h(qOXFg)|LA3ky@z)MO(|3*%(t z)HEY=6Jvu!%fyuAR7;~o0}D$NOH0#a69bdY7cSH@+8LP{nWiQsnWmYTq?#lfC7Kzf urkSUuq^6jsnV1`ySsEA`r(}`r$h$; delta 216 zcmbQXjCtBJW}yIYW)=|!5O6D<5S>f&VA}VgB9xF?{#=7W0+xC2n>U z@Z_63sqZ&?EDI|mXQS2RDg9wgr&%_??)P`#0BN3cB5A^8rweHsW`^d5X$BUC$%bYI zCP_(YNfycGiRQ*8#)hV8re>yTrpBq3=BWm0n=f3bXS6d*Nli0IG%+ztF-l4{Gc-0d vFfcMsO*Ar2Gd4B0v@l3bF;6oyGf7ga4e(}U5@AMl?d0A|76{wVUh)J0uPj82