diff --git a/picamera_coverage.zip b/picamera_coverage.zip index 18741417..cca9a968 100644 Binary files a/picamera_coverage.zip and b/picamera_coverage.zip differ diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index b1a88863..5a984bed 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. @@ -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( - 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/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 0ee5880b..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.""" @@ -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 - 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!" @@ -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( - self, - cancel: lt.deps.CancelHook, - logger: lt.deps.InvocationLogger, - 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, @@ -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._cancel.clear() - 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, ) @@ -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. - - 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, 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) 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( - self, logger: lt.deps.InvocationLogger, cancel: lt.deps.CancelHook - ) -> 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, cancel=cancel, scan_name=scan.name) + self.stitch_scan(scan_name=scan.name) 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") 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_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 f753250f..4a59e47e 100644 --- a/tests/test_smart_scan.py +++ b/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): - """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._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 - ) + 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( - 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", - ) + 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._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 + 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): @@ -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. - 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_autofocus = mocker.Mock() + smart_scan_thing._stage.position = MOCK_START_POS - smart_scan_thing._ongoing_scan = mock_ongoing_scan - smart_scan_thing._stage = mock_stage - smart_scan_thing._autofocus = mock_autofocus - return smart_scan_thing + mock_ongoing_scan = mocker.Mock() + mock_ongoing_scan.name = MOCK_SCAN_NAME + mock_ongoing_scan.images_dir = MOCK_SCAN_DIR + 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") 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): 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() 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"]