Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
520a4bc
Refactor `get_success(...)` to allow other threads to make progress
MadLittleMods Jun 19, 2026
65a1c59
Refactor `get_failure`
MadLittleMods Jun 19, 2026
fdeed9a
Fix `get_failure(...)` raises docstring
MadLittleMods Jun 19, 2026
5ca9050
Add changelog
MadLittleMods Jun 19, 2026
6e9b2a2
Fix `get_failure` lint
MadLittleMods Jun 19, 2026
c45774c
Reduce timeout so you don't have to wait as long when something goes …
MadLittleMods Jun 19, 2026
ae7e367
Fix test cases that don't need `by=`
MadLittleMods Jun 19, 2026
f54d0c0
Fix `tests/storage/test_background_update.py`
MadLittleMods Jun 20, 2026
997a160
Fix `tests/app/test_homeserver_shutdown.py`
MadLittleMods Jun 20, 2026
b501ad1
Fix `tests/handlers/test_presence.py`
MadLittleMods Jun 20, 2026
9cfd0f9
Fix `tests/handlers/test_send_email.py`
MadLittleMods Jun 20, 2026
66a515b
Explain why remove `get_success_or_raise`
MadLittleMods Jun 20, 2026
a1092da
Extract logic to `_wait_for_deferred`
MadLittleMods Jun 20, 2026
09c91d3
Fix FIXME comment grammar
MadLittleMods Jun 20, 2026
4357aa4
Use 1 second timeout default
MadLittleMods Jun 20, 2026
edce488
Use "deferred" in `_wait_for_deferred` docstring
MadLittleMods Jun 20, 2026
5cc4590
Add example if you need to advance time
MadLittleMods Jun 20, 2026
5b27102
Fix `tests.handlers.test_profile.ProfileTestCase.test_background_upda…
MadLittleMods Jun 20, 2026
44253df
No need to change background update in `tests/app/test_homeserver_shu…
MadLittleMods Jun 20, 2026
47297af
Fix `tests.handlers.test_user_directory.UserDirectoryTestCase.test_pr…
MadLittleMods Jun 20, 2026
cc2c27b
Fix `tests/handlers/test_typing.py`
MadLittleMods Jun 20, 2026
26dc512
Fix `trial tests.replication.test_federation_ack`
MadLittleMods Jun 20, 2026
2bce6e7
Fix lints
MadLittleMods Jun 20, 2026
3425d15
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jun 20, 2026
ecce873
Remove other `till_deferred_has_result`
MadLittleMods Jun 20, 2026
2c51142
Explain better
MadLittleMods Jun 20, 2026
999d22d
Fix `tests.storage.databases.main.test_events_worker.GetEventCancella…
MadLittleMods Jun 20, 2026
41642be
Remove `wait_on_thread`
MadLittleMods Jun 20, 2026
350b15f
Fix `trial-olddeps`: `builtins.TypeError: 'type' object is not subscr…
MadLittleMods Jun 20, 2026
60ddfc6
Fix `tests.replication.tcp.test_handler.ChannelsTestCase.test_wait_fo…
MadLittleMods Jun 20, 2026
167ad62
Run CI again
MadLittleMods Jun 22, 2026
ad0bbb6
Fix `tests.replication.tcp.test_handler.ChannelsTestCase.test_wait_fo…
MadLittleMods Jun 22, 2026
f0e968a
Run CI again
MadLittleMods Jun 22, 2026
85edec1
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jun 22, 2026
919a94c
Align default timeout with `get_success`
MadLittleMods Jun 22, 2026
2c8ea3a
Remove double space in comment
MadLittleMods Jun 22, 2026
2300a19
Run CI again
MadLittleMods Jun 22, 2026
a1170c6
Run CI again
MadLittleMods Jun 23, 2026
efcd574
Run CI again
MadLittleMods Jun 23, 2026
749ea8b
Run CI again
MadLittleMods Jun 23, 2026
77ddfcf
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jun 23, 2026
92346f2
Switch to cross-compatible `time.sleep()`
MadLittleMods Jun 23, 2026
ce1758c
Comment about Python's default thread switch interval
MadLittleMods Jun 23, 2026
0add7cd
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jun 25, 2026
a871de0
`time.sleep(0.001)` after a few loops
MadLittleMods Jun 25, 2026
74060df
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jun 25, 2026
28586c6
Remove real-time `timeout`
MadLittleMods Jun 26, 2026
cb91056
Fix lints
MadLittleMods Jun 26, 2026
a975f83
Refactor to make `CLOCK_SCHEDULE_EPSILON` generic
MadLittleMods Jun 26, 2026
34f015f
Use `callLater(0)` for `FakeTransport`
MadLittleMods Jun 26, 2026
e7b09b3
Use `callLater(0)` for `FakeChannel`
MadLittleMods Jun 26, 2026
f11be8b
Remove some no longer necessary `advance(...)` in tests (related to `…
MadLittleMods Jun 26, 2026
453a296
Merge branch 'develop' into madlittlemods/better-get-success
MadLittleMods Jul 2, 2026
2bff819
Update references to `_EPSILON` (now called `CLOCK_SCHEDULE_EPSILON`)
MadLittleMods Jul 2, 2026
a12a9e0
Remove some no longer necessary `advance(...)` in tests (related to `…
MadLittleMods Jul 2, 2026
d034373
Better label fire-and-forget
MadLittleMods Jul 2, 2026
77184c8
Update docstring to be accurate after 100 loop refactor instead of re…
MadLittleMods Jul 2, 2026
5be3534
Better document plan to refactor and discussion where it spawned from
MadLittleMods Jul 2, 2026
77ce7a5
More accurate `test_redact_messages_all_rooms` description
MadLittleMods Jul 2, 2026
bc54b65
Advance by `CLOCK_SCHEDULE_EPSILON`
MadLittleMods Jul 2, 2026
3cb3c66
Fix `tests.crypto.test_keyring`
MadLittleMods Jul 2, 2026
c37b6ce
Fix `tests.util.test_task_scheduler.TestTaskScheduler.test_cancel_run…
MadLittleMods Jul 2, 2026
6e86cf3
Fix `tests.handlers.test_oidc.OidcHandlerTestCase.test_exchange_code_…
MadLittleMods Jul 2, 2026
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/19871.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update `HomeserverTestCase.get_success(...)` and friends to drive async Rust (Tokio runtime/thread pool).
21 changes: 20 additions & 1 deletion tests/app/test_homeserver_shutdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
from typing import Any
from unittest.mock import patch

from twisted.internet.defer import ensureDeferred

from synapse.app.homeserver import SynapseHomeServer
from synapse.logging.context import LoggingContext
from synapse.storage.background_updates import UpdaterStatus
Expand Down Expand Up @@ -76,6 +78,13 @@ async def shutdown() -> None:

self.get_success(shutdown())

# XXX: There can be a few already dispatched database queries (from normal
# background tasks in Synapse) and the threadless `ThreadPool` that we use in
# tests uses *untracked* clock calls to pass database results back so `shutdown`
# doesn't cancel those calls. This is a quirk of our test infrastructure
# (threadless `ThreadPool`) so this kind of "hack" is fine.
self.reactor.advance(0)
Comment on lines +79 to +84

@MadLittleMods MadLittleMods Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The explanation here is slightly hand-wavey.

The other comment explanations are more concrete at-least in the fact that I've traced some scheduled reactor callback.


# Cleanup the internal reference in our test case
del self.hs

Expand Down Expand Up @@ -106,7 +115,10 @@ def test_clean_homeserver_shutdown_mid_background_updates(self) -> None:
# Pump the background updates by a single iteration, just to ensure any extra
# resources it uses have been started.
store = weakref.proxy(self.hs.get_datastores().main)
self.get_success(store.db_pool.updates.do_next_background_update(False), by=0.1)
background_update_d = ensureDeferred(
store.db_pool.updates.do_next_background_update(False)
)
self.get_success(background_update_d)

hs_ref = weakref.ref(self.hs)

Expand All @@ -127,6 +139,13 @@ async def shutdown() -> None:

self.get_success(shutdown())

# XXX: There can be a few already dispatched database queries (from normal
# background tasks in Synapse) and the threadless `ThreadPool` that we use in
# tests uses *untracked* clock calls to pass database results back so `shutdown`
# doesn't cancel those calls. This is a quirk of our test infrastructure
# (threadless `ThreadPool`) so this kind of "hack" is fine.
self.reactor.advance(0)

# Cleanup the internal reference in our test case
del self.hs

Expand Down
1 change: 0 additions & 1 deletion tests/handlers/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ def create_invite() -> EventBase:
event.room_version,
),
exc=LimitExceededError,
by=0.5,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In a lot of cases, the by usage didn't seem necessary at all (test still passes) (no need to advance time in the reactor/clock)

)

def _build_and_send_join_event(
Expand Down
82 changes: 60 additions & 22 deletions tests/handlers/test_presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
get_verify_key,
)

from twisted.internet.defer import ensureDeferred
from twisted.internet.testing import MemoryReactor

from synapse.api.constants import EventTypes, Membership, PresenceState
Expand Down Expand Up @@ -58,6 +59,7 @@
from synapse.storage.keys import FetchKeyResult
from synapse.types import JsonDict, UserID, get_domain_from_id
from synapse.util.clock import Clock
from synapse.util.duration import Duration

from tests import unittest
from tests.replication._base import BaseMultiWorkerStreamTestCase
Expand Down Expand Up @@ -948,12 +950,17 @@ def test_external_process_timeout(self) -> None:
)
worker_presence_handler = worker_to_sync_against.get_presence_handler()

self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
self.user_id, self.device_id, True, PresenceState.ONLINE
),
by=0.1,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
Comment thread
erikjohnston marked this conversation as resolved.
Outdated
self.get_success(sync_d)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the main pattern I'm recommending if you need to advance time by an non-zero increment. ensureDeferred works well but the name is a bit non-obvious to describe that we want to make the task run in the background on its own.

run_in_background(...) would also work but it's usage is a bit awkward. I guess we could use run_coroutine_in_background(...) instead 🤔

The difference between ensureDeferred(...) vs run_in_background(...)/run_coroutine_in_background(...) is all of the extra LoggingContext (log context) handling. It doesn't matter for tests though.


# Check that if we wait a while without telling the handler the user has
# stopped syncing that their presence state doesn't get timed out.
Expand Down Expand Up @@ -1264,30 +1271,40 @@ def test_set_presence_from_syncing_multi_device(
worker_presence_handler = worker_to_sync_against.get_presence_handler()

# 1. Sync with the first device.
self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
user_id,
"dev-1",
affect_presence=dev_1_state != PresenceState.OFFLINE,
presence_state=dev_1_state,
),
by=0.01,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
self.get_success(sync_d)

# 2. Wait half the idle timer.
self.reactor.advance(IDLE_TIMER / 1000 / 2)
self.reactor.pump([0.1])

# 3. Sync with the second device.
self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
user_id,
"dev-2",
affect_presence=dev_2_state != PresenceState.OFFLINE,
presence_state=dev_2_state,
),
by=0.01,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
self.get_success(sync_d)

# 4. Assert the expected presence state.
state = self.get_success(
Expand All @@ -1305,15 +1322,21 @@ def test_set_presence_from_syncing_multi_device(
#
# This is due to EXTERNAL_PROCESS_EXPIRY being equivalent to IDLE_TIMER.
if test_with_workers:
with self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
f"@other-user:{self.hs.config.server.server_name}",
"dev-3",
affect_presence=True,
presence_state=PresenceState.ONLINE,
),
by=0.01,
):
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())

with self.get_success(sync_d):
pass

# 5. Advance such that the first device should be discarded (the idle timer),
Expand Down Expand Up @@ -1501,26 +1524,36 @@ def test_set_presence_from_non_syncing_multi_device(
worker_presence_handler = worker_to_sync_against.get_presence_handler()

# 1. Sync with the first device.
sync_1 = self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
user_id,
"dev-1",
affect_presence=dev_1_state != PresenceState.OFFLINE,
presence_state=dev_1_state,
),
by=0.1,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
sync_1 = self.get_success(sync_d)

# 2. Sync with the second device.
sync_2 = self.get_success(
sync_d = ensureDeferred(
worker_presence_handler.user_syncing(
user_id,
"dev-2",
affect_presence=dev_2_state != PresenceState.OFFLINE,
presence_state=dev_2_state,
),
by=0.1,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
sync_2 = self.get_success(sync_d)

# 3. Assert the expected presence state.
state = self.get_success(
Expand Down Expand Up @@ -1622,12 +1655,17 @@ def test_set_presence_from_syncing_keeps_busy(
# Perform a sync with a presence state other than busy. This should NOT change
# our presence status; we only change from busy if we explicitly set it via
# /presence/*.
self.get_success(
sync_d = ensureDeferred(
worker_to_sync_against.get_presence_handler().user_syncing(
self.user_id, self.device_id, True, PresenceState.ONLINE
),
by=0.1,
)
)
# `user_syncing` proxies the presence write to the main process over an HTTP
# replication request. The request body is streamed by a `Cooperator` that uses
# the clock to schedule each chunk at a tiny *non-zero* delay (`_EPSILON`), so
# we need to actually advance the clock for it to fire.
self.reactor.advance(Duration(microseconds=1).as_secs())
self.get_success(sync_d)

# Check against the main process that the user's presence did not change.
state = self.get_success(self.presence_handler.get_state(self.user_id_obj))
Expand Down
6 changes: 3 additions & 3 deletions tests/handlers/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ async def slow_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]:
self.assertEqual(membership[state_tuple].content["displayname"], "Frank")

# Let's be sure we are over the delay introduced by slow_update_membership
self.get_success(self.clock.sleep(Duration(milliseconds=20)), by=1)
self.reactor.advance(Duration(milliseconds=20).as_secs())

membership = self.get_success(
self.storage_controllers.state.get_current_state(
Expand Down Expand Up @@ -278,7 +278,7 @@ async def potentially_slow_update_membership(

# Let's be sure we are over the delay introduced by slow_update_membership
# and that the task was not executed as expected
self.get_success(self.clock.sleep(Duration(milliseconds=20)), by=1)
self.reactor.advance(Duration(milliseconds=20).as_secs())

membership = self.get_success(
self.storage_controllers.state.get_current_state(
Expand All @@ -300,7 +300,7 @@ async def potentially_slow_update_membership(
)

# Let's be sure we are over the delay introduced by slow_update_membership
self.get_success(self.clock.sleep(Duration(milliseconds=20)), by=1)
self.reactor.advance(Duration(milliseconds=20).as_secs())

# Updates should have been resumed from room 2 after the restart
# so room 1 should not have been updated this time
Expand Down
4 changes: 0 additions & 4 deletions tests/handlers/test_room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ def test_local_user_local_joins_contribute_to_limit_and_are_limited(self) -> Non
action=Membership.JOIN,
),
LimitExceededError,
by=0.5,
)

@override_config({"rc_joins_per_room": {"per_second": 0.1, "burst_count": 2}})
Expand Down Expand Up @@ -213,7 +212,6 @@ def test_remote_joins_contribute_to_rate_limit(self) -> None:
remote_room_hosts=[self.OTHER_SERVER_NAME],
),
LimitExceededError,
by=0.5,
)

# TODO: test that remote joins to a room are rate limited.
Expand Down Expand Up @@ -281,7 +279,6 @@ def test_local_users_joining_on_another_worker_contribute_to_rate_limit(
action=Membership.JOIN,
),
LimitExceededError,
by=0.5,
)

# Try to join as Chris on the original worker. Should get denied because Alice
Expand All @@ -294,7 +291,6 @@ def test_local_users_joining_on_another_worker_contribute_to_rate_limit(
action=Membership.JOIN,
),
LimitExceededError,
by=0.5,
)


Expand Down
12 changes: 10 additions & 2 deletions tests/handlers/test_send_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,12 @@ def test_send_email(self) -> None:
)
)

# This matches the two `callLater` delays in `FakeTransport.registerProducer`
self.reactor.advance(0)
self.reactor.advance(0.1)

# the message should now get delivered
self.get_success(d, by=0.1)
self.get_success(d)

# check it arrived
self.assertEqual(len(message_delivery.messages), 1)
Expand Down Expand Up @@ -212,8 +216,12 @@ def test_send_email_force_tls(self) -> None:
)
)

# This matches the two `callLater` delays in `FakeTransport.registerProducer`
self.reactor.advance(0)
self.reactor.advance(0.1)

# the message should now get delivered
self.get_success(d, by=0.1)
self.get_success(d)

# check it arrived
self.assertEqual(len(message_delivery.messages), 1)
Expand Down
Loading
Loading