From 678b29657abc445a0056d80127bc6a6f302d0862 Mon Sep 17 00:00:00 2001 From: "Paul J. Dorn" Date: Fri, 15 Dec 2023 11:00:02 +0100 Subject: [PATCH] importlib-style pytest explicit sys.path monkeypatching only where needed TODO: verify against CI / editable installes / tox venv --- Makefile | 11 +++-- pyproject.toml | 20 ++++++-- requirements_test.txt | 5 +- tests/{config => }/__init__.py | 0 tests/importable/config/__init__.py | 0 tests/{ => importable}/config/test_cfg.py | 0 tests/{ => importable}/config/test_cfg_alt.py | 0 .../config/test_cfg_with_wsgi_app.py | 0 tests/{ => importable}/support.py | 0 tests/test_config.py | 46 +++++++++++-------- tests/test_http.py | 2 +- tests/test_invalid_requests.py | 2 +- tests/test_util.py | 15 ++++-- tests/test_valid_requests.py | 2 +- tests/treq.py | 13 +++++- tox.ini | 4 +- 16 files changed, 81 insertions(+), 39 deletions(-) rename tests/{config => }/__init__.py (100%) create mode 100644 tests/importable/config/__init__.py rename tests/{ => importable}/config/test_cfg.py (100%) rename tests/{ => importable}/config/test_cfg_alt.py (100%) rename tests/{ => importable}/config/test_cfg_with_wsgi_app.py (100%) rename tests/{ => importable}/support.py (100%) diff --git a/Makefile b/Makefile index 3641cd5ab..5624704c5 100644 --- a/Makefile +++ b/Makefile @@ -4,14 +4,15 @@ build: venv/bin/pip install -r requirements_dev.txt test: - venv/bin/python setup.py test + venv/bin/python -m pytest coverage: - venv/bin/python setup.py test --cov + venv/bin/python -m coverage run -m pytest + venv/bin/python -m coverage xml clean: - @rm -rf .Python MANIFEST build dist venv* *.egg-info *.egg - @find . -type f -name "*.py[co]" -delete - @find . -type d -name "__pycache__" -delete + # like @rm -rf, but safer: only untracked git-ignored + # any desirable difference between the two should instead be added to .gitignore + @git clean -X -f -- .Python MANIFEST build dist "venv*" "*.egg-info" "*.egg" __pycache__ .PHONY: build clean coverage test diff --git a/pyproject.toml b/pyproject.toml index 3a1f43a76..d436bec0a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,6 @@ testing = [ "eventlet", "coverage", "pytest", - "pytest-cov", ] [project.scripts] @@ -71,11 +70,26 @@ gunicorn = "gunicorn.app.wsgiapp:run" [project.entry-points."paste.server_runner"] main = "gunicorn.app.pasterapp:serve" +[tool.coverage.report] +omit = [ + # tests will load such file as configuration + # silence warnings/errors when we delete its source + "*/gunicorn.conf.py", +] + [tool.pytest.ini_options] -# can override these: python -m pytest --override-ini="addopts=" +minversion = "7.2.0" +# seconds until still-running tests are dumped +faulthandler_timeout = 5 norecursedirs = ["examples", "lib", "local", "src"] testpaths = ["tests/"] -addopts = "--assert=plain --cov=gunicorn --cov-report=xml" +# --assert=plain stops rewriting asserts for better expression info +# --import-mode=importlib disables error-prone sys.path modifications +# --strict-markers raises on unknown markers +# can override this: python -m pytest --override-ini="addopts=" +addopts = "--import-mode=importlib --strict-markers" +# main selling point of pytest-cov was to workaround --import-mode=prepend +# required_plugins = pytest-cov [tool.setuptools] # FIXME: zip-safe = long-overdue cleanup diff --git a/requirements_test.txt b/requirements_test.txt index b618d1a73..9d0701969 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,5 +1,8 @@ gevent eventlet coverage +# pytest 8.0 intends to drop Python 3.7 +# pytest 7.2.0 starts using Python 3.11 stdlib tomllib pytest>=7.2.0 -pytest-cov +# pytest 6.0 supports modern importlib, so we do not need pytest-cov to fixup sys.path +# pytest-cov diff --git a/tests/config/__init__.py b/tests/__init__.py similarity index 100% rename from tests/config/__init__.py rename to tests/__init__.py diff --git a/tests/importable/config/__init__.py b/tests/importable/config/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/config/test_cfg.py b/tests/importable/config/test_cfg.py similarity index 100% rename from tests/config/test_cfg.py rename to tests/importable/config/test_cfg.py diff --git a/tests/config/test_cfg_alt.py b/tests/importable/config/test_cfg_alt.py similarity index 100% rename from tests/config/test_cfg_alt.py rename to tests/importable/config/test_cfg_alt.py diff --git a/tests/config/test_cfg_with_wsgi_app.py b/tests/importable/config/test_cfg_with_wsgi_app.py similarity index 100% rename from tests/config/test_cfg_with_wsgi_app.py rename to tests/importable/config/test_cfg_with_wsgi_app.py diff --git a/tests/support.py b/tests/importable/support.py similarity index 100% rename from tests/support.py rename to tests/importable/support.py diff --git a/tests/test_config.py b/tests/test_config.py index 00de75e4e..a895aae51 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,6 +5,7 @@ import os import re import sys +from pathlib import Path import pytest @@ -16,20 +17,20 @@ from gunicorn import glogging from gunicorn.instrument import statsd -dirname = os.path.dirname(__file__) +tests_dir = Path(__file__).parent +examples_dir = tests_dir / ".." / "examples" def cfg_module(): return 'config.test_cfg' def alt_cfg_module(): return 'config.test_cfg_alt' def cfg_file(): - return os.path.join(dirname, "config", "test_cfg.py") + return tests_dir / "importable" / "config" / "test_cfg.py" def alt_cfg_file(): - return os.path.join(dirname, "config", "test_cfg_alt.py") + return tests_dir / "importable" / "config" / "test_cfg_alt.py" def cfg_file_with_wsgi_app(): - return os.path.join(dirname, "config", "test_cfg_with_wsgi_app.py") -def paster_ini(): - return os.path.join(dirname, "..", "examples", "frameworks", "pylonstest", "nose.ini") - + return tests_dir / "importable" / "config" / "test_cfg_with_wsgi_app.py" +def paster_ni(): + return examples_dir / "frameworks" / "pylonstest" / "nose.ini" class AltArgs: def __init__(self, args=None): @@ -37,6 +38,7 @@ def __init__(self, args=None): self.orig = sys.argv def __enter__(self): + assert all(isinstance(arg, str) for arg in self.args) sys.argv = self.args def __exit__(self, exc_type, exc_inst, traceback): @@ -226,7 +228,7 @@ def test_app_config(): def test_load_config(): - with AltArgs(["prog_name", "-c", cfg_file()]): + with AltArgs(["prog_name", "-c", "%s" % cfg_file()]): app = NoConfigApp() assert app.cfg.bind == ["unix:/tmp/bar/baz"] assert app.cfg.workers == 3 @@ -241,7 +243,9 @@ def test_load_config_explicit_file(): assert app.cfg.proc_name == "fooey" -def test_load_config_module(): +def test_load_config_module(monkeypatch): + monkeypatch.syspath_prepend(Path(__file__).parent / "importable") + with AltArgs(["prog_name", "-c", "python:%s" % cfg_module()]): app = NoConfigApp() assert app.cfg.bind == ["unix:/tmp/bar/baz"] @@ -250,13 +254,15 @@ def test_load_config_module(): def test_cli_overrides_config(): - with AltArgs(["prog_name", "-c", cfg_file(), "-b", "blarney"]): + with AltArgs(["prog_name", "-c", "%s" % cfg_file(), "-b", "blarney"]): app = NoConfigApp() assert app.cfg.bind == ["blarney"] assert app.cfg.proc_name == "fooey" -def test_cli_overrides_config_module(): +def test_cli_overrides_config_module(monkeypatch): + monkeypatch.syspath_prepend(Path(__file__).parent / "importable") + with AltArgs(["prog_name", "-c", "python:%s" % cfg_module(), "-b", "blarney"]): app = NoConfigApp() assert app.cfg.bind == ["blarney"] @@ -369,15 +375,15 @@ def test_load_enviroment_variables_config(monkeypatch): assert app.cfg.workers == 4 def test_config_file_environment_variable(monkeypatch): - monkeypatch.setenv("GUNICORN_CMD_ARGS", "--config=" + alt_cfg_file()) + monkeypatch.setenv("GUNICORN_CMD_ARGS", "--config=%s" % alt_cfg_file()) with AltArgs(): app = NoConfigApp() assert app.cfg.proc_name == "not-fooey" - assert app.cfg.config == alt_cfg_file() - with AltArgs(["prog_name", "--config", cfg_file()]): + assert app.cfg.config == "%s" % alt_cfg_file() + with AltArgs(["prog_name", "--config", "%s" % cfg_file()]): app = NoConfigApp() assert app.cfg.proc_name == "fooey" - assert app.cfg.config == cfg_file() + assert app.cfg.config == "%s" % cfg_file() def test_invalid_enviroment_variables_config(monkeypatch, capsys): monkeypatch.setenv("GUNICORN_CMD_ARGS", "--foo=bar") @@ -390,16 +396,16 @@ def test_invalid_enviroment_variables_config(monkeypatch, capsys): def test_cli_overrides_enviroment_variables_module(monkeypatch): monkeypatch.setenv("GUNICORN_CMD_ARGS", "--workers=4") - with AltArgs(["prog_name", "-c", cfg_file(), "--workers", "3"]): + with AltArgs(["prog_name", "-c", "%s" % cfg_file(), "--workers", "3"]): app = NoConfigApp() assert app.cfg.workers == 3 @pytest.mark.parametrize("options, expected", [ (["app:app"], 'app:app'), - (["-c", cfg_file(), "app:app"], 'app:app'), - (["-c", cfg_file_with_wsgi_app(), "app:app"], 'app:app'), - (["-c", cfg_file_with_wsgi_app()], 'app1:app1'), + (["-c", "%s" % cfg_file(), "app:app"], 'app:app'), + (["-c", "%s" % cfg_file_with_wsgi_app(), "app:app"], 'app:app'), + (["-c", "%s" % cfg_file_with_wsgi_app()], 'app1:app1'), ]) def test_wsgi_app_config(options, expected): cmdline = ["prog_name"] @@ -411,7 +417,7 @@ def test_wsgi_app_config(options, expected): @pytest.mark.parametrize("options", [ ([]), - (["-c", cfg_file()]), + (["-c", "%s" % cfg_file()]), ]) def test_non_wsgi_app(options, capsys): cmdline = ["prog_name"] diff --git a/tests/test_http.py b/tests/test_http.py index 9bbbdd5bb..a9333c5cf 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -1,5 +1,5 @@ import io -import t +from . import t import pytest from unittest import mock diff --git a/tests/test_invalid_requests.py b/tests/test_invalid_requests.py index 63224d07d..0bd03a4b8 100644 --- a/tests/test_invalid_requests.py +++ b/tests/test_invalid_requests.py @@ -7,7 +7,7 @@ import pytest -import treq +from . import treq dirname = os.path.dirname(__file__) reqdir = os.path.join(dirname, "requests", "invalid") diff --git a/tests/test_util.py b/tests/test_util.py index 35e544f9e..92d9e0195 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,7 +1,8 @@ # # This file is part of gunicorn released under the MIT license. # See the NOTICE for more information. -import os + +from pathlib import Path import pytest @@ -70,14 +71,16 @@ def test_warn(capsys): "support:create_app(count=3)", ], ) -def test_import_app_good(value): +def test_import_app_good(monkeypatch, value): + monkeypatch.syspath_prepend(Path(__file__).parent / "importable") + assert util.import_app(value) @pytest.mark.parametrize( ("value", "exc_type", "msg"), [ - ("a:app", ImportError, "No module"), + ("nonexisting:app", ImportError, "No module"), ("support:create_app(", AppImportError, "Failed to parse"), ("support:create.app()", AppImportError, "Function reference"), ("support:create_app(Gunicorn)", AppImportError, "literal values"), @@ -89,7 +92,9 @@ def test_import_app_good(value): ("support:HOST", AppImportError, "callable"), ], ) -def test_import_app_bad(value, exc_type, msg): +def test_import_app_bad(monkeypatch, value, exc_type, msg): + monkeypatch.syspath_prepend(Path(__file__).parent / "importable") + with pytest.raises(exc_type) as exc_info: util.import_app(value) @@ -97,7 +102,7 @@ def test_import_app_bad(value, exc_type, msg): def test_import_app_py_ext(monkeypatch): - monkeypatch.chdir(os.path.dirname(__file__)) + monkeypatch.chdir(Path(__file__).parent / "importable") with pytest.raises(ImportError) as exc_info: util.import_app("support.py") diff --git a/tests/test_valid_requests.py b/tests/test_valid_requests.py index 2c71622c4..a846f94a8 100644 --- a/tests/test_valid_requests.py +++ b/tests/test_valid_requests.py @@ -7,7 +7,7 @@ import pytest -import treq +from . import treq dirname = os.path.dirname(__file__) reqdir = os.path.join(dirname, "requests", "valid") diff --git a/tests/treq.py b/tests/treq.py index eb5f78dcb..cbd9b50c4 100644 --- a/tests/treq.py +++ b/tests/treq.py @@ -14,7 +14,18 @@ from gunicorn.util import split_request_uri dirname = os.path.dirname(__file__) -random.seed() + +# Choices: +# a) stdlib secrets module +# will help expose more bad tests +# may flip-flip between same test working and failing +# b) stdlib random (library name is a misnomer) with fixed seed +# will produce repeatably results +# may miss bad tests aligning with some properties of the generator +# c) stdlib random with arbitrary seed +# only the downsides of a+b +# nonsensical +random.seed(a=0, version=2) def uri(data): diff --git a/tox.ini b/tox.ini index 94a36db00..1e336dd45 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,9 @@ skipsdist = false [testenv] use_develop = true -commands = pytest --cov=gunicorn {posargs} +commands = + python -m coverage run -m pytest {posargs} + python -m coverage xml deps = -rrequirements_test.txt