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

skip_on_cran() tests will reduce code coverage readout in assess_covr_coverage() #337

Open
PatrickRWright opened this issue Apr 2, 2024 · 1 comment
Labels
Difficulty: Medium Mild design and/or technical implementation challenges Enhancement New feature or request

Comments

@PatrickRWright
Copy link

PatrickRWright commented Apr 2, 2024

Some package test files will include testthat::skip_on_cran(). A use case I am aware of is to reduce runtime when deploying to CRAN and thus prevent package rejection. In the context of test coverage this means that tests are likely functional but omitted on CRAN only for technical reasons.

In the current setup, packages may return artificially low scores with respect to code coverage.

Here's a related SO thread for some additional context:
https://stackoverflow.com/questions/76153396/how-to-include-tests-with-skip-on-cran-when-calling-covrpackage-coverage

Example (22.84% vs. 95.18%):
This needs some minutes to complete.

library(dplyr)
library(tibble)
library(riskmetric)

system("wget -q https://cran.r-project.org/src/contrib/secuTrialR_1.1.1.tar.gz -P source_archives")
system("tar -zxf source_archives/secuTrialR_1.1.1.tar.gz -C source_archives")

st <- pkg_ref("source_archives/secuTrialR", source = "pkg_source")
st_assess <- st %>% 
  tibble::as_tibble() %>% 
  pkg_assess(assessments = list(assess_covr_coverage))

Sys.setenv(NOT_CRAN = "true")

st_not_cran <- pkg_ref("source_archives/secuTrialR", source = "pkg_source")
st_not_cran_assess <- st_not_cran %>% 
  tibble::as_tibble() %>% 
  pkg_assess(assessments = list(assess_covr_coverage))

Sys.setenv(NOT_CRAN = "")

st_assess$covr_coverage[[1]]$totalcoverage
# > [1] 22.84
st_not_cran_assess$covr_coverage[[1]]$totalcoverage
# > [1] 95.18
@PatrickRWright PatrickRWright changed the title skip_on_cran() tests will reduce code coverage readout in assess_covr_coverage skip_on_cran() tests will reduce code coverage readout in assess_covr_coverage() Apr 2, 2024
@dgkf
Copy link
Collaborator

dgkf commented Apr 3, 2024

This sounds reasonable to me!

I do anticipate that there will be other packages where changing this envvar will cause their coverage to fail. I'm imagining specifically situations where packages use skip_on_cran() to disable tests that run outside their own CI/CD processes - maybe requiring authenticated database access using secrets only available on CI, or packages not available on CRAN.

There are more appropriate ways of handling these situations (skip_on_ci(), skip_if(), specifying AdditionalRepositories), but skip_on_cran() can often be the most obvious path forward in such cases so it sometimes gets used as a catch-all.

I would say we should try to run tests as inclusively as possible, but then fall back to cran-style tests if there are issues.

@emilliman5 emilliman5 added Enhancement New feature or request Difficulty: Medium Mild design and/or technical implementation challenges labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Medium Mild design and/or technical implementation challenges Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants