Skip to content

Stop _gl_context from converting test failures into skips - #2565

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:gl-context-skip-swallows-failures
Open

Stop _gl_context from converting test failures into skips#2565
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:gl-context-skip-swallows-failures

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

_gl_context() yields from inside a try whose handler turns everything into a skip:

    try:
        ...create context and texture...
        yield int(tex_id.value), int(target)

    except Exception as e:
        # Convert any pyglet/GL creation failure into a clean skip
        pytest.skip(f"Could not create GL context/texture: {type(e).__name__}: {e}")

@contextmanager re-raises the with-body's exception at the yield point, so
except Exception also catches failures from the test body. AssertionError is an
Exception; pytest's Skipped is a BaseException, so the replacement skip escapes
cleanly and the run is reported as SKIPPED.

That means neither assertion in test_cuda_gl_register_image_smoketest can fail the suite:

        assert name in acceptable, f"cudaGraphicsGLRegisterImage returned {name}"
        if name == "cudaSuccess":
            assert int(resource) != 0

A genuinely wrong cudaGraphicsGLRegisterImage return is reported as
Could not create GL context/texture: AssertionError: .... The handler's own comment says
"creation failure", which is what it was meant to cover.

Demonstrated with a minimal standalone reproduction of the same shape:

test_yield_in_try.py::test_body_failure_under_main_shape   SKIPPED
    Could not create GL context/texture: AssertionError: this assertion should FAIL the test
test_yield_in_try.py::test_body_failure_under_fixed_shape  FAILED
    AssertionError: this assertion should FAIL the test

Both tests in this module are on the always-skipped list in #2077, so this has never been
visible in CI.

Fix

Move the context/texture creation into _create_gl_texture() so the handler wraps only
that, and keep the yield outside it. Cleanup stays in the outer finally, so a
partially built context is still torn down on the skip path. tex_id is now initialized,
which also removes an UnboundLocalError that the finally block was silently swallowing
when creation failed before the texture existed.

pytest.importorskip and the no-EGL pytest.skip inside _create_gl_texture still
propagate correctly — Skipped is a BaseException, so except Exception does not catch
them.

Test

test_gl_context_lets_body_failures_fail stubs _create_gl_texture() and asserts that an
AssertionError raised inside the with body propagates. No GL context, no display, no
GPU, no pyglet
— so unlike the two smoketests it actually runs in CI. I verified it
passes against the restructured contextmanager and that the pre-existing shape converts
the same failure into a skip.

ruff check and ruff format --check are clean.

_gl_context() yields from inside a try whose handler turns everything into a
skip:

    try:
        ...create context and texture...
        yield int(tex_id.value), int(target)
    except Exception as e:
        # Convert any pyglet/GL creation failure into a clean skip
        pytest.skip(f"Could not create GL context/texture: {type(e).__name__}: {e}")

@contextmanager re-raises the with-body's exception at the yield point, so
`except Exception` also catches failures from the test body. AssertionError
is an Exception; pytest's Skipped is a BaseException, so the replacement
skip escapes cleanly and the run is reported as SKIPPED.

That means neither assertion in test_cuda_gl_register_image_smoketest can
fail the suite:

    assert name in acceptable, f"cudaGraphicsGLRegisterImage returned {name}"
    assert int(resource) != 0

A genuinely wrong cudaGraphicsGLRegisterImage return is reported as
"Could not create GL context/texture: AssertionError: ...". The comment on
the handler says "creation failure", which is what it was meant to cover.

Move the context/texture creation into _create_gl_texture() so the handler
wraps only that, and keep the yield outside it. Cleanup stays in the outer
finally, so a partially built context is still torn down. tex_id is now
initialized, which also removes an UnboundLocalError that the finally block
was silently swallowing when creation failed early.

Adds test_gl_context_lets_body_failures_fail, which stubs
_create_gl_texture() and asserts an AssertionError raised in the with-body
propagates. It needs no GL context, no display and no GPU.
@copy-pr-bot

copy-pr-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant