Skip to content

Conversation

@rluo8
Copy link
Contributor

@rluo8 rluo8 commented Feb 6, 2026

This is to add code coverage support for Windows platform.

Here are the main steps:

  1. Run coverage on Linux runner - linux-amd64-gpu-a100-latest-1 , and upload coverage data and source to artifact.
  2. Run coverage on Windows runner - windows-amd64-gpu-a100-latest-1, and upload coverage to artifact.
  3. Download the coverage data from artifact, and then combine the coverage data. Generate coverage report using the combined data of Linux and Windows. Then upload the report to github pages.

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Feb 6, 2026

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@rluo8
Copy link
Contributor Author

rluo8 commented Feb 6, 2026

Hi @mdboom ,
Could you please take a look?
Thanks!

@mdboom
Copy link
Contributor

mdboom commented Feb 6, 2026

In order to test this before we merge it, I

  • pushed rluo8/main to upstream/windows-coverage
  • Manually triggered the coverage action on that branch

The job is running here: https://github.com/NVIDIA/cuda-python/actions/runs/21754529128

Copy link
Contributor

@mdboom mdboom left a comment

Choose a reason for hiding this comment

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

This is really great. A lot of hard problems solved here.

I just had the one question about whether we need to do a full clone of the repo. Other than resolving that question, LGTM.

- name: Checkout ${{ github.event.repository.name }}
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to fetch the whole history?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, since cuda_bindings and cuda_core both use setuptools_scm to get their version from git history, fetch-depth: 0 is required. Without full history, setuptools_scm may compute an incorrect version, which causes cuda.core.init.py's import-time check to fail with ImportError: cuda.bindings 12.x or 13.x must be installed.

Error like this without fetch-depth:0:

  • cd /__w/cuda-python/cuda-python/.venv/lib/python3.14/site-packages
  • /__w/cuda-python/cuda-python/.venv/bin/pytest -v --cov=./cuda --cov-append --cov-context=test --cov-config=/__w/cuda-python/cuda-python/.coveragerc /__w/cuda-python/cuda-python/cuda_core/tests
    ImportError while loading conftest '/__w/cuda-python/cuda-python/cuda_core/tests/conftest.py'.
    ../../../../cuda_core/tests/conftest.py:7: in
    import helpers
    ../../../../cuda_core/tests/helpers/init.py:10: in
    from cuda.core._utils.cuda_utils import handle_return
    cuda/core/init.py:14: in
    raise ImportError("cuda.bindings 12.x or 13.x must be installed")
    E ImportError: cuda.bindings 12.x or 13.x must be installed
    Error: Process completed with exit code 4.

@mdboom
Copy link
Contributor

mdboom commented Feb 10, 2026

/ok to test

@github-actions
Copy link

@leofang leofang added the CI/CD CI/CD infrastructure label Feb 10, 2026
with:
name: coverage
name: coverage-combined
path: docs/coverage/
Copy link
Member

@leofang leofang Feb 10, 2026

Choose a reason for hiding this comment

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

Q: Does each PR (and that built from main) overwrite with each other?

Copy link
Contributor Author

@rluo8 rluo8 Feb 11, 2026

Choose a reason for hiding this comment

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

Yes, they would overwrite with each other and only keep the latest report in docs/coverage.
The coverage workflow needs to be manually triggered for PR. And for main branch, it was scheduled to be nightly triggered. Current github page report was from this PR because the nightly main coverage run failed and couldn't generate a report.
Historical coverage data is available as artifacts uploaded by each coverage run, with a retention period of 7 days (configurable via retention-days in the workflow).
Do we need to keep historical coverage reports in github pages?

# Patch cuda_core pyproject.toml - add after cuda.core._cpp line
sed -i '/\"cuda\.core\._cpp\" = \[\".*\"\]/a \"*\" = [\"*.cpp\"]' cuda_core/pyproject.toml

echo "Applied coverage patches to cuda_core"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than doing these patches I think it would be better to add build commands into the projects that need it and pass the options through via pip install --config-settings or using an environment variable.

@kkraus14
Copy link
Collaborator

Also, should we consider using something like https://codecov.io instead of manually managing all of the coverage reports?

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

Labels

CI/CD CI/CD infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants