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

Type hints for tests.federation #14991

Merged
merged 7 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14991.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve type hints.
4 changes: 1 addition & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ exclude = (?x)
|synapse/storage/databases/main/cache.py
|synapse/storage/schema/

|tests/federation/test_federation_catch_up.py
|tests/federation/test_federation_sender.py
|tests/http/federation/test_matrix_federation_agent.py
|tests/http/federation/test_srv_resolver.py
|tests/http/test_proxyagent.py
Expand Down Expand Up @@ -89,7 +87,7 @@ disallow_untyped_defs = True
[mypy-tests.events.*]
disallow_untyped_defs = True

[mypy-tests.federation.transport.test_client]
[mypy-tests.federation.*]
disallow_untyped_defs = True

[mypy-tests.handlers.*]
Expand Down
18 changes: 9 additions & 9 deletions tests/federation/test_complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from synapse.api.errors import Codes, SynapseError
from synapse.rest import admin
from synapse.rest.client import login, room
from synapse.types import UserID
from synapse.types import JsonDict, UserID

from tests import unittest
from tests.test_utils import make_awaitable
Expand All @@ -31,12 +31,12 @@ class RoomComplexityTests(unittest.FederatingHomeserverTestCase):
login.register_servlets,
]

def default_config(self):
def default_config(self) -> JsonDict:
config = super().default_config()
config["limit_remote_rooms"] = {"enabled": True, "complexity": 0.05}
return config

def test_complexity_simple(self):
def test_complexity_simple(self) -> None:

u1 = self.register_user("u1", "pass")
u1_token = self.login("u1", "pass")
Expand Down Expand Up @@ -66,7 +66,7 @@ def test_complexity_simple(self):
complexity = channel.json_body["v1"]
self.assertEqual(complexity, 1.23)

def test_join_too_large(self):
def test_join_too_large(self) -> None:

u1 = self.register_user("u1", "pass")

Expand Down Expand Up @@ -95,7 +95,7 @@ def test_join_too_large(self):
self.assertEqual(f.value.code, 400, f.value)
self.assertEqual(f.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)

def test_join_too_large_admin(self):
def test_join_too_large_admin(self) -> None:
# Check whether an admin can join if option "admins_can_join" is undefined,
# this option defaults to false, so the join should fail.

Expand Down Expand Up @@ -126,7 +126,7 @@ def test_join_too_large_admin(self):
self.assertEqual(f.value.code, 400, f.value)
self.assertEqual(f.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)

def test_join_too_large_once_joined(self):
def test_join_too_large_once_joined(self) -> None:

u1 = self.register_user("u1", "pass")
u1_token = self.login("u1", "pass")
Expand Down Expand Up @@ -180,7 +180,7 @@ class RoomComplexityAdminTests(unittest.FederatingHomeserverTestCase):
login.register_servlets,
]

def default_config(self):
def default_config(self) -> JsonDict:
config = super().default_config()
config["limit_remote_rooms"] = {
"enabled": True,
Expand All @@ -189,7 +189,7 @@ def default_config(self):
}
return config

def test_join_too_large_no_admin(self):
def test_join_too_large_no_admin(self) -> None:
# A user which is not an admin should not be able to join a remote room
# which is too complex.

Expand Down Expand Up @@ -220,7 +220,7 @@ def test_join_too_large_no_admin(self):
self.assertEqual(f.value.code, 400, f.value)
self.assertEqual(f.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)

def test_join_too_large_admin(self):
def test_join_too_large_admin(self) -> None:
# An admin should be able to join rooms where a complexity check fails.

u1 = self.register_user("u1", "pass", admin=True)
Expand Down
52 changes: 31 additions & 21 deletions tests/federation/test_federation_catch_up.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
from typing import List, Tuple
from typing import Callable, List, Optional, Tuple
from unittest.mock import Mock

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes
from synapse.events import EventBase
from synapse.federation.sender import PerDestinationQueue, TransactionManager
from synapse.federation.units import Edu
from synapse.federation.units import Edu, Transaction
from synapse.rest import admin
from synapse.rest.client import login, room
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util import Clock
from synapse.util.retryutils import NotRetryingDestination

from tests.test_utils import event_injection, make_awaitable
Expand All @@ -28,23 +32,25 @@ class FederationCatchUpTestCases(FederatingHomeserverTestCase):
login.register_servlets,
]

def make_homeserver(self, reactor, clock):
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
return self.setup_test_homeserver(
federation_transport_client=Mock(spec=["send_transaction"]),
)

def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
# stub out get_current_hosts_in_room
state_handler = hs.get_state_handler()
state_storage_controller = hs.get_storage_controllers().state

# This mock is crucial for destination_rooms to be populated.
state_handler.get_current_hosts_in_room = Mock(
return_value=make_awaitable(["test", "host2"])
# TODO: this seems to no longer be the case---tests pass with this mock
# commented out.
state_storage_controller.get_current_hosts_in_room = Mock( # type: ignore[assignment]
return_value=make_awaitable({"test", "host2"})
)

# whenever send_transaction is called, record the pdu data
self.pdus = []
self.failed_pdus = []
self.pdus: List[JsonDict] = []
self.failed_pdus: List[JsonDict] = []
self.is_online = True
self.hs.get_federation_transport_client().send_transaction.side_effect = (
self.record_transaction
Expand All @@ -55,8 +61,13 @@ def default_config(self) -> JsonDict:
config["federation_sender_instances"] = None
return config

async def record_transaction(self, txn, json_cb):
if self.is_online:
async def record_transaction(
self, txn: Transaction, json_cb: Optional[Callable[[], JsonDict]]
) -> JsonDict:
if json_cb is None:
# The tests seem to expect that this method raises in this situation.
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
raise Exception("Blank json_cb")
elif self.is_online:
data = json_cb()
self.pdus.extend(data["pdus"])
return {}
Expand Down Expand Up @@ -92,7 +103,7 @@ def get_destination_room(self, room: str, destination: str = "host2") -> dict:
)[0]
return {"event_id": event_id, "stream_ordering": stream_ordering}

def test_catch_up_destination_rooms_tracking(self):
def test_catch_up_destination_rooms_tracking(self) -> None:
"""
Tests that we populate the `destination_rooms` table as needed.
"""
Expand All @@ -117,7 +128,7 @@ def test_catch_up_destination_rooms_tracking(self):
self.assertEqual(row_2["event_id"], event_id_2)
self.assertEqual(row_1["stream_ordering"], row_2["stream_ordering"] - 1)

def test_catch_up_last_successful_stream_ordering_tracking(self):
def test_catch_up_last_successful_stream_ordering_tracking(self) -> None:
"""
Tests that we populate the `destination_rooms` table as needed.
"""
Expand Down Expand Up @@ -174,7 +185,7 @@ def test_catch_up_last_successful_stream_ordering_tracking(self):
"Send succeeded but not marked as last_successful_stream_ordering",
)

def test_catch_up_from_blank_state(self):
def test_catch_up_from_blank_state(self) -> None:
"""
Runs an overall test of federation catch-up from scratch.
Further tests will focus on more narrow aspects and edge-cases, but I
Expand Down Expand Up @@ -261,16 +272,15 @@ async def fake_send(
destination_tm: str,
pending_pdus: List[EventBase],
_pending_edus: List[Edu],
) -> bool:
) -> None:
assert destination == destination_tm
results_list.extend(pending_pdus)
return True # success!

transaction_manager.send_new_transaction = fake_send
transaction_manager.send_new_transaction = fake_send # type: ignore[assignment]

return per_dest_queue, results_list

def test_catch_up_loop(self):
def test_catch_up_loop(self) -> None:
"""
Tests the behaviour of _catch_up_transmission_loop.
"""
Expand Down Expand Up @@ -334,7 +344,7 @@ def test_catch_up_loop(self):
event_5.internal_metadata.stream_ordering,
)

def test_catch_up_on_synapse_startup(self):
def test_catch_up_on_synapse_startup(self) -> None:
"""
Tests the behaviour of get_catch_up_outstanding_destinations and
_wake_destinations_needing_catchup.
Expand Down Expand Up @@ -412,7 +422,7 @@ def test_catch_up_on_synapse_startup(self):
# patch wake_destination to just count the destinations instead
woken = []

def wake_destination_track(destination):
def wake_destination_track(destination: str) -> None:
woken.append(destination)

self.hs.get_federation_sender().wake_destination = wake_destination_track
Expand All @@ -432,7 +442,7 @@ def wake_destination_track(destination):
# - all destinations are woken exactly once; they appear once in woken.
self.assertCountEqual(woken, server_names[:-1])

def test_not_latest_event(self):
def test_not_latest_event(self) -> None:
"""Test that we send the latest event in the room even if its not ours."""

per_dest_queue, sent_pdus = self.make_fake_destination_queue()
Expand Down
12 changes: 7 additions & 5 deletions tests/federation/test_federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ class FederationClientTest(FederatingHomeserverTestCase):
login.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer):
def prepare(
self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer
) -> None:
super().prepare(reactor, clock, homeserver)

# mock out the Agent used by the federation client, which is easier than
Expand All @@ -51,7 +53,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer):
self.creator = f"@creator:{self.OTHER_SERVER_NAME}"
self.test_room_id = "!room_id"

def test_get_room_state(self):
def test_get_room_state(self) -> None:
# mock up some events to use in the response.
# In real life, these would have things in `prev_events` and `auth_events`, but that's
# a bit annoying to mock up, and the code under test doesn't care, so we don't bother.
Expand Down Expand Up @@ -140,7 +142,7 @@ def test_get_room_state(self):
["m.room.create", "m.room.member", "m.room.power_levels"],
)

def test_get_pdu_returns_nothing_when_event_does_not_exist(self):
def test_get_pdu_returns_nothing_when_event_does_not_exist(self) -> None:
"""No event should be returned when the event does not exist"""
pulled_pdu_info = self.get_success(
self.hs.get_federation_client().get_pdu(
Expand All @@ -151,11 +153,11 @@ def test_get_pdu_returns_nothing_when_event_does_not_exist(self):
)
self.assertEqual(pulled_pdu_info, None)

def test_get_pdu(self):
def test_get_pdu(self) -> None:
"""Test to make sure an event is returned by `get_pdu()`"""
self._get_pdu_once()

def test_get_pdu_event_from_cache_is_pristine(self):
def test_get_pdu_event_from_cache_is_pristine(self) -> None:
"""Test that modifications made to events returned by `get_pdu()`
do not propagate back to to the internal cache (events returned should
be a copy).
Expand Down
Loading