From b28788325f24d8059770b8e1185012b616c299b6 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 17 Oct 2018 21:32:28 +0100 Subject: [PATCH] tests: Improve unit tests runtime performance This patch set does 2 things: 1) Improves unit test runtime peformance via pytest-xdist [0] 2) Reduces finnicky nature of `is_connected` helpers which sometimes skip even when there is access to the internet; logic has been added to make these checks more accurate to avoid skipping tests Note that while there are newer alternatives to pytest-xdist they are only compatible with much newer versions of Python. [0] https://pypi.org/project/pytest-xdist/ Change-Id: Ib04b48ebabca0551058e5e1065056f4e559fbfe6 --- test-requirements.txt | 1 + tests/unit/conftest.py | 17 +++++++++-------- tests/unit/test_utils.py | 28 +++++++++++++++++----------- tools/gate/run-unit-tests.sh | 6 ++++-- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/test-requirements.txt b/test-requirements.txt index 5a165747..efca744b 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,6 +2,7 @@ pytest==3.2.1 pytest-cov==2.5.1 testfixtures +pytest-xdist==1.23.2 mock==2.0.0 # Formatting diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 6dab893e..846e51a5 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -12,13 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +import atexit import copy - import os -import pytest import shutil import tempfile +import pytest + from pegleg import config """Fixtures that are applied to all unit tests.""" @@ -35,7 +36,10 @@ def restore_config(): config.GLOBAL_CONTEXT = original_global_context -@pytest.fixture(scope="module", autouse=True) +# NOTE(felipemonteiro): This uses `atexit` rather than a `pytest.fixture` +# decorator because 1) this only needs to be run exactly once and 2) this +# works across multiple test executors via `pytest -n ` +@atexit.register def clean_temporary_git_repos(): """Iterates through all temporarily created directories and deletes each one that was created for testing. @@ -51,8 +55,5 @@ def clean_temporary_git_repos(): if any(p.startswith('airship') for p in os.listdir(path)): yield path - try: - yield - finally: - for tempdir in temporary_git_repos(): - shutil.rmtree(tempdir, ignore_errors=True) + for tempdir in temporary_git_repos(): + shutil.rmtree(tempdir, ignore_errors=True) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 67438683..d8e28302 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -55,11 +55,14 @@ def is_connected(): :returns: True if connected else False. """ - try: - r = requests.get("http://www.github.com/", proxies={}, timeout=3) - return r.ok - except requests.exceptions.RequestException: - return False + for _ in range(3): + try: + r = requests.get("http://www.github.com/", proxies={}, timeout=3) + r.raise_for_status() + return True + except requests.exceptions.RequestException: + pass + return False def is_connected_behind_proxy(): @@ -67,9 +70,12 @@ def is_connected_behind_proxy(): :returns: True if connected else False. """ - try: - r = requests.get( - "http://www.github.com/", proxies=_PROXY_SERVERS, timeout=3) - return r.ok - except requests.exceptions.RequestException: - return False + for _ in range(3): + try: + r = requests.get( + "http://www.github.com/", proxies=_PROXY_SERVERS, timeout=3) + r.raise_for_status() + return True + except requests.exceptions.RequestException: + pass + return False diff --git a/tools/gate/run-unit-tests.sh b/tools/gate/run-unit-tests.sh index 15042fe8..61fbf44f 100755 --- a/tools/gate/run-unit-tests.sh +++ b/tools/gate/run-unit-tests.sh @@ -2,9 +2,11 @@ set -e posargs=$@ +# cross-platform way to derive the number of logical cores +readonly num_cores=$(python -c 'import multiprocessing as mp; print(mp.cpu_count())') if [ ${#posargs} -ge 1 ]; then - pytest -k ${posargs} + pytest -k ${posargs} -n $num_cores else - pytest + pytest -n $num_cores fi set +e