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

Document that test_if_offline() depends on curl #1854

Closed
cboettig opened this issue Sep 16, 2023 · 1 comment · Fixed by #1866
Closed

Document that test_if_offline() depends on curl #1854

cboettig opened this issue Sep 16, 2023 · 1 comment · Fixed by #1866
Labels
bug an unexpected problem or unintended behavior skip
Milestone

Comments

@cboettig
Copy link
Contributor

The documentation for test_if_offline() doesn't mention that this function requires the optional dependency, curl. Because of how skipping tests works and common patterns of use like r-lib/actions, this makes it easy to think tests are passing (i.e. gh-actions reports success) when in fact all tests that need network are being skipped because the package did not suggest curl in addition to suggesting testthat. I think I am not alone in not reading the testthat logs of every successful github action check, many of us look at logs closely only when the checks fail, not when they pass.

Yes, many packages that will have tests needing network access (and thus using skip_if_offline() will already depend directly or indirectly on the curl package and so not be bitten by this. But many will not, since networking is a feature of many R packages (and base R itself of course) that have no dependency on the R curl package. (I've hit this three times this week in separate packages -- for instance, arrow, duckdb, and the spatial packages binding gdal like terra, stars, sf, gdalcubes all provide network-based interfaces to remote data resources).

I think it might help to mention this in documentation. In general I think that's helpful for functions that pull in optional dependencies, especially when that function isn't used interactively. (In this case interactive testing doesn't help, since curl can be available on the host system but not the github actions tests).

Or perhaps the testthat::test_if_offline() should trigger a warning if curl is not available?

@hadley
Copy link
Member

hadley commented Sep 17, 2023

Yeah I think that should be check_installed("curl") not skip_if_not_installed("curl").

@hadley hadley added bug an unexpected problem or unintended behavior skip labels Sep 17, 2023
hadley added a commit that referenced this issue Sep 22, 2023
@hadley hadley modified the milestones: 3.1.8, v3.2.0 Sep 22, 2023
hadley added a commit that referenced this issue Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior skip
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants