Skip to content

feat: implement coverage reporting#582

Merged
sayakpaul merged 5 commits into
mainfrom
coverage
May 27, 2026
Merged

feat: implement coverage reporting#582
sayakpaul merged 5 commits into
mainfrom
coverage

Conversation

@sayakpaul
Copy link
Copy Markdown
Member

@sayakpaul sayakpaul commented May 25, 2026

Some in-line comments.

Should we also run coverage on main pushes and keep a status on README? I think we don't need it. Open to suggestions.

Comment on lines +126 to +129
PCT_DISPLAY=$(printf '%.1f' "$PCT")
if [ "$PCT_INT" -ge 80 ]; then
EMOJI=":white_check_mark:"
elif [ "$PCT_INT" -ge 70 ]; then
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Open to other percentages.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment thread .github/workflows/test_kernels.yaml Outdated

env:
UV_PYTHON_PREFERENCE: only-managed
COVERAGE_CELL: ${{ matrix.python-version == '3.10' && matrix.torch-version == '2.12.0' }}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only run for a single path. But can increase it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doing a single path sounds good to me!

danieldk
danieldk previously approved these changes May 26, 2026
Copy link
Copy Markdown
Member

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Looks good, some nits about the naming, feel free to ignore.

Comment thread .github/workflows/coverage_comment.yaml Outdated
fi

EXISTING=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --paginate \
--jq 'first(.[] | select(.body | startswith("<!-- coverage-report -->")) | .id) // empty')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if a user makes a comment with that marker? I don't think there is an issue, since it'd just overwrite their comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. Addressed.

Comment thread .github/workflows/test_kernels.yaml Outdated

env:
UV_PYTHON_PREFERENCE: only-managed
COVERAGE_CELL: ${{ matrix.python-version == '3.10' && matrix.torch-version == '2.12.0' }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doing a single path sounds good to me!

Comment thread .github/workflows/test_kernels.yaml Outdated
Comment thread .github/workflows/test_kernels.yaml Outdated
Comment thread .github/workflows/test_kernels.yaml Outdated
sayakpaul and others added 2 commits May 26, 2026 13:30
@sayakpaul
Copy link
Copy Markdown
Member Author

There is an unrelated failure in the CI. I will fix it later. @danieldk

@sayakpaul sayakpaul merged commit 868a49d into main May 27, 2026
68 checks passed
@sayakpaul sayakpaul deleted the coverage branch May 27, 2026 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants