From b2fe7be1d035bcf07c024856869960c0510ee6fe Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Thu, 31 Oct 2019 18:20:13 +0400 Subject: [PATCH] Unify timeout values in test suite (#1717) The goal is to be able to to tweak the timeout values globally, and to use larger timeouts on CI where running time is less predictable. --- test/__init__.py | 10 +++ test/appengine/test_gae_manager.py | 7 +- test/contrib/test_socks.py | 5 +- test/test_connectionpool.py | 15 ++-- test/with_dummyserver/test_connectionpool.py | 14 +--- test/with_dummyserver/test_https.py | 12 +-- test/with_dummyserver/test_poolmanager.py | 6 +- .../test_proxy_poolmanager.py | 16 ++-- test/with_dummyserver/test_socketlevel.py | 81 +++++++++---------- 9 files changed, 88 insertions(+), 78 deletions(-) diff --git a/test/__init__.py b/test/__init__.py index c9a9806a91..1943ca96bb 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -28,6 +28,16 @@ # RFC 3849: 2001:db8::/32 is for documentation only. INVALID_SOURCE_ADDRESSES = [("192.0.2.255", 0), ("2001:db8::1", 0)] +# We use timeouts in three different ways in our tests +# +# 1. To make sure that the operation timeouts, we can use a short timeout. +# 2. To make sure that the test does not hang even if the operation should succeed, we +# want to use a long timeout, even more so on CI where tests can be really slow +# 3. To test our timeout logic by using two different values, eg. by using different +# values at the pool level and at the request level. +SHORT_TIMEOUT = 0.001 +LONG_TIMEOUT = 0.5 if os.environ.get("CI") else 0.01 + def clear_warnings(cls=HTTPWarning): new_filters = [] diff --git a/test/appengine/test_gae_manager.py b/test/appengine/test_gae_manager.py index c41ec141ff..22f564da29 100644 --- a/test/appengine/test_gae_manager.py +++ b/test/appengine/test_gae_manager.py @@ -7,6 +7,7 @@ import urllib3.util.retry from test.with_dummyserver import test_connectionpool +from test import SHORT_TIMEOUT # This class is used so we can re-use the tests from the connection pool. @@ -46,7 +47,11 @@ def setup_method(self, method): def test_exceptions(self): # DeadlineExceededError -> TimeoutError with pytest.raises(urllib3.exceptions.TimeoutError): - self.pool.request("GET", "/sleep?seconds=0.005", timeout=0.001) + self.pool.request( + "GET", + "/sleep?seconds={}".format(5 * SHORT_TIMEOUT), + timeout=SHORT_TIMEOUT, + ) # InvalidURLError -> ProtocolError with pytest.raises(urllib3.exceptions.ProtocolError): diff --git a/test/contrib/test_socks.py b/test/contrib/test_socks.py index bf1bd090e3..d70eb7cff6 100644 --- a/test/contrib/test_socks.py +++ b/test/contrib/test_socks.py @@ -8,6 +8,7 @@ from dummyserver.testcase import IPV4SocketDummyServerTestCase import pytest +from test import SHORT_TIMEOUT try: import ssl @@ -314,7 +315,9 @@ def request_handler(listener): proxy_url = "socks5h://%s:%s" % (self.host, self.port) with socks.SOCKSProxyManager(proxy_url) as pm: with pytest.raises(ConnectTimeoutError): - pm.request("GET", "http://example.com", timeout=0.001, retries=False) + pm.request( + "GET", "http://example.com", timeout=SHORT_TIMEOUT, retries=False + ) event.set() def test_connection_failure(self): diff --git a/test/test_connectionpool.py b/test/test_connectionpool.py index 532d244526..3cd215304f 100644 --- a/test/test_connectionpool.py +++ b/test/test_connectionpool.py @@ -31,6 +31,7 @@ from ssl import SSLError as BaseSSLError from dummyserver.server import DEFAULT_CA +from test import SHORT_TIMEOUT class HTTPUnixConnection(HTTPConnection): @@ -212,13 +213,13 @@ def test_not_same_host_custom_protocol(self, a, b): def test_max_connections(self): with HTTPConnectionPool(host="localhost", maxsize=1, block=True) as pool: - pool._get_conn(timeout=0.01) + pool._get_conn(timeout=SHORT_TIMEOUT) with pytest.raises(EmptyPoolError): - pool._get_conn(timeout=0.01) + pool._get_conn(timeout=SHORT_TIMEOUT) with pytest.raises(EmptyPoolError): - pool.request("GET", "/", pool_timeout=0.01) + pool.request("GET", "/", pool_timeout=SHORT_TIMEOUT) assert pool.num_connections == 1 @@ -288,7 +289,7 @@ def _test(exception, expect, reason=None): # See: https://github.com/urllib3/urllib3/issues/76 pool._make_request = lambda *args, **kwargs: _raise(HTTPException) with pytest.raises(MaxRetryError): - pool.request("GET", "/", retries=1, pool_timeout=0.01) + pool.request("GET", "/", retries=1, pool_timeout=SHORT_TIMEOUT) assert pool.pool.qsize() == POOL_SIZE def test_assert_same_host(self): @@ -348,9 +349,9 @@ def test_pool_timeouts(self): assert pool.timeout._connect == Timeout.DEFAULT_TIMEOUT assert pool.timeout.total is None - pool = HTTPConnectionPool(host="localhost", timeout=3) - assert pool.timeout._read == 3 - assert pool.timeout._connect == 3 + pool = HTTPConnectionPool(host="localhost", timeout=SHORT_TIMEOUT) + assert pool.timeout._read == SHORT_TIMEOUT + assert pool.timeout._connect == SHORT_TIMEOUT assert pool.timeout.total is None def test_no_host(self): diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py index bd7fec1afe..b0c63e1cbe 100644 --- a/test/with_dummyserver/test_connectionpool.py +++ b/test/with_dummyserver/test_connectionpool.py @@ -25,6 +25,7 @@ from urllib3.util.retry import Retry, RequestHistory from urllib3.util.timeout import Timeout +from test import SHORT_TIMEOUT, LONG_TIMEOUT from dummyserver.testcase import HTTPDummyServerTestCase, SocketDummyServerTestCase from dummyserver.server import NoIPv6Warning, HAS_IPV6_AND_DNS @@ -37,10 +38,6 @@ log.addHandler(logging.StreamHandler(sys.stdout)) -SHORT_TIMEOUT = 0.001 -LONG_TIMEOUT = 0.03 - - def wait_for_socket(ready_event): ready_event.wait() ready_event.clear() @@ -51,19 +48,16 @@ def test_timeout_float(self): block_event = Event() ready_event = self.start_basic_handler(block_send=block_event, num=2) - # Pool-global timeout - with HTTPConnectionPool( - self.host, self.port, timeout=SHORT_TIMEOUT, retries=False - ) as pool: + with HTTPConnectionPool(self.host, self.port, retries=False) as pool: wait_for_socket(ready_event) with pytest.raises(ReadTimeoutError): - pool.request("GET", "/") + pool.request("GET", "/", timeout=SHORT_TIMEOUT) block_event.set() # Release block # Shouldn't raise this time wait_for_socket(ready_event) block_event.set() # Pre-release block - pool.request("GET", "/") + pool.request("GET", "/", timeout=LONG_TIMEOUT) def test_conn_closed(self): block_event = Event() diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 9c3077f8b4..9c4a502ca8 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -39,6 +39,8 @@ requiresTLSv1_2, requiresTLSv1_3, TARPIT_HOST, + SHORT_TIMEOUT, + LONG_TIMEOUT, ) from urllib3 import HTTPSConnectionPool from urllib3.connection import VerifiedHTTPSConnection, RECENT_DATE @@ -503,7 +505,7 @@ def test_good_fingerprint_and_hostname_mismatch(self): @requires_network def test_https_timeout(self): - timeout = Timeout(total=None, connect=0.001) + timeout = Timeout(total=None, connect=SHORT_TIMEOUT) with HTTPSConnectionPool( TARPIT_HOST, self.port, @@ -553,7 +555,7 @@ def test_enhanced_timeout(self): with HTTPSConnectionPool( TARPIT_HOST, self.port, - timeout=Timeout(connect=0.001), + timeout=Timeout(connect=SHORT_TIMEOUT), retries=False, cert_reqs="CERT_REQUIRED", ) as https_pool: @@ -569,12 +571,12 @@ def test_enhanced_timeout(self): with HTTPSConnectionPool( TARPIT_HOST, self.port, - timeout=Timeout(connect=5), + timeout=Timeout(connect=LONG_TIMEOUT), retries=False, cert_reqs="CERT_REQUIRED", ) as https_pool: with pytest.raises(ConnectTimeoutError): - https_pool.request("GET", "/", timeout=Timeout(connect=0.001)) + https_pool.request("GET", "/", timeout=Timeout(connect=SHORT_TIMEOUT)) with HTTPSConnectionPool( TARPIT_HOST, @@ -587,7 +589,7 @@ def test_enhanced_timeout(self): try: with pytest.raises(ConnectTimeoutError): https_pool.request( - "GET", "/", timeout=Timeout(total=None, connect=0.001) + "GET", "/", timeout=Timeout(total=None, connect=SHORT_TIMEOUT) ) finally: conn.close() diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py index c7af72badb..cbd6dd7dc0 100644 --- a/test/with_dummyserver/test_poolmanager.py +++ b/test/with_dummyserver/test_poolmanager.py @@ -9,6 +9,8 @@ from urllib3.exceptions import MaxRetryError from urllib3.util.retry import Retry +from test import LONG_TIMEOUT + # Retry failed tests pytestmark = pytest.mark.flaky @@ -88,7 +90,7 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.base_url, fields={"target": cross_host_location}, - timeout=1, + timeout=LONG_TIMEOUT, retries=0, ) @@ -96,7 +98,7 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.base_url, fields={"target": "%s/echo?a=b" % self.base_url_alt}, - timeout=1, + timeout=LONG_TIMEOUT, retries=1, ) diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py index dde862ed23..014df62af7 100644 --- a/test/with_dummyserver/test_proxy_poolmanager.py +++ b/test/with_dummyserver/test_proxy_poolmanager.py @@ -12,6 +12,8 @@ from urllib3.exceptions import MaxRetryError, SSLError, ProxyError, ConnectTimeoutError from urllib3.connectionpool import connection_from_url, VerifiedHTTPSConnection +from test import SHORT_TIMEOUT, LONG_TIMEOUT + # Retry failed tests pytestmark = pytest.mark.flaky @@ -54,7 +56,7 @@ def test_nagle_proxy(self): def test_proxy_conn_fail(self): host, port = get_unreachable_address() with proxy_from_url( - "http://%s:%s/" % (host, port), retries=1, timeout=0.05 + "http://%s:%s/" % (host, port), retries=1, timeout=LONG_TIMEOUT ) as http: with pytest.raises(MaxRetryError): http.request("GET", "%s/" % self.https_url) @@ -134,7 +136,7 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": cross_host_location}, - timeout=1, + timeout=LONG_TIMEOUT, retries=0, ) @@ -142,7 +144,7 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": "%s/echo?a=b" % self.http_url_alt}, - timeout=1, + timeout=LONG_TIMEOUT, retries=1, ) assert r._pool.host != self.http_host_alt @@ -155,7 +157,7 @@ def test_cross_protocol_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": cross_protocol_location}, - timeout=1, + timeout=LONG_TIMEOUT, retries=0, ) @@ -163,7 +165,7 @@ def test_cross_protocol_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": "%s/echo?a=b" % self.https_url}, - timeout=1, + timeout=LONG_TIMEOUT, retries=1, ) assert r._pool.host == self.https_host @@ -321,14 +323,14 @@ def test_proxy_pooling_ext(self): def test_https_proxy_timeout(self): with proxy_from_url("https://{host}".format(host=TARPIT_HOST)) as https: with pytest.raises(MaxRetryError) as e: - https.request("GET", self.http_url, timeout=0.001) + https.request("GET", self.http_url, timeout=SHORT_TIMEOUT) assert type(e.value.reason) == ConnectTimeoutError @pytest.mark.timeout(0.5) @requires_network def test_https_proxy_pool_timeout(self): with proxy_from_url( - "https://{host}".format(host=TARPIT_HOST), timeout=0.001 + "https://{host}".format(host=TARPIT_HOST), timeout=SHORT_TIMEOUT ) as https: with pytest.raises(MaxRetryError) as e: https.request("GET", self.http_url) diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py index b9c9ae5c6a..e2d6bf919f 100644 --- a/test/with_dummyserver/test_socketlevel.py +++ b/test/with_dummyserver/test_socketlevel.py @@ -47,7 +47,12 @@ class MimeToolMessage(object): import pytest -from test import fails_on_travis_gce, requires_ssl_context_keyfile_password +from test import ( + fails_on_travis_gce, + requires_ssl_context_keyfile_password, + SHORT_TIMEOUT, + LONG_TIMEOUT, +) # Retry failed tests pytestmark = pytest.mark.flaky @@ -374,7 +379,12 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPConnectionPool( - self.host, self.port, timeout=0.01, retries=False, maxsize=3, block=True + self.host, + self.port, + timeout=SHORT_TIMEOUT, + retries=False, + maxsize=3, + block=True, ) as http: try: with pytest.raises(ReadTimeoutError): @@ -395,7 +405,7 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPConnectionPool( - self.host, self.port, timeout=0.01, retries=True + self.host, self.port, timeout=SHORT_TIMEOUT, retries=True ) as pool: try: with pytest.raises(ReadTimeoutError): @@ -417,7 +427,7 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPSConnectionPool( - self.host, self.port, timeout=0.01, retries=False + self.host, self.port, timeout=SHORT_TIMEOUT, retries=False ) as pool: try: with pytest.raises(ReadTimeoutError): @@ -461,7 +471,7 @@ def socket_handler(listener): try: self._start_server(socket_handler) - t = Timeout(connect=0.001, read=0.01) + t = Timeout(connect=SHORT_TIMEOUT, read=LONG_TIMEOUT) with HTTPConnectionPool(self.host, self.port, timeout=t) as pool: response = pool.request("GET", "/", retries=1) assert response.status == 200 @@ -498,7 +508,7 @@ def socket_handler(listener): "/", retries=0, preload_content=False, - timeout=Timeout(connect=1, read=0.01), + timeout=Timeout(connect=1, read=SHORT_TIMEOUT), ) try: with pytest.raises(ReadTimeoutError): @@ -531,9 +541,8 @@ def socket_handler(listener): with HTTPConnectionPool(self.host, self.port) as pool: try: with pytest.raises(ReadTimeoutError): - pool.urlopen( - "GET", "/", retries=False, timeout=Timeout(connect=1, read=0.01) - ) + timeout = Timeout(connect=LONG_TIMEOUT, read=SHORT_TIMEOUT) + pool.urlopen("GET", "/", retries=False, timeout=timeout) finally: timed_out.set() @@ -641,12 +650,9 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPConnectionPool(self.host, self.port) as pool: poolsize = pool.pool.qsize() + timeout = Timeout(connect=LONG_TIMEOUT, read=SHORT_TIMEOUT) response = pool.urlopen( - "GET", - "/", - retries=0, - preload_content=False, - timeout=Timeout(connect=1, read=0.01), + "GET", "/", retries=0, preload_content=False, timeout=timeout ) try: with pytest.raises(ReadTimeoutError): @@ -745,11 +751,7 @@ def socket_handler(listener): with HTTPConnectionPool(self.host, self.port) as pool: # First request should fail. response = pool.urlopen( - "GET", - "/", - retries=0, - preload_content=False, - timeout=Timeout(connect=1, read=0.1), + "GET", "/", retries=0, preload_content=False, timeout=LONG_TIMEOUT ) try: with pytest.raises(ReadTimeoutError): @@ -759,11 +761,7 @@ def socket_handler(listener): # Second should succeed. response = pool.urlopen( - "GET", - "/", - retries=0, - preload_content=False, - timeout=Timeout(connect=1, read=1), + "GET", "/", retries=0, preload_content=False, timeout=LONG_TIMEOUT ) assert len(response.read()) == 8 @@ -788,11 +786,11 @@ def socket_handler(listener): ) # Wait for the socket to close. - done_closing.wait(timeout=1) + done_closing.wait(timeout=LONG_TIMEOUT) # Look for the empty string to show that the connection got closed. # Don't get stuck in a timeout. - sock.settimeout(1) + sock.settimeout(LONG_TIMEOUT) new_data = sock.recv(65536) assert not new_data sock.close() @@ -805,7 +803,7 @@ def socket_handler(listener): response.close() done_closing.set() # wait until the socket in our pool gets closed - successful = complete.wait(timeout=1) + successful = complete.wait(timeout=LONG_TIMEOUT) assert successful, "Timed out waiting for connection close" def test_release_conn_param_is_respected_after_timeout_retry(self): @@ -863,7 +861,7 @@ def socket_handler(listener): retries=1, release_conn=False, preload_content=False, - timeout=Timeout(connect=1, read=0.01), + timeout=Timeout(connect=LONG_TIMEOUT, read=SHORT_TIMEOUT), ) # The connection should still be on the response object, and none @@ -991,7 +989,7 @@ def echo_socket_handler(listener): ) assert r.status == 200 - close_event.wait(timeout=1) + close_event.wait(timeout=LONG_TIMEOUT) with pytest.raises(ProxyError): conn.urlopen( "GET", @@ -1180,11 +1178,7 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPSConnectionPool(self.host, self.port, ca_certs=DEFAULT_CA) as pool: response = pool.urlopen( - "GET", - "/", - retries=0, - preload_content=False, - timeout=Timeout(connect=1, read=0.01), + "GET", "/", retries=0, preload_content=False, timeout=LONG_TIMEOUT ) try: with pytest.raises(ReadTimeoutError): @@ -1223,12 +1217,9 @@ def request(): self.host, self.port, assert_fingerprint=fingerprint ) try: + timeout = Timeout(connect=LONG_TIMEOUT, read=SHORT_TIMEOUT) response = pool.urlopen( - "GET", - "/", - preload_content=False, - timeout=Timeout(connect=1, read=0.01), - retries=0, + "GET", "/", preload_content=False, retries=0, timeout=timeout ) response.read() finally: @@ -1327,7 +1318,7 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPSConnectionPool(self.host, self.port) as pool: with pytest.raises(MaxRetryError): - pool.request("GET", "/", timeout=0.01) + pool.request("GET", "/", timeout=SHORT_TIMEOUT) context.load_default_certs.assert_called_with() def test_ssl_dont_load_default_certs_when_given(self): @@ -1375,7 +1366,7 @@ def socket_handler(listener): with HTTPSConnectionPool(self.host, self.port, **kwargs) as pool: with pytest.raises(MaxRetryError): - pool.request("GET", "/", timeout=0.01) + pool.request("GET", "/", timeout=SHORT_TIMEOUT) context.load_default_certs.assert_not_called() @@ -1631,7 +1622,7 @@ def test_chunked_head_response_does_not_hang(self): b"\r\n" ) with HTTPConnectionPool(self.host, self.port, retries=False) as pool: - r = pool.request("HEAD", "/", timeout=1, preload_content=False) + r = pool.request("HEAD", "/", timeout=LONG_TIMEOUT, preload_content=False) # stream will use the read_chunked method here. assert [] == list(r.stream()) @@ -1644,7 +1635,7 @@ def test_empty_head_response_does_not_hang(self): b"\r\n" ) with HTTPConnectionPool(self.host, self.port, retries=False) as pool: - r = pool.request("HEAD", "/", timeout=1, preload_content=False) + r = pool.request("HEAD", "/", timeout=LONG_TIMEOUT, preload_content=False) # stream will use the read method here. assert [] == list(r.stream()) @@ -1673,7 +1664,7 @@ def socket_handler(listener): self._start_server(socket_handler) with HTTPConnectionPool(self.host, self.port, retries=False) as pool: - r = pool.request("GET", "/", timeout=1, preload_content=False) + r = pool.request("GET", "/", timeout=LONG_TIMEOUT, preload_content=False) # Stream should read to the end. assert [b"hello, world"] == list(r.stream(None)) @@ -1699,7 +1690,7 @@ def socket_handler(listener): b"\r\n" b"hello, world" ) - done_event.wait(1) + done_event.wait(LONG_TIMEOUT) sock.close() self._start_server(socket_handler)