-
Notifications
You must be signed in to change notification settings - Fork 340
Automatically skip missing packages on CRAN #2173
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
Conversation
Practically, this should mean that you never need to add `skip_if_not_installed()` for suggested packages. It's already the tidyverse style to not do this, but now this works everywhere. Fixes #1585
R/test-that.R
Outdated
| expectation = handle_expectation, | ||
| packageNotFoundError = function(e) { | ||
| if (on_cran()) { | ||
| skip(paste0(e$package, " is not installed.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe sync it exactly with the skip_if_not_installed() message?
Line 107 in 6099802
| error = function(e) skip(paste0("{", pkg, "} is not installed")) |
| } | ||
| }, | ||
| expectation = handle_expectation, | ||
| packageNotFoundError = function(e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a great & reasonable compromise -- minimal implementation & fits well with existing approach that always sets !on_cran in package GHA.
one possible concern is that it doesn't make any effort to distinguish "package missing in current environment" from "you forgot to add a package to your Suggests". I guess R CMD check is inspecting tests/ files for pkg:: usage so it should be caught anyway.
no I think it's fine. thanks for the consideration! AIUI it will now work exactly as One advantage of the |
Practically, this should mean that you never need to add
skip_if_not_installed()for suggested packages. It's already the tidyverse style to not do this, but now this works everywhere.Fixes #1598
@MichaelChirico is there anyway we could make this more convenient for google infrastructure? Maybe consult another env var?