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

Fix logged errors from unit tests. #10939

Merged
merged 5 commits into from
Sep 30, 2021
Merged

Fix logged errors from unit tests. #10939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
909343d
send_transaction must be async
clokep Sep 20, 2021
c9e814b
Properly call defer.fail.
clokep Sep 20, 2021
18bdc81
Fix error in scheduler tests.
clokep Sep 20, 2021
adcd37e
Fix-up calls in appservice testing.
clokep Sep 20, 2021
b2e7e22
Newsfragment
clokep Sep 29, 2021
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/10939.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix logged errors in unit tests.
40 changes: 19 additions & 21 deletions tests/appservice/test_scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from synapse.logging.context import make_deferred_yieldable

from tests import unittest
from tests.test_utils import make_awaitable
from tests.test_utils import simple_async_mock

from ..utils import MockClock

Expand All @@ -49,11 +49,10 @@ def test_single_service_up_txn_sent(self):
txn = Mock(id=txn_id, service=service, events=events)

# mock methods
self.store.get_appservice_state = Mock(
return_value=defer.succeed(ApplicationServiceState.UP)
)
txn.send = Mock(return_value=make_awaitable(True))
self.store.create_appservice_txn = Mock(return_value=defer.succeed(txn))
self.store.get_appservice_state = simple_async_mock(ApplicationServiceState.UP)
txn.send = simple_async_mock(True)
txn.complete = simple_async_mock(True)
self.store.create_appservice_txn = simple_async_mock(txn)
Comment on lines -52 to +55
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any change in behaviour here, or is this just a cleaner way of doing it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one bugfix in here, which is also handling txn.complete, which previously ended up as just a Mock which can't be awaited. The rest is just refactoring to be a bit cleaner. (Sorry for not separating those!)

While I was in here I also converted defer.succeed since the mocked functions don't return Deferreds anymore. Note that we (annoyingly) have two ways of making async mocks at the moment (simple_async_mock and make_awaitable), essentially you can switch defer.succeed -> make_awaitable OR you can switch Mock(return_value=defer.succeed(...)) -> simple_async_mock(...). Sad times.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. Thanks!


# actual call
self.successResultOf(defer.ensureDeferred(self.txnctrl.send(service, events)))
Expand All @@ -71,10 +70,10 @@ def test_single_service_down(self):
events = [Mock(), Mock()]

txn = Mock(id="idhere", service=service, events=events)
self.store.get_appservice_state = Mock(
return_value=defer.succeed(ApplicationServiceState.DOWN)
self.store.get_appservice_state = simple_async_mock(
ApplicationServiceState.DOWN
)
self.store.create_appservice_txn = Mock(return_value=defer.succeed(txn))
self.store.create_appservice_txn = simple_async_mock(txn)

# actual call
self.successResultOf(defer.ensureDeferred(self.txnctrl.send(service, events)))
Expand All @@ -94,12 +93,10 @@ def test_single_service_up_txn_not_sent(self):
txn = Mock(id=txn_id, service=service, events=events)

# mock methods
self.store.get_appservice_state = Mock(
return_value=defer.succeed(ApplicationServiceState.UP)
)
self.store.set_appservice_state = Mock(return_value=defer.succeed(True))
txn.send = Mock(return_value=make_awaitable(False)) # fails to send
self.store.create_appservice_txn = Mock(return_value=defer.succeed(txn))
self.store.get_appservice_state = simple_async_mock(ApplicationServiceState.UP)
self.store.set_appservice_state = simple_async_mock(True)
txn.send = simple_async_mock(False) # fails to send
self.store.create_appservice_txn = simple_async_mock(txn)

# actual call
self.successResultOf(defer.ensureDeferred(self.txnctrl.send(service, events)))
Expand All @@ -122,7 +119,7 @@ def setUp(self):
self.as_api = Mock()
self.store = Mock()
self.service = Mock()
self.callback = Mock()
self.callback = simple_async_mock()
self.recoverer = _Recoverer(
clock=self.clock,
as_api=self.as_api,
Expand All @@ -144,8 +141,8 @@ def take_txn(*args, **kwargs):
self.recoverer.recover()
# shouldn't have called anything prior to waiting for exp backoff
self.assertEquals(0, self.store.get_oldest_unsent_txn.call_count)
txn.send = Mock(return_value=make_awaitable(True))
txn.complete.return_value = make_awaitable(None)
txn.send = simple_async_mock(True)
txn.complete = simple_async_mock(None)
# wait for exp backoff
self.clock.advance_time(2)
self.assertEquals(1, txn.send.call_count)
Expand All @@ -170,8 +167,8 @@ def take_txn(*args, **kwargs):

self.recoverer.recover()
self.assertEquals(0, self.store.get_oldest_unsent_txn.call_count)
txn.send = Mock(return_value=make_awaitable(False))
txn.complete.return_value = make_awaitable(None)
txn.send = simple_async_mock(False)
txn.complete = simple_async_mock(None)
self.clock.advance_time(2)
self.assertEquals(1, txn.send.call_count)
self.assertEquals(0, txn.complete.call_count)
Expand All @@ -184,7 +181,7 @@ def take_txn(*args, **kwargs):
self.assertEquals(3, txn.send.call_count)
self.assertEquals(0, txn.complete.call_count)
self.assertEquals(0, self.callback.call_count)
txn.send = Mock(return_value=make_awaitable(True)) # successfully send the txn
txn.send = simple_async_mock(True) # successfully send the txn
pop_txn = True # returns the txn the first time, then no more.
self.clock.advance_time(16)
self.assertEquals(1, txn.send.call_count) # new mock reset call count
Expand All @@ -195,6 +192,7 @@ def take_txn(*args, **kwargs):
class ApplicationServiceSchedulerQueuerTestCase(unittest.TestCase):
def setUp(self):
self.txn_ctrl = Mock()
self.txn_ctrl.send = simple_async_mock()
self.queuer = _ServiceQueuer(self.txn_ctrl, MockClock())

def test_send_single_event_no_queue(self):
Expand Down
7 changes: 6 additions & 1 deletion tests/events/test_presence_router.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from synapse.types import JsonDict, StreamToken, create_requester

from tests.handlers.test_sync import generate_sync_config
from tests.test_utils import simple_async_mock
from tests.unittest import FederatingHomeserverTestCase, TestCase, override_config


Expand Down Expand Up @@ -133,8 +134,12 @@ class PresenceRouterTestCase(FederatingHomeserverTestCase):
]

def make_homeserver(self, reactor, clock):
# Mock out the calls over federation.
fed_transport_client = Mock(spec=["send_transaction"])
fed_transport_client.send_transaction = simple_async_mock({})

hs = self.setup_test_homeserver(
federation_transport_client=Mock(spec=["send_transaction"]),
federation_transport_client=fed_transport_client,
)
# Load the modules into the homeserver
module_api = hs.get_module_api()
Expand Down
6 changes: 3 additions & 3 deletions tests/federation/test_federation_sender.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ def test_unreachable_server(self):
recovery
"""
mock_send_txn = self.hs.get_federation_transport_client().send_transaction
mock_send_txn.side_effect = lambda t, cb: defer.fail("fail")
mock_send_txn.side_effect = lambda t, cb: defer.fail(AssertionError("fail"))

# create devices
u1 = self.register_user("user", "pass")
Expand Down Expand Up @@ -376,7 +376,7 @@ def test_prune_outbound_device_pokes1(self):
This case tests the behaviour when the server has never been reachable.
"""
mock_send_txn = self.hs.get_federation_transport_client().send_transaction
mock_send_txn.side_effect = lambda t, cb: defer.fail("fail")
mock_send_txn.side_effect = lambda t, cb: defer.fail(AssertionError("fail"))

# create devices
u1 = self.register_user("user", "pass")
Expand Down Expand Up @@ -429,7 +429,7 @@ def test_prune_outbound_device_pokes2(self):

# now the server goes offline
mock_send_txn = self.hs.get_federation_transport_client().send_transaction
mock_send_txn.side_effect = lambda t, cb: defer.fail("fail")
mock_send_txn.side_effect = lambda t, cb: defer.fail(AssertionError("fail"))

self.login("user", "pass", device_id="D2")
self.login("user", "pass", device_id="D3")
Expand Down
7 changes: 6 additions & 1 deletion tests/module_api/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from tests.events.test_presence_router import send_presence_update, sync_presence
from tests.replication._base import BaseMultiWorkerStreamTestCase
from tests.test_utils import simple_async_mock
from tests.test_utils.event_injection import inject_member_event
from tests.unittest import HomeserverTestCase, override_config
from tests.utils import USE_POSTGRES_FOR_TESTS
Expand All @@ -46,8 +47,12 @@ def prepare(self, reactor, clock, homeserver):
self.auth_handler = homeserver.get_auth_handler()

def make_homeserver(self, reactor, clock):
# Mock out the calls over federation.
fed_transport_client = Mock(spec=["send_transaction"])
fed_transport_client.send_transaction = simple_async_mock({})

return self.setup_test_homeserver(
federation_transport_client=Mock(spec=["send_transaction"]),
federation_transport_client=fed_transport_client,
)

def test_can_register_user(self):
Expand Down