cuda.core: fix a few test issues (#2714)

Address PR #2714 review findings 1-8 and sharpen the test guidance:

* Drop GLException from the GL-unavailable set; restrict the skip catch to context creation so GL allocation errors propagate.
* Recognize Linux GL/EGL ImportError and Windows opengl32 FileNotFoundError/AttributeError as genuine GL-unavailable cases.
* Consolidate the GL availability predicate into cuda_python_test_helpers/graphics.py with focused tests.
* Fix supports_ipc_mempool to inspect the raw CUresult; only CUDA_ERROR_NOT_SUPPORTED means unsupported.
* Fix nvfatbin and NVRTC probes to catch loader/symbol failures (DynamicLibNotFoundError/FunctionNotFoundError), let genuine API errors propagate (2, 3).
* Clean up partial GL resources on setup failure.
* Share the conditional-handle skip helper between helpers/graph_kernels.py and the integration tests.
* Move is_gl_context_unavailable tests to cuda_core/tests/test_helpers.py with provenance markers so CI runs them.
* Update AGENTS.md with the corrected examples and new guidance.

Author: juenglin

cuda_bindings/tests/test_graphics_apis.py

@@ -9,73 +9,95 @@
 
 import pyglet
 import pytest
+from cuda_python_test_helpers.graphics import is_gl_context_unavailable
 
 from cuda.bindings import runtime as cudart
 
 
-def _configure_pyglet_headless(pyglet):
+def _configure_pyglet_headless():
     """On headless Linux: enable EGL mode or skip if EGL is absent."""
     if sys.platform.startswith("linux") and not (os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY")):
         if ctypes.util.find_library("EGL") is None:
             pytest.skip("No DISPLAY and no EGL runtime available for headless context.")
         pyglet.options["headless"] = True
 
 
-def _setup_gl_texture(pyglet):
-    """Open a GL context and allocate a 2-D RGBA8 texture. Returns (win, tex_id, target)."""
+def _open_gl_window():
+    """Open a hidden window (or configure EGL headless). Returns the window or None.
+
+    Closes the window if switch_to() fails so a partially-constructed window does not leak.
+    """
     if not pyglet.options.get("headless"):
         # Hidden window path (WGL on Windows, GLX/WLS on Linux)
         from pyglet import gl
 
         config = gl.Config(double_buffer=False)
         win = pyglet.window.Window(visible=False, config=config)
-        win.switch_to()
+        try:
+            win.switch_to()
+        except Exception:
+            with contextlib.suppress(Exception):
+                win.close()
+            raise
+        return win
     else:
         # Headless EGL path; pyglet will arrange a pbuffer-like headless context
         from pyglet.gl import headless  # noqa: F401
 
-        win = None
+        return None
+
+
+def _allocate_gl_texture(win):
+    """Allocate a 2-D RGBA8 texture. Caller must have a current GL context.
 
-    # Make a tiny texture so we have a real GL object to register
+    Deletes the generated texture if a later GL call fails, so a partial
+    resource does not leak.
+    """
     from pyglet.gl import gl as _gl
 
     tex_id = _gl.GLuint(0)
-    _gl.glGenTextures(1, ctypes.byref(tex_id))
-    target = _gl.GL_TEXTURE_2D
-    _gl.glBindTexture(target, tex_id.value)
-    _gl.glTexParameteri(target, _gl.GL_TEXTURE_MIN_FILTER, _gl.GL_NEAREST)
-    _gl.glTexParameteri(target, _gl.GL_TEXTURE_MAG_FILTER, _gl.GL_NEAREST)
-    width, height = 16, 16
-    _gl.glTexImage2D(target, 0, _gl.GL_RGBA8, width, height, 0, _gl.GL_RGBA, _gl.GL_UNSIGNED_BYTE, None)
-    return win, tex_id, target
+    try:
+        _gl.glGenTextures(1, ctypes.byref(tex_id))
+        target = _gl.GL_TEXTURE_2D
+        _gl.glBindTexture(target, tex_id.value)
+        _gl.glTexParameteri(target, _gl.GL_TEXTURE_MIN_FILTER, _gl.GL_NEAREST)
+        _gl.glTexParameteri(target, _gl.GL_TEXTURE_MAG_FILTER, _gl.GL_NEAREST)
+        width, height = 16, 16
+        _gl.glTexImage2D(target, 0, _gl.GL_RGBA8, width, height, 0, _gl.GL_RGBA, _gl.GL_UNSIGNED_BYTE, None)
+        return tex_id, target
+    except Exception:
+        if tex_id.value:
+            with contextlib.suppress(Exception):
+                _gl.glDeleteTextures(1, ctypes.byref(tex_id))
+        raise
 
 
 @contextlib.contextmanager
 def _gl_context():
     """Yield ``(tex_id, tex_target)`` with a current GL context, or skip if GL is unavailable."""
-    _configure_pyglet_headless(pyglet)
+    _configure_pyglet_headless()
 
     try:
-        win, tex_id, target = _setup_gl_texture(pyglet)
+        win = _open_gl_window()
     except Exception as e:
-        pytest.skip(f"Could not create GL context/texture: {type(e).__name__}: {e}")
+        if is_gl_context_unavailable(e):
+            pytest.skip(f"Could not create GL context: {type(e).__name__}: {e}")
+        raise
 
+    tex_id = None
     try:
+        tex_id, target = _allocate_gl_texture(win)
         yield int(tex_id.value), int(target)
     finally:
-        # Best-effort cleanup
-        try:
-            from pyglet.gl import gl as _gl
+        if tex_id is not None:
+            with contextlib.suppress(Exception):
+                from pyglet.gl import gl as _gl
 
-            if tex_id.value:
-                _gl.glDeleteTextures(1, ctypes.byref(tex_id))
-        except Exception:  # noqa: S110
-            pass
-        try:
+                if tex_id.value:
+                    _gl.glDeleteTextures(1, ctypes.byref(tex_id))
+        with contextlib.suppress(Exception):
             if win is not None:
                 win.close()
-        except Exception:  # noqa: S110
-            pass
 
 
 @pytest.mark.parametrize(

cuda_core/tests/AGENTS.md

@@ -83,10 +83,166 @@ Follow these rules when adding or moving shared test code:
   `tests/helpers/` instead.
 - Import helpers explicitly from the test root, for example:
   `from helpers.memory import create_managed_memory_resource_or_skip`.
+- Search `tests/helpers/` and `cuda_python_test_helpers/` for prior art
+  before adding a new helper; consolidate duplicates across
+
+  packages into `cuda_python_test_helpers` (both `cuda_core` and
+  `cuda_bindings` test environments already depend on it).
 - Fixtures in a nested `conftest.py` are available to tests in its directory
   and descendants; fixtures from applicable parent `conftest.py` files remain
   available.
 - Do not add `__init__.py` solely because a test directory contains a
   `conftest.py`.
 - In directories without `__init__.py`, keep test-module basenames unique
   within this test suite.
+
+## Skip only real setup failures
+
+`pytest.skip(reason)` records the test as SKIPPED with `reason` in the
+report. A helper that wraps `yield` in `except Exception: pytest.skip(...)`
+therefore records every test-body failure as a skip — a real regression, a
+`TypeError`, an `AttributeError` all become "SKIPPED: <reason>" instead of
+"FAILED", and the suite goes green regardless of whether the code under
+test works.
+
+Catch only the specific exception that legitimately means "not available",
+and only around the setup call — never around `yield`:
+
+```python
+@contextlib.contextmanager
+def _gl_context():
+    try:
+        win, tex_id = _setup_gl_texture()      # setup only
+    except (pyglet.NoSuchConfigException, GLContextError) as e:
+        pytest.skip(f"GL unavailable: {e}")
+    try:
+        yield tex_id                          # body exceptions propagate
+    finally:
+        _cleanup(win, tex_id)
+```
+
+The exception names in the example are illustrative — `GLContextError`
+is not a real pyglet class. Match real pyglet exception names by type, or
+use the shared `is_gl_context_unavailable` helper in
+`cuda_python_test_helpers.graphics`.
+
+`GLException` is pyglet's generic GL-error class, raised after any GL call that reports an error
+(`GL_INVALID_ENUM`, etc.). Do **not** include it in the "GL unavailable" set — it hides real bugs in GL allocation code as skips.
+
+Platform-specific "library not loadable" manifestations (genuine "GL unavailable"):
+
+- Linux without libGL/libEGL: `ImportError('Library "GL" not found.')` / `ImportError('Library "EGL" not found.')` from `pyglet/lib.py`.
+- Windows without opengl32.dll: `FileNotFoundError` from `ctypes.windll.opengl32`; on Python 3.12+ `ctypes.LibraryLoader` re-raises `AttributeError("opengl32")`.
+
+When a CUDA call's error means "feature refused by this driver" (e.g.
+`CUDA_ERROR_OPERATING_SYSTEM` for CUDA-GL interop on WSL), skip at the call
+site with a narrow catch on the specific error, not inside the GL helper —
+see `_register_gl_buffer` / `_register_gl_image` in `tests/test_graphics.py`.
+
+## `importorskip` is for optional dependencies only
+
+`pytest.importorskip("X")` is correct when `X` is genuinely optional
+(platform-gated binding, parametrized "test each available module"). It is
+dead code when `X` is a declared test or runtime dependency: the skip then
+fires only when the environment is broken, which is the case you want to fail
+loudly, not hide. Use a bare top-level `import` for declared deps.
+
+Before adding `importorskip`, check `cuda_core/pyproject.toml`'s `test`
+and `test-cu*` groups and `cuda_core`'s `dependencies`. If the target is
+listed, import it directly.
+
+## Capability probes must not swallow real bugs
+
+A probe function that answers "is feature X available?" by catching
+`Exception` and returning `False` will report "not available" even when the
+probed API failed for a real, unexpected reason — silently enabling a skip
+that hides the bug. Catch only the exception that genuinely means "not
+available", and split the checks so each catch is narrow:
+
+```python
+def _is_nvfatbin_available():
+    from cuda.bindings._internal.utils import FunctionNotFoundError
+    from cuda.pathfinder import DynamicLibNotFoundError
+
+    try:
+        from cuda.bindings import nvfatbin
+    except ImportError:
+        return False
+    try:
+        nvfatbin.version()
+    except (DynamicLibNotFoundError, FunctionNotFoundError):
+        # libnvfatbin not loadable, or nvFatbinVersion symbol missing.
+        return False
+    return True
+```
+
+Catch only the exceptions that mean "not installed / not loadable".
+A genuine API-status failure (e.g. `nvfatbin.nvFatbinError` from a
+successfully loaded library) must propagate so a real bug is not hidden
+as "unavailable".
+
+For a probe that calls a CUDA API returning a `CUresult`, let `handle_return`
+raise on any non-success result and classify only a successful bitmask
+lacking the documented bit as `False`; other failures propagate so a real driver bug is not hidden as "unsupported":
+
+```python
+@functools.cache
+def supports_ipc_mempool(device_id):
+    from cuda.bindings import driver
+
+    handle_return(driver.cuInit(0))
+    dev_id = int(getattr(device_id, "device_id", device_id))
+    attr = driver.CUdevice_attribute.CU_DEVICE_ATTRIBUTE_MEMPOOL_SUPPORTED_HANDLE_TYPES
+    mask = handle_return(driver.cuDeviceGetAttribute(attr, dev_id))
+    posix_fd = driver.CUmemAllocationHandleType.CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR
+    return (int(mask) & int(posix_fd)) != 0
+```
+
+Do not catch `ImportError` for a hard runtime dependency (e.g.
+`cuda.bindings` for `cuda.core`) — that is a broken environment and should
+surface at collection time.
+
+## Clean up partial setup on failure
+
+If a setup step allocates a resource (GL object, window, file handle) and a later
+step fails, clean up the partial resource before re-raising so it does not
+leak. Wrap the allocation in `try/except` and delete the generated object in
+the `except` before re-raising:
+
+```python
+def _allocate_gl_buffer(win, nbytes):
+    from pyglet.gl import gl as _gl
+
+    buf_id = _gl.GLuint(0)
+    try:
+        _gl.glGenBuffers(1, ctypes.byref(buf_id))
+        _gl.glBindBuffer(_gl.GL_ARRAY_BUFFER, buf_id.value)
+        _gl.glBufferData(_gl.GL_ARRAY_BUFFER, nbytes, None, _gl.GL_DYNAMIC_DRAW)
+        return buf_id
+    except Exception:
+        if buf_id.value:
+            with contextlib.suppress(Exception):
+                _gl.glDeleteBuffers(1, ctypes.byref(buf_id))
+        raise
+```
+
+Initialize handles to `None` before the protected region so the `finally`
+cleanup does not `NameError` when allocation raises before returning a handle.
+
+## Tests that touch CUDA must establish their own context
+
+The `init_cuda` fixture pops the CUDA context on teardown, so a test
+that calls a CUDA API without `init_cuda` (or an explicit
+`Device.set_current()`) inherits whatever context the previous test happened
+to leave current on the thread — possibly none. With `pytest-randomly` that
+makes the pass/fail outcome depend on test order, so it moves seed to seed and
+looks like flakiness. Request `init_cuda` for any test that calls into the
+driver, or set up and tear down a context yourself.
+
+## Assert on behavior, not implementation
+
+Pin on observable behavior the contract guarantees — return values, raised
+exception types, public state transitions. Avoid asserting on internal
+call counts, private helper invocation order, or error message substrings
+that are not part of the contract. A refactor that preserves behavior but
+changes internals should not break the test.

cuda_core/tests/graph/test_device_launch.py

@@ -7,6 +7,7 @@
 import pytest
 from cuda_python_test_helpers.marks import requires_module
 
+import cuda.pathfinder as pathfinder
 from cuda.core import (
     Device,
     LaunchConfig,
@@ -48,7 +49,6 @@ def _compile_device_launcher_kernel():
 
     Raises pytest.skip if libcudadevrt.a cannot be found.
     """
-    pathfinder = pytest.importorskip("cuda.pathfinder")
     try:
         cudadevrt_path = pathfinder.find_static_lib("cudadevrt")
     except pathfinder.StaticLibNotFoundError as e:

cuda_core/tests/graph/test_graph_definition_integration.py

@@ -7,10 +7,11 @@
 
 import numpy as np
 import pytest
+from helpers.graph_kernels import skip_if_nvrtc_lacks_conditional_handle
 from helpers.memory import xfail_on_graph_mempool_oom
 
 from cuda.core import Device, EventOptions, LaunchConfig, Program, ProgramOptions
-from cuda.core._utils.cuda_utils import driver, handle_return
+from cuda.core._utils.cuda_utils import CUDAError, driver, handle_return
 from cuda.core.graph import GraphDefinition
 
 SIZEOF_FLOAT = 4
@@ -128,8 +129,9 @@ def _compile_heat_kernels():
             "cubin",
             name_expressions=("heat_step", "countdown"),
         )
-    except Exception:
-        pytest.skip("NVRTC does not support cudaGraphConditionalHandle")
+    except CUDAError as exc:
+        skip_if_nvrtc_lacks_conditional_handle(exc)
+        raise
     return mod.get_kernel("heat_step"), mod.get_kernel("countdown")
 
 
@@ -145,8 +147,9 @@ def _compile_bisect_kernels():
     prog = Program(_BISECT_KERNEL_SOURCE, code_type="c++", options=_nvrtc_opts())
     try:
         mod = prog.compile("cubin", name_expressions=names)
-    except Exception:
-        pytest.skip("NVRTC does not support cudaGraphConditionalHandle")
+    except CUDAError as exc:
+        skip_if_nvrtc_lacks_conditional_handle(exc)
+        raise
     return tuple(mod.get_kernel(n) for n in names)
 
 

cuda_core/tests/helpers/__init__.py

@@ -28,26 +28,29 @@ def supports_ipc_mempool(device_id: int | object) -> bool:
     Uses cuDeviceGetAttribute(CU_DEVICE_ATTRIBUTE_MEMPOOL_SUPPORTED_HANDLE_TYPES)
     to check for CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR support. Does not
     require an active CUDA context.
+
+    Unsupported handle types are represented by a successful query whose bitmask
+    lacks the POSIX-FD bit, so the check below naturally returns False.
+    Other driver errors (invalid device, deinitialized driver) propagate via
+    handle_return so a real bug is not hidden as "unsupported".
     """
     if IS_WSL:
         return False
 
-    try:
-        # Lazy import to avoid hard dependency when not running GPU tests
-        from cuda.bindings import driver  # type: ignore
+    # Lazy import to avoid hard dependency when not running GPU tests
+    from cuda.bindings import driver  # type: ignore
 
-        # Initialize CUDA
-        handle_return(driver.cuInit(0))
+    # Initialize CUDA
+    handle_return(driver.cuInit(0))
 
-        # Resolve device id from int or Device-like object
-        dev_id = int(getattr(device_id, "device_id", device_id))
+    # Resolve device id from int or Device-like object
+    dev_id = int(getattr(device_id, "device_id", device_id))
 
-        # Query supported mempool handle types bitmask
-        attr = driver.CUdevice_attribute.CU_DEVICE_ATTRIBUTE_MEMPOOL_SUPPORTED_HANDLE_TYPES
-        mask = handle_return(driver.cuDeviceGetAttribute(attr, dev_id))
+    # Query supported mempool handle types bitmask. Unsupported handle types are
+    # represented by a successful query whose bitmask lacks the POSIX-FD bit.
+    attr = driver.CUdevice_attribute.CU_DEVICE_ATTRIBUTE_MEMPOOL_SUPPORTED_HANDLE_TYPES
+    mask = handle_return(driver.cuDeviceGetAttribute(attr, dev_id))
 
-        # Check POSIX FD handle type support via bitmask
-        posix_fd = driver.CUmemAllocationHandleType.CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR
-        return (int(mask) & int(posix_fd)) != 0
-    except Exception:
-        return False
+    # Check POSIX FD handle type support via bitmask
+    posix_fd = driver.CUmemAllocationHandleType.CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR
+    return (int(mask) & int(posix_fd)) != 0