Merge branch 'main' of https://github.com/willmiao/ComfyUI-Lora-Manager

docs/testing/coverage_analysis.md

@@ -0,0 +1,26 @@
+# Backend Test Coverage Notes
+
+## Pytest Execution
+- Command: `python -m pytest`
+- Result: All 283 collected tests passed in the current environment.
+- Coverage tooling (``pytest-cov``/``coverage``) is unavailable in the offline sandbox, so line-level metrics could not be generated. The earlier attempt to install ``pytest-cov`` failed because the package index cannot be reached from the container.
+
+## High-Priority Gaps to Address
+
+### 1. Standalone server bootstrapping
+* **Source:** [`standalone.py`](../../standalone.py)
+* **Why it matters:** The standalone entry point wires together the aiohttp application, static asset routes, model-route registration, and configuration validation. None of these behaviours are covered by automated tests, leaving regressions in bootstrapping logic undetected.
+* **Suggested coverage:** Add integration-style tests that instantiate `StandaloneServer`/`StandaloneLoraManager` with temporary settings and assert that routes (HTTP + websocket) are registered, configuration warnings fire for missing paths, and the mock ComfyUI shims behave as expected.
+
+### 2. Model service registration factory
+* **Source:** [`py/services/model_service_factory.py`](../../py/services/model_service_factory.py)
+* **Why it matters:** The factory coordinates which model services and routes the API exposes, including error handling when unknown model types are requested. No current tests verify registration, memoization of route instances, or the logging path on failures.
+* **Suggested coverage:** Unit tests that exercise `register_model_type`, `get_route_instance`, error branches in `get_service_class`/`get_route_class`, and `setup_all_routes` when a route setup raises. Use lightweight fakes to confirm the logger is called and state is cleared via `clear_registrations`.
+
+### 3. Server-side i18n helper
+* **Source:** [`py/services/server_i18n.py`](../../py/services/server_i18n.py)
+* **Why it matters:** Template rendering relies on the `ServerI18nManager` to load locale JSON, perform key lookups, and format parameters. The fallback logic (dot-notation lookup, English fallbacks, placeholder substitution) is untested, so malformed locale files or regressions in placeholder handling would slip through.
+* **Suggested coverage:** Tests that load fixture locale dictionaries, assert `set_locale` fallbacks, verify nested key resolution and placeholder substitution, and ensure missing keys return the original identifier.
+
+## Next Steps
+Prioritize creating focused unit tests around these modules, then re-run pytest once coverage tooling is available to confirm the new tests close the identified gaps.

tests/services/test_model_service_factory.py

@@ -0,0 +1,103 @@
+"""Unit tests for :mod:`py.services.model_service_factory`."""
+
+from __future__ import annotations
+
+from typing import List
+
+import pytest
+
+from py.services.model_service_factory import ModelServiceFactory
+
+
+class _RecorderRoute:
+    """Route double capturing setup invocations."""
+
+    def __init__(self) -> None:
+        self.calls: List[object] = []
+
+    def setup_routes(self, app):
+        self.calls.append(app)
+
+
+class _FailingRoute:
+    def setup_routes(self, app):  # pragma: no cover - used in exception path
+        raise RuntimeError("boom")
+
+
+class _DummyService:
+    pass
+
+
+@pytest.fixture(autouse=True)
+def _reset_model_service_factory():
+    """Ensure each test receives an isolated factory registry."""
+
+    ModelServiceFactory.clear_registrations()
+    yield
+    ModelServiceFactory.clear_registrations()
+
+
+def test_register_and_retrieve_model_type():
+    """Registering a model type exposes its service and cached routes."""
+
+    ModelServiceFactory.register_model_type("demo", _DummyService, _RecorderRoute)
+
+    assert ModelServiceFactory.get_service_class("demo") is _DummyService
+    assert ModelServiceFactory.get_route_class("demo") is _RecorderRoute
+
+    first_instance = ModelServiceFactory.get_route_instance("demo")
+    second_instance = ModelServiceFactory.get_route_instance("demo")
+
+    assert isinstance(first_instance, _RecorderRoute)
+    assert first_instance is second_instance, "route instances should be memoized"
+
+
+def test_get_unknown_model_type_raises():
+    """Unknown model types raise ``ValueError`` for both accessors."""
+
+    with pytest.raises(ValueError):
+        ModelServiceFactory.get_service_class("missing")
+
+    with pytest.raises(ValueError):
+        ModelServiceFactory.get_route_class("missing")
+
+
+def test_setup_all_routes_invokes_registered_routes():
+    """``setup_all_routes`` delegates to each registered route instance."""
+
+    ModelServiceFactory.register_model_type("demo", _DummyService, _RecorderRoute)
+    app = object()
+
+    ModelServiceFactory.setup_all_routes(app)
+
+    route = ModelServiceFactory.get_route_instance("demo")
+    assert route.calls == [app]
+
+
+def test_setup_all_routes_logs_failures(caplog):
+    """Failures while binding a route are logged and do not interrupt others."""
+
+    ModelServiceFactory.register_model_type("ok", _DummyService, _RecorderRoute)
+    ModelServiceFactory.register_model_type("broken", _DummyService, _FailingRoute)
+    app = object()
+
+    caplog.set_level("ERROR")
+    ModelServiceFactory.setup_all_routes(app)
+
+    route = ModelServiceFactory.get_route_instance("ok")
+    assert route.calls == [app]
+    assert any("Failed to setup routes for broken" in record.message for record in caplog.records)
+
+
+def test_clear_registrations_resets_all_state():
+    """``clear_registrations`` removes services, routes, and cached instances."""
+
+    ModelServiceFactory.register_model_type("demo", _DummyService, _RecorderRoute)
+
+    assert ModelServiceFactory.is_registered("demo")
+    ModelServiceFactory.clear_registrations()
+    assert not ModelServiceFactory.get_registered_types()
+    assert not ModelServiceFactory.is_registered("demo")
+
+    with pytest.raises(ValueError):
+        ModelServiceFactory.get_service_class("demo")

tests/standalone/test_standalone_server.py

@@ -0,0 +1,215 @@
+"""Tests covering the standalone bootstrap flow."""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from types import ModuleType, SimpleNamespace
+from typing import List, Tuple
+
+import pytest
+from aiohttp import web
+
+from py.utils.settings_paths import ensure_settings_file
+
+
+@pytest.fixture
+def standalone_module(monkeypatch) -> ModuleType:
+    """Load the ``standalone`` module with a lightweight ``LoraManager`` stub."""
+
+    import importlib
+    import sys
+    from types import ModuleType
+
+    original_lora_manager = sys.modules.get("py.lora_manager")
+
+    stub_module = ModuleType("py.lora_manager")
+
+    class _StubLoraManager:
+        @classmethod
+        def add_routes(cls):  # pragma: no cover - compatibility shim
+            return None
+
+        @classmethod
+        async def _initialize_services(cls):  # pragma: no cover - compatibility shim
+            return None
+
+        @classmethod
+        async def _cleanup(cls, app):  # pragma: no cover - compatibility shim
+            return None
+
+    stub_module.LoraManager = _StubLoraManager
+    sys.modules["py.lora_manager"] = stub_module
+
+    module = importlib.import_module("standalone")
+    yield module
+
+    sys.modules.pop("standalone", None)
+    if original_lora_manager is not None:
+        sys.modules["py.lora_manager"] = original_lora_manager
+    else:
+        sys.modules.pop("py.lora_manager", None)
+
+
+def _write_settings(contents: dict) -> Path:
+    """Persist *contents* into the isolated settings.json."""
+
+    settings_path = Path(ensure_settings_file())
+    settings_path.write_text(json.dumps(contents))
+    return settings_path
+
+
+async def test_standalone_server_sets_up_routes(tmp_path, standalone_module):
+    """``StandaloneServer.setup`` wires the HTTP routes and lifecycle hooks."""
+
+    example_images_dir = tmp_path / "example_images"
+    example_images_dir.mkdir()
+    (example_images_dir / "preview.png").write_text("placeholder")
+
+    _write_settings({"example_images_path": str(example_images_dir)})
+
+    server = standalone_module.StandaloneServer()
+    await server.setup()
+
+    canonical_routes = {resource.canonical for resource in server.app.router.resources()}
+
+    assert "/" in canonical_routes, "status endpoint should be registered"
+    assert (
+        "/example_images_static" in canonical_routes
+    ), "static example image route should be exposed when the directory exists"
+    assert server.app.on_startup, "startup callbacks must be attached"
+    assert server.app.on_shutdown, "shutdown callbacks must be attached"
+
+
+def test_validate_settings_warns_for_missing_model_paths(caplog, standalone_module):
+    """Missing model folders trigger the configuration warning."""
+
+    caplog.set_level("WARNING")
+
+    _write_settings(
+        {
+            "folder_paths": {
+                "loras": ["/non/existent"],
+                "checkpoints": [],
+                "embeddings": [],
+            }
+        }
+    )
+
+    assert standalone_module.validate_settings() is False
+    warning_lines = [record.message for record in caplog.records if record.levelname == "WARNING"]
+    assert any("CONFIGURATION WARNING" in line for line in warning_lines)
+
+
+def test_standalone_lora_manager_registers_routes(monkeypatch, tmp_path, standalone_module):
+    """``StandaloneLoraManager.add_routes`` registers static and websocket routes."""
+
+    app = web.Application()
+    route_calls: List[Tuple[str, dict]] = []
+    app["route_calls"] = route_calls
+
+    locales_dir = tmp_path / "locales"
+    locales_dir.mkdir()
+    static_dir = tmp_path / "static"
+    static_dir.mkdir()
+
+    monkeypatch.setattr(
+        standalone_module,
+        "config",
+        SimpleNamespace(i18n_path=str(locales_dir), static_path=str(static_dir)),
+    )
+
+    register_calls: List[str] = []
+
+    import py.services.model_service_factory as factory_module
+
+    def fake_register_default_model_types() -> None:
+        register_calls.append("called")
+
+    monkeypatch.setattr(
+        factory_module,
+        "register_default_model_types",
+        fake_register_default_model_types,
+    )
+
+    def fake_setup_all_routes(cls, app_arg):
+        route_calls.append(("ModelServiceFactory.setup_all_routes", {"app": app_arg}))
+
+    monkeypatch.setattr(
+        factory_module.ModelServiceFactory,
+        "setup_all_routes",
+        classmethod(fake_setup_all_routes),
+    )
+
+    class DummyRecipeRoutes:
+        @staticmethod
+        def setup_routes(app_arg):
+            route_calls.append(("RecipeRoutes", {}))
+
+    class DummyUpdateRoutes:
+        @staticmethod
+        def setup_routes(app_arg):
+            route_calls.append(("UpdateRoutes", {}))
+
+    class DummyMiscRoutes:
+        @staticmethod
+        def setup_routes(app_arg):
+            route_calls.append(("MiscRoutes", {}))
+
+    class DummyExampleImagesRoutes:
+        @staticmethod
+        def setup_routes(app_arg, **kwargs):
+            route_calls.append(("ExampleImagesRoutes", kwargs))
+
+    class DummyPreviewRoutes:
+        @staticmethod
+        def setup_routes(app_arg):
+            route_calls.append(("PreviewRoutes", {}))
+
+    class DummyStatsRoutes:
+        def setup_routes(self, app_arg):
+            route_calls.append(("StatsRoutes", {}))
+
+    monkeypatch.setattr("py.routes.recipe_routes.RecipeRoutes", DummyRecipeRoutes)
+    monkeypatch.setattr("py.routes.update_routes.UpdateRoutes", DummyUpdateRoutes)
+    monkeypatch.setattr("py.routes.misc_routes.MiscRoutes", DummyMiscRoutes)
+    monkeypatch.setattr(
+        "py.routes.example_images_routes.ExampleImagesRoutes",
+        DummyExampleImagesRoutes,
+    )
+    monkeypatch.setattr("py.routes.preview_routes.PreviewRoutes", DummyPreviewRoutes)
+    monkeypatch.setattr("py.routes.stats_routes.StatsRoutes", DummyStatsRoutes)
+
+    ws_manager_stub = SimpleNamespace(
+        handle_connection=lambda request: None,
+        handle_download_connection=lambda request: None,
+        handle_init_connection=lambda request: None,
+    )
+    monkeypatch.setattr("py.services.websocket_manager.ws_manager", ws_manager_stub)
+
+    server = SimpleNamespace(app=app)
+
+    standalone_module.StandaloneLoraManager.add_routes(server)
+
+    assert register_calls, "default model types should be registered"
+
+    canonical_routes = {resource.canonical for resource in app.router.resources()}
+    assert "/locales" in canonical_routes
+    assert "/loras_static" in canonical_routes
+
+    websocket_paths = {route.resource.canonical for route in app.router.routes() if "ws" in route.resource.canonical}
+    assert {
+        "/ws/fetch-progress",
+        "/ws/download-progress",
+        "/ws/init-progress",
+    } <= websocket_paths
+
+    assert any(call[0] == "ModelServiceFactory.setup_all_routes" for call in route_calls)
+    assert any(call[0] == "RecipeRoutes" for call in route_calls)
+    assert any(call[0] == "StatsRoutes" for call in route_calls)
+
+    prompt_server = pytest.importorskip("server").PromptServer
+    assert getattr(prompt_server, "instance", None) is server
+
+    assert app.on_startup, "service initialization hook should be scheduled"
+    assert app.on_shutdown, "cleanup hook should be scheduled"