CMLDEV-990 Add docstrings and type annotations across the code base and tests (#196)

Author: tmikuska

tests/conftest.py

@@ -40,6 +40,11 @@
 
 
 def client_library_patched_system_info(version: str) -> Iterator[MagicMock]:
+    """Patch ClientLibrary.system_info to return a fixed version.
+
+    :param version: Version string to return from system_info.
+    :yields: The patch object for ClientLibrary.system_info.
+    """
     with patch.object(
         ClientLibrary, "system_info", return_value={"version": version, "ready": True}
     ) as cl:
@@ -48,44 +53,65 @@ def client_library_patched_system_info(version: str) -> Iterator[MagicMock]:
 
 @pytest.fixture
 def client_library_server_current() -> Iterator[MagicMock]:
+    """Simulate a controller running the current CML version.
+
+    :yields: The patch object for ClientLibrary.system_info.
+    """
     yield from client_library_patched_system_info(version=CURRENT_VERSION)
 
 
 @pytest.fixture
 def client_library_server_2_0_0() -> Iterator[MagicMock]:
+    """Simulate a controller running CML version 2.0.0.
+
+    :yields: The patch object for ClientLibrary.system_info.
+    """
     yield from client_library_patched_system_info(version="2.0.0")
 
 
 @pytest.fixture
 def client_library_server_2_19_0() -> Iterator[MagicMock]:
+    """Simulate a controller running CML version 2.19.0.
+
+    :yields: The patch object for ClientLibrary.system_info.
+    """
     yield from client_library_patched_system_info(version="2.19.0")
 
 
 @pytest.fixture
 def client_library_server_2_9_0() -> Iterator[MagicMock]:
-    """Simulate a controller running CML version 2.9.0."""
+    """Simulate a controller running CML version 2.9.0.
 
+    :yields: The patch object for ClientLibrary.system_info.
+    """
     yield from client_library_patched_system_info(version="2.9.0")
 
 
 @pytest.fixture
 def mocked_session() -> Iterator[MagicMock]:
+    """Patch authentication.CustomClient for tests that need a mock HTTP session.
+
+    :yields: The patched CustomClient class (MagicMock).
+    """
     with patch.object(authentication, "CustomClient", autospec=True) as session:
         yield session
 
 
 @pytest.fixture(scope="session")
 def test_data_dir() -> Path:
+    """Path to the test_data directory containing JSON fixtures.
+
+    :returns: Path to the test_data directory.
+    """
     return Path(__file__).parent / "test_data"
 
 
 def resp_body_from_file(test_data_dir: Path, request: httpx.Request) -> httpx.Response:
-    """
-    A callback that returns the contents of a file based on the request.
+    """Return response body from a file based on the request URL path.
 
-    :param request: The request that contains the title to search for
-    :returns: A Response that has `content` set to the contents of a file
-        that corresponds to a response body for the request.
+    :param test_data_dir: Directory containing JSON fixture files.
+    :param request: The HTTP request; URL path determines which file to load.
+    :returns: An httpx.Response with content set to the matching fixture file.
     """
     endpoint_parts = request.url.path.split("/")[3:]
     filename = "not initialized"
@@ -99,11 +125,14 @@ def resp_body_from_file(test_data_dir: Path, request: httpx.Request) -> httpx.Re
 
 
 @pytest.fixture
-def respx_mock_with_labs(respx_mock: MockRouter, test_data_dir: Path):
-    """
-    A test fixture that provides basic lab data with respx_mock so that unit tests can
-    call ``client.all_labs`` or ``client.join_existing_lab``.  The sample data includes
-    some runtime data, like node states and simulation_statistics.
+def respx_mock_with_labs(respx_mock: MockRouter, test_data_dir: Path) -> None:
+    """Provide basic lab data with respx_mock for unit tests.
+
+    Enables tests to call ``client.all_labs`` or ``client.join_existing_lab``.
+    Sample data includes runtime data (node states, simulation_statistics).
+
+    :param respx_mock: The respx mock router to configure.
+    :param test_data_dir: Directory containing JSON fixture files.
     """
     respx_mock.get(FAKE_HOST_API + "system_information").respond(
         json={"version": CURRENT_VERSION, "ready": True, "oui": "52:54:00:00:00:00"},
@@ -185,7 +214,12 @@ def respx_mock_with_labs(respx_mock: MockRouter, test_data_dir: Path):
 
 
 @pytest.fixture
-def client_library(respx_mock_with_labs: MockRouter) -> Iterator[ClientLibrary]:
+def client_library(respx_mock_with_labs: None) -> Iterator[ClientLibrary]:
+    """Provide a ClientLibrary instance with mocked lab API responses.
+
+    :param respx_mock_with_labs: Fixture that configures respx (consumed for setup).
+    :yields: A ClientLibrary connected to FAKE_HOST with test credentials.
+    """
     _ = respx_mock_with_labs
     client = ClientLibrary(url=FAKE_HOST, username="test", password="pa$$")
     yield client

tests/test_auth_management.py

@@ -18,6 +18,8 @@
 # limitations under the License.
 #
 
+from __future__ import annotations
+
 import time
 from unittest.mock import MagicMock
 
@@ -33,6 +35,11 @@
 
 
 def make_auth_management(settings: dict) -> tuple[AuthManagement, MagicMock]:
+    """Create AuthManagement instance with mocked session and given settings.
+
+    :param settings: Auth configuration dict (method, ldap/radius settings).
+    :returns: Tuple of (AuthManagement, mocked session).
+    """
     session = MagicMock()
     auth_management = AuthManagement(session, auto_sync=False)
     auth_management._settings = settings.copy()
@@ -46,20 +53,29 @@ def make_auth_management(settings: dict) -> tuple[AuthManagement, MagicMock]:
         ("radius", RADIUSManager),
     ],
 )
-def test_manager_returns_expected_manager(method: str, manager_cls):
+def test_manager_returns_expected_manager(
+    method: str, manager_cls: type[LDAPManager] | type[RADIUSManager]
+) -> None:
+    """Manager property returns LDAPManager or RADIUSManager based on method.
+
+    :param method: Auth method ("ldap" or "radius").
+    :param manager_cls: Expected manager class.
+    """
     auth_management, _ = make_auth_management({"method": method})
 
     assert isinstance(auth_management.manager, manager_cls)
 
 
-def test_update_settings_no_args_raises():
+def test_update_settings_no_args_raises() -> None:
+    """update_settings with no args raises TypeError."""
     auth_management, _ = make_auth_management({"method": "ldap"})
 
     with pytest.raises(TypeError, match="No settings to update"):
         auth_management.update_settings()
 
 
-def test_sync_updates_settings_and_timestamp():
+def test_sync_updates_settings_and_timestamp() -> None:
+    """Sync fetches config from server and updates _last_sync_time."""
     auth_management, session = make_auth_management({"method": "ldap"})
     session.get.return_value.json.return_value = {"method": "local"}
 
@@ -73,7 +89,11 @@ def test_sync_updates_settings_and_timestamp():
 
 
 @pytest.mark.parametrize("search_filter", [None, "(cn=admins)"])
-def test_get_ldap_groups(search_filter: str | None):
+def test_get_ldap_groups(search_filter: str | None) -> None:
+    """get_ldap_groups returns groups, optionally filtered.
+
+    :param search_filter: Optional LDAP filter string.
+    """
     auth_management, session = make_auth_management({"method": "ldap"})
     session.get.return_value.json.return_value = ["group-1", "group-2"]
 
@@ -88,15 +108,17 @@ def test_get_ldap_groups(search_filter: str | None):
         )
 
 
-def test_refresh_ldap_groups():
+def test_refresh_ldap_groups() -> None:
+    """refresh_ldap_groups sends PUT to system/auth/refresh."""
     auth_management, session = make_auth_management({"method": "ldap"})
 
     auth_management.refresh_ldap_groups()
 
     session.put.assert_called_once_with("system/auth/refresh")
 
 
-def test_test_auth_with_user_credentials():
+def test_auth_with_user_credentials() -> None:
+    """test_auth with username/password sends auth-data in request."""
     auth_management, session = make_auth_management({"method": "ldap"})
     session.post.return_value.json.return_value = {"auth_ok": True}
 
@@ -114,7 +136,8 @@ def test_test_auth_with_user_credentials():
     )
 
 
-def test_test_auth_with_group_name():
+def test_auth_with_group_name() -> None:
+    """test_auth with group_name sends group-data in request."""
     auth_management, session = make_auth_management({"method": "ldap"})
     session.post.return_value.json.return_value = {"auth_ok": True}
 
@@ -132,7 +155,8 @@ def test_test_auth_with_group_name():
     )
 
 
-def test_test_current_auth_includes_manager_password():
+def test_current_auth_includes_manager_password() -> None:
+    """test_current_auth includes manager_password in auth-config."""
     auth_management, session = make_auth_management(
         {"method": "ldap", "verify_tls": True}
     )
@@ -183,7 +207,12 @@ def test_test_current_auth_includes_manager_password():
         ("verify_tls", False),
     ],
 )
-def test_ldap_settings_update(setting: str, value):
+def test_ldap_settings_update(setting: str, value: str | bool | float) -> None:
+    """LDAP manager setter PATCHes config with setting and value.
+
+    :param setting: LDAP setting name.
+    :param value: Value to set.
+    """
     auth_management, session = make_auth_management({"method": "ldap", setting: value})
     manager = auth_management._managers["ldap"]
 
@@ -195,14 +224,19 @@ def test_ldap_settings_update(setting: str, value):
     assert auth_management._settings[setting] == value
 
 
-def test_ldap_timeout_inactive_method_raises():
+def test_ldap_timeout_inactive_method_raises() -> None:
+    """Accessing LDAP timeout when method is local raises MethodNotActive.
+
+    :raises MethodNotActive: When LDAP is not the active auth method.
+    """
     auth_management, _ = make_auth_management({"method": "local", "timeout": 5})
 
     with pytest.raises(MethodNotActive):
         _ = auth_management._managers["ldap"].timeout
 
 
-def test_ldap_resource_pool_accepts_resource_pool_instance():
+def test_ldap_resource_pool_accepts_instance() -> None:
+    """LDAP resource_pool setter accepts ResourcePool instance, uses its id."""
     auth_management, session = make_auth_management(
         {"method": "ldap", "resource_pool": "old"}
     )
@@ -240,7 +274,12 @@ def test_ldap_resource_pool_accepts_resource_pool_instance():
         ("resource_pool", "pool-id"),
     ],
 )
-def test_radius_settings_update(setting: str, value):
+def test_radius_settings_update(setting: str, value: str | int | float) -> None:
+    """RADIUS manager setter PATCHes config with setting and value.
+
+    :param setting: RADIUS setting name.
+    :param value: Value to set.
+    """
     auth_management, session = make_auth_management(
         {"method": "radius", setting: value}
     )
@@ -254,7 +293,8 @@ def test_radius_settings_update(setting: str, value):
     assert auth_management._settings[setting] == value
 
 
-def test_radius_secret_setter_updates_setting():
+def test_radius_secret_setter_updates_setting() -> None:
+    """RADIUS secret setter PATCHes config."""
     auth_management, session = make_auth_management({"method": "radius"})
     manager = auth_management._managers["radius"]
 
@@ -265,14 +305,19 @@ def test_radius_secret_setter_updates_setting():
     )
 
 
-def test_radius_timeout_inactive_method_raises():
+def test_radius_timeout_inactive_method_raises() -> None:
+    """Accessing RADIUS timeout when method is local raises MethodNotActive.
+
+    :raises MethodNotActive: When RADIUS is not the active auth method.
+    """
     auth_management, _ = make_auth_management({"method": "local", "timeout": 5})
 
     with pytest.raises(MethodNotActive):
         _ = auth_management._managers["radius"].timeout
 
 
-def test_radius_resource_pool_accepts_resource_pool_instance():
+def test_radius_resource_pool_accepts_instance() -> None:
+    """RADIUS resource_pool setter accepts ResourcePool instance, uses its id."""
     auth_management, session = make_auth_management(
         {"method": "radius", "resource_pool": "old"}
     )
@@ -300,7 +345,8 @@ def test_radius_resource_pool_accepts_resource_pool_instance():
     assert auth_management._settings["resource_pool"] == "pool-456"
 
 
-def test_ldap_manager_password_setter_updates_setting():
+def test_ldap_manager_password_setter_updates() -> None:
+    """LDAP manager_password setter PATCHes config."""
     auth_management, session = make_auth_management({"method": "ldap"})
     manager = auth_management._managers["ldap"]
 
@@ -311,7 +357,8 @@ def test_ldap_manager_password_setter_updates_setting():
     )
 
 
-def test_update_settings_precedence_and_sync_called():
+def test_update_settings_precedence_and_sync() -> None:
+    """Keyword args override dict args; sync fetches updated config."""
     auth_management, session = make_auth_management({"method": "ldap"})
     session.get.return_value.json.return_value = {"method": "ldap", "verify_tls": False}
 
@@ -324,7 +371,11 @@ def test_update_settings_precedence_and_sync_called():
     assert auth_management._settings["verify_tls"] is False
 
 
-def test_sync_if_outdated_triggers_sync(monkeypatch: pytest.MonkeyPatch):
+def test_sync_if_outdated_triggers_sync(monkeypatch: pytest.MonkeyPatch) -> None:
+    """sync_if_outdated triggers sync when interval exceeded.
+
+    :param monkeypatch: Pytest monkeypatch fixture.
+    """
     auth_management, session = make_auth_management({"method": "ldap"})
     session.get.return_value.json.return_value = {"method": "ldap"}
     auth_management.auto_sync = True
@@ -338,7 +389,11 @@ def test_sync_if_outdated_triggers_sync(monkeypatch: pytest.MonkeyPatch):
     session.get.assert_called_once_with("system/auth/config")
 
 
-def test_sync_if_outdated_skips_when_recent(monkeypatch: pytest.MonkeyPatch):
+def test_sync_if_outdated_skips_when_recent(monkeypatch: pytest.MonkeyPatch) -> None:
+    """sync_if_outdated skips sync when within interval.
+
+    :param monkeypatch: Pytest monkeypatch fixture.
+    """
     auth_management, session = make_auth_management({"method": "ldap"})
     auth_management.auto_sync = True
     auth_management.auto_sync_interval = 10.0
@@ -351,7 +406,11 @@ def test_sync_if_outdated_skips_when_recent(monkeypatch: pytest.MonkeyPatch):
     session.get.assert_not_called()
 
 
-def test_accessing_wrong_manager_raises():
+def test_accessing_wrong_manager_raises() -> None:
+    """Accessing RADIUS manager when LDAP is active raises MethodNotActive.
+
+    :raises MethodNotActive: When the requested manager is not active.
+    """
     auth_management, _ = make_auth_management({"method": "ldap"})
 
     with pytest.raises(MethodNotActive):