From aa7e278aba2c9c69363f6d1ee5eb4e8f55243782 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 14:40:54 -0500 Subject: [PATCH 1/7] test_ttlcache --- mypy.ini | 1 - tests/util/caches/test_ttlcache.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/mypy.ini b/mypy.ini index 4cd61e048457..61485382c371 100644 --- a/mypy.ini +++ b/mypy.ini @@ -63,7 +63,6 @@ exclude = (?x) |tests/util/caches/test_deferred_cache.py |tests/util/caches/test_descriptors.py |tests/util/caches/test_response_cache.py - |tests/util/caches/test_ttlcache.py |tests/util/test_async_helpers.py |tests/util/test_batching_queue.py |tests/util/test_dict_cache.py diff --git a/tests/util/caches/test_ttlcache.py b/tests/util/caches/test_ttlcache.py index fe8314057da1..13dfd40771a9 100644 --- a/tests/util/caches/test_ttlcache.py +++ b/tests/util/caches/test_ttlcache.py @@ -22,7 +22,7 @@ class CacheTestCase(unittest.TestCase): def setUp(self): self.mock_timer = Mock(side_effect=lambda: 100.0) - self.cache = TTLCache("test_cache", self.mock_timer) + self.cache: TTLCache[str, str] = TTLCache("test_cache", self.mock_timer) def test_get(self): """simple set/get tests""" From f36528b82978eee47bb5f7ef76f7906d49222b06 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 14:51:55 -0500 Subject: [PATCH 2/7] test_response_cache --- mypy.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index 61485382c371..2f54f93c8831 100644 --- a/mypy.ini +++ b/mypy.ini @@ -62,7 +62,6 @@ exclude = (?x) |tests/util/caches/test_cached_call.py |tests/util/caches/test_deferred_cache.py |tests/util/caches/test_descriptors.py - |tests/util/caches/test_response_cache.py |tests/util/test_async_helpers.py |tests/util/test_batching_queue.py |tests/util/test_dict_cache.py From b138850296618f41ea3aa236e0cef7388d2d53b1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 15:00:01 -0500 Subject: [PATCH 3/7] test_descriptors --- mypy.ini | 1 - tests/util/caches/test_descriptors.py | 22 ++++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/mypy.ini b/mypy.ini index 2f54f93c8831..2e43dd628e47 100644 --- a/mypy.ini +++ b/mypy.ini @@ -61,7 +61,6 @@ exclude = (?x) |tests/test_terms_auth.py |tests/util/caches/test_cached_call.py |tests/util/caches/test_deferred_cache.py - |tests/util/caches/test_descriptors.py |tests/util/test_async_helpers.py |tests/util/test_batching_queue.py |tests/util/test_dict_cache.py diff --git a/tests/util/caches/test_descriptors.py b/tests/util/caches/test_descriptors.py index 43475a307f9b..13f1edd5332c 100644 --- a/tests/util/caches/test_descriptors.py +++ b/tests/util/caches/test_descriptors.py @@ -13,11 +13,12 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import Iterable, Set, Tuple +from typing import Iterable, Set, Tuple, cast from unittest import mock from twisted.internet import defer, reactor from twisted.internet.defer import CancelledError, Deferred +from twisted.internet.interfaces import IReactorTime from synapse.api.errors import SynapseError from synapse.logging.context import ( @@ -37,8 +38,8 @@ def run_on_reactor(): - d = defer.Deferred() - reactor.callLater(0, d.callback, 0) + d: "Deferred[int]" = defer.Deferred() + cast(IReactorTime, reactor).callLater(0, d.callback, 0) return make_deferred_yieldable(d) @@ -224,7 +225,8 @@ def fn(self, arg1): callbacks: Set[str] = set() # set off an asynchronous request - obj.result = origin_d = defer.Deferred() + origin_d: Deferred = defer.Deferred() + obj.result = origin_d d1 = obj.fn(1, on_invalidate=lambda: callbacks.add("d1")) self.assertFalse(d1.called) @@ -262,7 +264,7 @@ def test_cache_logcontexts(self): """Check that logcontexts are set and restored correctly when using the cache.""" - complete_lookup = defer.Deferred() + complete_lookup: Deferred = defer.Deferred() class Cls: @descriptors.cached() @@ -772,10 +774,14 @@ def fn(self, arg1, arg2): @descriptors.cachedList(cached_method_name="fn", list_name="args1") async def list_fn(self, args1, arg2): - assert current_context().name == "c1" + context = current_context() + assert isinstance(context, LoggingContext) + assert context.name == "c1" # we want this to behave like an asynchronous function await run_on_reactor() - assert current_context().name == "c1" + context = current_context() + assert isinstance(context, LoggingContext) + assert context.name == "c1" return self.mock(args1, arg2) with LoggingContext("c1") as c1: @@ -834,7 +840,7 @@ def list_fn(self, args1) -> "Deferred[dict]": return self.mock(args1) obj = Cls() - deferred_result = Deferred() + deferred_result: "Deferred[dict]" = Deferred() obj.mock.return_value = deferred_result # start off several concurrent lookups of the same key From 6cbb38cbcd269eaa7cda2d2112b58882adfb2c39 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 15:16:17 -0500 Subject: [PATCH 4/7] test_deferred_cache --- mypy.ini | 1 - tests/util/caches/test_deferred_cache.py | 35 ++++++++++++------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/mypy.ini b/mypy.ini index 2e43dd628e47..2899d5ebfd4d 100644 --- a/mypy.ini +++ b/mypy.ini @@ -60,7 +60,6 @@ exclude = (?x) |tests/test_state.py |tests/test_terms_auth.py |tests/util/caches/test_cached_call.py - |tests/util/caches/test_deferred_cache.py |tests/util/test_async_helpers.py |tests/util/test_batching_queue.py |tests/util/test_dict_cache.py diff --git a/tests/util/caches/test_deferred_cache.py b/tests/util/caches/test_deferred_cache.py index 02b99b466a26..0d5ea93bf4c6 100644 --- a/tests/util/caches/test_deferred_cache.py +++ b/tests/util/caches/test_deferred_cache.py @@ -13,6 +13,7 @@ # limitations under the License. from functools import partial +from typing import List, Tuple from twisted.internet import defer @@ -23,19 +24,19 @@ class DeferredCacheTestCase(TestCase): def test_empty(self): - cache = DeferredCache("test") + cache: DeferredCache[str, int] = DeferredCache("test") with self.assertRaises(KeyError): cache.get("foo") def test_hit(self): - cache = DeferredCache("test") + cache: DeferredCache[str, int] = DeferredCache("test") cache.prefill("foo", 123) self.assertEqual(self.successResultOf(cache.get("foo")), 123) def test_hit_deferred(self): - cache = DeferredCache("test") - origin_d = defer.Deferred() + cache: DeferredCache[str, int] = DeferredCache("test") + origin_d: "defer.Deferred[int]" = defer.Deferred() set_d = cache.set("k1", origin_d) # get should return an incomplete deferred @@ -57,14 +58,14 @@ def check1(r): def test_callbacks(self): """Invalidation callbacks are called at the right time""" - cache = DeferredCache("test") + cache: DeferredCache[str, int] = DeferredCache("test") callbacks = set() # start with an entry, with a callback cache.prefill("k1", 10, callback=lambda: callbacks.add("prefill")) # now replace that entry with a pending result - origin_d = defer.Deferred() + origin_d: "defer.Deferred[int]" = defer.Deferred() set_d = cache.set("k1", origin_d, callback=lambda: callbacks.add("set")) # ... and also make a get request @@ -90,14 +91,14 @@ def test_callbacks(self): self.assertEqual(callbacks, {"set", "get"}) def test_set_fail(self): - cache = DeferredCache("test") + cache: DeferredCache[str, int] = DeferredCache("test") callbacks = set() # start with an entry, with a callback cache.prefill("k1", 10, callback=lambda: callbacks.add("prefill")) # now replace that entry with a pending result - origin_d = defer.Deferred() + origin_d: defer.Deferred = defer.Deferred() set_d = cache.set("k1", origin_d, callback=lambda: callbacks.add("set")) # ... and also make a get request @@ -127,8 +128,8 @@ def test_set_fail(self): self.assertEqual(callbacks, {"prefill", "get2"}) def test_get_immediate(self): - cache = DeferredCache("test") - d1 = defer.Deferred() + cache: DeferredCache[str, int] = DeferredCache("test") + d1: "defer.Deferred[int]" = defer.Deferred() cache.set("key1", d1) # get_immediate should return default @@ -143,7 +144,7 @@ def test_get_immediate(self): self.assertEqual(v, 2) def test_invalidate(self): - cache = DeferredCache("test") + cache: DeferredCache[Tuple[str], int] = DeferredCache("test") cache.prefill(("foo",), 123) cache.invalidate(("foo",)) @@ -151,7 +152,7 @@ def test_invalidate(self): cache.get(("foo",)) def test_invalidate_all(self): - cache = DeferredCache("testcache") + cache: DeferredCache[str, str] = DeferredCache("testcache") callback_record = [False, False] @@ -159,10 +160,10 @@ def record_callback(idx): callback_record[idx] = True # add a couple of pending entries - d1 = defer.Deferred() + d1: "defer.Deferred[str]" = defer.Deferred() cache.set("key1", d1, partial(record_callback, 0)) - d2 = defer.Deferred() + d2: "defer.Deferred[str]" = defer.Deferred() cache.set("key2", d2, partial(record_callback, 1)) # lookup should return pending deferreds @@ -194,7 +195,7 @@ def record_callback(idx): cache.get("key1", None) def test_eviction(self): - cache = DeferredCache( + cache: DeferredCache[int, str] = DeferredCache( "test", max_entries=2, apply_cache_factor_from_config=False ) @@ -209,7 +210,7 @@ def test_eviction(self): cache.get(3) def test_eviction_lru(self): - cache = DeferredCache( + cache: DeferredCache[int, str] = DeferredCache( "test", max_entries=2, apply_cache_factor_from_config=False ) @@ -228,7 +229,7 @@ def test_eviction_lru(self): cache.get(3) def test_eviction_iterable(self): - cache = DeferredCache( + cache: DeferredCache[int, List[str]] = DeferredCache( "test", max_entries=3, apply_cache_factor_from_config=False, From 4d4b1755c421bd1f7ae23f2b1140edff6d456519 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 15:17:41 -0500 Subject: [PATCH 5/7] test_cached_call --- mypy.ini | 1 - tests/util/caches/test_cached_call.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/mypy.ini b/mypy.ini index 2899d5ebfd4d..620b24de52bf 100644 --- a/mypy.ini +++ b/mypy.ini @@ -59,7 +59,6 @@ exclude = (?x) |tests/server_notices/test_resource_limits_server_notices.py |tests/test_state.py |tests/test_terms_auth.py - |tests/util/caches/test_cached_call.py |tests/util/test_async_helpers.py |tests/util/test_batching_queue.py |tests/util/test_dict_cache.py diff --git a/tests/util/caches/test_cached_call.py b/tests/util/caches/test_cached_call.py index 80b97167bac0..5756d3bcad07 100644 --- a/tests/util/caches/test_cached_call.py +++ b/tests/util/caches/test_cached_call.py @@ -28,7 +28,7 @@ def test_get(self): Happy-path test case: makes a couple of calls and makes sure they behave correctly """ - d = Deferred() + d: "Deferred[int]" = Deferred() async def f(): return await d @@ -95,7 +95,7 @@ class RetryOnExceptionCachedCallTestCase(TestCase): def test_get(self): # set up the RetryOnExceptionCachedCall around a function which will fail # (after a while) - d = Deferred() + d: "Deferred[int]" = Deferred() async def f1(): await d From 8f6cf38710f0d40163c1e2d0dcdc64dab3c51c2f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 16:06:04 -0500 Subject: [PATCH 6/7] Require type hints in tests.util.caches (except test_descriptors). --- mypy.ini | 6 ++++++ tests/util/caches/test_cached_call.py | 19 +++++++++-------- tests/util/caches/test_deferred_cache.py | 26 ++++++++++++------------ tests/util/caches/test_response_cache.py | 16 +++++++-------- tests/util/caches/test_ttlcache.py | 6 +++--- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/mypy.ini b/mypy.ini index 620b24de52bf..25b3c9374885 100644 --- a/mypy.ini +++ b/mypy.ini @@ -128,6 +128,12 @@ disallow_untyped_defs = True [mypy-tests.federation.transport.test_client] disallow_untyped_defs = True +[mypy-tests.util.caches.*] +disallow_untyped_defs = True + +[mypy-tests.util.caches.test_descriptors] +disallow_untyped_defs = False + [mypy-tests.utils] disallow_untyped_defs = True diff --git a/tests/util/caches/test_cached_call.py b/tests/util/caches/test_cached_call.py index 5756d3bcad07..9266f12590cb 100644 --- a/tests/util/caches/test_cached_call.py +++ b/tests/util/caches/test_cached_call.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import NoReturn from unittest.mock import Mock from twisted.internet import defer @@ -23,14 +24,14 @@ class CachedCallTestCase(TestCase): - def test_get(self): + def test_get(self) -> None: """ Happy-path test case: makes a couple of calls and makes sure they behave correctly """ d: "Deferred[int]" = Deferred() - async def f(): + async def f() -> int: return await d slow_call = Mock(side_effect=f) @@ -43,7 +44,7 @@ async def f(): # now fire off a couple of calls completed_results = [] - async def r(): + async def r() -> None: res = await cached_call.get() completed_results.append(res) @@ -69,12 +70,12 @@ async def r(): self.assertEqual(r3, 123) slow_call.assert_not_called() - def test_fast_call(self): + def test_fast_call(self) -> None: """ Test the behaviour when the underlying function completes immediately """ - async def f(): + async def f() -> int: return 12 fast_call = Mock(side_effect=f) @@ -92,12 +93,12 @@ async def f(): class RetryOnExceptionCachedCallTestCase(TestCase): - def test_get(self): + def test_get(self) -> None: # set up the RetryOnExceptionCachedCall around a function which will fail # (after a while) d: "Deferred[int]" = Deferred() - async def f1(): + async def f1() -> NoReturn: await d raise ValueError("moo") @@ -110,7 +111,7 @@ async def f1(): # now fire off a couple of calls completed_results = [] - async def r(): + async def r() -> None: try: await cached_call.get() except Exception as e1: @@ -137,7 +138,7 @@ async def r(): # to the getter d = Deferred() - async def f2(): + async def f2() -> int: return await d slow_call.reset_mock() diff --git a/tests/util/caches/test_deferred_cache.py b/tests/util/caches/test_deferred_cache.py index 0d5ea93bf4c6..f74d82b1dcd5 100644 --- a/tests/util/caches/test_deferred_cache.py +++ b/tests/util/caches/test_deferred_cache.py @@ -23,18 +23,18 @@ class DeferredCacheTestCase(TestCase): - def test_empty(self): + def test_empty(self) -> None: cache: DeferredCache[str, int] = DeferredCache("test") with self.assertRaises(KeyError): cache.get("foo") - def test_hit(self): + def test_hit(self) -> None: cache: DeferredCache[str, int] = DeferredCache("test") cache.prefill("foo", 123) self.assertEqual(self.successResultOf(cache.get("foo")), 123) - def test_hit_deferred(self): + def test_hit_deferred(self) -> None: cache: DeferredCache[str, int] = DeferredCache("test") origin_d: "defer.Deferred[int]" = defer.Deferred() set_d = cache.set("k1", origin_d) @@ -44,7 +44,7 @@ def test_hit_deferred(self): self.assertFalse(get_d.called) # add a callback that will make sure that the set_d gets called before the get_d - def check1(r): + def check1(r: str) -> str: self.assertTrue(set_d.called) return r @@ -56,7 +56,7 @@ def check1(r): self.assertEqual(self.successResultOf(set_d), 99) self.assertEqual(self.successResultOf(get_d), 99) - def test_callbacks(self): + def test_callbacks(self) -> None: """Invalidation callbacks are called at the right time""" cache: DeferredCache[str, int] = DeferredCache("test") callbacks = set() @@ -90,7 +90,7 @@ def test_callbacks(self): cache.prefill("k1", 30) self.assertEqual(callbacks, {"set", "get"}) - def test_set_fail(self): + def test_set_fail(self) -> None: cache: DeferredCache[str, int] = DeferredCache("test") callbacks = set() @@ -127,7 +127,7 @@ def test_set_fail(self): cache.prefill("k1", 30) self.assertEqual(callbacks, {"prefill", "get2"}) - def test_get_immediate(self): + def test_get_immediate(self) -> None: cache: DeferredCache[str, int] = DeferredCache("test") d1: "defer.Deferred[int]" = defer.Deferred() cache.set("key1", d1) @@ -143,7 +143,7 @@ def test_get_immediate(self): v = cache.get_immediate("key1", 1) self.assertEqual(v, 2) - def test_invalidate(self): + def test_invalidate(self) -> None: cache: DeferredCache[Tuple[str], int] = DeferredCache("test") cache.prefill(("foo",), 123) cache.invalidate(("foo",)) @@ -151,12 +151,12 @@ def test_invalidate(self): with self.assertRaises(KeyError): cache.get(("foo",)) - def test_invalidate_all(self): + def test_invalidate_all(self) -> None: cache: DeferredCache[str, str] = DeferredCache("testcache") callback_record = [False, False] - def record_callback(idx): + def record_callback(idx: int) -> None: callback_record[idx] = True # add a couple of pending entries @@ -194,7 +194,7 @@ def record_callback(idx): with self.assertRaises(KeyError): cache.get("key1", None) - def test_eviction(self): + def test_eviction(self) -> None: cache: DeferredCache[int, str] = DeferredCache( "test", max_entries=2, apply_cache_factor_from_config=False ) @@ -209,7 +209,7 @@ def test_eviction(self): cache.get(2) cache.get(3) - def test_eviction_lru(self): + def test_eviction_lru(self) -> None: cache: DeferredCache[int, str] = DeferredCache( "test", max_entries=2, apply_cache_factor_from_config=False ) @@ -228,7 +228,7 @@ def test_eviction_lru(self): cache.get(1) cache.get(3) - def test_eviction_iterable(self): + def test_eviction_iterable(self) -> None: cache: DeferredCache[int, List[str]] = DeferredCache( "test", max_entries=3, diff --git a/tests/util/caches/test_response_cache.py b/tests/util/caches/test_response_cache.py index 025b73e32f90..f09eeecadabe 100644 --- a/tests/util/caches/test_response_cache.py +++ b/tests/util/caches/test_response_cache.py @@ -35,7 +35,7 @@ class ResponseCacheTestCase(TestCase): (These have cache with a short timeout_ms=, shorter than will be tested through advancing the clock) """ - def setUp(self): + def setUp(self) -> None: self.reactor, self.clock = get_clock() def with_cache(self, name: str, ms: int = 0) -> ResponseCache: @@ -49,7 +49,7 @@ async def delayed_return(self, o: str) -> str: await self.clock.sleep(1) return o - def test_cache_hit(self): + def test_cache_hit(self) -> None: cache = self.with_cache("keeping_cache", ms=9001) expected_result = "howdy" @@ -74,7 +74,7 @@ def test_cache_hit(self): "cache should still have the result", ) - def test_cache_miss(self): + def test_cache_miss(self) -> None: cache = self.with_cache("trashing_cache", ms=0) expected_result = "howdy" @@ -90,7 +90,7 @@ def test_cache_miss(self): ) self.assertCountEqual([], cache.keys(), "cache should not have the result now") - def test_cache_expire(self): + def test_cache_expire(self) -> None: cache = self.with_cache("short_cache", ms=1000) expected_result = "howdy" @@ -115,7 +115,7 @@ def test_cache_expire(self): self.reactor.pump((2,)) self.assertCountEqual([], cache.keys(), "cache should not have the result now") - def test_cache_wait_hit(self): + def test_cache_wait_hit(self) -> None: cache = self.with_cache("neutral_cache") expected_result = "howdy" @@ -131,7 +131,7 @@ def test_cache_wait_hit(self): self.assertEqual(expected_result, self.successResultOf(wrap_d)) - def test_cache_wait_expire(self): + def test_cache_wait_expire(self) -> None: cache = self.with_cache("medium_cache", ms=3000) expected_result = "howdy" @@ -162,7 +162,7 @@ def test_cache_wait_expire(self): self.assertCountEqual([], cache.keys(), "cache should not have the result now") @parameterized.expand([(True,), (False,)]) - def test_cache_context_nocache(self, should_cache: bool): + def test_cache_context_nocache(self, should_cache: bool) -> None: """If the callback clears the should_cache bit, the result should not be cached""" cache = self.with_cache("medium_cache", ms=3000) @@ -170,7 +170,7 @@ def test_cache_context_nocache(self, should_cache: bool): call_count = 0 - async def non_caching(o: str, cache_context: ResponseCacheContext[int]): + async def non_caching(o: str, cache_context: ResponseCacheContext[int]) -> str: nonlocal call_count call_count += 1 await self.clock.sleep(1) diff --git a/tests/util/caches/test_ttlcache.py b/tests/util/caches/test_ttlcache.py index 13dfd40771a9..679d1eb36bd9 100644 --- a/tests/util/caches/test_ttlcache.py +++ b/tests/util/caches/test_ttlcache.py @@ -20,11 +20,11 @@ class CacheTestCase(unittest.TestCase): - def setUp(self): + def setUp(self) -> None: self.mock_timer = Mock(side_effect=lambda: 100.0) self.cache: TTLCache[str, str] = TTLCache("test_cache", self.mock_timer) - def test_get(self): + def test_get(self) -> None: """simple set/get tests""" self.cache.set("one", "1", 10) self.cache.set("two", "2", 20) @@ -59,7 +59,7 @@ def test_get(self): self.assertEqual(self.cache._metrics.hits, 4) self.assertEqual(self.cache._metrics.misses, 5) - def test_expiry(self): + def test_expiry(self) -> None: self.cache.set("one", "1", 10) self.cache.set("two", "2", 20) self.cache.set("three", "3", 30) From 0c49e09fe99ef110a0907f173df220bf1977777a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 22 Nov 2022 16:10:40 -0500 Subject: [PATCH 7/7] Newsfragment --- changelog.d/14529.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14529.misc diff --git a/changelog.d/14529.misc b/changelog.d/14529.misc new file mode 100644 index 000000000000..d44571b73149 --- /dev/null +++ b/changelog.d/14529.misc @@ -0,0 +1 @@ +Add missing type hints.