Skip to content

Commit

Permalink
Unify timeout values
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pquentin committed Oct 29, 2019
1 parent d0521c3 commit 74b9c62
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 78 deletions.
10 changes: 10 additions & 0 deletions test/__init__.py
Expand Up @@ -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.1 if os.environ.get("CI") else 0.01


def clear_warnings(cls=HTTPWarning):
new_filters = []
Expand Down
3 changes: 2 additions & 1 deletion test/appengine/test_gae_manager.py
Expand Up @@ -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.
Expand Down Expand Up @@ -46,7 +47,7 @@ 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=0.005", timeout=SHORT_TIMEOUT)

# InvalidURLError -> ProtocolError
with pytest.raises(urllib3.exceptions.ProtocolError):
Expand Down
5 changes: 4 additions & 1 deletion test/contrib/test_socks.py
Expand Up @@ -8,6 +8,7 @@
from dummyserver.testcase import IPV4SocketDummyServerTestCase

import pytest
from test import SHORT_TIMEOUT

try:
import ssl
Expand Down Expand Up @@ -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):
Expand Down
15 changes: 8 additions & 7 deletions test/test_connectionpool.py
Expand Up @@ -31,6 +31,7 @@
from ssl import SSLError as BaseSSLError

from dummyserver.server import DEFAULT_CA
from test import SHORT_TIMEOUT


class HTTPUnixConnection(HTTPConnection):
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -288,7 +289,7 @@ def _test(exception, expect, reason=None):
# See: https://github.com/shazow/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):
Expand Down Expand Up @@ -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):
Expand Down
14 changes: 4 additions & 10 deletions test/with_dummyserver/test_connectionpool.py
Expand Up @@ -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

Expand All @@ -35,10 +36,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()
Expand All @@ -49,19 +46,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()
Expand Down
12 changes: 7 additions & 5 deletions test/with_dummyserver/test_https.py
Expand Up @@ -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
Expand Down Expand Up @@ -501,7 +503,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,
Expand Down Expand Up @@ -551,7 +553,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:
Expand All @@ -567,12 +569,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,
Expand All @@ -585,7 +587,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()
Expand Down
6 changes: 4 additions & 2 deletions test/with_dummyserver/test_poolmanager.py
Expand Up @@ -9,6 +9,8 @@
from urllib3.exceptions import MaxRetryError
from urllib3.util.retry import Retry

from test import LONG_TIMEOUT


class TestPoolManager(HTTPDummyServerTestCase):
@classmethod
Expand Down Expand Up @@ -85,15 +87,15 @@ def test_cross_host_redirect(self):
"GET",
"%s/redirect" % self.base_url,
fields={"target": cross_host_location},
timeout=1,
timeout=LONG_TIMEOUT,
retries=0,
)

r = http.request(
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/echo?a=b" % self.base_url_alt},
timeout=1,
timeout=LONG_TIMEOUT,
retries=1,
)

Expand Down
16 changes: 9 additions & 7 deletions test/with_dummyserver/test_proxy_poolmanager.py
Expand Up @@ -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


class TestHTTPProxyManager(HTTPDummyProxyTestCase):
@classmethod
Expand Down Expand Up @@ -51,7 +53,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)
Expand Down Expand Up @@ -131,15 +133,15 @@ def test_cross_host_redirect(self):
"GET",
"%s/redirect" % self.http_url,
fields={"target": cross_host_location},
timeout=1,
timeout=LONG_TIMEOUT,
retries=0,
)

r = http.request(
"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
Expand All @@ -152,15 +154,15 @@ def test_cross_protocol_redirect(self):
"GET",
"%s/redirect" % self.http_url,
fields={"target": cross_protocol_location},
timeout=1,
timeout=LONG_TIMEOUT,
retries=0,
)

r = http.request(
"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
Expand Down Expand Up @@ -318,14 +320,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)
Expand Down

0 comments on commit 74b9c62

Please sign in to comment.