Add unit tests for RelativeDataPath and save_from_memory

src/openflexure_microscope_server/things/__init__.py

@@ -82,7 +82,6 @@ class OFMThing(lt.Thing):
         return rel_data_path
 
 
-# TODO Write tests for this
 class RelativeDataPath(RootModel[str]):
     """A relative path that is validated, and can have a Thing assigned to it.
 

src/openflexure_microscope_server/things/camera/__init__.py

@@ -679,7 +679,6 @@ class BaseCamera(OFMThing, ABC):
             image = image.resize(save_resolution, Image.Resampling.BOX)
         try:
             save_kwargs: dict[str, Any] = {}
-            # TODO: Test that the save_kwargs are called as expected for different formats.
             jpeg_exts = BASE_IMAGE_FORMATS["jpeg"].supported_extensions
             if resolved_path.lower().endswith(jpeg_exts):
                 # Per PIL documentation,

tests/unit_tests/test_base_camera.py

@@ -5,10 +5,16 @@ test_simulated_camera.py and for testing the consistency of camera APIs see
 test_cameras.py.
 """
 
+import os
+from dataclasses import dataclass, field
+from typing import Optional
+
 import numpy as np
 import pytest
 from PIL import Image
 
+from openflexure_microscope_server.things import RelativeDataPath
+from openflexure_microscope_server.things.camera import CaptureMode
 from openflexure_microscope_server.things.camera.simulation import SimulatedCamera
 from openflexure_microscope_server.things.stage.dummy import DummyStage
 
@@ -61,3 +67,68 @@ def test_handle_broken_frame(test_env):
     for _i in range(15):
         array = camera.grab_as_array()
         assert isinstance(array, np.ndarray)
+
+
+@dataclass
+class MemorySaveTestCase:
+    """Inputs and expected outputs for testing ``save_from_memory``.
+
+    The default save kwargs assume a jpeg.
+    """
+
+    filename: str = "foobar.jpeg"
+    save_resolution: Optional[tuple[int, int]] = None
+    resize_needed: bool = False
+    save_kwargs: dict[str, int] = field(
+        default_factory=lambda: {"quality": 95, "subsampling": 0}
+    )
+
+
+SAVE_TEST_CASES = [
+    # Default test case is a jpeg, ckec it works with all extensions.
+    MemorySaveTestCase("foobar.jpeg"),
+    MemorySaveTestCase("foobar.jpg"),
+    MemorySaveTestCase("foobar.JPEG"),
+    MemorySaveTestCase("foobar.JPG"),
+    MemorySaveTestCase("foobar.png.jpeg"),
+    MemorySaveTestCase("foobar.png", save_kwargs={}),
+    MemorySaveTestCase("foobar.PNG", save_kwargs={}),
+    MemorySaveTestCase("foobar.jpeg.png", save_kwargs={}),
+    MemorySaveTestCase(save_resolution=None, resize_needed=False),
+    MemorySaveTestCase(save_resolution=(1000, 1200), resize_needed=False),
+    MemorySaveTestCase(save_resolution=(2000, 2400), resize_needed=True),
+]
+
+
+@pytest.mark.parametrize("test_case", SAVE_TEST_CASES)
+def test_save_from_memory(test_case, test_env, mocker):
+    """Check the correct timage is retrieved and saved with correct settings."""
+    camera = test_env.get_thing_by_type(SimulatedCamera)
+    camera._memory_buffer = mocker.Mock()
+    camera._add_metadata_to_capture = mocker.Mock()
+
+    mode = CaptureMode(description="foo", save_resolution=test_case.save_resolution)
+    type(camera).capture_modes = mocker.PropertyMock(return_value={"standard": mode})
+
+    mock_image = mocker.Mock()
+    # Make resize return itself so we can track further calls of the Image object after
+    # a resize
+    mock_image.resize.return_value = mock_image
+    mock_image.size = (1000, 1200)
+
+    camera._memory_buffer.get_image.return_value = (
+        mock_image,
+        {"meta": "data"},
+        "standard",
+    )
+
+    camera._data_dir = os.path.normpath("/fake/data/dir")
+
+    camera.save_from_memory(RelativeDataPath(test_case.filename), 33)
+
+    assert camera._memory_buffer.get_image.call_count == 1
+    assert camera._memory_buffer.get_image.call_args.args == (33,)
+    assert camera._add_metadata_to_capture.call_count == 1
+    assert mock_image.resize.call_count == (1 if test_case.resize_needed else 0)
+    assert mock_image.save.call_count == 1
+    assert mock_image.save.call_args.kwargs == test_case.save_kwargs

tests/unit_tests/test_ofm_thing.py

@@ -0,0 +1,128 @@
+"""Tests for the for thing defined in the base __init__ of the things dir."""
+
+import os
+from tempfile import TemporaryDirectory
+
+import pytest
+from pydantic import ValidationError
+
+from labthings_fastapi.testing import create_thing_without_server
+
+from openflexure_microscope_server.things import OFMThing, RelativeDataPath
+
+
+@pytest.mark.parametrize(
+    ("path", "valid"),
+    [
+        ("foo.png", True),
+        ("foo/bar/", True),
+        ("./foo/bar", True),
+        ("/foo/bar", False),
+        ("../foo/bar", False),
+        ("foo/../bar", False),
+    ],
+)
+def test_rel_data_path_validation(path, valid):
+    """Check validation for a number of cases."""
+    if valid:
+        path_obj = RelativeDataPath(path)
+        assert path_obj.root == os.path.normpath(path)
+    else:
+        with pytest.raises(ValidationError):
+            RelativeDataPath(path)
+
+
+def test_setting_saving_things():
+    """Check that the saving thing can be set."""
+    thing1 = create_thing_without_server(OFMThing)
+    thing1._data_dir = os.path.join("data", "thing1")
+    thing2 = create_thing_without_server(OFMThing)
+    thing2._data_dir = os.path.join("data", "thing2")
+    path = RelativeDataPath("foo.png")
+
+    assert not path.save_location_set
+
+    with pytest.raises(
+        RuntimeError, match="The saving Thing for the relative path was never set"
+    ):
+        path.abs_data_path
+
+    # Set the saving thing and check the response is as expected
+    path.set_saving_thing(thing1)
+    assert path.save_location_set
+    assert path.abs_data_path == os.path.join("data", "thing1", "foo.png")
+    assert path._saving_thing is thing1
+
+    # Check there is an error is trying to overwrite it with a new thing...
+    with pytest.raises(
+        RuntimeError, match="The saving Thing for the relative path is already set"
+    ):
+        path.set_saving_thing(thing2)
+    # or a temporary dir
+    with pytest.raises(
+        RuntimeError, match="The saving Thing for the relative path is already set"
+    ):
+        path.save_to_tempdir()
+
+    # Check no error when using set_saving_thing_if_unset
+    path.set_saving_thing_if_unset(thing2)
+    assert path.save_location_set
+    assert path.abs_data_path == os.path.join("data", "thing1", "foo.png")
+    assert path._saving_thing is thing1
+
+    # Check set_saving_thing_if_unset works on a new path.
+    path2 = RelativeDataPath("foo2.png")
+    assert not path2.save_location_set
+    path2.set_saving_thing_if_unset(thing2)
+    assert path2.save_location_set
+    assert path2.abs_data_path == os.path.join("data", "thing2", "foo2.png")
+    assert path2._saving_thing is thing2
+
+
+def test_saving_thing_propagates_on_join():
+    """Check when joining to a path the saving thing propagates to the new path."""
+    thing = create_thing_without_server(OFMThing)
+    thing._data_dir = os.path.join("data", "thing")
+    path = RelativeDataPath("foo")
+
+    # No thing set
+    assert not path.save_location_set
+
+    # After joining no thing set
+    path2 = path.join("bar")
+    assert not path2.save_location_set
+
+    # Set thing and check joiend path is as expected
+    path2.set_saving_thing(thing)
+    assert path2.save_location_set
+    assert path2.abs_data_path == os.path.join("data", "thing", "foo", "bar")
+
+    # Do another join. Set thing remains
+    path3 = path2.join("file.png")
+    assert path3.save_location_set
+    assert path3.abs_data_path == os.path.join(
+        "data", "thing", "foo", "bar", "file.png"
+    )
+
+
+def test_temp_dir_for_saving_thing():
+    """Check that the saving thing can actually be a temporary directory."""
+    path = RelativeDataPath("foo.png")
+    tmpdir = path.save_to_tempdir()
+    assert isinstance(tmpdir, TemporaryDirectory)
+    assert path.save_location_set
+    assert path.abs_data_path == os.path.join(tmpdir.name, "foo.png")
+
+
+def test_create_data_path_from_thing():
+    """Check the create_data_path method of ``OFMThing``.
+
+    It should create a RelativeDataPath with itself set as the saving thing.
+    """
+    thing = create_thing_without_server(OFMThing)
+    thing._data_dir = os.path.join("data", "thing")
+    path = thing.create_data_path("file.png")
+
+    assert path.save_location_set
+    assert path.abs_data_path == os.path.join("data", "thing", "file.png")
+    assert path._saving_thing is thing