Skip to content

coverage: add tests for memory, launcher, linker, program, and utils coverage gaps#2130

Merged
rwgk merged 5 commits into
NVIDIA:mainfrom
rluo8:main
May 30, 2026
Merged

coverage: add tests for memory, launcher, linker, program, and utils coverage gaps#2130
rwgk merged 5 commits into
NVIDIA:mainfrom
rluo8:main

Conversation

@rluo8
Copy link
Copy Markdown
Contributor

@rluo8 rluo8 commented May 22, 2026

Description

Adds focused unit tests that target previously-untested branches across cuda_core.

Here is the per-file coverage rate changes:
cuda/core/_memory/_legacy.py 64.52% -> 98.39% (+33.87%, -21 miss)
cuda/core/_launch_config.pyx 56.34% -> 77.46% (+21.13%, -15 miss)
cuda/core/_linker.pyx 64.64% -> 81.49% (+16.85%, -61 miss)
cuda/core/_memory/_peer_access_utils.pyx 22.02% -> 37.16% (+15.14%, -33 miss)
cuda/core/_memory/_device_memory_resource.pyx 68.06% -> 81.94% (+13.89%, -10 miss)
cuda/core/_memory/_graph_memory_resource.pyx 80.26% -> 88.16% (+7.89%, -6 miss)
cuda/core/_memoryview.pyx 62.99% -> 65.82% (+2.82%, -20 miss)
cuda/core/_memory/_virtual_memory_resource.py 89.58% -> 91.67% (+2.08%, -5 miss)
cuda/core/system/_device.pyx 76.47% -> 77.65% (+1.18%, -3 miss)
cuda/core/_program.pyx 86.06% -> 86.96% (+0.90%, -6 miss)

Total cuda/core/ 77.80% -> 79.53% (+1.73%, -180 miss)

The tests were run and could pass locally.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label May 22, 2026
@rluo8
Copy link
Copy Markdown
Contributor Author

rluo8 commented May 22, 2026

Hi @mdboom @rwgk , could you please help review ? Thanks!

@github-actions

This comment has been minimized.

@rwgk rwgk added this to the cuda.core next milestone May 29, 2026
@rwgk rwgk added the P1 Medium priority - Should do label May 29, 2026
@rwgk rwgk self-requested a review May 29, 2026 04:24
@rwgk
Copy link
Copy Markdown
Contributor

rwgk commented May 29, 2026

Hi @rluo8, I generated the finding and DRY (don't repeat yourself) note with GPT-5.5. Could you please address?

  Findings first:

  • Medium: cuda_core/tests/test_memory.py:1022 calls _grow_allocation_fast_path() with a separately reserved, non-contiguous new_ptr,
    even though that helper’s contract is specifically “reserved contiguously after the existing buffer.” The test then manually tears
    down mappings because normal Buffer.close() would be unsafe. This gives coverage, but it tests an invalid state and can mask
    regressions in the real fast path. I’d prefer driving this through modify_allocation() with a controlled contiguous reservation, or
    using fakes/mocks that preserve the helper’s preconditions.

  DRY note, not a blocker:

  • cuda_core/tests/test_launcher.py:469 and cuda_core/tests/test_launcher.py:523 duplicate most of the scalar compile/launch setup from
     the existing scalar argument test. A small helper or parametrized test over subclass type, dtype, C++ type, and expected value
    would reduce the duplication without making the tests harder to read.

@rluo8
Copy link
Copy Markdown
Contributor Author

rluo8 commented May 29, 2026

Hi @rluo8, I generated the finding and DRY (don't repeat yourself) note with GPT-5.5. Could you please address?

  Findings first:

  • Medium: cuda_core/tests/test_memory.py:1022 calls _grow_allocation_fast_path() with a separately reserved, non-contiguous new_ptr,
    even though that helper’s contract is specifically “reserved contiguously after the existing buffer.” The test then manually tears
    down mappings because normal Buffer.close() would be unsafe. This gives coverage, but it tests an invalid state and can mask
    regressions in the real fast path. I’d prefer driving this through modify_allocation() with a controlled contiguous reservation, or
    using fakes/mocks that preserve the helper’s preconditions.

  DRY note, not a blocker:

  • cuda_core/tests/test_launcher.py:469 and cuda_core/tests/test_launcher.py:523 duplicate most of the scalar compile/launch setup from
     the existing scalar argument test. A small helper or parametrized test over subclass type, dtype, C++ type, and expected value
    would reduce the duplication without making the tests harder to read.

Thanks Ralf. I'll fix this.

Copy link
Copy Markdown
Contributor

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rluo8 for iterating on this! I’m comfortable approving this even though some of the added coverage targets private/internal helpers: the tests are narrowly scoped, and several of these paths are difficult to exercise reliably through only public end-to-end behavior. Given the PR’s explicit coverage goal, this seems like a reasonable tradeoff.

I'll hold off merging right now because we're preparing for a patch release. I'll merge when that's done.

@rwgk rwgk merged commit e350c5a into NVIDIA:main May 30, 2026
96 checks passed
@github-actions
Copy link
Copy Markdown

Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module P1 Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants