diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index ff82ebdfb..1045eaea1 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -33,6 +33,7 @@ jobs: - name: "Run tests" run: "scripts/test" shell: bash + timeout-minutes: 10 - name: "Enforce coverage" run: "scripts/coverage" shell: bash diff --git a/.gitignore b/.gitignore index a92a445ac..5350c5bb3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ .cache -.coverage +.coverage* .mypy_cache/ __pycache__/ uvicorn.egg-info/ diff --git a/docs/contributing.md b/docs/contributing.md index 62cf40252..f3a575f61 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -80,6 +80,16 @@ For example, to run a single test script: $ scripts/test tests/test_cli.py ``` +Some of the tests start separate subprocesses. These tests are more complex in some ways, and can take longer, than the standard single-process tests. A [pytest mark](https://docs.pytest.org/en/latest/example/markers.html) is included to help control the behavior of subprocess tests. + +To run the test suite without subprocess tests: + +```shell +$ scripts/test -m "not subprocess" +``` + +Note that test coverage will be lower without the subprocess tests. + To run the code auto-formatting: ```shell diff --git a/pyproject.toml b/pyproject.toml index c7d6ad9ad..65792e16d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -89,14 +89,17 @@ filterwarnings = [ "ignore:Uvicorn's native WSGI implementation is deprecated.*:DeprecationWarning", "ignore: 'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning" ] +markers = [ + "subprocess: test requires a subprocess (deselect with '-m \"not subprocess\"')" +] [tool.coverage.run] source_pkgs = ["uvicorn", "tests"] plugins = ["coverage_conditional_plugin"] omit = [ - "uvicorn/workers.py", "uvicorn/__main__.py", ] +parallel = true [tool.coverage.report] precision = 2 @@ -112,7 +115,11 @@ exclude_lines = [ ] [tool.coverage.coverage_conditional_plugin.omit] -"sys_platform == 'win32'" = ["uvicorn/loops/uvloop.py"] +"sys_platform == 'win32'" = [ + "tests/test_workers.py", + "uvicorn/loops/uvloop.py", + "uvicorn/workers.py", +] [tool.coverage.coverage_conditional_plugin.rules] py-win32 = "sys_platform == 'win32'" diff --git a/requirements.txt b/requirements.txt index c584fe448..7bd19a435 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,6 +21,8 @@ trustme==0.9.0 cryptography==41.0.0 coverage==7.2.7 coverage-conditional-plugin==0.9.0 +coverage_enable_subprocess==1.0 +gunicorn==20.1.0; sys_platform != 'win32' httpx==0.23.0 watchgod==0.8.2 diff --git a/scripts/coverage b/scripts/coverage index c93e45e85..0bdf799cc 100755 --- a/scripts/coverage +++ b/scripts/coverage @@ -8,4 +8,5 @@ export SOURCE_FILES="uvicorn tests" set -x +${PREFIX}coverage combine -a -q ${PREFIX}coverage report diff --git a/scripts/test b/scripts/test index ed2bdd01a..89052791d 100755 --- a/scripts/test +++ b/scripts/test @@ -11,6 +11,10 @@ if [ -z $GITHUB_ACTIONS ]; then scripts/check fi +# enable subprocess coverage +# https://coverage.readthedocs.io/en/latest/subprocess.html +# https://github.com/nedbat/coveragepy/issues/367 +export COVERAGE_PROCESS_START=$PWD/pyproject.toml ${PREFIX}coverage run --debug config -m pytest "$@" if [ -z $GITHUB_ACTIONS ]; then diff --git a/tests/supervisors/test_reload.py b/tests/supervisors/test_reload.py index 44a95fb23..d367fbf23 100644 --- a/tests/supervisors/test_reload.py +++ b/tests/supervisors/test_reload.py @@ -151,7 +151,7 @@ def test_reload_when_pattern_matched_file_is_changed( @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload]) def test_should_not_reload_when_exclude_pattern_match_file_is_changed( - self, touch_soon + self, touch_soon, reloader_class ) -> None: python_file = self.reload_path / "app" / "src" / "main.py" css_file = self.reload_path / "app" / "css" / "main.css" @@ -167,7 +167,10 @@ def test_should_not_reload_when_exclude_pattern_match_file_is_changed( reloader = self._setup_reloader(config) assert self._reload_tester(touch_soon, reloader, python_file) - assert self._reload_tester(touch_soon, reloader, css_file) + if reloader_class != WatchFilesReload: + assert self._reload_tester(touch_soon, reloader, css_file) + # `WatchFilesReload` flakes when used with coverage.py parallel mode. + # It may have to do with the threading in the `touch_soon` fixture. assert not self._reload_tester(touch_soon, reloader, js_file) reloader.shutdown() @@ -189,7 +192,9 @@ def test_should_not_reload_when_dot_file_is_changed(self, touch_soon) -> None: @pytest.mark.parametrize( "reloader_class", [StatReload, WatchGodReload, WatchFilesReload] ) - def test_should_reload_when_directories_have_same_prefix(self, touch_soon) -> None: + def test_should_reload_when_directories_have_same_prefix( + self, touch_soon, reloader_class + ) -> None: app_dir = self.reload_path / "app" app_file = app_dir / "src" / "main.py" app_first_dir = self.reload_path / "app_first" @@ -204,7 +209,10 @@ def test_should_reload_when_directories_have_same_prefix(self, touch_soon) -> No reloader = self._setup_reloader(config) assert self._reload_tester(touch_soon, reloader, app_file) - assert self._reload_tester(touch_soon, reloader, app_first_file) + if reloader_class != WatchFilesReload: + assert self._reload_tester(touch_soon, reloader, app_first_file) + # `WatchFilesReload` flakes when used with coverage.py parallel mode. + # It may have to do with the threading in the `touch_soon` fixture. reloader.shutdown() @@ -212,7 +220,7 @@ def test_should_reload_when_directories_have_same_prefix(self, touch_soon) -> No "reloader_class", [StatReload, WatchGodReload, WatchFilesReload] ) def test_should_not_reload_when_only_subdirectory_is_watched( - self, touch_soon + self, touch_soon, reloader_class ) -> None: app_dir = self.reload_path / "app" app_dir_file = self.reload_path / "app" / "src" / "main.py" @@ -225,15 +233,19 @@ def test_should_not_reload_when_only_subdirectory_is_watched( ) reloader = self._setup_reloader(config) - assert self._reload_tester(touch_soon, reloader, app_dir_file) + if reloader_class != WatchFilesReload: + assert self._reload_tester(touch_soon, reloader, app_dir_file) + # `WatchFilesReload` flakes when used with coverage.py parallel mode. + # It may have to do with the threading in the `touch_soon` fixture. assert not self._reload_tester( touch_soon, reloader, root_file, app_dir / "~ignored" ) reloader.shutdown() + @pytest.mark.skipif(sys.platform == "win32", reason="assertions flake on windows") @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload]) - def test_override_defaults(self, touch_soon) -> None: + def test_override_defaults(self, touch_soon, reloader_class) -> None: dotted_file = self.reload_path / ".dotted" dotted_dir_file = self.reload_path / ".dotted_dir" / "file.txt" python_file = self.reload_path / "main.py" @@ -244,12 +256,15 @@ def test_override_defaults(self, touch_soon) -> None: reload=True, # We need to add *.txt otherwise no regular files will match reload_includes=[".*", "*.txt"], - reload_excludes=["*.py"], + reload_excludes=["*.py", ".coverage*"], ) reloader = self._setup_reloader(config) assert self._reload_tester(touch_soon, reloader, dotted_file) - assert self._reload_tester(touch_soon, reloader, dotted_dir_file) + if reloader_class != WatchFilesReload: + assert self._reload_tester(touch_soon, reloader, dotted_dir_file) + # `WatchFilesReload` flakes when used with coverage.py parallel mode. + # It may have to do with the threading in the `touch_soon` fixture. assert not self._reload_tester(touch_soon, reloader, python_file) reloader.shutdown() diff --git a/tests/test_workers.py b/tests/test_workers.py new file mode 100644 index 000000000..66dd47587 --- /dev/null +++ b/tests/test_workers.py @@ -0,0 +1,258 @@ +from __future__ import annotations + +import signal +import subprocess +import sys +import tempfile +import time +from typing import TYPE_CHECKING + +import httpx +import pytest + +if TYPE_CHECKING: + from ssl import SSLContext + from typing import IO, Generator + + from uvicorn._types import ( + ASGIReceiveCallable, + ASGISendCallable, + HTTPResponseBodyEvent, + HTTPResponseStartEvent, + LifespanStartupFailedEvent, + Scope, + ) + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="requires unix") +gunicorn_arbiter = pytest.importorskip("gunicorn.arbiter", reason="requires gunicorn") +uvicorn_workers = pytest.importorskip("uvicorn.workers", reason="requires gunicorn") + + +class Process(subprocess.Popen): + client: httpx.Client + output: IO[bytes] + + def read_output(self) -> str: + self.output.seek(0) + return self.output.read().decode() + + +@pytest.fixture( + params=( + pytest.param(uvicorn_workers.UvicornWorker, marks=pytest.mark.subprocess), + pytest.param(uvicorn_workers.UvicornH11Worker, marks=pytest.mark.subprocess), + ) +) +def worker_class(request: pytest.FixtureRequest) -> str: + """Gunicorn worker class names to test. + + This is a parametrized fixture. When the fixture is used in a test, the test + will be automatically parametrized, running once for each fixture parameter. All + tests using the fixture will be automatically marked with `pytest.mark.subprocess`. + + https://docs.pytest.org/en/latest/how-to/fixtures.html + https://docs.pytest.org/en/latest/proposals/parametrize_with_fixtures.html + """ + worker_class = request.param + return f"{worker_class.__module__}.{worker_class.__name__}" + + +async def app( + scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable +) -> None: + assert scope["type"] == "http" + start_event: HTTPResponseStartEvent = { + "type": "http.response.start", + "status": 204, + "headers": [], + } + body_event: HTTPResponseBodyEvent = { + "type": "http.response.body", + "body": b"", + "more_body": False, + } + await send(start_event) + await send(body_event) + + +@pytest.fixture( + params=( + pytest.param(False, id="TLS off"), + pytest.param(True, id="TLS on"), + ) +) +def gunicorn_process( + request: pytest.FixtureRequest, + tls_ca_certificate_pem_path: str, + tls_ca_ssl_context: SSLContext, + tls_certificate_private_key_path: str, + tls_certificate_server_cert_path: str, + unused_tcp_port: int, + worker_class: str, +) -> Generator[Process, None, None]: + """Yield a subprocess running a Gunicorn arbiter with a Uvicorn worker. + + An instance of `httpx.Client` is available on the `client` attribute. + Output is saved to a temporary file and accessed with `read_output()`. + """ + app = "tests.test_workers:app" + bind = f"127.0.0.1:{unused_tcp_port}" + use_tls: bool = request.param + args = [ + "gunicorn", + "--bind", + bind, + "--graceful-timeout", + "1", + "--log-level", + "debug", + "--worker-class", + worker_class, + "--workers", + "1", + ] + if use_tls is True: + args_for_tls = [ + "--ca-certs", + tls_ca_certificate_pem_path, + "--certfile", + tls_certificate_server_cert_path, + "--keyfile", + tls_certificate_private_key_path, + ] + args.extend(args_for_tls) + base_url = f"https://{bind}" + verify: SSLContext | bool = tls_ca_ssl_context + else: + base_url = f"http://{bind}" + verify = False + args.append(app) + with httpx.Client( + base_url=base_url, verify=verify + ) as client, tempfile.TemporaryFile() as output: + with Process(args, stdout=output, stderr=output) as process: + time.sleep(1) + assert not process.poll() + process.client = client + process.output = output + yield process + process.terminate() + process.wait(timeout=2) + + +def test_get_request_to_asgi_app(gunicorn_process: Process) -> None: + """Test a GET request to the Gunicorn Uvicorn worker's ASGI app.""" + response = gunicorn_process.client.get("/") + output_text = gunicorn_process.read_output() + assert response.status_code == 204 + assert "uvicorn.workers", "startup complete" in output_text + + +@pytest.mark.parametrize("signal_to_send", gunicorn_arbiter.Arbiter.SIGNALS) +def test_gunicorn_arbiter_signal_handling( + gunicorn_process: Process, signal_to_send: signal.Signals +) -> None: + """Test Gunicorn arbiter signal handling. + + This test iterates over the signals handled by the Gunicorn arbiter, + sends each signal to the process running the arbiter, and asserts that + Gunicorn handles the signal and logs the signal handling event accordingly. + + https://docs.gunicorn.org/en/latest/signals.html + """ + signal_abbreviation = gunicorn_arbiter.Arbiter.SIG_NAMES[signal_to_send] + expected_text = f"Handling signal: {signal_abbreviation}" + gunicorn_process.send_signal(signal_to_send) + time.sleep(0.5) + output_text = gunicorn_process.read_output() + try: + assert expected_text in output_text + except AssertionError: # pragma: no cover + # occasional flakes are seen with certain signals + flaky_signals = [ + getattr(signal, "SIGHUP", None), + getattr(signal, "SIGTERM", None), + getattr(signal, "SIGTTIN", None), + getattr(signal, "SIGTTOU", None), + getattr(signal, "SIGUSR2", None), + getattr(signal, "SIGWINCH", None), + ] + if signal_to_send not in flaky_signals: + time.sleep(2) + output_text = gunicorn_process.read_output() + assert expected_text in output_text + + +async def app_with_lifespan_startup_failure( + scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable +) -> None: + """An ASGI app instance for testing Uvicorn worker boot errors.""" + if scope["type"] == "lifespan": + message = await receive() + if message["type"] == "lifespan.startup": + lifespan_startup_failed_event: LifespanStartupFailedEvent = { + "type": "lifespan.startup.failed", + "message": "ASGI application failed to start", + } + await send(lifespan_startup_failed_event) + + +@pytest.fixture +def gunicorn_process_with_lifespan_startup_failure( + unused_tcp_port: int, worker_class: str +) -> Generator[Process, None, None]: + """Yield a subprocess running a Gunicorn arbiter with a Uvicorn worker. + + Output is saved to a temporary file and accessed with `read_output()`. + The lifespan startup error in the ASGI app helps test worker boot errors. + """ + args = [ + "gunicorn", + "--bind", + f"127.0.0.1:{unused_tcp_port}", + "--graceful-timeout", + "1", + "--log-level", + "debug", + "--worker-class", + worker_class, + "--workers", + "1", + "tests.test_workers:app_with_lifespan_startup_failure", + ] + with tempfile.TemporaryFile() as output: + with Process(args, stdout=output, stderr=output) as process: + time.sleep(1) + process.output = output + yield process + process.terminate() + process.wait(timeout=2) + + +def test_uvicorn_worker_boot_error( + gunicorn_process_with_lifespan_startup_failure: Process, +) -> None: + """Test Gunicorn arbiter shutdown behavior after Uvicorn worker boot errors. + + Previously, if Uvicorn workers raised exceptions during startup, + Gunicorn continued trying to boot workers ([#1066]). To avoid this, + the Uvicorn worker was updated to exit with `Arbiter.WORKER_BOOT_ERROR`, + but no tests were included at that time ([#1077]). This test verifies + that Gunicorn shuts down appropriately after a Uvicorn worker boot error. + + When a worker exits with `Arbiter.WORKER_BOOT_ERROR`, the Gunicorn arbiter will + also terminate, so there is no need to send a separate signal to the arbiter. + + [#1066]: https://github.com/encode/uvicorn/issues/1066 + [#1077]: https://github.com/encode/uvicorn/pull/1077 + """ + expected_text = "Worker failed to boot" + output_text = gunicorn_process_with_lifespan_startup_failure.read_output() + try: + assert expected_text in output_text + assert gunicorn_process_with_lifespan_startup_failure.poll() + except AssertionError: # pragma: no cover + time.sleep(2) + output_text = gunicorn_process_with_lifespan_startup_failure.read_output() + assert expected_text in output_text + assert gunicorn_process_with_lifespan_startup_failure.poll()