Merge branch 'final-dependency-removals' into 'v3'

Final dependency removals

See merge request openflexure/openflexure-microscope-server!453

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.
@@ -240,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__
@@ -281,10 +285,7 @@ class FinalStitcher(BaseStitcher):
                 )
         return overlap, correlation_resize
 
-    def run(
+    def run(self) -> None:
-        self,
-        cancel: lt.deps.CancelHook,
-    ) -> 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(
+    def _log_ongoing(self, process: subprocess.Popen) -> None:
-        self,
-        process: subprocess.Popen,
-        cancel: lt.deps.CancelHook,
-    ) -> 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
+                # This should act exactly like sleep if not started in a LabThings
-                # be thrown
+                # action thread. In an action thread it will raise
-                cancel.sleep(0.2)
+                # 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()

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.
 

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."""
@@ -80,13 +75,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
+        """Only start the requested method if the scan is running."""
-        if self._scan_logger is not None:
+        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!"
@@ -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,34 +119,13 @@ 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._cancel: Optional[lt.deps.CancelHook] = 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(
+    def sample_scan(self, scan_name: str = "") -> None:
-        self,
-        cancel: lt.deps.CancelHook,
-        logger: lt.deps.InvocationLogger,
-        autofocus: AutofocusDep,
-        stage: StageDep,
-        cam: CameraClient,
-        csm: CSMDep,
-        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,
@@ -157,13 +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._cancel = cancel
-        self._scan_logger = logger
-        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.
@@ -180,7 +152,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()
@@ -188,12 +160,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
-            self._cam = None
-            self._csm = None
             self._ongoing_scan = None
             self._scan_data = None
             self._scan_lock.release()
@@ -202,7 +168,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:
@@ -226,7 +192,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 "
@@ -254,7 +220,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],
@@ -309,19 +275,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"
             )
@@ -390,15 +356,13 @@ 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.logger.info("Stopping scan because it was cancelled.")
-            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:
             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,
             )
@@ -463,7 +427,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(
@@ -488,7 +452,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
@@ -505,21 +469,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(self._cancel)
+            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,
-                cancel=self._cancel,
                 scan_name=self._ongoing_scan.name,
                 correlation_resize=self._scan_data.correlation_resize,
                 overlap=self._scan_data.overlap,
@@ -628,17 +590,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")
 
@@ -646,7 +608,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
@@ -654,30 +616,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)
             )
@@ -730,36 +692,31 @@ class SmartScanThing(lt.Thing):
     @lt.action
     def stitch_scan(
         self,
-        logger: lt.deps.InvocationLogger,
-        cancel: lt.deps.CancelHook,
         scan_name: str,
         correlation_resize: Optional[float] = None,
         overlap: Optional[float] = None,
     ) -> None:
-        """Generate a stitched image based on stage position metadata.
+        """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
-        """
         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,
             scan_data_dict=scan_data_dict,
         )
         try:
-            # start the final stitch, providing the cancel hook to allow aborting
+            final_stitcher.run()
-            final_stitcher.run(cancel)
         except lt.exceptions.InvocationCancelledError:
             # 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(
@@ -774,16 +731,13 @@ class SmartScanThing(lt.Thing):
         return ZipBlob.from_file(zip_fname)
 
     @lt.action
-    def stitch_all_scans(
+    def stitch_all_scans(self) -> None:
-        self, logger: lt.deps.InvocationLogger, cancel: lt.deps.CancelHook
-    ) -> 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, cancel=cancel, scan_name=scan.name)
+                self.stitch_scan(scan_name=scan.name)

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

tests/mock_things/__init__.py

@@ -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.
-"""

tests/mock_things/mock_autofocus.py

@@ -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

tests/mock_things/mock_camera.py

@@ -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"},
-            )
-        )

tests/mock_things/mock_csm.py

@@ -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],
-    ]

tests/mock_things/mock_stage.py

@@ -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}

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

tests/test_smart_scan.py

@@ -35,14 +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
-
-# 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")
@@ -57,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):
@@ -92,16 +86,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 +114,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 +145,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)
@@ -159,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
@@ -173,76 +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
-    cancel_mock = 1  # not called
-    af_mock = MockAutoFocusThing()
-    stage_mock = MockStageThing()
-    cam_mock = MockCameraThing()
-    csm_mock = MockCSMThing()
 
-    class MockedSmartScanThing(SmartScanThing):
+    def check_locked(*_args, **_kwargs):
-        """Mocked version of SmartScanThing with a patched _run_scan method."""
+        """Check the scan is locked."""
+        assert smart_scan_thing._scan_lock.locked()
 
-        # Counter for checking functions were called
+    mocker.patch.object(smart_scan_thing, "_run_scan", side_effect=check_locked)
-        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._cancel is cancel_mock
-            assert self._scan_logger is LOGGER
-            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
-    )
 
     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(
+        smart_scan_thing.sample_scan(scan_name="FooBar")
-            cancel=cancel_mock,  # Shouldn't be used, can be checked
+
-            logger=LOGGER,
-            autofocus=af_mock,
-            stage=stage_mock,
-            cam=cam_mock,
-            csm=csm_mock,
-            scan_name="FooBar",
-        )
     except Exception as e:
         exec_info = e
 
-    assert mock_ss_thing._scan_lock.acquire(timeout=0.1)
+    assert not smart_scan_thing._scan_lock.locked()
-    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
-    assert mock_ss_thing._cam is None
-    assert mock_ss_thing._csm is None
 
     # 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):
@@ -250,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"
@@ -284,23 +245,22 @@ 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.
+    # Set the lock so it thinks the scan is running
-    smart_scan_thing._scan_logger = LOGGER
+    with smart_scan_thing._scan_lock:
-    mocker.patch.object(
+        mocker.patch.object(
-        smart_scan_thing, "_calc_displacement_from_test_image", return_value=[100, 100]
+            smart_scan_thing,
-    )
+            "_calc_displacement_from_test_image",
-    mock_ongoing_scan = mocker.Mock()
+            return_value=[100, 100],
-    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)
 
-    mock_autofocus = mocker.Mock()
+        smart_scan_thing._stage.position = MOCK_START_POS
 
-    smart_scan_thing._ongoing_scan = mock_ongoing_scan
+        mock_ongoing_scan = mocker.Mock()
-    smart_scan_thing._stage = mock_stage
+        mock_ongoing_scan.name = MOCK_SCAN_NAME
-    smart_scan_thing._autofocus = mock_autofocus
+        mock_ongoing_scan.images_dir = MOCK_SCAN_DIR
-    return smart_scan_thing
+        smart_scan_thing._ongoing_scan = mock_ongoing_scan
+
+        yield smart_scan_thing
 
 
 def test_collect_scan_data(scan_thing_mocked_for_scan_data):
@@ -341,8 +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, "_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")

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

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
+from .utilities.lt_test_utils import LabThingsTestEnv
-LOGGER = logging.getLogger("mock-invocation_logger")
+
+# 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):
+    t_start = time.time()
-        stitcher = PreviewStitcher(FAKE_DIR, overlap=0.1, correlation_resize=0.5)
+    # Start the action
-        # Send in the argument HANG to mock-stitch and it just hang for 10s
+    response = stitching_test_env.start_action("stitcher", "run_preview")
-        stitcher._extra_args = ["HANG"]
+    # Sleep long enough for at least 1 log.
-        cancel_hook = lt.deps.CancelHook(id=uuid.uuid4())
+    time.sleep(0.5)
-        t_start = time.time()
+    # Cancel using a DELETE request
-        # Start stitching
+    stitching_test_env.cancel_action(response)
-        stitcher.start()
+    invocation_data = stitching_test_env.poll_action(response)
-        # Run wait() in a thread.
+
-        thread = threading.Thread(target=stitcher.wait, kwargs={"cancel": cancel_hook})
+    # If it wasn't cancelled it would hang for 10 s. Here we check the cancel killed
-        thread.start()
+    # it within 2s.
-        # Sleep long enough for at least 1 log.
+    assert time.time() - t_start < 2
-        time.sleep(0.5)
+    assert invocation_data["status"] == "cancelled"
-        # use set() to cancel!
+    logs = invocation_data["log"]
-        cancel_hook.set()
+    assert len(logs) == 1
-        thread.join()
+    assert re.match(r"^Invocation [0-9a-f-]+ was cancelled", logs[0]["message"])
-        # 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
 
 
 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):
+    # Start the action
-        stitcher = FinalStitcher(FAKE_DIR, logger=LOGGER)
+    response = stitching_test_env.start_action("stitcher", "run_final")
-        cancel_hook = lt.deps.CancelHook(id=uuid.uuid4())
+    # 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.
+    assert invocation_data["status"] == "cancelled"
-        thread = threading.Thread(target=stitcher.run, kwargs={"cancel": cancel_hook})
+    logs = invocation_data["log"]
-        thread.start()
+    assert len(logs) < len(FINAL_EXPECTED_COMMAND) + 1
-        # Sleep long enough for at least 1 log.
+    assert logs[-2]["message"] == "Stitching cancelled by user"
-        time.sleep(0.5)
+    assert re.match(r"^Invocation [0-9a-f-]+ was cancelled", logs[-1]["message"])
-        # 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"
 
 
 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()

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