Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Require types in tests.storage. (#14646)
Browse files Browse the repository at this point in the history
Adds missing type hints to `tests.storage` package
and does not allow untyped definitions.
  • Loading branch information
clokep committed Dec 9, 2022
1 parent 94bc21e commit 3ac412b
Show file tree
Hide file tree
Showing 36 changed files with 489 additions and 341 deletions.
1 change: 1 addition & 0 deletions changelog.d/14646.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add missing type hints.
14 changes: 4 additions & 10 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ disallow_untyped_defs = False
[mypy-tests.*]
disallow_untyped_defs = False

[mypy-tests.handlers.test_sso]
disallow_untyped_defs = True

[mypy-tests.handlers.test_user_directory]
disallow_untyped_defs = True

Expand All @@ -103,16 +106,7 @@ disallow_untyped_defs = True
[mypy-tests.state.test_profile]
disallow_untyped_defs = True

[mypy-tests.storage.test_id_generators]
disallow_untyped_defs = True

[mypy-tests.storage.test_profile]
disallow_untyped_defs = True

[mypy-tests.handlers.test_sso]
disallow_untyped_defs = True

[mypy-tests.storage.test_user_directory]
[mypy-tests.storage.*]
disallow_untyped_defs = True

[mypy-tests.rest.*]
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/end_to_end_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ async def get_e2e_device_keys_for_federation_query(
@cancellable
async def get_e2e_device_keys_for_cs_api(
self,
query_list: List[Tuple[str, Optional[str]]],
query_list: Collection[Tuple[str, Optional[str]]],
include_displaynames: bool = True,
) -> Dict[str, Dict[str, JsonDict]]:
"""Fetch a list of device keys, formatted suitably for the C/S API.
Expand Down
10 changes: 7 additions & 3 deletions tests/storage/databases/main/test_deviceinbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from twisted.test.proto_helpers import MemoryReactor

from synapse.rest import admin
from synapse.rest.client import devices
from synapse.server import HomeServer
from synapse.util import Clock

from tests.unittest import HomeserverTestCase

Expand All @@ -25,11 +29,11 @@ class DeviceInboxBackgroundUpdateStoreTestCase(HomeserverTestCase):
devices.register_servlets,
]

def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store = hs.get_datastores().main
self.user_id = self.register_user("foo", "pass")

def test_background_remove_deleted_devices_from_device_inbox(self):
def test_background_remove_deleted_devices_from_device_inbox(self) -> None:
"""Test that the background task to delete old device_inboxes works properly."""

# create a valid device
Expand Down Expand Up @@ -89,7 +93,7 @@ def test_background_remove_deleted_devices_from_device_inbox(self):
self.assertEqual(1, len(res))
self.assertEqual(res[0], "cur_device")

def test_background_remove_hidden_devices_from_device_inbox(self):
def test_background_remove_hidden_devices_from_device_inbox(self) -> None:
"""Test that the background task to delete hidden devices
from device_inboxes works properly."""

Expand Down
27 changes: 14 additions & 13 deletions tests/storage/databases/main/test_events_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class HaveSeenEventsTestCase(unittest.HomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.hs = hs
self.store: EventsWorkerStore = hs.get_datastores().main

Expand All @@ -68,7 +68,7 @@ def prepare(self, reactor, clock, hs):

self.event_ids.append(event.event_id)

def test_simple(self):
def test_simple(self) -> None:
with LoggingContext(name="test") as ctx:
res = self.get_success(
self.store.have_seen_events(
Expand All @@ -90,7 +90,7 @@ def test_simple(self):
self.assertEqual(res, {self.event_ids[0]})
self.assertEqual(ctx.get_resource_usage().db_txn_count, 0)

def test_persisting_event_invalidates_cache(self):
def test_persisting_event_invalidates_cache(self) -> None:
"""
Test to make sure that the `have_seen_event` cache
is invalidated after we persist an event and returns
Expand Down Expand Up @@ -138,7 +138,7 @@ def test_persisting_event_invalidates_cache(self):
# That should result in a single db query to lookup
self.assertEqual(ctx.get_resource_usage().db_txn_count, 1)

def test_invalidate_cache_by_room_id(self):
def test_invalidate_cache_by_room_id(self) -> None:
"""
Test to make sure that all events associated with the given `(room_id,)`
are invalidated in the `have_seen_event` cache.
Expand Down Expand Up @@ -175,7 +175,7 @@ class EventCacheTestCase(unittest.HomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store: EventsWorkerStore = hs.get_datastores().main

self.user = self.register_user("user", "pass")
Expand All @@ -189,7 +189,7 @@ def prepare(self, reactor, clock, hs):
# Reset the event cache so the tests start with it empty
self.get_success(self.store._get_event_cache.clear())

def test_simple(self):
def test_simple(self) -> None:
"""Test that we cache events that we pull from the DB."""

with LoggingContext("test") as ctx:
Expand All @@ -198,7 +198,7 @@ def test_simple(self):
# We should have fetched the event from the DB
self.assertEqual(ctx.get_resource_usage().evt_db_fetch_count, 1)

def test_event_ref(self):
def test_event_ref(self) -> None:
"""Test that we reuse events that are still in memory but have fallen
out of the cache, rather than requesting them from the DB.
"""
Expand All @@ -223,7 +223,7 @@ def test_event_ref(self):
# from the DB
self.assertEqual(ctx.get_resource_usage().evt_db_fetch_count, 0)

def test_dedupe(self):
def test_dedupe(self) -> None:
"""Test that if we request the same event multiple times we only pull it
out once.
"""
Expand All @@ -241,7 +241,7 @@ def test_dedupe(self):
class DatabaseOutageTestCase(unittest.HomeserverTestCase):
"""Test event fetching during a database outage."""

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store: EventsWorkerStore = hs.get_datastores().main

self.room_id = f"!room:{hs.hostname}"
Expand Down Expand Up @@ -377,7 +377,7 @@ class GetEventCancellationTestCase(unittest.HomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store: EventsWorkerStore = hs.get_datastores().main

self.user = self.register_user("user", "pass")
Expand Down Expand Up @@ -412,7 +412,8 @@ def blocking_get_event_calls(
unblock: "Deferred[None]" = Deferred()
original_runWithConnection = self.store.db_pool.runWithConnection

async def runWithConnection(*args, **kwargs):
# Don't bother with the types here, we just pass into the original function.
async def runWithConnection(*args, **kwargs): # type: ignore[no-untyped-def]
await unblock
return await original_runWithConnection(*args, **kwargs)

Expand Down Expand Up @@ -441,7 +442,7 @@ async def get_event(ctx: LoggingContext) -> None:
self.assertEqual(ctx1.get_resource_usage().evt_db_fetch_count, 1)
self.assertEqual(ctx2.get_resource_usage().evt_db_fetch_count, 0)

def test_first_get_event_cancelled(self):
def test_first_get_event_cancelled(self) -> None:
"""Test cancellation of the first `get_event` call sharing a database fetch.
The first `get_event` call is the one which initiates the fetch. We expect the
Expand All @@ -467,7 +468,7 @@ def test_first_get_event_cancelled(self):
# The second `get_event` call should complete successfully.
self.get_success(get_event2)

def test_second_get_event_cancelled(self):
def test_second_get_event_cancelled(self) -> None:
"""Test cancellation of the second `get_event` call sharing a database fetch."""
with self.blocking_get_event_calls() as (unblock, get_event1, get_event2):
# Cancel the second `get_event` call.
Expand Down
18 changes: 10 additions & 8 deletions tests/storage/databases/main/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,28 @@
from twisted.internet import defer, reactor
from twisted.internet.base import ReactorBase
from twisted.internet.defer import Deferred
from twisted.test.proto_helpers import MemoryReactor

from synapse.server import HomeServer
from synapse.storage.databases.main.lock import _LOCK_TIMEOUT_MS
from synapse.util import Clock

from tests import unittest


class LockTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor, clock, hs: HomeServer):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store = hs.get_datastores().main

def test_acquire_contention(self):
def test_acquire_contention(self) -> None:
# Track the number of tasks holding the lock.
# Should be at most 1.
in_lock = 0
max_in_lock = 0

release_lock: "Deferred[None]" = Deferred()

async def task():
async def task() -> None:
nonlocal in_lock
nonlocal max_in_lock

Expand Down Expand Up @@ -76,7 +78,7 @@ async def task():
# At most one task should have held the lock at a time.
self.assertEqual(max_in_lock, 1)

def test_simple_lock(self):
def test_simple_lock(self) -> None:
"""Test that we can take out a lock and that while we hold it nobody
else can take it out.
"""
Expand All @@ -103,7 +105,7 @@ def test_simple_lock(self):
self.get_success(lock3.__aenter__())
self.get_success(lock3.__aexit__(None, None, None))

def test_maintain_lock(self):
def test_maintain_lock(self) -> None:
"""Test that we don't time out locks while they're still active"""

lock = self.get_success(self.store.try_acquire_lock("name", "key"))
Expand All @@ -119,7 +121,7 @@ def test_maintain_lock(self):

self.get_success(lock.__aexit__(None, None, None))

def test_timeout_lock(self):
def test_timeout_lock(self) -> None:
"""Test that we time out locks if they're not updated for ages"""

lock = self.get_success(self.store.try_acquire_lock("name", "key"))
Expand All @@ -139,7 +141,7 @@ def test_timeout_lock(self):

self.assertFalse(self.get_success(lock.is_still_valid()))

def test_drop(self):
def test_drop(self) -> None:
"""Test that dropping the context manager means we stop renewing the lock"""

lock = self.get_success(self.store.try_acquire_lock("name", "key"))
Expand All @@ -153,7 +155,7 @@ def test_drop(self):
lock2 = self.get_success(self.store.try_acquire_lock("name", "key"))
self.assertIsNotNone(lock2)

def test_shutdown(self):
def test_shutdown(self) -> None:
"""Test that shutting down Synapse releases the locks"""
# Acquire two locks
lock = self.get_success(self.store.try_acquire_lock("name", "key1"))
Expand Down
8 changes: 4 additions & 4 deletions tests/storage/databases/main/test_receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ReceiptsBackgroundUpdateStoreTestCase(HomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store = hs.get_datastores().main
self.user_id = self.register_user("foo", "pass")
self.token = self.login("foo", "pass")
Expand All @@ -47,7 +47,7 @@ def _test_background_receipts_unique_index(
table: str,
receipts: Dict[Tuple[str, str, str], Sequence[Dict[str, Any]]],
expected_unique_receipts: Dict[Tuple[str, str, str], Optional[Dict[str, Any]]],
):
) -> None:
"""Test that the background update to uniqueify non-thread receipts in
the given receipts table works properly.
Expand Down Expand Up @@ -154,7 +154,7 @@ def drop_receipts_unique_index(txn: LoggingTransaction) -> None:
f"Background update did not remove all duplicate receipts from {table}",
)

def test_background_receipts_linearized_unique_index(self):
def test_background_receipts_linearized_unique_index(self) -> None:
"""Test that the background update to uniqueify non-thread receipts in
`receipts_linearized` works properly.
"""
Expand All @@ -177,7 +177,7 @@ def test_background_receipts_linearized_unique_index(self):
},
)

def test_background_receipts_graph_unique_index(self):
def test_background_receipts_graph_unique_index(self) -> None:
"""Test that the background update to uniqueify non-thread receipts in
`receipts_graph` works properly.
"""
Expand Down
10 changes: 7 additions & 3 deletions tests/storage/databases/main/test_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@

import json

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import RoomTypes
from synapse.rest import admin
from synapse.rest.client import login, room
from synapse.server import HomeServer
from synapse.storage.databases.main.room import _BackgroundUpdates
from synapse.util import Clock

from tests.unittest import HomeserverTestCase

Expand All @@ -30,7 +34,7 @@ class RoomBackgroundUpdateStoreTestCase(HomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store = hs.get_datastores().main
self.user_id = self.register_user("foo", "pass")
self.token = self.login("foo", "pass")
Expand All @@ -40,7 +44,7 @@ def _generate_room(self) -> str:

return room_id

def test_background_populate_rooms_creator_column(self):
def test_background_populate_rooms_creator_column(self) -> None:
"""Test that the background update to populate the rooms creator column
works properly.
"""
Expand Down Expand Up @@ -95,7 +99,7 @@ def test_background_populate_rooms_creator_column(self):
)
self.assertEqual(room_creator_after, self.user_id)

def test_background_add_room_type_column(self):
def test_background_add_room_type_column(self) -> None:
"""Test that the background update to populate the `room_type` column in
`room_stats_state` works properly.
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test__base.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def test_upsert_many(self) -> None:
{(1, "user1", "hello"), (2, "user2", "bleb")},
)

def test_simple_update_many(self):
def test_simple_update_many(self) -> None:
"""
simple_update_many performs many updates at once.
"""
Expand Down
Loading

0 comments on commit 3ac412b

Please sign in to comment.