Fix client reader sharding tests#7853
Conversation
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
clokep
left a comment
There was a problem hiding this comment.
Looks good overall! Thanks for fixing all this up, I think it'll greatly improve being able to make other tests with multiple homeservers.
tests/replication/_base.py
Outdated
|
|
||
| return resource | ||
|
|
||
| def make_worker_hs(self, worker_app: str, extra_config={}) -> HomeServer: |
There was a problem hiding this comment.
Is it worth mentioning that this uses a mock for the HTTP client that can be used to assert calls back to the primary process?
While we're here can we add a type hint for extra_config?
There was a problem hiding this comment.
I've docced and typed this properly now. I actually decided that it was cleaner to have a **kwargs that we can pass through to setup_test_homeserver so that different tests can easily mock out different things (e.g. the pusher sharding test I'm writing needs to mock out a different http client)
There was a problem hiding this comment.
Yeah, that makes sense. Could also be better to sub-class this in a test-case if you want them all to have the same config.
* commit 'a973bcb8a': Add some tiny type annotations (#7870) Remove obsolete comment. Ensure that calls to `json.dumps` are compatible with the standard library json. (#7836) Avoid brand new rooms in `delete_old_current_state_events` (#7854) Allow accounts to be re-activated from the admin APIs. (#7847) Fix tests Fix typo Newsfile Use get_users_in_room rather than state handler in typing for speed Fix client reader sharding tests (#7853) Convert E2E key and room key handlers to async/await. (#7851) Return the proper 403 Forbidden error during errors with JWT logins. (#7844) remove `retry_on_integrity_error` wrapper for persist_events (#7848)
This fixes a bug in the client reader sharding tests where the register requests were always going to master instead of the workers (due to a typo in using
self.resourcerather thanresource). Fixing that leads to timeouts as the/registerpaths try and send a HTTP replication request to master, which we didn't handle.Currently, the tests are set up so that you need to explicitly call
self.handle_http_replication_attempt()after a connection attempt has been made. This is impossible to do if sending a request, asrenderblocks until the request is completed. We could solve this by trying and making it possible torendercallself.handle_http_replication_attemptat the same time, but it feels nicer to instead have the test base class automatically handle the http replication call.All in all, this PR:
tests/replication/)tests/server.py)tests/replication/_base.py)synapse/http/client.py)