extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
181 stars 27 forks source link

[WIP] Use `{covr}` #260

Closed Ilia-Kosenkov closed 1 year ago

Ilia-Kosenkov commented 1 year ago

Add {covr} workflow & register {rextendr} at [codecov.io]. The coverage can be viewed at codecov.io.

Note that permission was issued to codecov.io on my behalf to access extendr repositories. This can be revoked at any time.

codecov[bot] commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@84f62d6). Click here to learn what that means. Patch has no changes to coverable lines.

:exclamation: Current head 18ccc95 differs from pull request most recent head be561e9. Consider uploading reports for the commit be561e9 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #260 +/- ## ======================================= Coverage ? 82.01% ======================================= Files ? 22 Lines ? 1034 Branches ? 0 ======================================= Hits ? 848 Misses ? 186 Partials ? 0 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=extendr). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=extendr)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Ilia-Kosenkov commented 1 year ago

This is an experiment & I want your feedback.

multimeric commented 1 year ago

It looks good, I have two questions though:

a) Can we see the report that has hopefully been generated from this PR?, and b) Will this ever cause the build to fail?

Ilia-Kosenkov commented 1 year ago

Good questions. Just to clarify, I saw this tool used a lot in different projects but never configured it myself for a project I contribute on GitHub. Thus 1) Unlikely, until we merge it and get the first coverage for main branch. It needs a source of truth to compare to. Initial setup is always rough 2) I think we can configure it, e.g. desired/minimum level of coverage, wether it is optional or not, or even disable it altogether.

Since I cannot answer these questions without experimenting, my suggestion is to try it out and see if we can get a little bit more visibility of test coverage in our code base. This, and some better public look because we report testing & have high coverage (which is always cool). I have a couple of PRs planned for the end of this week, so we can check if we get any value out of it. If not, we can disable integration.

Note: If I understand correctly, submitting code coverage to the platform to get observability and allowing the platform to provide feedback/checks are two completely different features. The first one is widely used in R repositories all over the tidyverse, the second is not, so we can easily disable it.

multimeric commented 1 year ago

Uploading coverage is the weird part to me, but if you think it would provide visibility I'm not against it. Mostly I want this for CI coverage reports. Can you get covr to output its report to stdout so we can see it for builds?

Ilia-Kosenkov commented 1 year ago

It already prints it out in the CI, if that is what you ask. https://github.com/extendr/rextendr/actions/runs/4546712566/jobs/8015979450?pr=260#step:5:37 image

multimeric commented 1 year ago

Okay, good enough for me.

yutannihilation commented 1 year ago

all over the tidyverse

I think most of them disables the comment feature. Honestly, I feel codecov's comments a bit annoying as they overwrite the notification. Do you want the comments?

https://github.com/search?q=org%3Atidyverse+comment+path%3Acodecov.yml&type=code

Ilia-Kosenkov commented 1 year ago

I will go ahead and merge it bypassing failing ci -- these two are R-devel which fail because of https://github.com/extendr/extendr/issues/524