Merge branch 'test-server-startup' into 'v3'

Test server startup

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

src/openflexure_microscope_server/server/legacy_api.py

@@ -4,8 +4,20 @@ import labthings_fastapi as lt
 from fastapi import Response
 from socket import gethostname
 
+FAKE_ROUTES = [
+    "/api/v2/",
+    "/api/v2/streams/snapshot",
+    "/api/v2/instrument/settings/name",
+]
 
-def add_v2_endpoints(thing_server: lt.ThingServer):
+
+class JPEGResponse(Response):
+    """A FastAPI response with media_type set for a JPEG image."""
+
+    media_type = "image/jpeg"
+
+
+def add_v2_endpoints(thing_server: lt.ThingServer) -> None:
     """Add the v2 API endpoints for OpenFlexure Connect discoverability."""
     app = thing_server.app
 
@@ -19,15 +31,7 @@ def add_v2_endpoints(thing_server: lt.ThingServer):
 
         This is used by OF Connect to identify the microscope.
         """
-        fake_routes = [
+        return {url: {"url": url, "methods": ["GET"]} for url in FAKE_ROUTES}
-            "/api/v2/",
-            "/api/v2/streams/snapshot",
-            "/api/v2/instrument/settings/name",
-        ]
-        return {url: {"url": url, "methods": ["GET"]} for url in fake_routes}
-
-    class JPEGResponse(Response):
-        media_type = "image/jpeg"
 
     @app.get("/api/v2/streams/snapshot")
     @app.head("/api/v2/streams/snapshot")

tests/test_legacy_api.py

@@ -0,0 +1,63 @@
+"""Test server booting and shut down."""
+
+from socket import gethostname
+import asyncio
+
+# Import as ofm server to attempt to minimise confusion with server as a var in other
+# functions and also FastAPI `Server`.
+from openflexure_microscope_server.server import legacy_api
+
+
+def test_v2_endpoints(mocker):
+    """Check that the expected v2 endpoints are added."""
+    mock_server = mocker.Mock()
+    # Mock the camera thing to mocke the lores_mjpeg stream get_frame()
+    mock_server.things = {"/camera/": mocker.Mock()}
+    mock_server.things["/camera/"].lores_mjpeg_stream.grab_frame = mocker.AsyncMock(
+        return_value="Mock Frame"
+    )
+    mock_app = mock_server.app
+    # The wrapper returned for app.get so we can see what functions are decorated.
+    get_wrapper = mock_app.get.return_value
+    head_wrapper = mock_app.head.return_value
+
+    legacy_api.add_v2_endpoints(mock_server)
+
+    assert get_wrapper.call_count == 3
+    assert head_wrapper.call_count == 1
+
+    # The calls for the get decorator and the internal wrapper
+    get_and_wrapper_calls = zip(
+        mock_app.get.call_args_list, get_wrapper.call_args_list, strict=True
+    )
+    # Pull out the first arg of each to get the route and wrapped function, save as a
+    # dictionary - route: function
+    routes = {
+        get_call.args[0]: wrapper_call.args[0]
+        for get_call, wrapper_call in get_and_wrapper_calls
+    }
+
+    # Check the fake routes are set
+    assert "/routes" in routes
+    fake_routes_dict = routes["/routes"]()
+    assert isinstance(fake_routes_dict, dict)
+    for fake_route in legacy_api.FAKE_ROUTES:
+        assert fake_route in fake_routes_dict
+        assert fake_routes_dict[fake_route]["url"] == fake_route
+        assert fake_routes_dict[fake_route]["methods"] == ["GET"]
+
+    # Check snapshot returns a jupeg response from the async get_frame of the
+    # lores_mjpeg_stream
+    assert "/api/v2/streams/snapshot" in routes
+    # First get the async frame function from the head wrapper
+    get_frames_func = head_wrapper.call_args.args[0]()
+    assert routes["/api/v2/streams/snapshot"] == head_wrapper()
+
+    jpg_response = asyncio.run(get_frames_func)
+    assert isinstance(jpg_response, legacy_api.JPEGResponse)
+    # Value should be set as mocked rather than a frame
+    assert jpg_response.body.decode() == "Mock Frame"
+
+    # Also check the name is the hostname.
+    assert "/api/v2/instrument/settings/name" in routes
+    assert routes["/api/v2/instrument/settings/name"]() == gethostname()

tests/test_serve_static_files.py

@@ -11,18 +11,6 @@ from starlette.responses import FileResponse, RedirectResponse
 from openflexure_microscope_server.server import serve_static_files
 
 
-@pytest.fixture
-def mock_app(mocker):
-    """Return a mock FastAPI app.
-
-    This is just a mock, where the get method returns a mock.
-    """
-    wrapper = mocker.Mock()
-    app = mocker.Mock()
-    app.get.return_value = wrapper
-    return app
-
-
 @pytest.fixture
 def mock_static_dir():
     """Return the path of a mock static directory.
@@ -46,12 +34,13 @@ def mock_static_dir():
         ["fontfile.woff2", True],
     ],
 )
-def test_add_static_file(filename, allow_cache, mock_app):
+def test_add_static_file(filename, allow_cache, mocker):
     """Test running add_static_file with both font and non-font files.
 
     This should creates a wrapped function that returns a FileResponse, this
     should be have the path on disk and the no cache headers if not a woff2 font.
     """
+    mock_app = mocker.Mock()
     # Get the wrapper function from the mocked decorator
     wrapper = mock_app.get.return_value
     serve_static_files.add_static_file(mock_app, fname=filename, folder="bar")
@@ -83,8 +72,9 @@ def test_add_static_file(filename, allow_cache, mock_app):
             assert response.headers[key] == value
 
 
-def test_add_static_with_no_static_dir(mock_app, mocker):
+def test_add_static_with_no_static_dir(mocker):
     """Test that a FileNotFound error is raised if the static dir does not exist."""
+    mock_app = mocker.Mock()
     # Mock STATIC_PATH to something that doesn't exist
     mocker.patch(
         "openflexure_microscope_server.server.serve_static_files.STATIC_PATH",
@@ -128,8 +118,9 @@ def test_check_static_dir(mock_static_dir, mocker):
         serve_static_files.check_static_dir()
 
 
-def test_add_static_files(mock_app, mock_static_dir, mocker):
+def test_add_static_files(mock_static_dir, mocker):
     """Check add_static_files mounts the internal webapp dirs, and sets endpoints for files."""
+    mock_app = mocker.Mock()
     mocker.patch(
         "openflexure_microscope_server.server.serve_static_files.STATIC_PATH",
         mock_static_dir,
@@ -170,8 +161,9 @@ def test_add_static_files(mock_app, mock_static_dir, mocker):
     assert os.path.join(mock_static_dir, "js") in mounted_dirs
 
 
-def test_add_static_files_with_scan_dir(mock_app, mock_static_dir, mocker):
+def test_add_static_files_with_scan_dir(mock_static_dir, mocker):
     """Check the scan dir mounts if supplied."""
+    mock_app = mocker.Mock()
     mocker.patch(
         "openflexure_microscope_server.server.serve_static_files.STATIC_PATH",
         mock_static_dir,

tests/test_server_cli.py

@@ -0,0 +1,104 @@
+"""Test server booting and shut down."""
+
+import logging
+
+import pytest
+from fastapi import FastAPI
+
+# Import as ofm server to attempt to minimise confusion with server as a var in other
+# functions and also FastAPI `Server`.
+from openflexure_microscope_server import server as ofm_server
+
+from .test_server_config import FULL_CONFIG
+
+
+def test_no_config():
+    """Check that an error is thrown if no configuration is set for the microspe via CLI."""
+    with pytest.raises(RuntimeError, match="No configuration"):
+        ofm_server.serve_from_cli([])
+
+
+def test_successful_start(mocker, caplog):
+    """Check some basic things from the microscope setup.
+
+    This test checks that logging is set up and that the actual function used for
+    handling shutdown events shuts down the mjpeg streams.
+    """
+    mock_server = mocker.Mock()
+    # Create a mock for the camera so we can check the MJPEG streams are closed on
+    # shutdown.
+    mock_camera = mocker.Mock()
+    mock_server.things = {"/camera/": mock_camera}
+    # Mock the LabThings function that returns the server so we have a mock server
+    mocker.patch(
+        "openflexure_microscope_server.server.lt.cli.server_from_config",
+        return_value=mock_server,
+    )
+    # Also mock customisation or it will try to access the hard drive and make files.
+    mock_customise = mocker.patch.object(ofm_server, "customise_server")
+    # And mock uvicorn.run as we don't want to start a server process
+    mock_uvicorn_run = mocker.patch("openflexure_microscope_server.server.uvicorn.run")
+
+    # Run the mock CLI
+    ofm_server.serve_from_cli(["-c", FULL_CONFIG])
+
+    # Check that the server was customised and the run
+    assert mock_customise.call_count == 1
+    assert mock_uvicorn_run.call_count == 1
+    # Read the log config that was set
+    log_config = mock_uvicorn_run.call_args.kwargs["log_config"]
+    # Check both uvicorn loggers were set to propagate
+    assert log_config["loggers"]["uvicorn"]["propagate"]
+    assert log_config["loggers"]["uvicorn.access"]["propagate"]
+
+    # Now Mock a shutdown signal being sent
+    ofm_server.Server.handle_exit(mock_server.app, "mock_signal", "mock_frame")
+
+    # Check
+    assert mock_camera.kill_mjpeg_streams.call_count == 1
+
+    # Mock shutdown again but with the MJPEG stream throwing a BaseException
+    mock_camera.kill_mjpeg_streams.side_effect = BaseException("mock-54321")
+
+    # This should log at error level but even a BaseException will not cause an error
+    # that stops the shutdown signal propagating
+    with caplog.at_level(logging.ERROR):
+        ofm_server.Server.handle_exit(mock_server.app, "mock_signal", "mock_frame")
+    # Assert there is one error log
+    assert len(caplog.records) == 1
+    # And that it is the message raised by the kill_mjpeg_streams mock
+    assert str(caplog.records[0].msg) == "mock-54321"
+
+
+def test_failed_customise(mocker):
+    """Check what happens if there is an error before the server is started by uvicorn.
+
+    This differs based on whether the --fallback flag is set.
+    """
+    mock_server = mocker.Mock()
+    # Mock the LabThings function that returns the server so we have a mock server
+    mocker.patch(
+        "openflexure_microscope_server.server.lt.cli.server_from_config",
+        return_value=mock_server,
+    )
+    # Also mock customisation or it will try to access the hard drive and make files.
+    mocker.patch.object(
+        ofm_server, "customise_server", side_effect=RuntimeError("Can't touch this")
+    )
+    # And mock uvicorn.run as we don't want to start a server process
+    mock_uvicorn_run = mocker.patch("openflexure_microscope_server.server.uvicorn.run")
+
+    # Running the mock CLI will error
+    with pytest.raises(RuntimeError, match="Can't touch this"):
+        ofm_server.serve_from_cli(["-c", FULL_CONFIG])
+
+    # But with the fallback flag uvicorn run will be run
+    ofm_server.serve_from_cli(["-c", FULL_CONFIG, "--fallback"])
+
+    assert mock_uvicorn_run.call_count == 1
+    # Get the fallback app passed to uvicorn tun
+    fallback_app = mock_uvicorn_run.call_args.args[0]
+    # Check it really is a fastapi
+    assert isinstance(fallback_app, FastAPI)
+    # An that it has the error to display
+    assert str(fallback_app.labthings_error) == "Can't touch this"

tests/test_server_config.py

@@ -0,0 +1,129 @@
+"""Test server booting and shut down."""
+
+import os
+import json
+from copy import deepcopy
+
+import pytest
+
+# Import as ofm server to attempt to minimise confusion with server as a var in other
+# functions and also FastAPI `Server`.
+from openflexure_microscope_server import server as ofm_server
+
+THIS_DIR = os.path.dirname(os.path.abspath(__file__))
+REPO_ROOT = os.path.dirname(THIS_DIR)
+FULL_CONFIG = os.path.join(REPO_ROOT, "ofm_config_full.json")
+SIM_CONFIG = os.path.join(REPO_ROOT, "ofm_config_simulation.json")
+
+
+@pytest.mark.parametrize("side_effect", [None, RuntimeError("Mock")])
+def test_monkey_patched_handle_exit(side_effect, mocker):
+    """Test that handle exit is monkey_patched."""
+    mock_app = mocker.Mock()
+    # Setup the same for any side_effect
+    # Create mocks for FastAPI.Server, and for our custom shutdown function
+    mock_shutdown_func = mocker.Mock(side_effect=side_effect)
+    mock_fastapi_server = mocker.patch.object(ofm_server, "Server")
+    original_mock_handle_exit = mock_fastapi_server.handle_exit
+    handle_exit_args = (mock_app, "mock_signal", "mock_frame")
+
+    ofm_server.set_shutdown_function(mock_shutdown_func)
+
+    if side_effect is None:
+        # In normal operation, both the custom shutdown function and the original
+        # handle_exit should be called
+        ofm_server.Server.handle_exit(*handle_exit_args)
+
+        assert mock_shutdown_func.call_count == 1
+        assert original_mock_handle_exit.call_count == 1
+        # Check arguments are propagated correctly.
+        assert original_mock_handle_exit.call_args.args == handle_exit_args
+    else:
+        # Use an error side effect to check that our custom shutdown function is called
+        # before the standard `handle_exit`
+        with pytest.raises(RuntimeError, match="Mock"):
+            ofm_server.Server.handle_exit(*handle_exit_args)
+        # The error was raised so we know our custom function was run, check that
+        # handle_exit wasn't.
+        # Note: that the non-mocked shutdown function is entirely wrapped in a
+        # try/except BaseExcpetion, so that handle_exit will always run.
+        assert original_mock_handle_exit.call_count == 0
+
+
+def test_customise_server(mocker):
+    """Check that all server customisation stages are called."""
+    mock_server = mocker.Mock()
+    # Mock everything to be called in customisation, partly setup may not be complete,
+    # but also to limit excess coverage reporting.
+    mocked_add_static = mocker.patch.object(ofm_server, "add_static_files")
+    mocked_v2_endpoints = mocker.patch.object(ofm_server, "add_v2_endpoints")
+    mocked_configure_logging = mocker.patch.object(ofm_server, "configure_logging")
+    mocked_retrieve_log = mocker.patch.object(ofm_server, "retrieve_log")
+    mocked_retrieve_log_file = mocker.patch.object(ofm_server, "retrieve_log_from_file")
+
+    mock_app = mock_server.app
+    # The wrapper returned for app.get so we can see what functions are decorated.
+    wrapper = mock_app.get.return_value
+
+    # Finally we can run it!
+    ofm_server.customise_server(mock_server, "mock_log_folder", "mock_scan_folder")
+
+    # Check each internal customisation function is called
+    assert mocked_add_static.call_count == 1
+    assert mocked_v2_endpoints.call_count == 1
+    assert mocked_configure_logging.call_count == 1
+    # Check app.get adds two routes
+    assert mock_app.get.call_count == 2
+    assert wrapper.call_count == 2
+
+    # Check the routes and functions are as expected.
+    added_routes = [call.args[0] for call in mock_app.get.call_args_list]
+    assert "/log/" in added_routes
+    assert "/logfile/" in added_routes
+    wrapped_functions = [call.args[0] for call in wrapper.call_args_list]
+    assert mocked_retrieve_log in wrapped_functions
+    assert mocked_retrieve_log_file
+
+
+@pytest.mark.parametrize(
+    "config_file, expected_scan_dir",
+    [[FULL_CONFIG, "/var/openflexure/scans/"], [SIM_CONFIG, "./openflexure/scans/"]],
+)
+def test_get_scans_dir_ok(config_file, expected_scan_dir):
+    """Test the _get_scans_dir function and also check the standard config files."""
+    with open(config_file, "r", encoding="utf-8") as f_obj:
+        config_dict = json.load(f_obj)
+    assert ofm_server._get_scans_dir(config_dict) == expected_scan_dir
+
+
+def test_get_scans_dir_no_smart_scan():
+    """Test _get_scans_dir with no SmartScanThing."""
+    # Load the standard config dict
+    with open(FULL_CONFIG, "r", encoding="utf-8") as f_obj:
+        config_dict = json.load(f_obj)
+    # Delete smart scan
+    del config_dict["things"]["/smart_scan/"]
+    # No SmartScanThing, should return None
+    assert ofm_server._get_scans_dir(config_dict) is None
+
+
+def test_get_scans_dir_bad_smart_scan_config():
+    """Test _get_scans_dir with a SmartScanThing that doesn't set a config dir."""
+    # Load the standard config dict
+    with open(FULL_CONFIG, "r", encoding="utf-8") as f_obj:
+        config_dict = json.load(f_obj)
+
+    # Copy the dictionary
+    broken_config = deepcopy(config_dict)
+    # Delete all the smart scan kwargs
+    del broken_config["things"]["/smart_scan/"]["kwargs"]
+    # Creates an Error
+    with pytest.raises(RuntimeError):
+        ofm_server._get_scans_dir(broken_config)
+
+    # Same thing should happen if just the scans_folder key is deleted
+    broken_config = deepcopy(config_dict)
+    del broken_config["things"]["/smart_scan/"]["kwargs"]
+    # Creates an Error
+    with pytest.raises(RuntimeError):
+        ofm_server._get_scans_dir(broken_config)