Merge pull request #8761 from matrix-org/rav/test_request_rendering

Make `make_request` actually render the request

changelog.d/8761.misc

@@ -0,0 +1 @@
+ Refactor test utilities for injecting HTTP requests.

tests/app/test_frontend_proxy.py

@@ -15,7 +15,7 @@
 
 from synapse.app.generic_worker import GenericWorkerServer
 
-from tests.server import make_request, render
+from tests.server import make_request
 from tests.unittest import HomeserverTestCase
 
 
@@ -56,10 +56,8 @@ def test_listen_http_with_presence_enabled(self):
         # Grab the resource from the site that was told to listen
         self.assertEqual(len(self.reactor.tcpServers), 1)
         site = self.reactor.tcpServers[0][1]
-        resource = site.resource.children[b"_matrix"].children[b"client"]
 
-        request, channel = make_request(self.reactor, site, "PUT", "presence/a/status")
-        render(request, resource, self.reactor)
+        _, channel = make_request(self.reactor, site, "PUT", "presence/a/status")
 
         # 400 + unrecognised, because nothing is registered
         self.assertEqual(channel.code, 400)
@@ -78,10 +76,8 @@ def test_listen_http_with_presence_disabled(self):
         # Grab the resource from the site that was told to listen
         self.assertEqual(len(self.reactor.tcpServers), 1)
         site = self.reactor.tcpServers[0][1]
-        resource = site.resource.children[b"_matrix"].children[b"client"]
 
-        request, channel = make_request(self.reactor, site, "PUT", "presence/a/status")
-        render(request, resource, self.reactor)
+        _, channel = make_request(self.reactor, site, "PUT", "presence/a/status")
 
         # 401, because the stub servlet still checks authentication
         self.assertEqual(channel.code, 401)

tests/app/test_openid_listener.py

@@ -20,7 +20,7 @@
 from synapse.app.homeserver import SynapseHomeServer
 from synapse.config.server import parse_listener_def
 
-from tests.server import make_request, render
+from tests.server import make_request
 from tests.unittest import HomeserverTestCase
 
 
@@ -67,16 +67,15 @@ def test_openid_listener(self, names, expectation):
         # Grab the resource from the site that was told to listen
         site = self.reactor.tcpServers[0][1]
         try:
-            resource = site.resource.children[b"_matrix"].children[b"federation"]
+            site.resource.children[b"_matrix"].children[b"federation"]
         except KeyError:
             if expectation == "no_resource":
                 return
             raise
 
-        request, channel = make_request(
+        _, channel = make_request(
             self.reactor, site, "GET", "/_matrix/federation/v1/openid/userinfo"
         )
-        render(request, resource, self.reactor)
 
         self.assertEqual(channel.code, 401)
 
@@ -116,15 +115,14 @@ def test_openid_listener(self, names, expectation):
         # Grab the resource from the site that was told to listen
         site = self.reactor.tcpServers[0][1]
         try:
-            resource = site.resource.children[b"_matrix"].children[b"federation"]
+            site.resource.children[b"_matrix"].children[b"federation"]
         except KeyError:
             if expectation == "no_resource":
                 return
             raise
 
-        request, channel = make_request(
+        _, channel = make_request(
             self.reactor, site, "GET", "/_matrix/federation/v1/openid/userinfo"
         )
-        render(request, resource, self.reactor)
 
         self.assertEqual(channel.code, 401)

tests/federation/test_complexity.py

@@ -51,7 +51,6 @@ def test_complexity_simple(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/unstable/rooms/%s/complexity" % (room_1,)
         )
-        self.render(request)
         self.assertEquals(200, channel.code)
         complexity = channel.json_body["v1"]
         self.assertTrue(complexity > 0, complexity)
@@ -64,7 +63,6 @@ def test_complexity_simple(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/unstable/rooms/%s/complexity" % (room_1,)
         )
-        self.render(request)
         self.assertEquals(200, channel.code)
         complexity = channel.json_body["v1"]
         self.assertEqual(complexity, 1.23)

tests/federation/test_federation_server.py

@@ -51,7 +51,6 @@ def test_bad_request(self, query_content):
             "/_matrix/federation/v1/get_missing_events/%s" % (room_1,),
             query_content,
         )
-        self.render(request)
         self.assertEquals(400, channel.code, channel.result)
         self.assertEqual(channel.json_body["errcode"], "M_NOT_JSON")
 
@@ -99,7 +98,6 @@ def test_without_event_id(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/v1/state/%s" % (room_1,)
         )
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
 
         self.assertEqual(
@@ -132,7 +130,6 @@ def test_needs_to_be_in_room(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/v1/state/%s" % (room_1,)
         )
-        self.render(request)
         self.assertEquals(403, channel.code, channel.result)
         self.assertEqual(channel.json_body["errcode"], "M_FORBIDDEN")
 

tests/federation/transport/test_server.py

@@ -40,13 +40,11 @@ def test_blocked_public_room_list_over_federation(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/v1/publicRooms"
         )
-        self.render(request)
         self.assertEquals(403, channel.code)
 
     @override_config({"allow_public_rooms_over_federation": True})
     def test_open_public_room_list_over_federation(self):
         request, channel = self.make_request(
             "GET", "/_matrix/federation/v1/publicRooms"
         )
-        self.render(request)
         self.assertEquals(200, channel.code)

tests/handlers/test_directory.py

@@ -412,7 +412,6 @@ def test_denied(self):
             b"directory/room/%23test%3Atest",
             ('{"room_id":"%s"}' % (room_id,)).encode("ascii"),
         )
-        self.render(request)
         self.assertEquals(403, channel.code, channel.result)
 
     def test_allowed(self):
@@ -423,7 +422,6 @@ def test_allowed(self):
             b"directory/room/%23unofficial_test%3Atest",
             ('{"room_id":"%s"}' % (room_id,)).encode("ascii"),
         )
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
 
 
@@ -438,7 +436,6 @@ def prepare(self, reactor, clock, hs):
         request, channel = self.make_request(
             "PUT", b"directory/list/room/%s" % (room_id.encode("ascii"),), b"{}"
         )
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
 
         self.room_list_handler = hs.get_room_list_handler()
@@ -452,7 +449,6 @@ def test_disabling_room_list(self):
 
         # Room list is enabled so we should get some results
         request, channel = self.make_request("GET", b"publicRooms")
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
         self.assertTrue(len(channel.json_body["chunk"]) > 0)
 
@@ -461,7 +457,6 @@ def test_disabling_room_list(self):
 
         # Room list disabled so we should get no results
         request, channel = self.make_request("GET", b"publicRooms")
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
         self.assertTrue(len(channel.json_body["chunk"]) == 0)
 
@@ -470,5 +465,4 @@ def test_disabling_room_list(self):
         request, channel = self.make_request(
             "PUT", b"directory/list/room/%s" % (room_id.encode("ascii"),), b"{}"
         )
-        self.render(request)
         self.assertEquals(403, channel.code, channel.result)

tests/handlers/test_message.py

@@ -209,5 +209,4 @@ def test_deny_redact_server_acl(self):
         request, channel = self.make_request(
             "POST", path, content={}, access_token=self.access_token
         )
-        self.render(request)
         self.assertEqual(int(channel.result["code"]), 403)

tests/handlers/test_typing.py

@@ -228,7 +228,6 @@ def test_started_typing_remote_recv(self):
             ),
             federation_auth_origin=b"farm",
         )
-        self.render(request)
         self.assertEqual(channel.code, 200)
 
         self.on_new_event.assert_has_calls([call("typing_key", 1, rooms=[ROOM_ID])])

tests/handlers/test_user_directory.py

@@ -537,7 +537,6 @@ def test_disabling_room_list(self):
         request, channel = self.make_request(
             "POST", b"user_directory/search", b'{"search_term":"user2"}'
         )
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
         self.assertTrue(len(channel.json_body["results"]) > 0)
 
@@ -546,6 +545,5 @@ def test_disabling_room_list(self):
         request, channel = self.make_request(
             "POST", b"user_directory/search", b'{"search_term":"user2"}'
         )
-        self.render(request)
         self.assertEquals(200, channel.code, channel.result)
         self.assertTrue(len(channel.json_body["results"]) == 0)

tests/http/test_additional_resource.py

@@ -17,7 +17,7 @@
 from synapse.http.additional_resource import AdditionalResource
 from synapse.http.server import respond_with_json
 
-from tests.server import FakeSite, make_request, render
+from tests.server import FakeSite, make_request
 from tests.unittest import HomeserverTestCase
 
 
@@ -47,7 +47,6 @@ def test_async(self):
         resource = AdditionalResource(self.hs, handler)
 
         request, channel = make_request(self.reactor, FakeSite(resource), "GET", "/")
-        render(request, resource, self.reactor)
 
         self.assertEqual(request.code, 200)
         self.assertEqual(channel.json_body, {"some_key": "some_value_async"})
@@ -57,7 +56,6 @@ def test_sync(self):
         resource = AdditionalResource(self.hs, handler)
 
         request, channel = make_request(self.reactor, FakeSite(resource), "GET", "/")
-        render(request, resource, self.reactor)
 
         self.assertEqual(request.code, 200)
         self.assertEqual(channel.json_body, {"some_key": "some_value_sync"})

tests/replication/_base.py

@@ -36,7 +36,7 @@
 from synapse.util import Clock
 
 from tests import unittest
-from tests.server import FakeTransport, render
+from tests.server import FakeTransport
 
 try:
     import hiredis
@@ -347,9 +347,6 @@ def _get_worker_hs_config(self) -> dict:
         config["worker_replication_http_port"] = "8765"
         return config
 
-    def render_on_worker(self, worker_hs: HomeServer, request: SynapseRequest):
-        render(request, self._hs_to_site[worker_hs].resource, self.reactor)
-
     def replicate(self):
         """Tell the master side of replication that something has happened, and then
         wait for the replication to occur.

tests/replication/test_client_reader_shard.py

@@ -55,7 +55,6 @@ def test_register_single_worker(self):
             "register",
             {"username": "user", "type": "m.login.password", "password": "bar"},
         )  # type: SynapseRequest, FakeChannel
-        self.render_on_worker(worker_hs, request_1)
         self.assertEqual(request_1.code, 401)
 
         # Grab the session
@@ -69,7 +68,6 @@ def test_register_single_worker(self):
             "register",
             {"auth": {"session": session, "type": "m.login.dummy"}},
         )  # type: SynapseRequest, FakeChannel
-        self.render_on_worker(worker_hs, request_2)
         self.assertEqual(request_2.code, 200)
 
         # We're given a registered user.
@@ -89,7 +87,6 @@ def test_register_multi_worker(self):
             "register",
             {"username": "user", "type": "m.login.password", "password": "bar"},
         )  # type: SynapseRequest, FakeChannel
-        self.render_on_worker(worker_hs_1, request_1)
         self.assertEqual(request_1.code, 401)
 
         # Grab the session
@@ -104,7 +101,6 @@ def test_register_multi_worker(self):
             "register",
             {"auth": {"session": session, "type": "m.login.dummy"}},
         )  # type: SynapseRequest, FakeChannel
-        self.render_on_worker(worker_hs_2, request_2)
         self.assertEqual(request_2.code, 200)
 
         # We're given a registered user.

tests/replication/test_multi_media_repo.py

@@ -68,15 +68,15 @@ def _get_media_req(
             the media which the caller should respond to.
         """
         resource = hs.get_media_repository_resource().children[b"download"]
-        request, channel = make_request(
+        _, channel = make_request(
             self.reactor,
             FakeSite(resource),
             "GET",
             "/{}/{}".format(target, media_id),
             shorthand=False,
             access_token=self.access_token,
+            await_result=False,
         )
-        request.render(resource)
         self.pump()
 
         clients = self.reactor.tcpClients

tests/replication/test_sharded_event_persister.py

@@ -183,7 +183,6 @@ def test_vector_clock_token(self):
         request, channel = make_request(
             self.reactor, sync_hs_site, "GET", "/sync", access_token=access_token
         )
-        self.render_on_worker(sync_hs, request)
         next_batch = channel.json_body["next_batch"]
 
         # We now gut wrench into the events stream MultiWriterIdGenerator on
@@ -214,7 +213,6 @@ def test_vector_clock_token(self):
             "/sync?since={}".format(next_batch),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
 
         # We should only see the new event and nothing else
         self.assertIn(room_id1, channel.json_body["rooms"]["join"])
@@ -245,7 +243,6 @@ def test_vector_clock_token(self):
             "/sync?since={}".format(vector_clock_token),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
 
         self.assertNotIn(room_id1, channel.json_body["rooms"]["join"])
         self.assertIn(room_id2, channel.json_body["rooms"]["join"])
@@ -271,7 +268,6 @@ def test_vector_clock_token(self):
             "/sync?since={}".format(next_batch),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
 
         prev_batch1 = channel.json_body["rooms"]["join"][room_id1]["timeline"][
             "prev_batch"
@@ -292,7 +288,6 @@ def test_vector_clock_token(self):
             ),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
         self.assertListEqual([], channel.json_body["chunk"])
 
         # Paginating back on the second room should produce the first event
@@ -306,7 +301,6 @@ def test_vector_clock_token(self):
             ),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
         self.assertEqual(len(channel.json_body["chunk"]), 1)
         self.assertEqual(
             channel.json_body["chunk"][0]["event_id"], first_event_in_room2
@@ -322,7 +316,6 @@ def test_vector_clock_token(self):
             ),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
         self.assertListEqual([], channel.json_body["chunk"])
 
         request, channel = make_request(
@@ -334,7 +327,6 @@ def test_vector_clock_token(self):
             ),
             access_token=access_token,
         )
-        self.render_on_worker(sync_hs, request)
         self.assertEqual(len(channel.json_body["chunk"]), 1)
         self.assertEqual(
             channel.json_body["chunk"][0]["event_id"], first_event_in_room2