From 29d69332aa658a96698a456e044763321f4bbd82 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sun, 31 May 2026 22:06:33 -0700 Subject: [PATCH] [BugFix] Fix `_has_module` to verify native deps via trial import (#44035) Signed-off-by: esmeetu Signed-off-by: Jeffrey Wang Signed-off-by: Nick Hill Co-authored-by: esmeetu Co-authored-by: Nick Hill --- tests/utils_/test_import_utils.py | 95 ++++++++++++++++++++++++++++++- vllm/utils/import_utils.py | 20 +++++-- 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/tests/utils_/test_import_utils.py b/tests/utils_/test_import_utils.py index d42685b3fc9..d1f822037ac 100644 --- a/tests/utils_/test_import_utils.py +++ b/tests/utils_/test_import_utils.py @@ -1,8 +1,10 @@ # SPDX-License-Identifier: Apache-2.0 # SPDX-FileCopyrightText: Copyright contributors to the vLLM project +from unittest.mock import MagicMock, patch + import pytest -from vllm.utils.import_utils import PlaceholderModule +from vllm.utils.import_utils import PlaceholderModule, _has_module def _raises_module_not_found(): @@ -44,3 +46,94 @@ def test_placeholder_module_error_handling(): with _raises_module_not_found(): # Test conflict with internal __module attribute _ = placeholder_attr.module + + +class TestHasModule: + """Tests for _has_module with trial import verification.""" + + def setup_method(self): + # Clear the @cache between tests so each test gets a fresh call + _has_module.cache_clear() + + def test_returns_true_for_importable_stdlib_module(self): + assert _has_module("json") is True + + def test_returns_false_for_nonexistent_module(self): + assert _has_module("nonexistent_module_xyz_12345") is False + + def test_returns_false_when_find_spec_succeeds_but_import_fails(self): + """Simulate a native extension whose shared library is missing. + + ``find_spec`` finds the package on disk, but the actual import + raises ``ImportError`` (e.g. missing ``libcudart.so``). + """ + fake_spec = MagicMock() + + with ( + patch( + "vllm.utils.import_utils.importlib.util.find_spec", + return_value=fake_spec, + ), + patch( + "vllm.utils.import_utils.importlib.import_module", + side_effect=ImportError( + "libcudart.so.12: cannot open shared object file" + ), + ), + ): + assert _has_module("fake_native_ext") is False + + def test_returns_false_on_os_error_during_import(self): + """Some shared-library failures surface as ``OSError``.""" + fake_spec = MagicMock() + + with ( + patch( + "vllm.utils.import_utils.importlib.util.find_spec", + return_value=fake_spec, + ), + patch( + "vllm.utils.import_utils.importlib.import_module", + side_effect=OSError("cannot load library"), + ), + ): + assert _has_module("fake_native_ext_os") is False + + def test_returns_false_on_unexpected_error_during_import(self): + """A broken extension may raise a non-import error (e.g. ``RuntimeError``). + + Such modules are not usable, so ``_has_module`` should still return + ``False`` rather than letting the exception propagate. + """ + fake_spec = MagicMock() + + with ( + patch( + "vllm.utils.import_utils.importlib.util.find_spec", + return_value=fake_spec, + ), + patch( + "vllm.utils.import_utils.importlib.import_module", + side_effect=RuntimeError("CUDA driver version is insufficient"), + ), + ): + assert _has_module("fake_broken_ext") is False + + def test_returns_false_when_find_spec_raises(self): + """``find_spec`` itself can raise for dotted names whose parent package + fails to import. This should be treated as the module being unavailable. + """ + with patch( + "vllm.utils.import_utils.importlib.util.find_spec", + side_effect=ModuleNotFoundError("No module named 'fake_parent'"), + ): + assert _has_module("fake_parent.child") is False + + def test_result_is_cached(self): + """Verify the @cache decorator prevents repeated imports.""" + _has_module("json") # prime the cache + + with patch("vllm.utils.import_utils.importlib.util.find_spec") as mock_spec: + result = _has_module("json") # should hit cache + mock_spec.assert_not_called() + assert result is True diff --git a/vllm/utils/import_utils.py b/vllm/utils/import_utils.py index e97228bfa60..e008e17d806 100644 --- a/vllm/utils/import_utils.py +++ b/vllm/utils/import_utils.py @@ -392,12 +392,24 @@ class LazyLoader(ModuleType): # Optional dependency detection utilities @cache def _has_module(module_name: str) -> bool: - """Return True if *module_name* can be found in the current environment. + """Return True if *module_name* can be imported in the current environment. - The result is cached so that subsequent queries for the same module incur - no additional overhead. + Uses ``importlib.util.find_spec`` as a fast pre-check, then performs a + trial import to verify that native dependencies (shared libraries, etc.) + are also satisfied. Any failure during the trial import is treated as the + module being unavailable. The result is cached so that subsequent queries + for the same module incur no additional overhead. """ - return importlib.util.find_spec(module_name) is not None + try: + if importlib.util.find_spec(module_name) is None: + return False + importlib.import_module(module_name) + except ImportError: + logger.warning( + "Module %s was found but failed to import", module_name, exc_info=True + ) + return False + return True def has_deep_ep() -> bool: