Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report Code Coverage for Tests #69

Closed
wants to merge 10 commits into from

Conversation

saulshanabrook
Copy link
Member

This PR changes the tests to record their code coverage, using the Tarpaulin tool and source-based coverage (xd009642/tarpaulin#549).

Local Usage

Now when running make test it will record the code coverage, and print that out:

Nov 29 13:02:10.910  INFO cargo_tarpaulin::report: Coverage Results:
|| Tested/Total Lines:
|| src/ast/expr.rs: 15/40 +0.00%
|| src/ast/mod.rs: 26/26 +0.00%
|| src/extract.rs: 58/76 +0.00%
|| src/gj.rs: 243/270 +0.00%
|| src/lib.rs: 482/616 +0.16%
|| src/main.rs: 0/30 +0.00%
|| src/sort/i64.rs: 31/31 +0.00%
|| src/sort/macros.rs: 18/20 +0.00%
|| src/sort/map.rs: 142/160 +0.00%
|| src/sort/mod.rs: 11/13 +0.00%
|| src/sort/rational.rs: 70/70 +0.00%
|| src/sort/string.rs: 13/15 +0.00%
|| src/sort/unit.rs: 15/19 +0.00%
|| src/typecheck.rs: 313/395 +0.25%
|| src/unionfind.rs: 83/131 +0.00%
|| src/util.rs: 8/12 +0.00%
|| src/value.rs: 12/12 +0.00%
|| 
79.55% coverage, 1540/1936 lines covered, +0.10% change in coverage

It will also create a tarpaulin-report.html file, which can be opened to see the coverage line-by-line.
Screenshot 2022-11-29 at 1 24 13 PM

I was also able to install the Coverage Gutters VS Code extension, which picked up the coverage file and showed the information inline:

Screenshot 2022-11-29 at 1 04 22 PM

Codecov

It uploads the results to Codecov so that it is easy to see over time how coverage evolves, and also for each PR how it impacts the code coverage. This is useful when refactoring or implementing new features, to make sure all branches are covered.

I enabled it on my fork, to test it out, which you can browse here:

Screenshot 2022-11-29 at 1 26 35 PM

In order to upload to Codecov, I found I had to set the token, otherwise, I would get a 404 error. This is a known bug with Codecov (codecov/codecov-action#557, codecov/feedback#126). I made the token public, instead of setting it privately so that it would be used on any PRs from forks as well (codecov/codecov-action#557 (comment)).

Before merging this, if you choose to keep this approach, you would have to make an account on Codecov, add this project, and update the token in this PR with your token for this repo. Making the token public does present some security risk (anyone would be able to submit coverage for this repo) which would have to be weighed against the advantage of allowing PRs from forks to have their coverage submitted (and compared).

Alternative using grcov

I did want to note that I did try using the RUSTFLAGS='-Cinstrument-coverage' env variable directly and then using grcov to convert to lcov and HTML, instead of using Tarpaulin. This does have support for branches, which is nice, but I found the lcov generation to inaccurate (mozilla/grcov#860).

It also requires installing the llvm-tools-preview component, since it uses that to do the coverage generation, whereas Tarpaulin reimplemented what they needed in rust.

Performance

I also noticed that using this coverage increased the testing time from ~2 second to ~13 second on my local machine, once things were cached. The alternative above using instrument-coverage directly takes ~5 seconds.

@mwillsey
Copy link
Member

mwillsey commented Dec 6, 2022

Thanks for the PR! This a little more than I want to commit too at the moment. But it's great to have this PR standing by once we stabilize some more!

@mwillsey mwillsey closed this Dec 6, 2022
@saulshanabrook
Copy link
Member Author

saulshanabrook commented Dec 7, 2022

@mwillsey Sounds good! I was surprised that it worked quite easily, it just took a while to figure out the right way to do it.

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.

2 participants