Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Aug 5, 2025

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?

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
@hadley hadley requested a review from lionel- August 5, 2025 16:17
R/test-that.R Outdated
expectation = handle_expectation,
packageNotFoundError = function(e) {
if (on_cran()) {
skip(paste0(e$package, " is not installed."))
Copy link
Contributor

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?

error = function(e) skip(paste0("{", pkg, "} is not installed"))

}
},
expectation = handle_expectation,
packageNotFoundError = function(e) {
Copy link
Contributor

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.

@MichaelChirico
Copy link
Contributor

@MichaelChirico is there anyway we could make this more convenient for google infrastructure? Maybe consult another env var?

no I think it's fine. thanks for the consideration! AIUI it will now work exactly as skip_if_not_installed() does (since we test on_cran()) -- the test will pass and it's the importer's responsibility to add requirements as needed.

One advantage of the skip_if_not_installed() approach is we can easily do static analysis of testthat/ files and pull out which packages are needed. With this, we'll need to pull that information out from the test logs, but that's basically how it goes in the current status quo.

@hadley hadley merged commit 86480e2 into main Aug 6, 2025
10 of 11 checks passed
@hadley hadley deleted the auto-skip-missing-package branch August 6, 2025 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically skip in case of packageNotFoundError?

4 participants