Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import time
import warnings
from collections.abc import Callable, Iterable, Sequence
from contextlib import ExitStack, contextmanager
from contextlib import ExitStack, contextmanager, suppress
from multiprocessing import Process
from pathlib import Path
from typing import Any, Literal
Expand Down Expand Up @@ -1511,13 +1511,18 @@
return wrapper


<<<<<<< HEAD

Check failure on line 1514 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1514:7: invalid-syntax: Expected a statement

Check failure on line 1514 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1514:5: invalid-syntax: Expected a statement

Check failure on line 1514 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1514:3: invalid-syntax: Expected a statement

Check failure on line 1514 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1514:1: invalid-syntax: Expected a statement
def spawn_new_process_for_each_test(f: Callable[_P, None]) -> Callable[_P, None]:
"""Decorator to spawn a new process for each test function.

Uses subprocess with cloudpickle to serialize the test function and
propagates exceptions back to the parent, so test failures are never
silently swallowed (fixes https://github.com/vllm-project/vllm/issues/41415).
"""
=======

Check failure on line 1522 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1522:7: invalid-syntax: Expected a statement

Check failure on line 1522 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1522:5: invalid-syntax: Expected a statement

Check failure on line 1522 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1522:3: invalid-syntax: Expected a statement

Check failure on line 1522 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1522:1: invalid-syntax: Expected a statement

Check failure on line 1523 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1522:8: invalid-syntax: Expected a statement
"""Decorator to spawn a new process for each test function."""

Check failure on line 1524 in tests/utils.py

View workflow job for this annotation

GitHub Actions / pre-commit

Ruff (invalid-syntax)

tests/utils.py:1524:1: invalid-syntax: Unexpected indentation
>>>>>>> 2c4dc247b ([Bugfix] Fix spawn_new_process_for_each_test silently swallowing failures)
Comment thread
dzhengAP marked this conversation as resolved.
Outdated

@functools.wraps(f)
def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> None:
Expand Down Expand Up @@ -1574,6 +1579,64 @@

return wrapper

def spawn_new_process_for_each_test(f: Callable[_P, None]) -> Callable[_P, None]:
"""Decorator to spawn a new process for each test function.

Uses subprocess with cloudpickle to serialize the test function and
propagates exceptions back to the parent, so test failures are never
silently swallowed (fixes https://github.com/vllm-project/vllm/issues/41415).
"""

@functools.wraps(f)
def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> None:
with tempfile.NamedTemporaryFile(
delete=False, suffix=".tb", mode="wb"
) as tmp:
tb_file = tmp.name

try:
# Serialize the function + args with cloudpickle so closures work
payload = cloudpickle.dumps((f, args, kwargs, tb_file))

child_script = (
"import sys, cloudpickle, traceback\n"
"f, args, kwargs, tb_file = cloudpickle.loads(sys.stdin.buffer.read())\n"
"try:\n"
" f(*args, **kwargs)\n"
"except BaseException:\n"
" open(tb_file, 'w').write(traceback.format_exc())\n"
" sys.exit(1)\n"
)

repo_root = str(VLLM_PATH.resolve())
env = os.environ.copy()
env["PYTHONPATH"] = repo_root + os.pathsep + env.get("PYTHONPATH", "")

result = subprocess.run(
[sys.executable, "-c", child_script],
input=payload,
capture_output=True,
env=env,
)

if result.returncode != 0:
# Read traceback written by child, fall back to stderr
tb = ""
if os.path.exists(tb_file) and os.path.getsize(tb_file) > 0:
with open(tb_file) as fp:
tb = fp.read()
else:
tb = result.stderr.decode()
raise RuntimeError(
f"Test subprocess '{f.__name__}' failed "
f"(exit code {result.returncode}):\n{tb}"
)
finally:
with contextlib.suppress(OSError):
os.remove(tb_file)

return wrapper


def create_new_process_for_each_test(
method: Literal["spawn", "fork"] | None = None,
Expand Down
4 changes: 2 additions & 2 deletions tests/v1/logits_processors/test_custom_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pytest
import pytest_asyncio

from tests.utils import RemoteOpenAIServerCustom, create_new_process_for_each_test
from tests.utils import RemoteOpenAIServerCustom
from tests.v1.logits_processors.utils import (
DUMMY_LOGITPROC_ARG,
DUMMY_LOGITPROC_FQCN,
Expand Down Expand Up @@ -119,7 +119,7 @@ async def client(server):
}


@create_new_process_for_each_test()
@pytest.mark.asyncio
@pytest.mark.parametrize(
"model_name",
[MODEL_NAME],
Expand Down
Loading