Fix exception test

src/palace/manager/service/redis/models/patron_activity.py

@@ -1,4 +1,4 @@
-from datetime import datetime, timedelta
+import datetime
 from enum import IntEnum
 from functools import cached_property
 from types import TracebackType
@@ -62,7 +62,7 @@ def redis(self) -> str:
             return f"{self.start}, {self.redis_end}"
 
     STATE_FIELD_LEN = 2
-    TIMESTAMP_FIELD_LEN = 25
+    TIMESTAMP_FIELD_LEN = 19
 
     SEPERATOR = "::"
     SEPERATOR_LEN = len(SEPERATOR)
@@ -81,7 +81,7 @@ def __init__(
         *,
         state: State,
         task_id: str,
-        timestamp: datetime | None = None,
+        timestamp: datetime.datetime | None = None,
     ):
         self.state = state
         self.task_id = task_id
@@ -94,12 +94,15 @@ def __init__(
     def from_redis(cls, data: str) -> Self:
         state = data[cls.STATE_OFFSET.slice]
         timestamp = data[cls.TIMESTAMP_OFFSET.slice]
+        aware_timestamp = datetime.datetime.fromisoformat(timestamp).replace(
+            tzinfo=datetime.timezone.utc
+        )
         task_id = data[cls.TASK_ID_OFFSET.slice]
 
         return cls(
             state=cls.State(int(state)),
             task_id=task_id,
-            timestamp=datetime.fromisoformat(timestamp),
+            timestamp=aware_timestamp,
         )
 
     def to_redis(self) -> str:
@@ -109,7 +112,11 @@ def to_redis(self) -> str:
                 f"State field is not the correct length: {state_str}. Expected {self.STATE_FIELD_LEN} characters."
             )
 
-        timestamp_str = self.timestamp.isoformat(timespec="seconds")
+        # Convert the timestamp to UTC before converting to a string.
+        utc_local = self.timestamp.astimezone(datetime.timezone.utc).replace(
+            tzinfo=None
+        )
+        timestamp_str = utc_local.isoformat(timespec="seconds")
         if len(timestamp_str) != self.TIMESTAMP_FIELD_LEN:
             raise ValueError(
                 f"Timestamp field is not the correct length: {timestamp_str}. Expected {self.TIMESTAMP_FIELD_LEN} characters."
@@ -126,7 +133,7 @@ def __eq__(self, other: Any) -> bool:
             and self.task_id == other.task_id
             # Since we truncate the timestamp to the second when converting to a string,
             # we need to compare the timestamps with a second of tolerance.
-            and self.timestamp - other.timestamp < timedelta(seconds=1)
+            and self.timestamp - other.timestamp < datetime.timedelta(seconds=1)
         )
 
 

tests/manager/celery/tasks/test_patron_activity.py

@@ -122,9 +122,9 @@ def test_collection_not_found(
     def test_exception(
         self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture
     ):
-        sync_task_fixture.collection.protocol = "unknown"
+        sync_task_fixture.mock_registry.from_collection.side_effect = Exception("Boom!")
 
-        with pytest.raises(KeyError):
+        with pytest.raises(Exception, match="Boom!"):
             sync_patron_activity.apply_async(
                 (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin")
             ).wait()
@@ -134,6 +134,7 @@ def test_exception(
         assert task_status.state == PatronActivityStatus.State.FAILED
 
         assert "An exception occurred during the patron activity sync" in caplog.text
+        assert "Boom!" in caplog.text
 
     def test_not_supported(
         self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture

tests/manager/service/redis/models/test_patron_activity.py

@@ -1,4 +1,5 @@
 import datetime
+from unittest.mock import patch
 
 import pytest
 from freezegun import freeze_time
@@ -37,9 +38,9 @@ def test_init(self):
             assert status.timestamp == utc_now()
 
     def test_offsets(self):
-        now = utc_now()
+        time = datetime_utc(2020, 1, 1, 1, 1)
         status = PatronActivityStatus(
-            state=PatronActivityStatus.State.LOCKED, task_id="abc", timestamp=now
+            state=PatronActivityStatus.State.LOCKED, task_id="abc", timestamp=time
         )
 
         data_str = status.to_redis()
@@ -49,13 +50,15 @@ def test_offsets(self):
         task_id = data_str[PatronActivityStatus.TASK_ID_OFFSET.slice]
 
         assert state == str(PatronActivityStatus.State.LOCKED)
-        assert timestamp == now.isoformat(timespec="seconds")
+        assert timestamp == "2020-01-01T01:01:00"
         assert task_id == "abc"
 
     def test_offsets_redis(self, redis_fixture: RedisFixture):
-        now = utc_now()
+        time = datetime_utc(1919, 1, 2, 3, 4)
         status = PatronActivityStatus(
-            state=PatronActivityStatus.State.NOT_SUPPORTED, task_id="abc", timestamp=now
+            state=PatronActivityStatus.State.NOT_SUPPORTED,
+            task_id="abc",
+            timestamp=time,
         )
         client = redis_fixture.client
 
@@ -67,11 +70,14 @@ def test_offsets_redis(self, redis_fixture: RedisFixture):
             PatronActivityStatus.STATE_OFFSET.start,
             PatronActivityStatus.STATE_OFFSET.redis_end,
         ) == str(PatronActivityStatus.State.NOT_SUPPORTED)
-        assert redis_fixture.client.getrange(
-            key,
-            PatronActivityStatus.TIMESTAMP_OFFSET.start,
-            PatronActivityStatus.TIMESTAMP_OFFSET.redis_end,
-        ) == now.isoformat(timespec="seconds")
+        assert (
+            redis_fixture.client.getrange(
+                key,
+                PatronActivityStatus.TIMESTAMP_OFFSET.start,
+                PatronActivityStatus.TIMESTAMP_OFFSET.redis_end,
+            )
+            == "1919-01-02T03:04:00"
+        )
         assert (
             redis_fixture.client.getrange(
                 key,
@@ -88,6 +94,25 @@ def test_round_trip(self):
         status_from_redis = PatronActivityStatus.from_redis(status.to_redis())
         assert status_from_redis == status
 
+    def test_to_redis(self):
+        # If the state cannot be converted into a 2 character string, we should raise an error
+        time = datetime_utc(2024, 6, 11, 1, 24)
+        with freeze_time(time):
+            status = PatronActivityStatus(
+                state=PatronActivityStatus.State.FAILED, task_id="test-123"
+            )
+        with patch.object(status, "state", "foo bar baz"):
+            with pytest.raises(ValueError):
+                status.to_redis()
+
+        # If the timestamp cannot be converted into a 19 character string, we should raise an error
+        with patch.object(status, "timestamp") as mock_timestamp:
+            mock_timestamp.isoformat.return_value = "foo bar baz"
+            with pytest.raises(ValueError):
+                status.to_redis()
+
+        assert status.to_redis() == "05::2024-06-11T01:24:00::test-123"
+
 
 class PatronActivityFixture:
     def __init__(self, db: DatabaseTransactionFixture, redis_fixture: RedisFixture):