Skip to content

Add github action for llvm-cov coverage#2361

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
pool2win:add-code-coverage
Jan 20, 2024
Merged

Add github action for llvm-cov coverage#2361
apoelstra merged 2 commits intorust-bitcoin:masterfrom
pool2win:add-code-coverage

Conversation

@pool2win
Copy link
Copy Markdown
Contributor

There has been discussion around generating coverage reports for tests. See #1853 and #2353.

This PR adds an action using llvm-cov and coveralls.

The action generates lcov coverage report using llvm-cov and uploads the generated report to coveralls. You can see sample reports for my fork here: https://coveralls.io/github/pool2win/rust-bitcoin

I am using the cargo-llvm-cov wrapper around llvm-cov.

I also use the coveralls official github action to push the coverage report to coveralls. It removes the need to deal with repository secrets etc, making the action easy to run for all contributors on their forks.

We can move this action later to rust.yml if we need to.

The action reports the generated report to coveralls.
@pool2win pool2win mentioned this pull request Jan 19, 2024
@apoelstra
Copy link
Copy Markdown
Member

Neat! Looks like it's working. This PR's report appears here https://coveralls.io/github/rust-bitcoin/rust-bitcoin

@apoelstra
Copy link
Copy Markdown
Member

Do you know if there's a way we can signal to coveralls to ignore the fuzz/ directory?

apoelstra
apoelstra previously approved these changes Jan 19, 2024
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 50ff685

@pool2win
Copy link
Copy Markdown
Contributor Author

Do you know if there's a way we can signal to coveralls to ignore the fuzz/ directory?

Seems like we can: https://github.com/taiki-e/cargo-llvm-cov#exclude-file-from-coverage

Here is the change that makes it work: pool2win@47b476e

Here is a coverage report without the fuzz directory: https://coveralls.io/github/pool2win/rust-bitcoin?branch=ignore-fuzz-dir-for-coveralls

I can do a separate PR for the patch or push the commit to this PR - whatever you prefer.

@Kixunil
Copy link
Copy Markdown
Collaborator

Kixunil commented Jan 19, 2024

Do you want to update this PR to have fuzz excluded? (Sorry, my brain is missing some gears today.)

I think just updating the PR is simplest. ACK once the change is in.

@pool2win
Copy link
Copy Markdown
Contributor Author

Do you want to update this PR to have fuzz excluded? (Sorry, my brain is missing some gears today.)

I think just updating the PR is simplest. ACK once the change is in.

Updated the PR to ignore the fuzz directory.

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 47b476e

Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 47b476e

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Legend, thanks!

ACK 47b476e

@apoelstra apoelstra merged commit 782c2d1 into rust-bitcoin:master Jan 20, 2024
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.

4 participants