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

Commit

Permalink
Modernize unit tests configuration settings for workers. (#14568)
Browse files Browse the repository at this point in the history
Use the newer foo_instances configuration instead of the
deprecated flags to enable specific features (e.g. start_pushers).
  • Loading branch information
realtyem authored Dec 1, 2022
1 parent 7aefc7e commit 854a688
Show file tree
Hide file tree
Showing 18 changed files with 123 additions and 81 deletions.
1 change: 1 addition & 0 deletions changelog.d/14568.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Modernize unit tests configuration related to workers.
16 changes: 12 additions & 4 deletions tests/events/test_presence_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ def parse_config(config_dict: dict) -> PresenceRouterTestConfig:


class PresenceRouterTestCase(FederatingHomeserverTestCase):
"""
Test cases using a custom PresenceRouter
By default in test cases, federation sending is disabled. This class re-enables it
for the main process by setting `federation_sender_instances` to None.
"""

servlets = [
admin.register_servlets,
login.register_servlets,
Expand All @@ -150,6 +157,11 @@ def prepare(self, reactor, clock, homeserver):
self.sync_handler = self.hs.get_sync_handler()
self.module_api = homeserver.get_module_api()

def default_config(self) -> JsonDict:
config = super().default_config()
config["federation_sender_instances"] = None
return config

@override_config(
{
"presence": {
Expand All @@ -162,7 +174,6 @@ def prepare(self, reactor, clock, homeserver):
},
}
},
"send_federation": True,
}
)
def test_receiving_all_presence_legacy(self):
Expand All @@ -180,7 +191,6 @@ def test_receiving_all_presence_legacy(self):
},
},
],
"send_federation": True,
}
)
def test_receiving_all_presence(self):
Expand Down Expand Up @@ -290,7 +300,6 @@ def receiving_all_presence_test_body(self):
},
}
},
"send_federation": True,
}
)
def test_send_local_online_presence_to_with_module_legacy(self):
Expand All @@ -310,7 +319,6 @@ def test_send_local_online_presence_to_with_module_legacy(self):
},
},
],
"send_federation": True,
}
)
def test_send_local_online_presence_to_with_module(self):
Expand Down
21 changes: 14 additions & 7 deletions tests/federation/test_federation_catch_up.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,21 @@
from synapse.federation.units import Edu
from synapse.rest import admin
from synapse.rest.client import login, room
from synapse.types import JsonDict
from synapse.util.retryutils import NotRetryingDestination

from tests.test_utils import event_injection, make_awaitable
from tests.unittest import FederatingHomeserverTestCase, override_config
from tests.unittest import FederatingHomeserverTestCase


class FederationCatchUpTestCases(FederatingHomeserverTestCase):
"""
Tests cases of catching up over federation.
By default for test cases federation sending is disabled. This Test class has it
re-enabled for the main process.
"""

servlets = [
admin.register_servlets,
room.register_servlets,
Expand Down Expand Up @@ -42,6 +50,11 @@ def prepare(self, reactor, clock, hs):
self.record_transaction
)

def default_config(self) -> JsonDict:
config = super().default_config()
config["federation_sender_instances"] = None
return config

async def record_transaction(self, txn, json_cb):
if self.is_online:
data = json_cb()
Expand Down Expand Up @@ -79,7 +92,6 @@ def get_destination_room(self, room: str, destination: str = "host2") -> dict:
)[0]
return {"event_id": event_id, "stream_ordering": stream_ordering}

@override_config({"send_federation": True})
def test_catch_up_destination_rooms_tracking(self):
"""
Tests that we populate the `destination_rooms` table as needed.
Expand All @@ -105,7 +117,6 @@ 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)

@override_config({"send_federation": True})
def test_catch_up_last_successful_stream_ordering_tracking(self):
"""
Tests that we populate the `destination_rooms` table as needed.
Expand Down Expand Up @@ -163,7 +174,6 @@ def test_catch_up_last_successful_stream_ordering_tracking(self):
"Send succeeded but not marked as last_successful_stream_ordering",
)

@override_config({"send_federation": True}) # critical to federate
def test_catch_up_from_blank_state(self):
"""
Runs an overall test of federation catch-up from scratch.
Expand Down Expand Up @@ -260,7 +270,6 @@ async def fake_send(

return per_dest_queue, results_list

@override_config({"send_federation": True})
def test_catch_up_loop(self):
"""
Tests the behaviour of _catch_up_transmission_loop.
Expand Down Expand Up @@ -325,7 +334,6 @@ def test_catch_up_loop(self):
event_5.internal_metadata.stream_ordering,
)

@override_config({"send_federation": True})
def test_catch_up_on_synapse_startup(self):
"""
Tests the behaviour of get_catch_up_outstanding_destinations and
Expand Down Expand Up @@ -424,7 +432,6 @@ def wake_destination_track(destination):
# - all destinations are woken exactly once; they appear once in woken.
self.assertCountEqual(woken, server_names[:-1])

@override_config({"send_federation": True})
def test_not_latest_event(self):
"""Test that we send the latest event in the room even if its not ours."""

Expand Down
27 changes: 22 additions & 5 deletions tests/federation/test_federation_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@
from synapse.types import JsonDict, ReadReceipt

from tests.test_utils import make_awaitable
from tests.unittest import HomeserverTestCase, override_config
from tests.unittest import HomeserverTestCase


class FederationSenderReceiptsTestCases(HomeserverTestCase):
"""
Test federation sending to update receipts.
By default for test cases federation sending is disabled. This Test class has it
re-enabled for the main process.
"""

def make_homeserver(self, reactor, clock):
hs = self.setup_test_homeserver(
federation_transport_client=Mock(spec=["send_transaction"]),
Expand All @@ -44,7 +51,11 @@ def make_homeserver(self, reactor, clock):

return hs

@override_config({"send_federation": True})
def default_config(self) -> JsonDict:
config = super().default_config()
config["federation_sender_instances"] = None
return config

def test_send_receipts(self):
mock_send_transaction = (
self.hs.get_federation_transport_client().send_transaction
Expand Down Expand Up @@ -87,7 +98,6 @@ def test_send_receipts(self):
],
)

@override_config({"send_federation": True})
def test_send_receipts_thread(self):
mock_send_transaction = (
self.hs.get_federation_transport_client().send_transaction
Expand Down Expand Up @@ -164,7 +174,6 @@ def test_send_receipts_thread(self):
],
)

@override_config({"send_federation": True})
def test_send_receipts_with_backoff(self):
"""Send two receipts in quick succession; the second should be flushed, but
only after 20ms"""
Expand Down Expand Up @@ -251,6 +260,13 @@ def test_send_receipts_with_backoff(self):


class FederationSenderDevicesTestCases(HomeserverTestCase):
"""
Test federation sending to update devices.
By default for test cases federation sending is disabled. This Test class has it
re-enabled for the main process.
"""

servlets = [
admin.register_servlets,
login.register_servlets,
Expand All @@ -265,7 +281,8 @@ def make_homeserver(self, reactor, clock):

def default_config(self):
c = super().default_config()
c["send_federation"] = True
# Enable federation sending on the main process.
c["federation_sender_instances"] = None
return c

def prepare(self, reactor, clock, hs):
Expand Down
3 changes: 2 additions & 1 deletion tests/handlers/test_presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,8 @@ def make_homeserver(self, reactor, clock):

def default_config(self):
config = super().default_config()
config["send_federation"] = True
# Enable federation sending on the main process.
config["federation_sender_instances"] = None
return config

def prepare(self, reactor, clock, hs):
Expand Down
6 changes: 4 additions & 2 deletions tests/handlers/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ def test_started_typing_local(self) -> None:
],
)

@override_config({"send_federation": True})
# Enable federation sending on the main process.
@override_config({"federation_sender_instances": None})
def test_started_typing_remote_send(self) -> None:
self.room_members = [U_APPLE, U_ONION]

Expand Down Expand Up @@ -305,7 +306,8 @@ def test_started_typing_remote_recv_not_in_room(self) -> None:
self.assertEqual(events[0], [])
self.assertEqual(events[1], 0)

@override_config({"send_federation": True})
# Enable federation sending on the main process.
@override_config({"federation_sender_instances": None})
def test_stopped_typing(self) -> None:
self.room_members = [U_APPLE, U_BANANA, U_ONION]

Expand Down
7 changes: 5 additions & 2 deletions tests/handlers/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):

def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
config = self.default_config()
config["update_user_directory"] = True
# Re-enables updating the user directory, as that function is needed below.
config["update_user_directory_from_worker"] = None

self.appservice = ApplicationService(
token="i_am_an_app_service",
Expand Down Expand Up @@ -1045,7 +1046,9 @@ class TestUserDirSearchDisabled(unittest.HomeserverTestCase):

def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
config = self.default_config()
config["update_user_directory"] = True
# Re-enables updating the user directory, as that function is needed below. It
# will be force disabled later
config["update_user_directory_from_worker"] = None
hs = self.setup_test_homeserver(config=config)

self.config = hs.config
Expand Down
3 changes: 2 additions & 1 deletion tests/module_api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,8 @@ def test_send_local_online_presence_to(self):
# Test sending local online presence to users from the main process
_test_sending_local_online_presence_to_local_user(self, test_with_workers=False)

@override_config({"send_federation": True})
# Enable federation sending on the main process.
@override_config({"federation_sender_instances": None})
def test_send_local_online_presence_to_federation(self):
"""Tests that send_local_presence_to_users sends local online presence to remote users."""
# Create a user who will send presence updates
Expand Down
1 change: 0 additions & 1 deletion tests/push/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ def make_homeserver(self, reactor, clock):
"riot_base_url": None,
}
config["public_baseurl"] = "http://aaa"
config["start_pushers"] = True

hs = self.setup_test_homeserver(config=config)

Expand Down
7 changes: 1 addition & 6 deletions tests/push/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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 Any, Dict, List, Optional, Tuple
from typing import List, Optional, Tuple
from unittest.mock import Mock

from twisted.internet.defer import Deferred
Expand Down Expand Up @@ -41,11 +41,6 @@ class HTTPPusherTests(HomeserverTestCase):
user_id = True
hijack_auth = False

def default_config(self) -> Dict[str, Any]:
config = super().default_config()
config["start_pushers"] = True
return config

def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
self.push_attempts: List[Tuple[Deferred, str, dict]] = []

Expand Down
2 changes: 1 addition & 1 deletion tests/replication/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def make_worker_hs(
stream to the master HS.
Args:
worker_app: Type of worker, e.g. `synapse.app.federation_sender`.
worker_app: Type of worker, e.g. `synapse.app.generic_worker`.
extra_config: Any extra config to use for this instances.
**kwargs: Options that get passed to `self.setup_test_homeserver`,
useful to e.g. pass some mocks for things like `federation_http_client`
Expand Down
5 changes: 2 additions & 3 deletions tests/replication/tcp/streams/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ class FederationStreamTestCase(BaseStreamTestCase):
def _get_worker_hs_config(self) -> dict:
# enable federation sending on the worker
config = super()._get_worker_hs_config()
# TODO: make it so we don't need both of these
config["send_federation"] = False
config["worker_app"] = "synapse.app.federation_sender"
config["worker_name"] = "federation_sender1"
config["federation_sender_instances"] = ["federation_sender1"]
return config

def test_catchup(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/replication/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def make_homeserver(self, reactor, clock):

def _get_worker_hs_config(self) -> dict:
config = self.default_config()
config["worker_app"] = "synapse.app.client_reader"
config["worker_app"] = "synapse.app.generic_worker"
config["worker_replication_host"] = "testserv"
config["worker_replication_http_port"] = "8765"

Expand All @@ -53,7 +53,7 @@ def _test_register(self) -> FakeChannel:
4. Return the final request.
"""
worker_hs = self.make_worker_hs("synapse.app.client_reader")
worker_hs = self.make_worker_hs("synapse.app.generic_worker")
site = self._hs_to_site[worker_hs]

channel_1 = make_request(
Expand Down
14 changes: 7 additions & 7 deletions tests/replication/test_client_reader_shard.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@


class ClientReaderTestCase(BaseMultiWorkerStreamTestCase):
"""Test using one or more client readers for registration."""
"""Test using one or more generic workers for registration."""

servlets = [register.register_servlets]

def _get_worker_hs_config(self) -> dict:
config = self.default_config()
config["worker_app"] = "synapse.app.client_reader"
config["worker_app"] = "synapse.app.generic_worker"
config["worker_replication_host"] = "testserv"
config["worker_replication_http_port"] = "8765"
return config

def test_register_single_worker(self):
"""Test that registration works when using a single client reader worker."""
worker_hs = self.make_worker_hs("synapse.app.client_reader")
"""Test that registration works when using a single generic worker."""
worker_hs = self.make_worker_hs("synapse.app.generic_worker")
site = self._hs_to_site[worker_hs]

channel_1 = make_request(
Expand Down Expand Up @@ -64,9 +64,9 @@ def test_register_single_worker(self):
self.assertEqual(channel_2.json_body["user_id"], "@user:test")

def test_register_multi_worker(self):
"""Test that registration works when using multiple client reader workers."""
worker_hs_1 = self.make_worker_hs("synapse.app.client_reader")
worker_hs_2 = self.make_worker_hs("synapse.app.client_reader")
"""Test that registration works when using multiple generic workers."""
worker_hs_1 = self.make_worker_hs("synapse.app.generic_worker")
worker_hs_2 = self.make_worker_hs("synapse.app.generic_worker")

site_1 = self._hs_to_site[worker_hs_1]
channel_1 = make_request(
Expand Down
5 changes: 3 additions & 2 deletions tests/replication/test_federation_ack.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
class FederationAckTestCase(HomeserverTestCase):
def default_config(self) -> dict:
config = super().default_config()
config["worker_app"] = "synapse.app.federation_sender"
config["send_federation"] = False
config["worker_app"] = "synapse.app.generic_worker"
config["worker_name"] = "federation_sender1"
config["federation_sender_instances"] = ["federation_sender1"]
return config

def make_homeserver(self, reactor, clock):
Expand Down
Loading

0 comments on commit 854a688

Please sign in to comment.