From 347eed7a3ee1da57850f5fc601819564af0de50d Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Thu, 31 Oct 2019 22:30:56 +0400 Subject: [PATCH 01/10] test_timeout: stop testing _make_request too This test is about timeouts, we don't need to test _make_request specifically here. --- test/with_dummyserver/test_connectionpool.py | 31 ++------------------ 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py index 0101d75a..fb870ce4 100644 --- a/test/with_dummyserver/test_connectionpool.py +++ b/test/with_dummyserver/test_connectionpool.py @@ -78,20 +78,13 @@ def test_conn_closed(self): def test_timeout(self): # Requests should time out when expected block_event = Event() - ready_event = self.start_basic_handler(block_send=block_event, num=6) + ready_event = self.start_basic_handler(block_send=block_event, num=3) # Pool-global timeout timeout = Timeout(read=SHORT_TIMEOUT) with HTTPConnectionPool( self.host, self.port, timeout=timeout, retries=False ) as pool: - wait_for_socket(ready_event) - conn = pool._get_conn() - with pytest.raises(ReadTimeoutError): - pool._make_request(conn, "GET", "/") - pool._put_conn(conn) - block_event.set() # Release request - wait_for_socket(ready_event) block_event.clear() with pytest.raises(ReadTimeoutError): @@ -102,18 +95,6 @@ def test_timeout(self): with HTTPConnectionPool( self.host, self.port, timeout=LONG_TIMEOUT, retries=False ) as pool: - conn = pool._get_conn() - wait_for_socket(ready_event) - now = time.time() - with pytest.raises(ReadTimeoutError): - pool._make_request(conn, "GET", "/", timeout=timeout) - delta = time.time() - now - block_event.set() # Release request - - message = "timeout was pool-level LONG_TIMEOUT rather than request-level SHORT_TIMEOUT" - assert delta < LONG_TIMEOUT, message - pool._put_conn(conn) - wait_for_socket(ready_event) now = time.time() with pytest.raises(ReadTimeoutError): @@ -124,20 +105,12 @@ def test_timeout(self): assert delta < LONG_TIMEOUT, message block_event.set() # Release request - # Timeout int/float passed directly to request and _make_request should - # raise a request timeout + # Timeout passed directly to request should raise a request timeout wait_for_socket(ready_event) with pytest.raises(ReadTimeoutError): pool.request("GET", "/", timeout=SHORT_TIMEOUT) block_event.set() # Release request - wait_for_socket(ready_event) - conn = pool._new_conn() - # FIXME: This assert flakes sometimes. Not sure why. - with pytest.raises(ReadTimeoutError): - pool._make_request(conn, "GET", "/", timeout=SHORT_TIMEOUT) - block_event.set() # Release request - def test_connect_timeout(self): url = "/" host, port = TARPIT_HOST, 80 From 32b88b7f457b7b9a4670e2177c33ef5355df752c Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Thu, 31 Oct 2019 22:52:50 +0400 Subject: [PATCH 02/10] test_timeout: rename timeout to short_timeout It's used throughout the test so it's better to explain that it's a short timeout. --- test/with_dummyserver/test_connectionpool.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py index fb870ce4..b37904c4 100644 --- a/test/with_dummyserver/test_connectionpool.py +++ b/test/with_dummyserver/test_connectionpool.py @@ -81,9 +81,9 @@ def test_timeout(self): ready_event = self.start_basic_handler(block_send=block_event, num=3) # Pool-global timeout - timeout = Timeout(read=SHORT_TIMEOUT) + short_timeout = Timeout(read=SHORT_TIMEOUT) with HTTPConnectionPool( - self.host, self.port, timeout=timeout, retries=False + self.host, self.port, timeout=short_timeout, retries=False ) as pool: wait_for_socket(ready_event) block_event.clear() @@ -98,7 +98,7 @@ def test_timeout(self): wait_for_socket(ready_event) now = time.time() with pytest.raises(ReadTimeoutError): - pool.request("GET", "/", timeout=timeout) + pool.request("GET", "/", timeout=short_timeout) delta = time.time() - now message = "timeout was pool-level LONG_TIMEOUT rather than request-level SHORT_TIMEOUT" From 2d60b07fac8f43b289640c587f7ff53ad278b60f Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Thu, 31 Oct 2019 23:06:18 +0400 Subject: [PATCH 03/10] Fix flaky test_timeout We had an interesting test failure in CI where this test failed twice in a row: we were expecting a timeout in 0.001 seconds, but both times it took more than one second! How can this happen? Well, when we ask a request to timeout after N seconds, here's what we know: * the request will time out, eventually * it won't timeout before those N seconds * it could timeout a long time after those N seconds, especially on a busy CI service Let's take advantage of those facts by specifying a request-specific timeout that's longer than the pool-level timeout and making sure that the timeout was long indeed. We still test that the request-specific timeout overrides the pool-level timeout, but will stop failing on busy CI services. --- test/with_dummyserver/test_connectionpool.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py index b37904c4..6b9cdecd 100644 --- a/test/with_dummyserver/test_connectionpool.py +++ b/test/with_dummyserver/test_connectionpool.py @@ -93,16 +93,16 @@ def test_timeout(self): # Request-specific timeouts should raise errors with HTTPConnectionPool( - self.host, self.port, timeout=LONG_TIMEOUT, retries=False + self.host, self.port, timeout=short_timeout, retries=False ) as pool: wait_for_socket(ready_event) now = time.time() with pytest.raises(ReadTimeoutError): - pool.request("GET", "/", timeout=short_timeout) + pool.request("GET", "/", timeout=LONG_TIMEOUT) delta = time.time() - now - message = "timeout was pool-level LONG_TIMEOUT rather than request-level SHORT_TIMEOUT" - assert delta < LONG_TIMEOUT, message + message = "timeout was pool-level SHORT_TIMEOUT rather than request-level LONG_TIMEOUT" + assert delta >= LONG_TIMEOUT, message block_event.set() # Release request # Timeout passed directly to request should raise a request timeout From 7e91933915c29e022150132908a8807618684ddb Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 05:16:40 +0000 Subject: [PATCH 04/10] Remove timeout in flaky proxy tests --- test/with_dummyserver/test_proxy_poolmanager.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py index a31a8baf..4ebe8a0b 100644 --- a/test/with_dummyserver/test_proxy_poolmanager.py +++ b/test/with_dummyserver/test_proxy_poolmanager.py @@ -131,7 +131,6 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": cross_host_location}, - timeout=LONG_TIMEOUT, retries=0, ) @@ -139,7 +138,6 @@ def test_cross_host_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": "%s/echo?a=b" % self.http_url_alt}, - timeout=LONG_TIMEOUT, retries=1, ) assert r._pool.host != self.http_host_alt @@ -152,7 +150,6 @@ def test_cross_protocol_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": cross_protocol_location}, - timeout=LONG_TIMEOUT, retries=0, ) @@ -160,7 +157,6 @@ def test_cross_protocol_redirect(self): "GET", "%s/redirect" % self.http_url, fields={"target": "%s/echo?a=b" % self.https_url}, - timeout=LONG_TIMEOUT, retries=1, ) assert r._pool.host == self.https_host From 011ca495a48be7b91bd5f9864a56b95accdf4161 Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 04:55:48 +0000 Subject: [PATCH 05/10] Bump cryptography to get Python 3.8 wheels --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 34d2fdda..bab27db7 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -9,7 +9,7 @@ pytest-random-order==1.0.4;python_version>="3.6" pytest-timeout==1.3.3 pytest-cov==2.7.1 h11==0.8.0 -cryptography==2.6.1 +cryptography==2.8 flaky==3.6.1 # https://github.com/GoogleCloudPlatform/python-repo-tools/issues/23 From ee86d8860ceb572e97e210084efb642fbe467a42 Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 05:19:27 +0000 Subject: [PATCH 06/10] Move cleancov in noxfile Both files are Python so it makes sense to merge them, and it will help with CI configuration. --- cleancov.py | 12 ------------ noxfile.py | 14 +++++++++++++- 2 files changed, 13 insertions(+), 13 deletions(-) delete mode 100644 cleancov.py diff --git a/cleancov.py b/cleancov.py deleted file mode 100644 index d36012a9..00000000 --- a/cleancov.py +++ /dev/null @@ -1,12 +0,0 @@ -from xml.etree import ElementTree as ET -import re -import sys - - -input_xml = ET.ElementTree(file=sys.argv[1]) -for class_ in input_xml.findall(".//class"): - filename = (class_.get("filename")) - filename = re.sub(".tox/.*/site-packages/", "src/", filename) - filename = re.sub("_sync", "_async", filename) - class_.set("filename", filename) -input_xml.write(sys.argv[1], xml_declaration=True) diff --git a/noxfile.py b/noxfile.py index fb7dacbe..1d5aa1a3 100644 --- a/noxfile.py +++ b/noxfile.py @@ -1,9 +1,21 @@ +from xml.etree import ElementTree as ET import os +import re import shutil +import sys import nox +def _clean_coverage(coverage_path): + input_xml = ET.ElementTree(file=coverage_path) + for class_ in input_xml.findall(".//class"): + filename = class_.get("filename") + filename = re.sub("_sync", "_async", filename) + class_.set("filename", filename) + input_xml.write(coverage_path, xml_declaration=True) + + def tests_impl(session, extras="socks,secure,brotli"): # Install deps and the package itself. session.install("-r", "dev-requirements.txt") @@ -28,7 +40,7 @@ def tests_impl(session, extras="socks,secure,brotli"): env={"PYTHONWARNINGS": "always::DeprecationWarning"} ) session.run("coverage", "xml") - session.run("python", "cleancov.py", "coverage.xml") + _clean_coverage("coverage.xml") @nox.session(python=["2.7", "3.5", "3.6", "3.7", "3.8", "pypy"]) From 7a31c47707da98ac0ab69f9f45863b13fb0f54b2 Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 04:15:55 +0000 Subject: [PATCH 07/10] Test Windows Python 3.x with GitHub Actions Keep testing 2.7 with AppVeyor as GitHub Actions makes it hard to test Python 2.7 on Windows and Nox. --- .github/workflows/ci.yml | 31 +++++++++++++++++++++++++++++++ appveyor.yml | 15 --------------- test/__init__.py | 2 +- test/conftest.py | 11 +++++++++++ 4 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..09dcb4c0 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,31 @@ +name: Continuous integration + +on: [push, pull_request] + +jobs: + build: + + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + python-version: [3.5, 3.6, 3.7, 3.8] + + steps: + - uses: actions/checkout@v1 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v1 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python.exe -m pip install --upgrade pip wheel nox + - name: Test + run: | + python.exe -m nox -s test-${{ matrix.python-version }} + - name: Coverage upload + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + run: | + python.exe -m pip install codecov + python.exe -m codecov diff --git a/appveyor.yml b/appveyor.yml index 18217236..9762e5b6 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,21 +12,6 @@ environment: PYTHON_ARCH: "64" NOX_SESSION: "test-2.7" - - PYTHON: "C:\\Python35-x64" - PYTHON_VERSION: "3.5.x" - PYTHON_ARCH: "64" - NOX_SESSION: "test-3.5" - - - PYTHON: "C:\\Python36-x64" - PYTHON_VERSION: "3.6.x" - PYTHON_ARCH: "64" - NOX_SESSION: "test-3.6" - - - PYTHON: "C:\\Python37-x64" - PYTHON_VERSION: "3.7.x" - PYTHON_ARCH: "64" - NOX_SESSION: "test-3.7" - cache: - C:\Users\appveyor\AppData\Local\pip\Cache diff --git a/test/__init__.py b/test/__init__.py index cb2f43e5..75a40ae1 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -36,7 +36,7 @@ # 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 +LONG_TIMEOUT = 0.5 if os.environ.get("CI") or os.environ.get("GITHUB_ACTIONS") else 0.01 def clear_warnings(cls=HTTPWarning): diff --git a/test/conftest.py b/test/conftest.py index 4763b50c..5d95ed19 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,5 +1,16 @@ +import platform import sys +import pytest + # We support Python 3.6+ for async code if sys.version_info[:2] < (3, 6): collect_ignore_glob = ["async/*.py", "with_dummyserver/async/*.py"] + +# The Python 3.8+ default loop on Windows breaks Tornado +@pytest.fixture(scope="session", autouse=True) +def configure_windows_event_loop(): + if sys.version_info >= (3, 8) and platform.system() == "Windows": + import asyncio + + asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) From cc368aa855cc27b8c9b2d49ee2ec5e80e4c10b86 Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 07:50:24 +0000 Subject: [PATCH 08/10] Test 2.7 from GitHub Actions too --- .github/workflows/ci.yml | 13 ++++++++++--- appveyor.yml | 38 -------------------------------------- 2 files changed, 10 insertions(+), 41 deletions(-) delete mode 100644 appveyor.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09dcb4c0..2f3f9f17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,20 +9,27 @@ jobs: strategy: fail-fast: false matrix: - python-version: [3.5, 3.6, 3.7, 3.8] + python-version: [2.7, 3.5, 3.6, 3.7, 3.8] steps: - uses: actions/checkout@v1 + - name: Set up Python 3.7 to run nox + uses: actions/setup-python@v1 + with: + python-version: 3.7 - name: Set up Python ${{ matrix.python-version }} + if: matrix.python_version != '3.7' uses: actions/setup-python@v1 with: python-version: ${{ matrix.python-version }} - name: Install dependencies run: | - python.exe -m pip install --upgrade pip wheel nox + # Work around https://github.com/theacodes/nox/issues/250 + Remove-Item C:\ProgramData\Chocolatey\bin\python2.7.exe + py -3.7 -m pip install nox - name: Test run: | - python.exe -m nox -s test-${{ matrix.python-version }} + nox -s test-${{ matrix.python-version }} - name: Coverage upload env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/appveyor.yml b/appveyor.yml deleted file mode 100644 index 9762e5b6..00000000 --- a/appveyor.yml +++ /dev/null @@ -1,38 +0,0 @@ -# AppVeyor.yml from https://github.com/ogrisel/python-appveyor-demo -# License: CC0 1.0 Universal: http://creativecommons.org/publicdomain/zero/1.0/ - -skip_branch_with_pr: true -build: off - -environment: - - matrix: - - PYTHON: "C:\\Python27-x64" - PYTHON_VERSION: "2.7.x" - PYTHON_ARCH: "64" - NOX_SESSION: "test-2.7" - -cache: - - C:\Users\appveyor\AppData\Local\pip\Cache - -install: - # Install Python (from the official .msi of http://python.org) and pip when - # not already installed. - - ps: if (-not(Test-Path($env:PYTHON))) { & _appveyor\install.ps1 } - - # Prepend newly installed Python to the PATH of this build (this cannot be - # done from inside the powershell script as it would require to restart - # the parent CMD process). - - SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH% - - # Upgrade to the latest version of pip to avoid it displaying warnings - # about it being out of date. - - C:\Python36-x64\python.exe -m pip install --upgrade pip wheel - - C:\Python36-x64\python.exe -m pip install nox - -test_script: - - C:\Python36-x64\python.exe -m nox -s "%NOX_SESSION%" - -on_success: - - pip install codecov - - codecov --env PLATFORM,TOXENV From 38161aa2296936958701336a28832c21398306db Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 09:00:24 +0000 Subject: [PATCH 09/10] Use "Windows" as build name on GitHub Action Windows CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f3f9f17..b63de4df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -name: Continuous integration +name: Windows on: [push, pull_request] From 2b6f5b7e78d5672aa2963f7b44d5b857610f702c Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Tue, 26 Nov 2019 10:06:35 +0000 Subject: [PATCH 10/10] ci: only mark build as Windows-only --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b63de4df..5a7aa985 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,9 +1,9 @@ -name: Windows +name: CI on: [push, pull_request] jobs: - build: + Windows: runs-on: windows-latest strategy: