Remove MockClock() (#18992)

Spawning from adding some logcontext debug logs in
#18966 and since we're not
logging at the `set_current_context(...)` level (see reasoning there),
this removes some usage of `set_current_context(...)`.

Specifically, `MockClock.call_later(...)` doesn't handle logcontexts
correctly. It uses the calling logcontext as the callback context
(wrong, as the logcontext could finish before the callback finishes) and
it didn't reset back to the sentinel context before handing back to the
reactor. It was like this since it was [introduced 10+ years
ago](38da988).
Instead of fixing the implementation which would just be a copy of our
normal `Clock`, we can just remove `MockClock`

Author: MadLittleMods

changelog.d/18992.misc

@@ -0,0 +1 @@
+Remove `MockClock()` in tests.

synapse/app/phone_stats_home.py

@@ -33,15 +33,17 @@
     run_as_background_process,
 )
 from synapse.types import JsonDict
-from synapse.util.constants import ONE_HOUR_SECONDS, ONE_MINUTE_SECONDS
+from synapse.util.constants import (
+    MILLISECONDS_PER_SECOND,
+    ONE_HOUR_SECONDS,
+    ONE_MINUTE_SECONDS,
+)
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 logger = logging.getLogger("synapse.app.homeserver")
 
-MILLISECONDS_PER_SECOND = 1000
-
 INITIAL_DELAY_BEFORE_FIRST_PHONE_HOME_SECONDS = 5 * ONE_MINUTE_SECONDS
 """
 We wait 5 minutes to send the first set of stats as the server can be quite busy the

synapse/util/constants.py

@@ -18,3 +18,5 @@
 # readability and catching bugs.
 ONE_MINUTE_SECONDS = 60
 ONE_HOUR_SECONDS = 60 * ONE_MINUTE_SECONDS
+
+MILLISECONDS_PER_SECOND = 1000

tests/appservice/test_scheduler.py

@@ -18,7 +18,7 @@
 # [This file includes modifications made by New Vector Limited]
 #
 #
-from typing import List, Optional, Sequence, Tuple, cast
+from typing import List, Optional, Sequence, Tuple
 from unittest.mock import AsyncMock, Mock
 
 from typing_extensions import TypeAlias
@@ -44,13 +44,12 @@
 from synapse.util.clock import Clock
 
 from tests import unittest
-
-from ..utils import MockClock
+from tests.server import get_clock
 
 
 class ApplicationServiceSchedulerTransactionCtrlTestCase(unittest.TestCase):
     def setUp(self) -> None:
-        self.clock = MockClock()
+        self.reactor, self.clock = get_clock()
         self.store = Mock()
         self.as_api = Mock()
 
@@ -170,14 +169,14 @@ def test_single_service_up_txn_not_sent(self) -> None:
 
 class ApplicationServiceSchedulerRecovererTestCase(unittest.TestCase):
     def setUp(self) -> None:
-        self.clock = MockClock()
+        self.reactor, self.clock = get_clock()
         self.as_api = Mock()
         self.store = Mock()
         self.service = Mock()
         self.callback = AsyncMock()
         self.recoverer = _Recoverer(
             server_name="test_server",
-            clock=cast(Clock, self.clock),
+            clock=self.clock,
             as_api=self.as_api,
             store=self.store,
             service=self.service,
@@ -202,7 +201,7 @@ def take_txn(
         txn.send = AsyncMock(return_value=True)
         txn.complete = AsyncMock(return_value=None)
         # wait for exp backoff
-        self.clock.advance_time(2)
+        self.reactor.advance(2)
         self.assertEqual(1, txn.send.call_count)
         self.assertEqual(1, txn.complete.call_count)
         # 2 because it needs to get None to know there are no more txns
@@ -229,21 +228,21 @@ def take_txn(
         self.assertEqual(0, self.store.get_oldest_unsent_txn.call_count)
         txn.send = AsyncMock(return_value=False)
         txn.complete = AsyncMock(return_value=None)
-        self.clock.advance_time(2)
+        self.reactor.advance(2)
         self.assertEqual(1, txn.send.call_count)
         self.assertEqual(0, txn.complete.call_count)
         self.assertEqual(0, self.callback.call_count)
-        self.clock.advance_time(4)
+        self.reactor.advance(4)
         self.assertEqual(2, txn.send.call_count)
         self.assertEqual(0, txn.complete.call_count)
         self.assertEqual(0, self.callback.call_count)
-        self.clock.advance_time(8)
+        self.reactor.advance(8)
         self.assertEqual(3, txn.send.call_count)
         self.assertEqual(0, txn.complete.call_count)
         self.assertEqual(0, self.callback.call_count)
         txn.send = AsyncMock(return_value=True)  # successfully send the txn
         pop_txn = True  # returns the txn the first time, then no more.
-        self.clock.advance_time(16)
+        self.reactor.advance(16)
         self.assertEqual(1, txn.send.call_count)  # new mock reset call count
         self.assertEqual(1, txn.complete.call_count)
         self.callback.assert_called_once_with(self.recoverer)
@@ -268,7 +267,7 @@ def take_txn(
         self.assertEqual(0, self.store.get_oldest_unsent_txn.call_count)
         txn.send = AsyncMock(return_value=False)
         txn.complete = AsyncMock(return_value=None)
-        self.clock.advance_time(2)
+        self.reactor.advance(2)
         self.assertEqual(1, txn.send.call_count)
         self.assertEqual(0, txn.complete.call_count)
         self.assertEqual(0, self.callback.call_count)

tests/config/test_oauth_delegation.py

@@ -231,7 +231,10 @@ def test_auth_providers_cannot_be_enabled(self) -> None:
         reactor, clock = get_clock()
         with self.assertRaises(ConfigError):
             setup_test_homeserver(
-                self.addCleanup, reactor=reactor, clock=clock, config=config
+                cleanup_func=self.addCleanup,
+                config=config,
+                reactor=reactor,
+                clock=clock,
             )
 
     def test_jwt_auth_cannot_be_enabled(self) -> None:
@@ -395,7 +398,10 @@ def test_auth_providers_cannot_be_enabled(self) -> None:
         reactor, clock = get_clock()
         with self.assertRaises(ConfigError):
             setup_test_homeserver(
-                self.addCleanup, reactor=reactor, clock=clock, config=config
+                cleanup_func=self.addCleanup,
+                config=config,
+                reactor=reactor,
+                clock=clock,
             )
 
     @skip_unless(HAS_AUTHLIB, "requires authlib")

tests/handlers/test_appservice.py

@@ -49,9 +49,9 @@
 from synapse.util.stringutils import random_string
 
 from tests import unittest
+from tests.server import get_clock
 from tests.test_utils import event_injection
 from tests.unittest import override_config
-from tests.utils import MockClock
 
 
 class AppServiceHandlerTestCase(unittest.TestCase):
@@ -61,14 +61,16 @@ def setUp(self) -> None:
         self.mock_store = Mock()
         self.mock_as_api = AsyncMock()
         self.mock_scheduler = Mock()
+        self.reactor, self.clock = get_clock()
+
         hs = Mock()
         hs.get_datastores.return_value = Mock(main=self.mock_store)
         self.mock_store.get_appservice_last_pos = AsyncMock(return_value=None)
         self.mock_store.set_appservice_last_pos = AsyncMock(return_value=None)
         self.mock_store.set_appservice_stream_type_pos = AsyncMock(return_value=None)
         hs.get_application_service_api.return_value = self.mock_as_api
         hs.get_application_service_scheduler.return_value = self.mock_scheduler
-        hs.get_clock.return_value = MockClock()
+        hs.get_clock.return_value = self.clock
         self.handler = ApplicationServicesHandler(hs)
         self.event_source = hs.get_event_sources()
 

tests/handlers/test_e2e_room_keys.py

@@ -21,7 +21,6 @@
 #
 
 import copy
-from unittest import mock
 
 from twisted.internet.testing import MemoryReactor
 
@@ -50,7 +49,7 @@
 
 class E2eRoomKeysHandlerTestCase(unittest.HomeserverTestCase):
     def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
-        return self.setup_test_homeserver(replication_layer=mock.Mock())
+        return self.setup_test_homeserver()
 
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.handler = hs.get_e2e_room_keys_handler()

tests/http/federation/test_srv_resolver.py

@@ -30,7 +30,7 @@
 from synapse.logging.context import LoggingContext, current_context
 
 from tests import unittest
-from tests.utils import MockClock
+from tests.server import get_clock
 
 
 class SrvResolverTestCase(unittest.TestCase):
@@ -105,7 +105,7 @@ def test_from_cache_expired_and_dns_fail(
 
     @defer.inlineCallbacks
     def test_from_cache(self) -> Generator["Deferred[object]", object, None]:
-        clock = MockClock()
+        reactor, clock = get_clock()
 
         dns_client_mock = Mock(spec_set=["lookupService"])
         dns_client_mock.lookupService = Mock(spec_set=[])

tests/http/test_matrixfederationclient.py

@@ -63,10 +63,6 @@ def check_logcontext(context: LoggingContextOrSentinel) -> None:
 
 
 class FederationClientTests(HomeserverTestCase):
-    def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
-        hs = self.setup_test_homeserver(reactor=reactor, clock=clock)
-        return hs
-
     def prepare(
         self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer
     ) -> None:

tests/media/test_media_retention.py

@@ -37,7 +37,6 @@
 
 from tests import unittest
 from tests.unittest import override_config
-from tests.utils import MockClock
 
 
 class MediaRetentionTestCase(unittest.HomeserverTestCase):
@@ -51,12 +50,6 @@ class MediaRetentionTestCase(unittest.HomeserverTestCase):
         admin.register_servlets_for_client_rest_resource,
     ]
 
-    def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
-        # We need to be able to test advancing time in the homeserver, so we
-        # replace the test homeserver's default clock with a MockClock, which
-        # supports advancing time.
-        return self.setup_test_homeserver(clock=MockClock())
-
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.remote_server_name = "remote.homeserver"
         self.store = hs.get_datastores().main