From 90cf1eec44940f8ede4a7f0490da43021c84a13a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 2 Dec 2020 15:12:02 +0000 Subject: [PATCH 1/4] Remove redundant mocking --- tests/api/test_filtering.py | 18 ++---------------- tests/handlers/test_directory.py | 2 -- tests/handlers/test_profile.py | 2 -- tests/storage/test_redaction.py | 11 +++-------- tests/storage/test_roommember.py | 8 -------- 5 files changed, 5 insertions(+), 36 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index bcf1a8010e..279c94a03d 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -16,8 +16,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from mock import Mock - import jsonschema from twisted.internet import defer @@ -28,7 +26,7 @@ from synapse.api.filtering import Filter from synapse.events import make_event_from_dict from tests import unittest -from tests.utils import DeferredMockCallable, MockHttpResource, setup_test_homeserver +from tests.utils import setup_test_homeserver user_localpart = "test_user" @@ -42,21 +40,9 @@ def MockEvent(**kwargs): class FilteringTestCase(unittest.TestCase): - @defer.inlineCallbacks def setUp(self): - self.mock_federation_resource = MockHttpResource() - - self.mock_http_client = Mock(spec=[]) - self.mock_http_client.put_json = DeferredMockCallable() - - hs = yield setup_test_homeserver( - self.addCleanup, - federation_http_client=self.mock_http_client, - keyring=Mock(), - ) - + hs = setup_test_homeserver(self.addCleanup) self.filtering = hs.get_filtering() - self.datastore = hs.get_datastore() def test_errors_on_invalid_filters(self): diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 2f1f2a5517..770d225ed5 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -42,8 +42,6 @@ class DirectoryTestCase(unittest.HomeserverTestCase): self.mock_registry.register_query_handler = register_query_handler hs = self.setup_test_homeserver( - federation_http_client=None, - resource_for_federation=Mock(), federation_client=self.mock_federation, federation_registry=self.mock_registry, ) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index ea2bcf2655..919547556b 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -44,8 +44,6 @@ class ProfileTestCase(unittest.TestCase): hs = yield setup_test_homeserver( self.addCleanup, - federation_http_client=None, - resource_for_federation=Mock(), federation_client=self.mock_federation, federation_server=Mock(), federation_registry=self.mock_registry, diff --git a/tests/storage/test_redaction.py b/tests/storage/test_redaction.py index fd0add5db3..a6303bf0ee 100644 --- a/tests/storage/test_redaction.py +++ b/tests/storage/test_redaction.py @@ -14,9 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - -from mock import Mock - from canonicaljson import json from twisted.internet import defer @@ -30,12 +27,10 @@ from tests.utils import create_room class RedactionTestCase(unittest.HomeserverTestCase): - def make_homeserver(self, reactor, clock): - config = self.default_config() + def default_config(self): + config = super().default_config() config["redaction_retention_period"] = "30d" - return self.setup_test_homeserver( - resource_for_federation=Mock(), federation_http_client=None, config=config - ) + return config def prepare(self, reactor, clock, hs): self.store = hs.get_datastore() diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 5ba1db2332..d2aed66f6d 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -14,8 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from unittest.mock import Mock - from synapse.api.constants import Membership from synapse.rest.admin import register_servlets_for_client_rest_resource from synapse.rest.client.v1 import login, room @@ -34,12 +32,6 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase): room.register_servlets, ] - def make_homeserver(self, reactor, clock): - hs = self.setup_test_homeserver( - resource_for_federation=Mock(), federation_http_client=None - ) - return hs - def prepare(self, reactor, clock, hs: TestHomeServer): # We can't test the RoomMemberStore on its own without the other event From c834f1d67a7feeaebc353d0170f99a618bf32b5b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 2 Dec 2020 17:40:31 +0000 Subject: [PATCH 2/4] remove unused `resource_for_federation` This is now only used in `test_typing`, so move it there. --- tests/handlers/test_typing.py | 14 +++++++++++++- tests/utils.py | 17 ----------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 36086ca836..294b71e8e2 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -21,12 +21,13 @@ from mock import ANY, Mock, call from twisted.internet import defer from synapse.api.errors import AuthError +from synapse.federation.transport import server as federation_server from synapse.types import UserID, create_requester +from synapse.util.ratelimitutils import FederationRateLimiter from tests import unittest from tests.test_utils import make_awaitable from tests.unittest import override_config -from tests.utils import register_federation_servlets # Some local users to test with U_APPLE = UserID.from_string("@apple:test") @@ -52,6 +53,17 @@ def _make_edu_transaction_json(edu_type, content): return json.dumps(_expect_edu_transaction(edu_type, content)).encode("utf8") +def register_federation_servlets(hs, resource): + federation_server.register_servlets( + hs, + resource=resource, + authenticator=federation_server.Authenticator(hs), + ratelimiter=FederationRateLimiter( + hs.get_clock(), config=hs.config.rc_federation + ), + ) + + class TypingNotificationsTestCase(unittest.HomeserverTestCase): servlets = [register_federation_servlets] diff --git a/tests/utils.py b/tests/utils.py index c8d3ffbaba..f5f56e5a79 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -34,7 +34,6 @@ from synapse.api.room_versions import RoomVersions from synapse.config.database import DatabaseConnectionConfig from synapse.config.homeserver import HomeServerConfig from synapse.config.server import DEFAULT_ROOM_VERSION -from synapse.federation.transport import server as federation_server from synapse.http.server import HttpServer from synapse.logging.context import current_context, set_current_context from synapse.server import HomeServer @@ -42,7 +41,6 @@ from synapse.storage import DataStore from synapse.storage.database import LoggingDatabaseConnection from synapse.storage.engines import PostgresEngine, create_engine from synapse.storage.prepare_database import prepare_database -from synapse.util.ratelimitutils import FederationRateLimiter # set this to True to run the tests against postgres instead of sqlite. # @@ -342,24 +340,9 @@ def setup_test_homeserver( hs.get_auth_handler().validate_hash = validate_hash - fed = kwargs.get("resource_for_federation", None) - if fed: - register_federation_servlets(hs, fed) - return hs -def register_federation_servlets(hs, resource): - federation_server.register_servlets( - hs, - resource=resource, - authenticator=federation_server.Authenticator(hs), - ratelimiter=FederationRateLimiter( - hs.get_clock(), config=hs.config.rc_federation - ), - ) - - def get_mock_call_args(pattern_func, mock_func): """ Return the arguments the mock function was called with interpreted by the pattern functions argument list. From b751624ff8cd110ebfd312f13b330c9ac7cd0d86 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 2 Dec 2020 15:13:05 +0000 Subject: [PATCH 3/4] remove unused DeferredMockCallable --- tests/utils.py | 91 +------------------------------------------------- 1 file changed, 1 insertion(+), 90 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index f5f56e5a79..977eeaf6ee 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -20,13 +20,12 @@ import os import time import uuid import warnings -from inspect import getcallargs from typing import Type from urllib import parse as urlparse from mock import Mock, patch -from twisted.internet import defer, reactor +from twisted.internet import defer from synapse.api.constants import EventTypes from synapse.api.errors import CodeMessageException, cs_error @@ -343,14 +342,6 @@ def setup_test_homeserver( return hs -def get_mock_call_args(pattern_func, mock_func): - """ Return the arguments the mock function was called with interpreted - by the pattern functions argument list. - """ - invoked_args, invoked_kargs = mock_func.call_args - return getcallargs(pattern_func, *invoked_args, **invoked_kargs) - - def mock_getRawHeaders(headers=None): headers = headers if headers is not None else {} @@ -536,86 +527,6 @@ class MockClock: return d -def _format_call(args, kwargs): - return ", ".join( - ["%r" % (a) for a in args] + ["%s=%r" % (k, v) for k, v in kwargs.items()] - ) - - -class DeferredMockCallable: - """A callable instance that stores a set of pending call expectations and - return values for them. It allows a unit test to assert that the given set - of function calls are eventually made, by awaiting on them to be called. - """ - - def __init__(self): - self.expectations = [] - self.calls = [] - - def __call__(self, *args, **kwargs): - self.calls.append((args, kwargs)) - - if not self.expectations: - raise ValueError( - "%r has no pending calls to handle call(%s)" - % (self, _format_call(args, kwargs)) - ) - - for (call, result, d) in self.expectations: - if args == call[1] and kwargs == call[2]: - d.callback(None) - return result - - failure = AssertionError( - "Was not expecting call(%s)" % (_format_call(args, kwargs)) - ) - - for _, _, d in self.expectations: - try: - d.errback(failure) - except Exception: - pass - - raise failure - - def expect_call_and_return(self, call, result): - self.expectations.append((call, result, defer.Deferred())) - - @defer.inlineCallbacks - def await_calls(self, timeout=1000): - deferred = defer.DeferredList( - [d for _, _, d in self.expectations], fireOnOneErrback=True - ) - - timer = reactor.callLater( - timeout / 1000, - deferred.errback, - AssertionError( - "%d pending calls left: %s" - % ( - len([e for e in self.expectations if not e[2].called]), - [e for e in self.expectations if not e[2].called], - ) - ), - ) - - yield deferred - - timer.cancel() - - self.calls = [] - - def assert_had_no_calls(self): - if self.calls: - calls = self.calls - self.calls = [] - - raise AssertionError( - "Expected not to received any calls, got:\n" - + "\n".join(["call(%s)" % _format_call(c[0], c[1]) for c in calls]) - ) - - async def create_room(hs, room_id: str, creator_id: str): """Creates and persist a creation event for the given room """ From 92ce4a52582e9561a2af2c36fe8d8eb07e94654d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 2 Dec 2020 15:14:35 +0000 Subject: [PATCH 4/4] changelog --- changelog.d/8861.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8861.misc diff --git a/changelog.d/8861.misc b/changelog.d/8861.misc new file mode 100644 index 0000000000..9821f804cf --- /dev/null +++ b/changelog.d/8861.misc @@ -0,0 +1 @@ +Remove some unnecessary stubbing from unit tests.