statrs-dev / statrs

Statistical computation library for Rust
https://docs.rs/statrs/latest/statrs/
MIT License
544 stars 78 forks source link

feat: reintroduce coverage and upload to codecov.io #229

Open YeungOnion opened 1 month ago

YeungOnion commented 1 month ago

Bringing back codecov #147

I went with tarpaulin because it seemed easy to setup and use, especially with the container on dockerhub. I don't believe that coverage reports need the same level of confidence as passing the tests that are written, but can be useful in deciding how to focus efforts.

But I'm new to this, so feedback would be appreciated.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.52%. Comparing base (49a5209) to head (0050684). Report is 190 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #229 +/- ## ========================================== - Coverage 91.76% 91.52% -0.25% ========================================== Files 44 50 +6 Lines 7360 10332 +2972 ========================================== + Hits 6754 9456 +2702 - Misses 606 876 +270 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

YeungOnion commented 1 month ago

It appears that tarpaulin is accurately reading tests that rely on the testing_boiler macro, I don't know where else to check for inaccuracy, so others' input would be helpful.

YeungOnion commented 1 month ago

Should we get tests back to >90% before the release or not fail ci to get the release out?

Note: since v14 we've been below 90%, thinking maybe changing it to 80%?

FreezyLemon commented 1 month ago

Does it make sense to enforce a coverage > x% as part of the CI pipeline for every PR? Codecov will create a report (see above) for all PRs which makes it obvious whether the coverage increases or decreases. Setting an arbitrary minimum could also lead to a situation like "current coverage is 95%, so it's fine if this PR drops it a little". But maybe I'm overthinking.

Higher coverage could still be a goal for the next release (or the one after that) even without the explicit CI check.

FreezyLemon commented 1 month ago

BTW: Looks like moving from nightly to stable caused a permissions error. Could be xd009642/tarpaulin#406 and may be solved by moving to a non-docker solution. FYI taiki-e/install-action supports tarpaulin, so that could be a good alternative.

YeungOnion commented 1 month ago

Note: Still looking into this option, but I force-pushed to test the action.

In looking into taiki-e/install-action I found their nextest project, which appears more friendly and designed for CI.

It and the wrapper they developed to compiling with instrument-coverage, cargo-llvm-cov it seems capable of removing the need to rerun tests for collecting coverage data.