From 4db2ea529eee2a5d6cfb28e513530218c5a43341 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 11 Jun 2024 13:45:46 -0300 Subject: [PATCH] Fix exception test --- .../service/redis/models/patron_activity.py | 19 +++++--- .../celery/tasks/test_patron_activity.py | 5 ++- .../redis/models/test_patron_activity.py | 45 ++++++++++++++----- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/palace/manager/service/redis/models/patron_activity.py b/src/palace/manager/service/redis/models/patron_activity.py index 6076cdf0f2..c6db76e777 100644 --- a/src/palace/manager/service/redis/models/patron_activity.py +++ b/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) ) diff --git a/tests/manager/celery/tasks/test_patron_activity.py b/tests/manager/celery/tasks/test_patron_activity.py index b139517ef6..69465a12cd 100644 --- a/tests/manager/celery/tasks/test_patron_activity.py +++ b/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 diff --git a/tests/manager/service/redis/models/test_patron_activity.py b/tests/manager/service/redis/models/test_patron_activity.py index ec11f9f04b..0a7b32e4f7 100644 --- a/tests/manager/service/redis/models/test_patron_activity.py +++ b/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):