From b1bf81c7cdf514a0b596e66e2f1d497b970a515e Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 14 Aug 2024 12:26:12 +0800 Subject: [PATCH 01/17] test(supervisors): ensure cleanup in test_multiprocess by using 'finally' Ensure proper cleanup in 'test_multiprocess' by wrapping the supervisor shutdown logic in a 'finally' block. This guarantees that the supervisor is properly terminated and all processes are joined, avoiding potential resource leaks or state residuebetween test runs. --- tests/supervisors/test_multiprocess.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index e1f594efe..022243388 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -86,12 +86,14 @@ def test_multiprocess_health_check() -> None: time.sleep(1) process = supervisor.processes[0] process.kill() - assert not process.is_alive() - time.sleep(1) - for p in supervisor.processes: - assert p.is_alive() - supervisor.signal_queue.append(signal.SIGINT) - supervisor.join_all() + try: + assert not process.is_alive() + time.sleep(1) + for p in supervisor.processes: + assert p.is_alive() + finally: + supervisor.signal_queue.append(signal.SIGINT) + supervisor.join_all() @new_console_in_windows From b4d8b09217650e80afd1ca16cf37711cf8023f1d Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 14 Aug 2024 17:18:08 +0800 Subject: [PATCH 02/17] test(supervisor): add timeout to multiprocess test subprocess A10-second timeout has been added to the subprocess call within the test_multiprocess.py to ensure it does not hang indefinitely in case of failures or slow systems. --- tests/supervisors/test_multiprocess.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 022243388..2b642282b 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -34,6 +34,7 @@ def new_function(): "-c", f"from {module} import {name}; {name}.__wrapped__()", ], + timeout=10, creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] ) From a65c61ad1b61a0eb6a2e3fc692a20fe1993f9a18 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 14 Aug 2024 17:24:43 +0800 Subject: [PATCH 03/17] try to solve --- tests/supervisors/test_multiprocess.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 2b642282b..93953772d 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -84,12 +84,13 @@ def test_multiprocess_health_check() -> None: config = Config(app=app, workers=2) supervisor = Multiprocess(config, target=run, sockets=[]) threading.Thread(target=supervisor.run, daemon=True).start() - time.sleep(1) + time.sleep(0.5) process = supervisor.processes[0] process.kill() + time.sleep(0.5) try: assert not process.is_alive() - time.sleep(1) + time.sleep(0.5) for p in supervisor.processes: assert p.is_alive() finally: From d832a5a156f73898ec2e887ea96f9cd4e502f6af Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 14 Aug 2024 17:33:16 +0800 Subject: [PATCH 04/17] try to solve --- tests/supervisors/test_multiprocess.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 93953772d..2f2a99540 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -84,13 +84,13 @@ def test_multiprocess_health_check() -> None: config = Config(app=app, workers=2) supervisor = Multiprocess(config, target=run, sockets=[]) threading.Thread(target=supervisor.run, daemon=True).start() - time.sleep(0.5) + time.sleep(1) process = supervisor.processes[0] process.kill() - time.sleep(0.5) + time.sleep(1) try: assert not process.is_alive() - time.sleep(0.5) + time.sleep(1) for p in supervisor.processes: assert p.is_alive() finally: From 27c696879038d7b0cbef86f68c183f6520a97189 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Thu, 15 Aug 2024 14:43:48 +0800 Subject: [PATCH 05/17] feat: add retry for unstable tests --- requirements.txt | 1 + tests/supervisors/test_multiprocess.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/requirements.txt b/requirements.txt index b3aba41c8..761ae30a1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,7 @@ twine==5.1.1 ruff==0.5.0 pytest==8.2.2 pytest-mock==3.14.0 +pytest-rerunfailures==14.0 mypy==1.10.1 types-click==7.1.8 types-pyyaml==6.0.12.20240311 diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 2f2a99540..55421dd54 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -76,6 +76,7 @@ def test_multiprocess_run() -> None: supervisor.join_all() +@pytest.mark.flaky(reruns=3, reruns_delay=2) @new_console_in_windows def test_multiprocess_health_check() -> None: """ @@ -84,13 +85,12 @@ def test_multiprocess_health_check() -> None: config = Config(app=app, workers=2) supervisor = Multiprocess(config, target=run, sockets=[]) threading.Thread(target=supervisor.run, daemon=True).start() - time.sleep(1) + time.sleep(1) # ensure server is up process = supervisor.processes[0] process.kill() - time.sleep(1) try: - assert not process.is_alive() - time.sleep(1) + assert not process.is_alive(1) + time.sleep(1) # release gil, ensure process restart. for p in supervisor.processes: assert p.is_alive() finally: From f0d496ec230964300c5cd75ace01b56b6fcff4aa Mon Sep 17 00:00:00 2001 From: vvanglro Date: Thu, 15 Aug 2024 16:55:40 +0800 Subject: [PATCH 06/17] test(supervisor): remove retry, ensure process close. --- requirements.txt | 1 - tests/supervisors/test_multiprocess.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/requirements.txt b/requirements.txt index 761ae30a1..b3aba41c8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,6 @@ twine==5.1.1 ruff==0.5.0 pytest==8.2.2 pytest-mock==3.14.0 -pytest-rerunfailures==14.0 mypy==1.10.1 types-click==7.1.8 types-pyyaml==6.0.12.20240311 diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 55421dd54..08433594f 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -76,7 +76,6 @@ def test_multiprocess_run() -> None: supervisor.join_all() -@pytest.mark.flaky(reruns=3, reruns_delay=2) @new_console_in_windows def test_multiprocess_health_check() -> None: """ @@ -88,9 +87,10 @@ def test_multiprocess_health_check() -> None: time.sleep(1) # ensure server is up process = supervisor.processes[0] process.kill() + process.join() try: - assert not process.is_alive(1) - time.sleep(1) # release gil, ensure process restart. + assert not process.is_alive(0.5) + time.sleep(1.5) # release gil, ensure process restart complete. for p in supervisor.processes: assert p.is_alive() finally: From ffe43cb02977e5e655ad4739d9b1a32c4c217a00 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Tue, 20 Aug 2024 20:28:56 +0800 Subject: [PATCH 07/17] test case --- tests/supervisors/test_multiprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 08433594f..d4f6a8002 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -34,7 +34,6 @@ def new_function(): "-c", f"from {module} import {name}; {name}.__wrapped__()", ], - timeout=10, creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] ) @@ -84,7 +83,8 @@ def test_multiprocess_health_check() -> None: config = Config(app=app, workers=2) supervisor = Multiprocess(config, target=run, sockets=[]) threading.Thread(target=supervisor.run, daemon=True).start() - time.sleep(1) # ensure server is up + # Ensure server is up. + time.sleep(1) process = supervisor.processes[0] process.kill() process.join() From 0b1b372157ca5a6208a5b4fae535d2a50e79608e Mon Sep 17 00:00:00 2001 From: vvanglro Date: Thu, 21 Nov 2024 16:12:15 +0800 Subject: [PATCH 08/17] test(supervisors): improve test_multiprocess reliability - Replace fixed sleep with retry mechanism for process status checks- Enhance test stability by allowing for variable process startup times --- tests/supervisors/test_multiprocess.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index d4f6a8002..b94ae4049 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -90,9 +90,15 @@ def test_multiprocess_health_check() -> None: process.join() try: assert not process.is_alive(0.5) - time.sleep(1.5) # release gil, ensure process restart complete. - for p in supervisor.processes: - assert p.is_alive() + while retry := 3: + try: + for p in supervisor.processes: + assert p.is_alive() + except AssertionError: + retry -= 1 + time.sleep(0.5) + else: + break finally: supervisor.signal_queue.append(signal.SIGINT) supervisor.join_all() From bd08e0e56739a03fb32d41fb44882b006c8f67fa Mon Sep 17 00:00:00 2001 From: vvanglro Date: Mon, 16 Dec 2024 17:53:47 +0800 Subject: [PATCH 09/17] test(supervisors): improve test_multiprocess reliability --- tests/supervisors/test_multiprocess.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index b94ae4049..955b2fc99 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -90,15 +90,17 @@ def test_multiprocess_health_check() -> None: process.join() try: assert not process.is_alive(0.5) - while retry := 3: + time.sleep(0.5) + start_time = time.time() + while time.time() - start_time < 6: try: for p in supervisor.processes: assert p.is_alive() - except AssertionError: - retry -= 1 - time.sleep(0.5) - else: break + except AssertionError: + time.sleep(1) + else: + raise RuntimeError() finally: supervisor.signal_queue.append(signal.SIGINT) supervisor.join_all() From f95cd0fe235eab1f92427bbe9145ddf966e8b543 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Mon, 16 Dec 2024 18:04:31 +0800 Subject: [PATCH 10/17] test(supervisors): improve test_multiprocess reliability --- tests/supervisors/test_multiprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 955b2fc99..5f3db539d 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -97,9 +97,9 @@ def test_multiprocess_health_check() -> None: for p in supervisor.processes: assert p.is_alive() break - except AssertionError: + except AssertionError: # pragma: no cover time.sleep(1) - else: + else: # pragma: no cover raise RuntimeError() finally: supervisor.signal_queue.append(signal.SIGINT) From 7c48f0498aeb91f05667a1f4168dce9c8cb27091 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Mon, 6 Jan 2025 16:26:53 +0800 Subject: [PATCH 11/17] Release gil to let the supervisor thread switch execution --- tests/supervisors/test_multiprocess.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 5f3db539d..296a76334 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -90,17 +90,10 @@ def test_multiprocess_health_check() -> None: process.join() try: assert not process.is_alive(0.5) - time.sleep(0.5) - start_time = time.time() - while time.time() - start_time < 6: - try: - for p in supervisor.processes: - assert p.is_alive() - break - except AssertionError: # pragma: no cover - time.sleep(1) - else: # pragma: no cover - raise RuntimeError() + time.sleep(0) # release gil. + time.sleep(1) # ensure process restart complete. + for p in supervisor.processes: + assert p.is_alive() finally: supervisor.signal_queue.append(signal.SIGINT) supervisor.join_all() From 418dc8440a8265be141532bbf712696c2195c3bd Mon Sep 17 00:00:00 2001 From: vvanglro Date: Mon, 6 Jan 2025 16:29:01 +0800 Subject: [PATCH 12/17] format --- tests/supervisors/test_multiprocess.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index 296a76334..a10041537 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -90,10 +90,10 @@ def test_multiprocess_health_check() -> None: process.join() try: assert not process.is_alive(0.5) - time.sleep(0) # release gil. - time.sleep(1) # ensure process restart complete. + time.sleep(0) # release gil. + time.sleep(1) # ensure process restart complete. for p in supervisor.processes: - assert p.is_alive() + assert p.is_alive() finally: supervisor.signal_queue.append(signal.SIGINT) supervisor.join_all() From 13c6970deec472e07e90f35322f8f1440c3155de Mon Sep 17 00:00:00 2001 From: vvanglro Date: Thu, 3 Jul 2025 10:55:31 +0800 Subject: [PATCH 13/17] test(supervisors): improve test_multiprocess reliability - Add retry mechanism for unstable test_multiprocess_health_check- Ensure server is running before killing the process - Add assertion to check process is alive before killing it --- tests/supervisors/test_multiprocess.py | 33 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index a10041537..e04049755 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -28,14 +28,26 @@ def new_function(): module = test_function.__module__ name = test_function.__name__ - subprocess.check_call( - [ - sys.executable, - "-c", - f"from {module} import {name}; {name}.__wrapped__()", - ], - creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] - ) + retry_num = 1 + # This test scenario is unstable, so the count is increased separately. + if name == "test_multiprocess_health_check": + retry_num = 3 + last_exception = None + for _ in range(retry_num): + try: + subprocess.check_call( + [ + sys.executable, + "-c", + f"from {module} import {name}; {name}.__wrapped__()", + ], + creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] + ) + return + except subprocess.CalledProcessError as e: + last_exception = e + + raise last_exception # type: ignore[misc] return new_function @@ -83,9 +95,10 @@ def test_multiprocess_health_check() -> None: config = Config(app=app, workers=2) supervisor = Multiprocess(config, target=run, sockets=[]) threading.Thread(target=supervisor.run, daemon=True).start() - # Ensure server is up. - time.sleep(1) + time.sleep(0) # release gil. + time.sleep(1) # ensure server is up. process = supervisor.processes[0] + assert process.is_alive() process.kill() process.join() try: From a447a5a138a0017f94b65af6497347a38eb9c991 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 16 Jul 2025 12:43:45 +0800 Subject: [PATCH 14/17] test(supervisors): implement a generic retry mechanism for tests - Add a `with_retry` decorator in tests/utils.py to handle retries - Apply the decorator to the `test_multiprocess_health_check` function - Remove the ad-hoc retry logic specific to `test_multiprocess_health_check` --- tests/supervisors/test_multiprocess.py | 30 +++++++-------------- tests/utils.py | 36 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/tests/supervisors/test_multiprocess.py b/tests/supervisors/test_multiprocess.py index e04049755..703151c56 100644 --- a/tests/supervisors/test_multiprocess.py +++ b/tests/supervisors/test_multiprocess.py @@ -10,6 +10,7 @@ import pytest +from tests.utils import with_retry from uvicorn import Config from uvicorn._types import ASGIReceiveCallable, ASGISendCallable, Scope from uvicorn.supervisors import Multiprocess @@ -28,26 +29,14 @@ def new_function(): module = test_function.__module__ name = test_function.__name__ - retry_num = 1 - # This test scenario is unstable, so the count is increased separately. - if name == "test_multiprocess_health_check": - retry_num = 3 - last_exception = None - for _ in range(retry_num): - try: - subprocess.check_call( - [ - sys.executable, - "-c", - f"from {module} import {name}; {name}.__wrapped__()", - ], - creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] - ) - return - except subprocess.CalledProcessError as e: - last_exception = e - - raise last_exception # type: ignore[misc] + subprocess.check_call( + [ + sys.executable, + "-c", + f"from {module} import {name}; {name}.__wrapped__()", + ], + creationflags=subprocess.CREATE_NO_WINDOW, # type: ignore[attr-defined] + ) return new_function @@ -88,6 +77,7 @@ def test_multiprocess_run() -> None: @new_console_in_windows +@with_retry(3) def test_multiprocess_health_check() -> None: """ Ensure that the health check works as expected. diff --git a/tests/utils.py b/tests/utils.py index 8145a2bd2..cddb2fc59 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,6 +1,8 @@ from __future__ import annotations import asyncio +import functools +import inspect import os import signal import sys @@ -8,6 +10,7 @@ from contextlib import asynccontextmanager, contextmanager from pathlib import Path from socket import socket +from typing import Any, Callable from uvicorn import Config, Server @@ -53,3 +56,36 @@ def get_asyncio_default_loop_per_os() -> type[asyncio.AbstractEventLoop]: return asyncio.ProactorEventLoop # type: ignore # pragma: nocover else: return asyncio.SelectorEventLoop # pragma: nocover + + +def with_retry(retry_count: int = 1) -> Callable[[Callable[..., Any]], Callable[..., Any]]: + def decorator(func: Callable[..., Any]) -> Callable[..., Any]: + if inspect.iscoroutinefunction(func): + + @functools.wraps(func) + async def async_wrapper(*args, **kwargs): + for attempt in range(retry_count): + try: + return await func(*args, **kwargs) + except Exception as e: + if attempt == retry_count - 1: + raise e + return None + + return async_wrapper + + else: + # Maintain the original calling method of the test case, e.g. test_multiprocess_health_check. + @functools.wraps(func) + def sync_wrapper(*args, **kwargs): + for attempt in range(retry_count): + try: + return func(*args, **kwargs) + except Exception as e: + if attempt == retry_count - 1: + raise e + return None + + return sync_wrapper + + return decorator From 68a59edd816d08850020b8dc37ef6dd10776963f Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 16 Jul 2025 12:47:48 +0800 Subject: [PATCH 15/17] test: add pragma nocover to retry decorator - Add pragma nocover comment to async_wrapper and sync_wrapper functions- This change excludes the decorator logic from code coverage metrics --- tests/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index cddb2fc59..5760f2f14 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -62,7 +62,7 @@ def with_retry(retry_count: int = 1) -> Callable[[Callable[..., Any]], Callable[ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: if inspect.iscoroutinefunction(func): - @functools.wraps(func) + @functools.wraps(func) # pragma: nocover async def async_wrapper(*args, **kwargs): for attempt in range(retry_count): try: @@ -76,7 +76,7 @@ async def async_wrapper(*args, **kwargs): else: # Maintain the original calling method of the test case, e.g. test_multiprocess_health_check. - @functools.wraps(func) + @functools.wraps(func) # pragma: nocover def sync_wrapper(*args, **kwargs): for attempt in range(retry_count): try: From 66fef01df6fd61027cb66de0c9b8cb46aed3de10 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 16 Jul 2025 12:49:00 +0800 Subject: [PATCH 16/17] lint --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 5760f2f14..19c3634eb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -62,7 +62,7 @@ def with_retry(retry_count: int = 1) -> Callable[[Callable[..., Any]], Callable[ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: if inspect.iscoroutinefunction(func): - @functools.wraps(func) # pragma: nocover + @functools.wraps(func) # pragma: nocover async def async_wrapper(*args, **kwargs): for attempt in range(retry_count): try: From e470d85303d5901f487545f5d570c9dd0d4ba3f0 Mon Sep 17 00:00:00 2001 From: vvanglro Date: Wed, 16 Jul 2025 12:51:20 +0800 Subject: [PATCH 17/17] test: add pragma nocover to async_wrapper return statement - Add pragma nocover comment to exclude async_wrapper return statement from coverage - This change helps to avoid unnecessary test coverage metrics --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 19c3634eb..a1cf59042 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -72,7 +72,7 @@ async def async_wrapper(*args, **kwargs): raise e return None - return async_wrapper + return async_wrapper # pragma: nocover else: # Maintain the original calling method of the test case, e.g. test_multiprocess_health_check.