Switch to using persistent connections in tests too.

src/allmydata/storage/http_client.py

@@ -282,19 +282,20 @@ class StorageClient(object):
     Low-level HTTP client that talks to the HTTP storage server.
     """
 
-    # If True, we're doing unit testing.
-    TEST_MODE = False
+    # If set, we're doing unit testing and we should call this with
+    # HTTPConnectionPool we create.
+    TEST_MODE_REGISTER_HTTP_POOL = None
 
     @classmethod
-    def start_test_mode(cls):
+    def start_test_mode(cls, callback):
         """Switch to testing mode.
 
-        In testing mode we disable persistent HTTP queries and have shorter
-        timeouts, to make certain tests work, but don't change the actual
-        semantic work being done—given a fast server, everything would work the
-        same.
+        In testing mode we register the pool with test system using the given
+        callback so it can Do Things, most notably killing off idle HTTP
+        connections at test shutdown and, in some tests, in the midddle of the
+        test.
         """
-        cls.TEST_MODE = True
+        cls.TEST_MODE_REGISTER_HTTP_POOL = callback
 
     # The URL is a HTTPS URL ("https://...").  To construct from a NURL, use
     # ``StorageClient.from_nurl()``.
@@ -318,13 +319,10 @@ class StorageClient(object):
         assert nurl.scheme == "pb"
         swissnum = nurl.path[0].encode("ascii")
         certificate_hash = nurl.user.encode("ascii")
+        pool = HTTPConnectionPool(reactor)
 
-        if cls.TEST_MODE:
-            pool = HTTPConnectionPool(reactor, persistent=False)
-            pool.retryAutomatically = False
-            pool.maxPersistentPerHost = 0
-        else:
-            pool = HTTPConnectionPool(reactor)
+        if cls.TEST_MODE_REGISTER_HTTP_POOL is not None:
+            cls.TEST_MODE_REGISTER_HTTP_POOL(pool)
 
         treq_client = HTTPClient(
             Agent(
@@ -403,8 +401,12 @@ class StorageClient(object):
         result = self._treq.request(method, url, headers=headers, **kwargs)
 
         # If we're in test mode, we want an aggressive timeout, e.g. for
-        # test_rref in test_system.py.
-        if self.TEST_MODE:
+        # test_rref in test_system.py. Unfortunately, test_rref results in the
+        # socket still listening(!), only without an HTTP server, due to limits
+        # in the relevant socket-binding test setup code. As a result, we don't
+        # get connection refused, the client will successfully connect. So we
+        # want a timeout so we notice that.
+        if self.TEST_MODE_REGISTER_HTTP_POOL is not None:
             result.addTimeout(5, self._clock)
 
         return result

src/allmydata/test/common_system.py

@@ -647,7 +647,8 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
     FORCE_FOOLSCAP_FOR_STORAGE : Optional[bool] = None
 
     def setUp(self):
-        http_client.StorageClient.start_test_mode()
+        self._http_client_pools = []
+        http_client.StorageClient.start_test_mode(self._http_client_pools.append)
         self.port_assigner = SameProcessStreamEndpointAssigner()
         self.port_assigner.setUp()
         self.addCleanup(self.port_assigner.tearDown)
@@ -655,10 +656,17 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         self.sparent = service.MultiService()
         self.sparent.startService()
 
+    def close_idle_http_connections(self):
+        """Close all HTTP client connections that are just hanging around."""
+        return defer.gatherResults(
+            [pool.closeCachedConnections() for pool in self._http_client_pools]
+        )
+
     def tearDown(self):
         log.msg("shutting down SystemTest services")
         d = self.sparent.stopService()
         d.addBoth(flush_but_dont_ignore)
+        d.addBoth(lambda x: self.close_idle_http_connections().addCallback(lambda _: x))
         d.addBoth(lambda x: deferLater(reactor, 0.01, lambda: x))
         return d
 

src/allmydata/test/test_system.py

@@ -1826,6 +1826,9 @@ class Connections(SystemTestMixin, unittest.TestCase):
         # now shut down the server
         d.addCallback(lambda ign: self.clients[1].disownServiceParent())
 
+        # kill any persistent http connections that might continue to work
+        d.addCallback(lambda ign: self.close_idle_http_connections())
+
         # and wait for the client to notice
         def _poll():
             return len(self.c0.storage_broker.get_connected_servers()) == 1