add comments to test functions in order to better document the intended design (a-luna#48)

Fixes a-luna#41

Author: a-luna

src/fastapi_redis_cache/cache.py

@@ -89,6 +89,7 @@ async def get_api_response_async(func, *args, **kwargs):
 
 
 def calculate_ttl(expire: Union[int, timedelta]) -> int:
+    """"Converts expire time to total seconds and ensures that ttl is capped at one year."""
     if isinstance(expire, timedelta):
         expire = int(expire.total_seconds())
     return min(expire, ONE_YEAR_IN_SECONDS)

src/fastapi_redis_cache/client.py

@@ -158,9 +158,3 @@ def get_etag(cached_data: Union[str, bytes, Dict]) -> str:
     def get_log_time():
         """Get a timestamp to include with a log message."""
         return datetime.now().strftime(LOG_TIMESTAMP)
-
-    @staticmethod
-    def hasmethod(obj, method_name):
-        """Return True if obj.method_name exists and is callable. Otherwise, return False."""
-        obj_method = getattr(obj, method_name, None)
-        return callable(obj_method) if obj_method else False

tests/main.py

@@ -16,9 +16,9 @@ def cache_never_expire(request: Request, response: Response):
 
 
 @app.get("/cache_expires")
-@cache(expire=timedelta(seconds=8))
+@cache(expire=timedelta(seconds=5))
 async def cache_expires():
-    return {"success": True, "message": "this data should be cached for eight seconds"}
+    return {"success": True, "message": "this data should be cached for five seconds"}
 
 
 @app.get("/cache_json_encoder")

tests/test_cache.py

@@ -4,7 +4,9 @@
 from datetime import datetime
 from decimal import Decimal
 
+import pytest
 from fastapi.testclient import TestClient
+from fastapi_redis_cache.client import HTTP_TIME
 
 from fastapi_redis_cache.util import deserialize_json
 from tests.main import app
@@ -14,13 +16,16 @@
 
 
 def test_cache_never_expire():
+    # Initial request, X-FastAPI-Cache header field should equal "Miss"
     response = client.get("/cache_never_expire")
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Send request to same endpoint, X-FastAPI-Cache header field should now equal "Hit"
     response = client.get("/cache_never_expire")
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
@@ -31,38 +36,72 @@ def test_cache_never_expire():
 
 
 def test_cache_expires():
-    start = datetime.now()
+    # Store time when response data was added to cache
+    added_at_utc = datetime.utcnow()
+
+    # Initial request, X-FastAPI-Cache header field should equal "Miss"
     response = client.get("/cache_expires")
     assert response.status_code == 200
-    assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"}
+    assert response.json() == {"success": True, "message": "this data should be cached for five seconds"}
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Store eTag value from response header
     check_etag = response.headers["etag"]
+
+    # Send request, X-FastAPI-Cache header field should now equal "Hit"
     response = client.get("/cache_expires")
     assert response.status_code == 200
-    assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"}
+    assert response.json() == {"success": True, "message": "this data should be cached for five seconds"}
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit"
-    assert "cache-control" in response.headers
-    assert "expires" in response.headers
+
+    # Verify eTag value matches the value stored from the initial response
     assert "etag" in response.headers
     assert response.headers["etag"] == check_etag
-    elapsed = (datetime.now() - start).total_seconds()
-    remaining = 8 - elapsed
-    if remaining > 0:
-        time.sleep(remaining)
+
+    # Store 'max-age' value of 'cache-control' header field
+    assert "cache-control" in response.headers
+    match = MAX_AGE_REGEX.search(response.headers.get("cache-control"))
+    assert match
+    ttl = int(match.groupdict()["ttl"])
+    assert ttl <= 5
+
+    # Store value of 'expires' header field
+    assert "expires" in response.headers
+    expire_at_utc = datetime.strptime(response.headers["expires"], HTTP_TIME)
+
+    # Wait until expire time has passed
+    now = datetime.utcnow()
+    while expire_at_utc > now:
+        time.sleep(1)
+        now = datetime.utcnow()
+
+    # Wait one additional second to ensure redis has deleted the expired response data
+    time.sleep(1)
+    second_request_utc = datetime.utcnow()
+
+    # Verify that the time elapsed since the data was added to the cache is greater than the ttl value
+    elapsed = (second_request_utc - added_at_utc).total_seconds()
+    assert elapsed > ttl
+
+    # Send request, X-FastAPI-Cache header field should equal "Miss" since the cached value has been evicted
     response = client.get("/cache_expires")
     assert response.status_code == 200
-    assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"}
+    assert response.json() == {"success": True, "message": "this data should be cached for five seconds"}
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Check eTag value again. Since data is the same, the value should still match
     assert response.headers["etag"] == check_etag
 
 
 def test_cache_json_encoder():
+    # In order to verify that our custom BetterJsonEncoder is working correctly, the  /cache_json_encoder
+    # endpoint returns a dict containing datetime.datetime, datetime.date and decimal.Decimal objects.
     response = client.get("/cache_json_encoder")
     assert response.status_code == 200
     response_json = response.json()
@@ -75,13 +114,18 @@ def test_cache_json_encoder():
             "val": "3.140000000000000124344978758017532527446746826171875",
         },
     }
+
+    # To verify that our custom object_hook function which deserializes types that are not typically
+    # JSON-serializable is working correctly, we test it with the serialized values sent in the response.
     json_dict = deserialize_json(json.dumps(response_json))
     assert json_dict["start_time"] == datetime(2021, 4, 20, 7, 17, 17)
     assert json_dict["finish_by"] == datetime(2021, 4, 21)
     assert json_dict["final_calc"] == Decimal(3.14)
 
 
 def test_cache_control_no_cache():
+    # Simple test that verifies if a request is recieved with the cache-control header field containing "no-cache",
+    # no caching behavior is performed
     response = client.get("/cache_never_expire", headers={"cache-control": "no-cache"})
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
@@ -92,6 +136,8 @@ def test_cache_control_no_cache():
 
 
 def test_cache_control_no_store():
+    # Simple test that verifies if a request is recieved with the cache-control header field containing "no-store",
+    # no caching behavior is performed
     response = client.get("/cache_never_expire", headers={"cache-control": "no-store"})
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
@@ -102,29 +148,39 @@ def test_cache_control_no_store():
 
 
 def test_if_none_match():
+    # Initial request, response data is added to cache
     response = client.get("/cache_never_expire")
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Store correct eTag value from response header
     etag = response.headers["etag"]
+    # Create another eTag value that is different from the correct value
     invalid_etag = "W/-5480454928453453778"
+
+    # Send request to same endpoint where If-None-Match header contains both valid and invalid eTag values
     response = client.get("/cache_never_expire", headers={"if-none-match": f"{etag}, {invalid_etag}"})
     assert response.status_code == 304
     assert not response.content
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Send request to same endpoint where If-None-Match header contains just the wildcard (*) character
     response = client.get("/cache_never_expire", headers={"if-none-match": "*"})
     assert response.status_code == 304
     assert not response.content
     assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit"
     assert "cache-control" in response.headers
     assert "expires" in response.headers
     assert "etag" in response.headers
+
+    # Send request to same endpoint where If-None-Match header contains only the invalid eTag value
     response = client.get("/cache_never_expire", headers={"if-none-match": invalid_etag})
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data can be cached indefinitely"}
@@ -135,6 +191,8 @@ def test_if_none_match():
 
 
 def test_partial_cache_one_hour():
+    # Simple test that verifies that the @cache_for_one_hour partial function version of the @cache decorator
+    # is working correctly.
     response = client.get("/cache_one_hour")
     assert response.status_code == 200
     assert response.json() == {"success": True, "message": "this data should be cached for one hour"}