Add tests for error_handling and director modules (#55)

Tests were written first against the 1.x API to verify correctness on
the existing codebase, then adjusted for the 2.0 snake_case renames and
sync error_handling refactor so that this commit applies cleanly on top
of the 2.x-refactor branch.

test_error_handling.py — covers all branches of check_response_for_error:
  - JSON and XML happy paths (no error keys)
  - C4ErrorResponse format: BadCredentials, Unauthorized, NotFound,
    unknown code fallback to C4Exception
  - Flat JSON code format: NotFound, BadCredentials (details priority)
  - Director error format: BadToken, Unauthorized, InvalidCategory,
    unknown error fallback to C4Exception
  - XML C4ErrorResponse parsing
  - Exception hierarchy and message preservation

test_director.py — covers get_item_variable_value edge cases:
  - Return types: int, bool, string, zero (not confused with None)
  - Undefined normalization to None
  - JSON null passthrough as None
  - Empty response and invalid format raise ValueError
  - List/tuple var_name joining
  - get_all_item_variable_value mixed Undefined normalization
  - get_all_item_info returns parsed JSON (2.0 behavior)

Also adds tests/conftest.py with shared director fixture and removes
the duplicate fixture from test_undefined_handling.py.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: davidrecordon

tests/conftest.py

@@ -0,0 +1,9 @@
+import pytest
+
+from pyControl4.director import C4Director
+
+
+@pytest.fixture
+def director():
+    """Create a C4Director with no real session (for mocked tests)."""
+    return C4Director("192.168.1.1", "test-token")

tests/test_director.py

@@ -0,0 +1,143 @@
+"""Tests for C4Director — get_item_variable_value, get_all_item_variable_value,
+and basic request wrappers.
+"""
+
+import json
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_int(director):
+    """Normal integer value is returned as-is."""
+    response = json.dumps([{"id": 100, "varName": "LIGHT_LEVEL", "value": 75}])
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_item_variable_value(100, "LIGHT_LEVEL")
+    assert result == 75
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_zero(director):
+    """Zero is returned as 0, not confused with None or falsy."""
+    response = json.dumps([{"id": 100, "varName": "LIGHT_LEVEL", "value": 0}])
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_item_variable_value(100, "LIGHT_LEVEL")
+    assert result == 0
+    assert result is not None
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_bool(director):
+    """Boolean value is returned as a Python bool."""
+    response = json.dumps([{"id": 100, "varName": "IS_ON", "value": True}])
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_item_variable_value(100, "IS_ON")
+    assert result is True
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_string(director):
+    """String value is returned as-is."""
+    response = json.dumps(
+        [{"id": 100, "varName": "PARTITION_STATE", "value": "ARMED_AWAY"}]
+    )
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_item_variable_value(100, "PARTITION_STATE")
+    assert result == "ARMED_AWAY"
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_null(director):
+    """JSON null value passes through as None (distinct from 'Undefined')."""
+    response = json.dumps([{"id": 100, "varName": "OPTIONAL_VAR", "value": None}])
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_item_variable_value(100, "OPTIONAL_VAR")
+    assert result is None
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_empty_response(director):
+    """Empty list response raises ValueError."""
+    with patch.object(director, "send_get_request", new=AsyncMock(return_value="[]")):
+        with pytest.raises(ValueError):
+            await director.get_item_variable_value(100, "NONEXISTENT")
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_invalid_format(director):
+    """Non-list JSON response raises ValueError (2.0 guard)."""
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value='{"value": 1}')
+    ):
+        with pytest.raises(ValueError):
+            await director.get_item_variable_value(100, "TEST")
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_list_var_name(director):
+    """List of var_names is joined with comma in the request URI."""
+    response = json.dumps([{"id": 100, "varName": "A", "value": 1}])
+    mock = AsyncMock(return_value=response)
+    with patch.object(director, "send_get_request", new=mock):
+        await director.get_item_variable_value(100, ["A", "B"])
+    uri = mock.call_args[0][0]
+    assert "varnames=A,B" in uri
+
+
+@pytest.mark.asyncio
+async def test_get_item_variable_value_tuple_var_name(director):
+    """Tuple of var_names is joined with comma in the request URI."""
+    response = json.dumps([{"id": 100, "varName": "X", "value": 42}])
+    mock = AsyncMock(return_value=response)
+    with patch.object(director, "send_get_request", new=mock):
+        await director.get_item_variable_value(100, ("X", "Y"))
+    uri = mock.call_args[0][0]
+    assert "varnames=X,Y" in uri
+
+
+@pytest.mark.asyncio
+async def test_get_all_item_variable_value_mixed(director):
+    """get_all_item_variable_value normalizes Undefined values in-place."""
+    response = json.dumps(
+        [
+            {"id": 1, "varName": "HUMIDITY", "value": "Undefined"},
+            {"id": 2, "varName": "HUMIDITY", "value": 45},
+            {"id": 3, "varName": "HUMIDITY", "value": 0},
+        ]
+    )
+    with patch.object(
+        director, "send_get_request", new=AsyncMock(return_value=response)
+    ):
+        result = await director.get_all_item_variable_value("HUMIDITY")
+    assert result[0]["value"] is None
+    assert result[1]["value"] == 45
+    assert result[2]["value"] == 0
+
+
+@pytest.mark.asyncio
+async def test_get_all_item_variable_value_empty(director):
+    """Empty list response raises ValueError."""
+    with patch.object(director, "send_get_request", new=AsyncMock(return_value="[]")):
+        with pytest.raises(ValueError):
+            await director.get_all_item_variable_value("NONEXISTENT")
+
+
+@pytest.mark.asyncio
+async def test_get_all_item_info_returns_parsed(director):
+    """get_all_item_info returns parsed list (2.0 returns parsed JSON)."""
+    raw = '[{"id": 1, "name": "Light"}]'
+    with patch.object(director, "send_get_request", new=AsyncMock(return_value=raw)):
+        result = await director.get_all_item_info()
+    assert isinstance(result, list)
+    assert result[0]["id"] == 1

tests/test_error_handling.py

@@ -0,0 +1,191 @@
+"""Tests for error_handling.py — all branches of check_response_for_error."""
+
+import json
+
+import pytest
+
+from pyControl4.error_handling import (
+    BadCredentials,
+    BadToken,
+    C4Exception,
+    InvalidCategory,
+    NotFound,
+    Unauthorized,
+    check_response_for_error,
+)
+
+# --- Happy paths (no exception raised) ---
+
+
+def test_json_no_error_keys():
+    """JSON response without error keys should not raise."""
+    check_response_for_error(json.dumps({"result": "ok"}))
+
+
+def test_xml_no_error_keys():
+    """XML response without error keys should not raise."""
+    check_response_for_error("<result>ok</result>")
+
+
+# --- C4ErrorResponse format ---
+
+
+def test_c4error_bad_credentials():
+    """C4ErrorResponse with matching details raises BadCredentials."""
+    payload = json.dumps(
+        {
+            "C4ErrorResponse": {
+                "code": 401,
+                "details": "Permission denied Bad credentials",
+                "message": "Permission denied",
+            }
+        }
+    )
+    with pytest.raises(BadCredentials):
+        check_response_for_error(payload)
+
+
+def test_c4error_401_no_matching_details():
+    """C4ErrorResponse with code 401 but non-matching details raises Unauthorized."""
+    payload = json.dumps(
+        {
+            "C4ErrorResponse": {
+                "code": 401,
+                "details": "",
+                "message": "Permission denied",
+            }
+        }
+    )
+    with pytest.raises(Unauthorized):
+        check_response_for_error(payload)
+
+
+def test_c4error_404():
+    """C4ErrorResponse with code 404 raises NotFound."""
+    payload = json.dumps(
+        {
+            "C4ErrorResponse": {
+                "code": 404,
+                "message": "Not found",
+            }
+        }
+    )
+    with pytest.raises(NotFound):
+        check_response_for_error(payload)
+
+
+def test_c4error_unknown_code_falls_back_to_base():
+    """C4ErrorResponse with unrecognized code raises exactly C4Exception (not a subclass)."""
+    payload = json.dumps(
+        {
+            "C4ErrorResponse": {
+                "code": 999,
+                "message": "Unknown error",
+            }
+        }
+    )
+    with pytest.raises(C4Exception) as exc_info:
+        check_response_for_error(payload)
+    assert type(exc_info.value) is C4Exception
+
+
+# --- Flat JSON code format ---
+
+
+def test_flat_json_404():
+    """Flat JSON with code 404 raises NotFound."""
+    payload = json.dumps({"code": 404, "message": "Account not found"})
+    with pytest.raises(NotFound):
+        check_response_for_error(payload)
+
+
+def test_flat_json_bad_credentials():
+    """Flat JSON with matching details raises BadCredentials (details take priority)."""
+    payload = json.dumps(
+        {
+            "code": 401,
+            "details": "Permission denied Bad credentials",
+            "message": "Permission denied",
+        }
+    )
+    with pytest.raises(BadCredentials):
+        check_response_for_error(payload)
+
+
+# --- Director error format ---
+
+
+def test_director_error_bad_token():
+    """Director error with matching details raises BadToken."""
+    payload = json.dumps(
+        {"error": "Unauthorized", "details": "Expired or invalid token"}
+    )
+    with pytest.raises(BadToken):
+        check_response_for_error(payload)
+
+
+def test_director_error_unauthorized_no_matching_details():
+    """Director 'Unauthorized' without matching details raises Unauthorized."""
+    payload = json.dumps({"error": "Unauthorized"})
+    with pytest.raises(Unauthorized):
+        check_response_for_error(payload)
+
+
+def test_director_error_invalid_category():
+    """Director 'Invalid category' raises InvalidCategory."""
+    payload = json.dumps({"error": "Invalid category"})
+    with pytest.raises(InvalidCategory):
+        check_response_for_error(payload)
+
+
+def test_director_error_unknown_falls_back_to_base():
+    """Director error with unrecognized string raises exactly C4Exception (not a subclass)."""
+    payload = json.dumps({"error": "Something else"})
+    with pytest.raises(C4Exception) as exc_info:
+        check_response_for_error(payload)
+    assert type(exc_info.value) is C4Exception
+
+
+# --- XML C4ErrorResponse ---
+
+
+def test_xml_c4error_401():
+    """XML C4ErrorResponse with code 401 raises Unauthorized."""
+    xml = (
+        '<?xml version="1.0" encoding="UTF-8" standalone="yes"?>'
+        "<C4ErrorResponse>"
+        "<code>401</code>"
+        "<details></details>"
+        "<message>Permission denied</message>"
+        "<subCode>0</subCode>"
+        "</C4ErrorResponse>"
+    )
+    with pytest.raises(Unauthorized):
+        check_response_for_error(xml)
+
+
+# --- Exception hierarchy and behavior ---
+
+
+def test_exception_hierarchy():
+    """Verify the exception inheritance chain."""
+    assert issubclass(BadCredentials, Unauthorized)
+    assert issubclass(BadToken, Unauthorized)
+    assert issubclass(Unauthorized, C4Exception)
+    assert issubclass(NotFound, C4Exception)
+    assert issubclass(InvalidCategory, C4Exception)
+    assert issubclass(C4Exception, Exception)
+
+
+def test_exception_stores_message():
+    """C4Exception stores the response text as .message."""
+    exc = C4Exception("some response text")
+    assert exc.message == "some response text"
+
+
+def test_raised_exception_preserves_response_text():
+    """Exception raised by check_response_for_error carries the original response."""
+    payload = json.dumps({"error": "Invalid category"})
+    with pytest.raises(InvalidCategory) as exc_info:
+        check_response_for_error(payload)
+    assert exc_info.value.message == payload

tests/test_undefined_handling.py

@@ -5,17 +5,10 @@
 
 import pytest
 
-from pyControl4.director import C4Director
 from pyControl4.light import C4Light
 from pyControl4.blind import C4Blind
 
 
-@pytest.fixture
-def director():
-    """Create a C4Director with a mocked session."""
-    return C4Director("192.168.1.1", "test-token")
-
-
 @pytest.mark.asyncio
 async def test_get_item_variable_value_undefined(director):
     """Test that get_item_variable_value normalizes 'Undefined' to None."""