Skip to content

Commit 03339f2

Browse files
Bernd VerstCopilot
andcommitted
Test that a non-positive signature cache bound is clamped
_EntityMethodSignatureCache clamps its bound with max(1, max_size), but nothing covered that clamp, so the PR description overstated the test coverage. Add regression tests for it. The clamp is not cosmetic. Without it a bound of 0 evicts each entry immediately after inserting it, so every lookup misses and the entity method reflection this cache exists to avoid runs on every operation again -- silently reintroducing the very problem this change fixes. A negative bound is worse: the eviction loop keeps popping past an already-empty dict and raises StopIteration. The new tests cover a zero and negative bound, that a clamped cache still behaves as a working size-one cache rather than a no-op, and that an executor handed a clamped cache still reflects only once. Verified red before green: with the clamp removed all five fail, including 'assert 3 == 1' on the reflection count. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2265f7b9-f8ad-4b50-a25d-19bd6f1ac96c
1 parent 1709591 commit 03339f2

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

tests/durabletask/test_entity_executor.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import logging
88
import threading
99

10+
import pytest
1011
from google.protobuf import wrappers_pb2
1112

1213
import durabletask.internal.orchestrator_service_pb2 as pb
@@ -523,3 +524,57 @@ def writer(worker_index: int) -> None:
523524
thread.join()
524525

525526
assert len(cache) == 8
527+
528+
@pytest.mark.parametrize("max_size", [0, -1, -1024])
529+
def test_non_positive_max_size_is_clamped_to_one(self, max_size: int):
530+
"""A non-positive bound must not produce a zero-capacity cache.
531+
532+
Without clamping, a bound of 0 would evict every entry immediately
533+
after inserting it, so every lookup would miss and the signature
534+
reflection this cache exists to avoid would run on every operation
535+
again. A negative bound is worse still: the eviction loop would keep
536+
popping past an already-empty dict.
537+
"""
538+
cache = _EntityMethodSignatureCache(max_size=max_size)
539+
540+
cache[(int, "op")] = True
541+
542+
assert len(cache) == 1
543+
assert cache.get((int, "op")) is True
544+
assert (int, "op") in cache
545+
546+
def test_clamped_cache_still_evicts_and_serves_hits(self):
547+
"""A clamped cache behaves as a working size-1 cache, not a no-op."""
548+
cache = _EntityMethodSignatureCache(max_size=0)
549+
550+
cache[(int, "op")] = True
551+
cache[(str, "op")] = False
552+
553+
# The newest entry is retained and readable; the older one is evicted
554+
# because the effective bound is one, not because caching is disabled.
555+
assert len(cache) == 1
556+
assert cache.get((str, "op")) is False
557+
assert cache.get((int, "op")) is None
558+
559+
def test_executor_with_clamped_cache_still_caches_reflection(self, monkeypatch):
560+
"""An executor handed a clamped cache must still avoid re-reflecting."""
561+
counting = _CountingInspect()
562+
monkeypatch.setattr(worker, "inspect", counting)
563+
564+
registry = _Registry()
565+
registry.add_entity(Counter)
566+
executor = _EntityExecutor(
567+
registry,
568+
logging.getLogger("test"),
569+
JsonDataConverter(),
570+
entity_method_cache=_EntityMethodSignatureCache(max_size=0),
571+
)
572+
573+
entity_id = entities.EntityInstanceId("Counter", "test-key")
574+
state = StateShim(None, JsonDataConverter())
575+
for _ in range(3):
576+
executor.execute("test-orch", entity_id, "add", state, "1")
577+
state.commit()
578+
579+
assert len(counting.signature_calls) == 1
580+
assert state.get_state(int) == 3

0 commit comments

Comments
 (0)