Apply suggestions from code review.

Thanks @bprobert97 - a couple of nice improvements to tests and docstrings.

Author: rwb27

src/labthings_fastapi/server/__init__.py

@@ -84,7 +84,7 @@ def __init__(
             arguments, and any connections to other `.Thing`\ s.
         :param settings_folder: the location on disk where `.Thing`
             settings will be saved.
-        :param api_prefix: An optional prefix for all API routes. This must either
+        :param api_prefix: A prefix for all API routes. This must either
             be empty, or start with a slash and not end with a slash.
         :param application_config: A mapping containing custom configuration for the
             application. This is not processed by LabThings. Each `.Thing` can access
@@ -176,10 +176,10 @@ def application_config(self) -> Mapping[str, Any] | None:
 
     @property
     def _api_prefix(self) -> str:
-        """A string that prefixes all URLs in the application.
+        r"""A string that prefixes all URLs in the application.
 
-        This must either be empty, or start with a slash and not
-        end with a slash.
+        This will either be empty, or start with a slash and not
+        end with a slash. Validation is performed in `.ThingServerConfig`\ .
         """
         return self._config.api_prefix
 

tests/test_server.py

@@ -73,27 +73,33 @@ def test_server_thing_descriptions():
             assert prop["forms"][0]["href"] == expected_href
 
 
-def test_api_prefix():
+@pytest.mark.parametrize("api_prefix", ["/api/v3", "/v1", "/custom/prefix"])
+def test_api_prefix(api_prefix):
     """Check we can add a prefix to the URLs on a server."""
 
     class Example(lt.Thing):
         """An example Thing"""
 
-    server = lt.ThingServer(things={"example": Example}, api_prefix="/api/v3")
+    server = lt.ThingServer(things={"example": Example}, api_prefix=api_prefix)
     paths = [route.path for route in server.app.routes if isinstance(route, Route)]
-    for expected_path in [
-        "/api/v3/action_invocations",
-        "/api/v3/action_invocations/{id}",
-        "/api/v3/action_invocations/{id}/output",
-        "/api/v3/action_invocations/{id}",
-        "/api/v3/blob/{blob_id}",
-        "/api/v3/thing_descriptions/",
-        "/api/v3/things/",
-        "/api/v3/example/",
-    ]:
+
+    # Dynamically generate expected paths based on the parametrized prefix
+    expected_paths = [
+        f"{api_prefix}/action_invocations",
+        f"{api_prefix}/action_invocations/{{id}}",
+        f"{api_prefix}/action_invocations/{{id}}/output",
+        f"{api_prefix}/blob/{{blob_id}}",
+        f"{api_prefix}/thing_descriptions/",
+        f"{api_prefix}/things/",
+        f"{api_prefix}/example/",
+    ]
+
+    for expected_path in expected_paths:
         assert expected_path in paths
 
-    unprefixed_paths = {p for p in paths if not p.startswith("/api/v3/")}
+    prefix_with_slash = f"{api_prefix}/"
+    unprefixed_paths = {p for p in paths if not p.startswith(prefix_with_slash)}
+
     assert unprefixed_paths == {
         "/openapi.json",
         "/docs",

tests/test_server_config_model.py

@@ -106,7 +106,7 @@ def test_ThingServerConfig():
         assert config.api_prefix == prefix
 
     # Check some bad prefixes
-    for prefix in ["api", "/api/", "api/v2", "/badchars!"]:
+    for prefix in ["api", "/api/", "api/", "api/v2", "/badchars!"]:
         with pytest.raises(ValidationError):
             ThingServerConfig(things={}, api_prefix=prefix)