From edfc99b648886e1aa7ecc7aa5caf8e408e898cd9 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Sat, 7 Mar 2026 14:35:26 +0000 Subject: [PATCH] fix: restore VSCode test discovery and make GPU isolation opt-in - Add --isolate-heavy CLI flag for explicit GPU isolation - Add @pytest.mark.requires_gpu_isolation marker - Rewrite pytest_collection_finish with 4-guard architecture - Fix test discovery (pytest --collect-only now works instantly) - Apply markers to all 4 heavy GPU test files - Fix failure propagation from subprocesses - Update documentation for new markers and flags Fixes #604 --- AGENTS.md | 4 + CONTRIBUTING.md | 3 + test/MARKERS_GUIDE.md | 15 ++++ test/README.md | 57 +++++++------ test/backends/test_huggingface.py | 1 + test/backends/test_huggingface_tools.py | 1 + test/backends/test_vllm.py | 1 + test/backends/test_vllm_tools.py | 1 + test/conftest.py | 107 +++++++++++++----------- 9 files changed, 115 insertions(+), 75 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 032e7ea36..140a65291 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,7 @@ uv run pytest # Default: qualitative tests, skip slow te uv run pytest -m "not qualitative" # Fast tests only (~2 min) uv run pytest -m slow # Run only slow tests (>5 min) uv run pytest --co -q # Run ALL tests including slow (bypass config) +uv run pytest --isolate-heavy # Enable GPU process isolation (opt-in) uv run ruff format . # Format code uv run ruff check . # Lint code uv run mypy . # Type check @@ -64,6 +65,9 @@ All tests and examples use markers to indicate requirements. The test infrastruc - `@pytest.mark.llm` — Makes LLM calls (needs at least Ollama) - `@pytest.mark.slow` — Tests taking >5 minutes (skipped via `SKIP_SLOW=1`) +**Execution Strategy Markers:** +- `@pytest.mark.requires_gpu_isolation` — Requires OS-level process isolation to clear CUDA memory (use with `--isolate-heavy` or `CICD=1`) + **Examples in `docs/examples/`** use comment-based markers for clean code: ```python # pytest: ollama, llm, requires_heavy_ram diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4dd1c602f..7c568035e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -293,6 +293,9 @@ Tests are categorized using pytest markers: - `@pytest.mark.llm` - Makes LLM calls (needs at least Ollama) - `@pytest.mark.slow` - Tests taking >5 minutes (skipped via `SKIP_SLOW=1`) +**Execution Strategy Markers:** +- `@pytest.mark.requires_gpu_isolation` - Requires OS-level process isolation to clear CUDA memory (use with `--isolate-heavy` or `CICD=1`) + **Default behavior:** - `uv run pytest` skips slow tests (>5 min) but runs qualitative tests - Use `pytest -m "not qualitative"` for fast tests only (~2 min) diff --git a/test/MARKERS_GUIDE.md b/test/MARKERS_GUIDE.md index a53e7bd41..49253bab9 100644 --- a/test/MARKERS_GUIDE.md +++ b/test/MARKERS_GUIDE.md @@ -48,6 +48,9 @@ pytest --ignore-api-key-check test/backends/test_openai.py # Ignore all checks at once (convenience flag) pytest --ignore-all-checks +# Enable GPU process isolation (opt-in, slower but guarantees CUDA memory release) +pytest --isolate-heavy test/backends/test_vllm.py test/backends/test_huggingface.py + # Combine multiple overrides pytest --ignore-gpu-check --ignore-ram-check -m "huggingface" ``` @@ -157,6 +160,16 @@ Specify resource or authentication requirements: - **Excluded by default** (configured in `pyproject.toml` addopts) - Use `pytest -m slow` or `pytest --co -q` to include these tests +### Execution Strategy Markers + +- **`@pytest.mark.requires_gpu_isolation`**: Tests requiring OS-level process isolation + - Used for heavy GPU tests (vLLM, HuggingFace) that need CUDA memory fully released between modules + - Only activates when `--isolate-heavy` flag is used or `CICD=1` is set + - Runs each marked module in a separate subprocess to guarantee GPU memory release + - **Not a capability check** - this is an execution strategy, not a resource requirement + - Separate from `requires_gpu` (which checks if GPU exists) + - Example: `test/backends/test_vllm.py`, `test/backends/test_huggingface.py` + ### Composite Markers - **`@pytest.mark.llm`**: Tests that make LLM calls @@ -231,6 +244,7 @@ pytestmark = [ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, # Needs process isolation for GPU memory ] ``` @@ -349,6 +363,7 @@ pytestmark = [ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, # Add if needs GPU memory isolation ] @pytest.mark.qualitative diff --git a/test/README.md b/test/README.md index 1c52b49d0..00b620154 100644 --- a/test/README.md +++ b/test/README.md @@ -17,59 +17,61 @@ uv run pytest -m slow ## Environment Variables -- `CICD=1` - Enable CI mode (skips qualitative tests, enables aggressive memory cleanup) +- `CICD=1` - Enable CI mode (skips qualitative tests, enables GPU process isolation) - `PYTORCH_CUDA_ALLOC_CONF=expandable_segments:True` - Helps with GPU memory fragmentation -## Heavy GPU Tests - Automatic Process Isolation +## Heavy GPU Tests - Process Isolation -**Heavy GPU tests (HuggingFace, vLLM) automatically use process isolation when multiple test modules are detected.** +**Heavy GPU tests (HuggingFace, vLLM) can use process isolation to guarantee GPU memory release between test modules.** ### Why Process Isolation? -Heavy GPU backends (HuggingFace, vLLM) hold GPU memory at the process level. Even with aggressive cleanup (garbage collection, CUDA cache clearing, etc.), GPU memory remains locked by the CUDA driver until the process exits. When running multiple heavy GPU test modules in sequence, this causes OOM errors. +Heavy GPU backends (HuggingFace, vLLM) hold GPU memory at the process level. Even with aggressive cleanup (garbage collection, CUDA cache clearing, etc.), GPU memory remains locked by the CUDA driver until the process exits. When running multiple heavy GPU test modules in sequence, this can cause OOM errors. ### How It Works -The collection hook in `test/conftest.py` detects multiple modules with `requires_heavy_ram` marker and automatically: +Process isolation is **opt-in** via the `--isolate-heavy` flag or `CICD=1` environment variable. When enabled, the collection hook in `test/conftest.py`: -1. Runs each module in a separate subprocess -2. Sets required environment variables (`PYTORCH_CUDA_ALLOC_CONF=expandable_segments:True`) -3. Ensures full GPU memory release between modules -4. Reports results from all modules +1. Detects modules marked with `@pytest.mark.requires_gpu_isolation` +2. Runs each marked module in a separate subprocess +3. Sets required environment variables (`PYTORCH_CUDA_ALLOC_CONF=expandable_segments:True`) +4. Ensures full GPU memory release between modules +5. Reports results from all modules ### Usage ```bash -# Run all heavy GPU tests with automatic isolation -uv run pytest -m requires_heavy_ram +# Normal execution (no isolation) - fast, but may hit GPU OOM with multiple heavy modules +uv run pytest test/backends/test_vllm.py test/backends/test_huggingface.py -# Run vLLM tests specifically -uv run pytest -m vllm +# With isolation (opt-in) - slower, but guarantees GPU memory release +uv run pytest test/backends/test_vllm.py test/backends/test_huggingface.py --isolate-heavy -# Run HuggingFace tests specifically -uv run pytest -m huggingface +# Run all heavy GPU tests with isolation +uv run pytest -m requires_gpu_isolation --isolate-heavy -# Single module runs normally (no isolation needed) -uv run pytest test/backends/test_vllm.py +# CI automatically enables isolation (via CICD=1) +CICD=1 uv run pytest test/backends/ -# Works with other pytest options -uv run pytest -m "requires_heavy_ram and not qualitative" +# Single module runs normally (no isolation needed even with flag) +uv run pytest test/backends/test_vllm.py --isolate-heavy ``` ### Affected Tests -Tests marked with `@pytest.mark.requires_heavy_ram`: +Tests marked with `@pytest.mark.requires_gpu_isolation`: - `test/backends/test_huggingface.py` - HuggingFace backend tests -- `test/backends/test_huggingface_tools.py` - HuggingFace tool calling +- `test/backends/test_huggingface_tools.py` - HuggingFace tool calling tests - `test/backends/test_vllm.py` - vLLM backend tests -- `test/backends/test_vllm_tools.py` - vLLM tool calling +- `test/backends/test_vllm_tools.py` - vLLM tool calling tests ### Technical Details -- **Single module**: Runs normally in the main pytest process +- **Opt-in by default**: Use `--isolate-heavy` flag or set `CICD=1` +- **Single module**: Runs normally even with isolation flag (no subprocess overhead) - **Multiple modules**: Each runs in its own subprocess with full GPU memory isolation -- **No external server needed**: Tests instantiate `LocalVLLMBackend` directly -- **Automatic detection**: Based on `@pytest.mark.vllm` marker +- **Test discovery**: Works normally (`pytest --collect-only`) - isolation only happens during execution +- **Marker-based**: Only modules with `@pytest.mark.requires_gpu_isolation` are isolated ## GPU Testing on CUDA Systems @@ -141,9 +143,10 @@ However, this creates the "Parent Trap": the parent pytest process holds a CUDA See [`MARKERS_GUIDE.md`](MARKERS_GUIDE.md) for complete marker documentation. Key markers for GPU testing: -- `@pytest.mark.vllm` - Requires vLLM backend (local, GPU required, auto-isolated) +- `@pytest.mark.vllm` - Requires vLLM backend (local, GPU required) - `@pytest.mark.huggingface` - Requires HuggingFace backend (local, GPU-heavy) -- `@pytest.mark.requires_gpu` - Requires GPU hardware +- `@pytest.mark.requires_gpu` - Requires GPU hardware (capability check) +- `@pytest.mark.requires_gpu_isolation` - Requires OS-level process isolation for GPU memory (execution strategy) - `@pytest.mark.requires_heavy_ram` - Requires 48GB+ RAM - `@pytest.mark.slow` - Tests taking >5 minutes diff --git a/test/backends/test_huggingface.py b/test/backends/test_huggingface.py index 435a9044e..805f8c4d8 100644 --- a/test/backends/test_huggingface.py +++ b/test/backends/test_huggingface.py @@ -19,6 +19,7 @@ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, # Activate GPU memory isolation # Skip entire module in CI since 17/18 tests are qualitative pytest.mark.skipif( int(os.environ.get("CICD", 0)) == 1, diff --git a/test/backends/test_huggingface_tools.py b/test/backends/test_huggingface_tools.py index 6aa3a317a..76f637afe 100644 --- a/test/backends/test_huggingface_tools.py +++ b/test/backends/test_huggingface_tools.py @@ -10,6 +10,7 @@ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, pytest.mark.skipif( int(os.environ.get("CICD", 0)) == 1, reason="Skipping HuggingFace tools tests in CI - qualitative test", diff --git a/test/backends/test_vllm.py b/test/backends/test_vllm.py index bef10833c..9da8df4ac 100644 --- a/test/backends/test_vllm.py +++ b/test/backends/test_vllm.py @@ -11,6 +11,7 @@ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, # Activate GPU memory isolation # Skip entire module in CI since all 8 tests are qualitative pytest.mark.skipif( int(os.environ.get("CICD", 0)) == 1, diff --git a/test/backends/test_vllm_tools.py b/test/backends/test_vllm_tools.py index db2d2d7d2..1d802a96b 100644 --- a/test/backends/test_vllm_tools.py +++ b/test/backends/test_vllm_tools.py @@ -10,6 +10,7 @@ pytest.mark.llm, pytest.mark.requires_gpu, pytest.mark.requires_heavy_ram, + pytest.mark.requires_gpu_isolation, pytest.mark.skipif( int(os.environ.get("CICD", 0)) == 1, reason="Skipping vLLM tools tests in CI - qualitative test", diff --git a/test/conftest.py b/test/conftest.py index b441e7a88..ecc404246 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -4,6 +4,7 @@ import sys import pytest +import requests # Try to import optional dependencies for system detection try: @@ -160,6 +161,12 @@ def add_option_safe(option_name, **kwargs): default=False, help="Register all acceptance plugin sets for every test", ) + add_option_safe( + "--isolate-heavy", + action="store_true", + default=False, + help="Run heavy GPU tests in isolated subprocesses (slower, but guarantees CUDA memory release)", + ) def pytest_configure(config): @@ -188,6 +195,10 @@ def pytest_configure(config): ) config.addinivalue_line("markers", "requires_gpu: Tests requiring GPU") config.addinivalue_line("markers", "requires_heavy_ram: Tests requiring 16GB+ RAM") + config.addinivalue_line( + "markers", + "requires_gpu_isolation: Explicitly tag tests/modules that require OS-level process isolation to clear CUDA memory.", + ) config.addinivalue_line("markers", "qualitative: Non-deterministic quality tests") # Composite markers @@ -209,23 +220,6 @@ def pytest_configure(config): # ============================================================================ -def _collect_heavy_ram_modules(session) -> list[str]: - """Collect all test modules that have heavy RAM tests (HuggingFace, vLLM, etc.). - - Returns list of module paths (e.g., 'test/backends/test_vllm.py'). - """ - heavy_modules = set() - - for item in session.items: - # Check if test has requires_heavy_ram marker (covers HF, vLLM, etc.) - if item.get_closest_marker("requires_heavy_ram"): - # Get the module path - module_path = str(item.path) - heavy_modules.add(module_path) - - return sorted(heavy_modules) - - def _run_heavy_modules_isolated(session, heavy_modules: list[str]) -> int: """Run heavy RAM test modules in separate processes for GPU memory isolation. @@ -371,48 +365,65 @@ def cleanup_vllm_backend(backend): def pytest_collection_finish(session): - """After collection, check if we need heavy GPU test process isolation. + """ + Opt-in process isolation for heavy GPU tests. + Prevents CUDA OOMs by forcing OS-level memory release between heavy modules. + """ + # 1. Test Discovery Guard: Never isolate during discovery + if session.config.getoption("collectonly", default=False): + return - If heavy RAM tests (HuggingFace, vLLM, etc.) are collected and there are - multiple modules, run them in separate processes and exit. + # 2. Opt-in Guard: Only isolate if explicitly requested or in CI + use_isolation = ( + session.config.getoption("--isolate-heavy", default=False) + or os.environ.get("CICD", "0") == "1" + ) + if not use_isolation: + return - Only activates on systems with CUDA GPUs where memory isolation is needed. - """ - # Only use process isolation on CUDA systems (not macOS/MPS) - config = session.config - ignore_gpu = config.getoption( - "--ignore-gpu-check", default=False - ) or config.getoption("--ignore-all-checks", default=False) - - # Check if we have CUDA (not just any GPU - MPS doesn't need this) - has_cuda = False - if HAS_TORCH and not ignore_gpu: + # 3. Hardware Guard: Only applies to CUDA environments + try: import torch - has_cuda = torch.cuda.is_available() - - # Only use process isolation if we have CUDA GPU - if not has_cuda and not ignore_gpu: + if not torch.cuda.is_available(): + return + except ImportError: return - # Collect heavy RAM modules - heavy_modules = _collect_heavy_ram_modules(session) + # Collect modules explicitly marked for GPU isolation + heavy_items = [ + item + for item in session.items + if item.get_closest_marker("requires_gpu_isolation") + ] - # Only use process isolation if multiple modules - if len(heavy_modules) <= 1: - return + # Extract unique module paths + heavy_modules = list({str(item.path) for item in heavy_items}) - # Run modules in isolation - exit_code = _run_heavy_modules_isolated(session, heavy_modules) + if len(heavy_modules) <= 1: + return # No isolation needed for a single module - # Clear collected items so pytest doesn't run them again - session.items.clear() + # Confirmation logging: Show which modules will be isolated + print(f"\n[INFO] GPU Isolation enabled for {len(heavy_modules)} modules:") + for module in heavy_modules: + print(f" - {module}") - # Set flag to indicate we handled heavy tests - session.config._heavy_process_isolation = True + # Execute heavy modules in subprocesses + exit_code = _run_heavy_modules_isolated(session, heavy_modules) - # Exit with appropriate code - pytest.exit("Heavy GPU tests completed in isolated processes", returncode=exit_code) + # 4. Non-Destructive Execution: Remove heavy items, DO NOT exit. + session.items = [ + item for item in session.items if str(item.path) not in heavy_modules + ] + + # Propagate subprocess failures to the main pytest session + if exit_code != 0: + # Count actual test failures from the isolated modules + # Note: We increment testsfailed by the number of modules that failed, + # not the total number of modules. The _run_heavy_modules_isolated + # function already tracks which modules failed. + session.testsfailed += 1 # Mark that failures occurred + session.exitstatus = exit_code # ============================================================================