Skip to content

Update FakeChannel.await_result(...) to drive async Rust (Tokio runtime/thread pool)#19879

Merged
MadLittleMods merged 30 commits into
developfrom
madlittlemods/try-fake-channel-await-result-rust-changes
Jul 7, 2026
Merged

Update FakeChannel.await_result(...) to drive async Rust (Tokio runtime/thread pool)#19879
MadLittleMods merged 30 commits into
developfrom
madlittlemods/try-fake-channel-await-result-rust-changes

Conversation

@MadLittleMods

@MadLittleMods MadLittleMods commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Update FakeChannel.await_result(...) to drive async Rust (Tokio runtime/thread pool)

Split off from #19878 (needed to drive requests in tests that use async Rust)

Follow-up to #19871

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

@MadLittleMods MadLittleMods changed the title Try FakeChannel.await_result(...) changes on their own Try FakeChannel.await_result(...) changes to drive async Rust on their own Jun 25, 2026
Comment on lines +569 to +571
# `notifier.wait_for_stream_token(from_token)` only checks every 500ms so we
# need to match that in order to make sure we hit the wake-up for sure.
channel.await_result(timeout_ms=500)

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.

Previously, this worked because await_result(...) would advance 0.1s past the specified timeout_ms and I guess that worked out to hit the 500ms notifier.wait_for_stream_token(from_token) check interval every time.

Now await_result(...) can timeout when we exactly hit the timeout_ms so we weren't hitting it.

I've updated the second wait to be 500ms to match notifier.wait_for_stream_token(from_token) so we know that we can always hit the interval regardless of the current Twisted clock time.

@MadLittleMods MadLittleMods changed the title Try FakeChannel.await_result(...) changes to drive async Rust on their own Update FakeChannel.await_result(...) and friends to drive async Rust (Tokio runtime/thread pool) Jun 25, 2026
Comment thread changelog.d/19879.misc
@@ -0,0 +1 @@
Update `HomeserverTestCase.get_success(...)` and friends to drive async Rust (Tokio runtime/thread pool).

@MadLittleMods MadLittleMods Jun 25, 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.

Same changelog as #19871 so they merge

We can consider FakeChannel.await_result(...) one of the "friends" of HomeserverTestCase.get_success(...)

Comment thread tests/server.py
Comment on lines +331 to 332
# TODO: Why?
self._reactor.run()

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.

Prior art. Something to figure out in a future PR. Just noting that I don't know. The reactor should already be running and we can potentially remove it in another PR.

Comment thread tests/server.py
@@ -301,15 +302,85 @@ def transport(self) -> "FakeChannel":
def await_result(self, timeout_ms: int = 1000) -> None:

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.

Similar structure to what we've done in #19871

@MadLittleMods MadLittleMods changed the title Update FakeChannel.await_result(...) and friends to drive async Rust (Tokio runtime/thread pool) Update FakeChannel.await_result(...) to drive async Rust (Tokio runtime/thread pool) Jun 25, 2026
Comment thread changelog.d/19879.misc
```
Error:
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/test_mau.py", line 74, in test_simple_deny_mau
    self.do_sync_for_user(token2)
  File "/home/runner/work/synapse/synapse/tests/test_mau.py", line 345, in do_sync_for_user
    raise HttpResponseException(
synapse.api.errors.ProxiedRequestError: 403: Monthly Active User Limit Exceeded

tests.test_mau.TestMauLimit.test_simple_deny_mau
```
Comment thread tests/server.py Outdated
if user_id:
timestamp = await self.store.user_last_seen_monthly_active(user_id)
if timestamp:
if timestamp is not None:

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.

Fixing a flawed check (0 is falsey) that was exposed by #19879 (comment)

self.assertEqual(channel.json_body["data"], "foo=bar")
self.assertEqual(channel.json_body["sequence_token"], sequence_token)
self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 100)
self.assertGreater(channel.json_body["expires_in_ms"], 0)

@MadLittleMods MadLittleMods Jul 7, 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.

Had to touch this because await_result(...) changed the minimum amount of time we advance by. These tests relied on that internal detail.


We don't need to test exact expires_in_ms values. We just care that it's > 0.

And we don't care about testing it on subsequent requests either. This test has nothing to do with expiration.

If we wanted to do this kind of thing, we should do something like the following:

duration_between_requests = Duration(milliseconds=100)

# Advance by some known amount
self.reactor.advance(duration_between_requests.as_secs())

# ...
self.assertGreater(channel.json_body["expires_in_ms"], 0)
self.assertLessEqual(
    channel.json_body["expires_in_ms"],
    prev_expires_in_ms - duration_between_requests.as_millis(),
)
prev_expires_in_ms = channel.json_body["expires_in_ms"]

Related discussion: #19539 (comment)

…elimit`

```
[FAIL]
Traceback (most recent call last):
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.10/lib/python3.10/site-packages/parameterized/parameterized.py", line 620, in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
  File "/home/runner/work/synapse/synapse/tests/rest/client/test_delayed_events.py", line 434, in test_send_delayed_event_ratelimit
    self.assertEqual(HTTPStatus.OK, channel.code, channel.result)
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.10/lib/python3.10/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
    super().assertEqual(first, second, msg)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 845, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 838, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: <HTTPStatus.OK: 200> != 429 : {'version': b'1.1', 'code': b'429', 'reason': b'Unknown Status', 'headers': Headers({b'Server': [b'1'], b'Date': [b'Tue, 07 Jul 2026 00:20:42 GMT'], b'Retry-After': [b'1'], b'Content-Type': [b'application/json'], b'Cache-Control': [b'no-cache, no-store, must-revalidate'], b'Access-Control-Allow-Origin': [b'*'], b'Access-Control-Allow-Methods': [b'GET, HEAD, POST, PUT, DELETE, OPTIONS'], b'Access-Control-Allow-Headers': [b'X-Requested-With, Content-Type, Authorization, Date'], b'Access-Control-Expose-Headers': [b'Synapse-Trace-Id, Server']}), 'body': b'{"errcode":"M_LIMIT_EXCEEDED","error":"Too Many Requests","retry_after_ms":396}', 'done': True}

tests.rest.client.test_delayed_events.DelayedEventsTestCase.test_send_delayed_event_ratelimit_0
tests.rest.client.test_delayed_events.DelayedEventsTestCase.test_send_delayed_event_ratelimit_1
```
…estCases`

```
[FAIL]
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/federation/test_federation_sender.py", line 790, in test_prune_outbound_device_pokes1
    self.assertGreaterEqual(mock_send_txn.call_count, 4)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 1250, in assertGreaterEqual
    self.fail(self._formatMessage(msg, standardMsg))
twisted.trial.unittest.FailTest: 2 not greater than or equal to 4

tests.federation.test_federation_sender.FederationSenderDevicesTestCases.test_prune_outbound_device_pokes1
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/federation/test_federation_sender.py", line 847, in test_prune_outbound_device_pokes2
    self.assertGreaterEqual(mock_send_txn.call_count, 3)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 1250, in assertGreaterEqual
    self.fail(self._formatMessage(msg, standardMsg))
twisted.trial.unittest.FailTest: 2 not greater than or equal to 3

tests.federation.test_federation_sender.FederationSenderDevicesTestCases.test_prune_outbound_device_pokes2
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/federation/test_federation_sender.py", line 743, in test_unreachable_server
    self.assertGreaterEqual(mock_send_txn.call_count, 4)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 1250, in assertGreaterEqual
    self.fail(self._formatMessage(msg, standardMsg))
twisted.trial.unittest.FailTest: 2 not greater than or equal to 4

tests.federation.test_federation_sender.FederationSenderDevicesTestCases.test_unreachable_server
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/federation/test_federation_sender.py", line 594, in test_upload_signatures
    self.assertEqual(len(self.edus), 2)
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.10/lib/python3.10/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
    super().assertEqual(first, second, msg)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 845, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/unittest/case.py", line 838, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 1 != 2

tests.federation.test_federation_sender.FederationSenderDevicesTestCases.test_upload_signatures
```
self.edus.extend(data["edus"])
return {}

def wait_for_device_list_updates_to_be_sent(self) -> None:

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.

Added wait_for_device_list_updates_to_be_sent to be more precise

Comment on lines +742 to +744
# Wait some time in between each device list update as we want each of them to
# be attempted to be sent in their own transaction
self.reactor.advance(Duration(seconds=1).as_secs())

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 main fixes to these tests was adding some time in between each device list update so they are sent in their own transaction. Tests assert self.assertGreaterEqual(mock_send_txn.call_count

Previously, we advanced 100ms on every login attempt because of await_result(...). await_result(...) now only advances 1ms and we need to space it out more.

Comment on lines +785 to +789
# Note: The extra 1ms is from `make_request(...)` -> `await_result(...)`
#
# FIXME: This test shouldn't care about this internal detail (don't
# assert exact timing)
last_seen=123456001,

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.

Something for a future PR

@MadLittleMods MadLittleMods marked this pull request as ready for review July 7, 2026 08:34
@MadLittleMods MadLittleMods requested a review from a team as a code owner July 7, 2026 08:34
@erikjohnston erikjohnston self-assigned this Jul 7, 2026

@erikjohnston erikjohnston left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@MadLittleMods MadLittleMods merged commit 026937b into develop Jul 7, 2026
45 of 47 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/try-fake-channel-await-result-rust-changes branch July 7, 2026 09:07
@MadLittleMods

Copy link
Copy Markdown
Contributor Author

Thanks for the review @erikjohnston 🐍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants