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

[R-package] Added instructions for generating R test coverage #2664

Merged
merged 4 commits into from
Jan 12, 2020

Conversation

jameslamb
Copy link
Collaborator

Really excited to say that I've discovered that covr (the most popular library for generating test coverage for R projects) now works well with lightgbm's R package! I had created jameslamb/lightgbm-r-coverage to work around the previous issues I had and I am VERY excited to archive that repo.

In this PR, I propose adding instructions to the R package's README explaining how contributors can generate coverage reports. I think this will be a valuable way for us to catch edge cases that are not covered by our current tests, and all the other good things you can do with code coverage.

At the time of creating this PR, the R package is 64.48% covered.

lightgbm Coverage: 64.48%
R/lgb.plot.importance.R: 0.00%
R/lgb.prepare_rules.R: 0.00%
R/lgb.prepare_rules2.R: 0.00%
R/lgb.prepare.R: 0.00%
R/lgb.prepare2.R: 0.00%
R/lgb.unloader.R: 0.00%
R/readRDS.lgb.Booster.R: 0.00%
R/saveRDS.lgb.Booster.R: 0.00%
R/lgb.cv.R: 57.14%
R/lgb.Predictor.R: 57.66%
R/lgb.Booster.R: 62.58%
R/callback.R: 67.48%
R/utils.R: 70.63%
R/lgb.Dataset.R: 72.02%
R/lgb.train.R: 75.94%
R/lightgbm.R: 95.45%
R/aliases.R: 100.00%
R/lgb.importance.R: 100.00%
R/lgb.interprete.R: 100.00%
R/lgb.model.dt.tree.R: 100.00%
R/lgb.plot.interpretation.R: 100.00%

I've attached a detailed coverage report for those who are curious (GitHub will not let you attach a .html file, so had to zip it).

coverage.html.zip

R-package/README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@jameslamb
Copy link
Collaborator Author

Just rebased to current master. I added two more files to .gitignore that were missed before.

@StrikerRUS normally I wouldn't merge an R-package PR with only your approval but I think for this once it's ok. Do you agree?

R-package/README.md Outdated Show resolved Hide resolved
@StrikerRUS
Copy link
Collaborator

@jameslamb

normally I wouldn't merge an R-package PR with only your approval but I think for this once it's ok. Do you agree?

Sure!

@jameslamb jameslamb merged commit f449a78 into microsoft:master Jan 12, 2020
@jameslamb jameslamb deleted the docs/r_coverage branch January 27, 2020 00:14
@StrikerRUS StrikerRUS added doc and removed maintenance labels Mar 1, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants