From 4fe0c4b5baf0b6578e2c6db183328b03f724270c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Cs=C3=A1rdi?= Date: Wed, 15 Mar 2023 11:58:06 +0100 Subject: [PATCH 1/4] Make optional test deps really optional Cf. #247. --- DESCRIPTION | 1 - R/test-helpers.R | 25 ++++++++++++++++++++++++ tests/testthat.R | 12 ++++++++++-- tests/testthat/helper.R | 5 +++++ tests/testthat/test-bugs.R | 1 + tests/testthat/test-clean-subprocess.R | 3 +++ tests/testthat/test-error.R | 6 ++++++ tests/testthat/test-eval.R | 2 ++ tests/testthat/test-libpath.R | 6 ++++++ tests/testthat/test-load-client.R | 2 ++ tests/testthat/test-messages.R | 2 ++ tests/testthat/test-presets.R | 2 ++ tests/testthat/test-r-session-messages.R | 2 ++ tests/testthat/test-r-session.R | 2 ++ tests/testthat/test-rcmd.R | 1 + tests/testthat/test-spelling.R | 1 + tests/testthat/test-utils.R | 2 ++ 17 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 R/test-helpers.R diff --git a/DESCRIPTION b/DESCRIPTION index b0e0ef99..7bf7176a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -22,7 +22,6 @@ Imports: Suggests: asciicast, cli (>= 1.1.0), - covr, mockery, ps, rprojroot, diff --git a/R/test-helpers.R b/R/test-helpers.R new file mode 100644 index 00000000..1ddeb7db --- /dev/null +++ b/R/test-helpers.R @@ -0,0 +1,25 @@ + +is_true_check_env_var <- function(x, default = "") { + # like utils:::str2logical + val <- Sys.getenv(x, default) + if (isTRUE(as.logical(val))) return(TRUE) + tolower(val) %in% c("1", "yes") +} + +isFALSE <- function(x) { + is.logical(x) && length(x) == 1L && !is.na(x) && !x +} + +is_false_check_env_var <- function(x, default = "") { + # like utils:::str2logical + val <- Sys.getenv(x, default) + if (isFALSE(as.logical(val))) return(TRUE) + tolower(val) %in% c("0", "no") +} + +# Only skip if _R_CHECK_FORCE_SUGGESTS_ is false + +skip_if_not_installed <- function(pkg) { + if (!is_false_check_env_var("_R_CHECK_FORCE_SUGGESTS_")) return() + testthat::skip_if_not_installed(pkg) +} diff --git a/tests/testthat.R b/tests/testthat.R index 75a67445..63961a8e 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -1,4 +1,12 @@ -library(testthat) + library(callr) -test_check("callr") +if (callr:::is_false_check_env_var("_R_CHECK_FORCE_SUGGESTS_")) { + if (requireNamespace("testthat", quietly = TRUE)) { + library(testthat) + test_check("callr") + } +} else { + library(testthat) + test_check("callr") +} diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index d37a4589..25677476 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -18,6 +18,7 @@ read_next <- function(x, timeout = 3000) { } has_locale <- function(l) { + skip_if_not_installed("withr") has <- TRUE tryCatch( withr::with_locale(c(LC_CTYPE = l), "foobar"), @@ -99,6 +100,7 @@ test_paths <- function(callr_drop, callr_keep, test_temp_file <- function(fileext = "", pattern = "test-file-", envir = parent.frame(), create = TRUE) { + skip_if_not_installed("withr") tmp <- tempfile(pattern = pattern, fileext = fileext) if (identical(envir, .GlobalEnv)) { message("Temporary files will _not_ be cleaned up") @@ -126,6 +128,7 @@ expect_error <- function(..., class = "error") { } test_package_root <- function() { + skip_if_not_installed("rprojroot") x <- tryCatch( rprojroot::find_package_root_file(), error = function(e) NULL) @@ -162,6 +165,8 @@ without_env <- function(f) { expect_r_process_snapshot <- function(..., interactive = TRUE, echo = TRUE, transform = NULL, variant = NULL) { + skip_if_not_installed("asciicast") + skip_if_not_installed("withr") # errors.R assumes non-interactive in testthat, but we don't want that withr::local_envvar(TESTTHAT = NA_character_) dots <- eval(substitute(alist(...))) diff --git a/tests/testthat/test-bugs.R b/tests/testthat/test-bugs.R index 77a95500..561b08c1 100644 --- a/tests/testthat/test-bugs.R +++ b/tests/testthat/test-bugs.R @@ -1,5 +1,6 @@ test_that("repos is a list, #82", { + skip_if_not_installed("withr") expect_true(withr::with_options( list(repos = list(CRAN = "https://cloud.r-project.org")), callr::r(function() inherits(getOption("repos"), "list")) diff --git a/tests/testthat/test-clean-subprocess.R b/tests/testthat/test-clean-subprocess.R index aa5f7ea1..bababa29 100644 --- a/tests/testthat/test-clean-subprocess.R +++ b/tests/testthat/test-clean-subprocess.R @@ -1,6 +1,7 @@ test_that("r() does not load anything", { skip_in_covr() + skip_if_not_installed("withr") pkgs <- withr::with_envvar( clean_envvars(), r(without_env(function() loadedNamespaces()))) @@ -11,6 +12,7 @@ test_that("r() does not load anything", { test_that("r_bg() does not load anything", { skip_in_covr() + skip_if_not_installed("withr") p <- withr::with_envvar( clean_envvars(), r_bg(without_env(function() loadedNamespaces()))) @@ -24,6 +26,7 @@ test_that("r_bg() does not load anything", { test_that("r_session does not load anything", { skip_in_covr() + skip_if_not_installed("withr") rs <- withr::with_envvar(clean_envvars(), r_session$new()) on.exit(rs$close(), add = TRUE) pkgs <- rs$run(without_env(function() loadedNamespaces())) diff --git a/tests/testthat/test-error.R b/tests/testthat/test-error.R index 086a70bd..58c78592 100644 --- a/tests/testthat/test-error.R +++ b/tests/testthat/test-error.R @@ -31,6 +31,7 @@ test_that("error stack is passed, .Last.error is set", { }) test_that("error behavior can be set using option", { + skip_if_not_installed("withr") withr::local_options(callr.error = "error") expect_snapshot( error = TRUE, @@ -51,6 +52,7 @@ test_that("error behavior can be set using option", { }) test_that("parent errors", { + skip_if_not_installed("withr") withr::local_options(list("callr.error" = "error")) expect_snapshot({ err <- tryCatch( @@ -62,6 +64,7 @@ test_that("parent errors", { }) test_that("parent errors, another level", { + skip_if_not_installed("withr") withr::local_options(list("callr.error" = "error")) expect_snapshot({ err <- tryCatch( @@ -85,6 +88,7 @@ test_that("error traces are printed recursively", { }) test_that("errors in r_bg() are merged", { + skip_if_not_installed("withr") withr::local_options(list("callr.error" = "error")) p <- r_bg(function() 1 + "A") @@ -98,6 +102,7 @@ test_that("errors in r_bg() are merged", { }) test_that("errors in r_process are merged", { + skip_if_not_installed("withr") withr::local_options(list("callr.error" = "error")) opts <- r_process_options(func = function() 1 + "A") @@ -195,6 +200,7 @@ test_that("format.call_status_error", { }) test_that("format.call_status_error 2", { + skip_if_not_installed("withr") expect_r_process_snapshot( withr::local_options(rlib_error_always_trace = TRUE), err <- tryCatch( diff --git a/tests/testthat/test-eval.R b/tests/testthat/test-eval.R index 14dfbd5a..e37f2cbb 100644 --- a/tests/testthat/test-eval.R +++ b/tests/testthat/test-eval.R @@ -82,6 +82,7 @@ test_that("stdout and stderr in the same file", { }) test_that("profiles are used as requested", { + skip_if_not_installed("withr") do <- function(system, user) { tmp1 <- tempfile() tmp2 <- tempfile() @@ -114,6 +115,7 @@ test_that("profiles are used as requested", { }) test_that(".Renviron is used, but lib path is set over it", { + skip_if_not_installed("withr") dir.create(tmp <- tempfile()) on.exit(unlink(tmp, recursive = TRUE), add = TRUE) withr::with_dir(tmp, { diff --git a/tests/testthat/test-libpath.R b/tests/testthat/test-libpath.R index fe2806b0..51fd1f6a 100644 --- a/tests/testthat/test-libpath.R +++ b/tests/testthat/test-libpath.R @@ -16,6 +16,7 @@ test_that(".Library.site", { }) test_that(".libPaths()", { + skip_if_not_installed("withr") dir.create(tmp <- tempfile()) on.exit(unlink(tmp, recursive = TRUE)) @@ -35,6 +36,7 @@ test_that("if .Renviron overrides R_PROFILE", { ## But we still need to use the proper lib path, as set in the fake ## profile skip_in_covr() + skip_if_not_installed("withr") cat("Sys.setenv(FOO='nope')\n", file = tmp_prof <- tempfile()) cat("R_PROFILE=\"", tmp_prof, "\"\n", file = tmp_env <- tempfile(), sep = "") @@ -64,6 +66,7 @@ test_that("libpath in system(), empty .Renviron", { # We remove the library with covr from the lib path, so this # cannot work in a subprocess. skip_in_covr() + skip_if_not_installed("withr") dir.create(tmpdrop <- tempfile("drop")) dir.create(tmpkeep <- tempfile("keep")) @@ -86,6 +89,7 @@ test_that("libpath in system, R_LIBS in .Renviron", { # We remove the library with covr from the lib path, so this # cannot work in a subprocess. skip_in_covr() + skip_if_not_installed("withr") dir.create(tmpdrop <- tempfile("drop")) dir.create(tmpkeep <- tempfile("keep")) @@ -109,6 +113,7 @@ test_that("libpath in system, R_LIBS", { # We remove the library with covr from the lib path, so this # cannot work in a subprocess. skip_in_covr() + skip_if_not_installed("withr") dir.create(tmpdrop <- tempfile("drop")) dir.create(tmpkeep <- tempfile("keep")) @@ -132,6 +137,7 @@ test_that("libpath in system, R_LIBS and .Renviron", { # We remove the library with covr from the lib path, so this # cannot work in a subprocess. skip_in_covr() + skip_if_not_installed("withr") dir.create(tmpdrop <- tempfile("drop")) dir.create(tmpkeep <- tempfile("keep")) diff --git a/tests/testthat/test-load-client.R b/tests/testthat/test-load-client.R index 44314cda..a4755fff 100644 --- a/tests/testthat/test-load-client.R +++ b/tests/testthat/test-load-client.R @@ -14,6 +14,7 @@ test_that("load_client_lib", { }) test_that("errors", { + skip_if_not_installed("mockery") mockery::stub(load_client_lib, "system.file", "") expect_error( load_client_lib(), @@ -22,6 +23,7 @@ test_that("errors", { }) test_that("errors 2", { + skip_if_not_installed("mockery") sofile <- system.file( "libs", .Platform$r_arch, paste0("client", .Platform$dynlib.ext), package = "processx" diff --git a/tests/testthat/test-messages.R b/tests/testthat/test-messages.R index f099ee1c..587d105c 100644 --- a/tests/testthat/test-messages.R +++ b/tests/testthat/test-messages.R @@ -1,5 +1,6 @@ test_that("messages in callr::r do not crash session", { + skip_if_not_installed("cli") ret <- r(function() { cli::cli_text("fooobar"); 1 + 1 }) expect_identical(ret, 2) gc() @@ -8,6 +9,7 @@ test_that("messages in callr::r do not crash session", { test_that("messages in callr::r_bg do not crash session", { skip_in_covr() # TODO: what wrong with this on Windows? skip_on_cran() + skip_if_not_installed("cli") rx <- r_bg(function() { cli::cli_text("fooobar"); 1 + 1 }) rx$wait(5000) diff --git a/tests/testthat/test-presets.R b/tests/testthat/test-presets.R index 4d7119dd..4d274e6a 100644 --- a/tests/testthat/test-presets.R +++ b/tests/testthat/test-presets.R @@ -1,5 +1,6 @@ test_that("r", { + skip_if_not_installed("withr") withr::with_options( list(repos = "foobar"), @@ -34,6 +35,7 @@ test_that("r_safe", { ## https://github.com/r-lib/callr/issues/66 test_that("names of getOption('repos') are preserved", { + skip_if_not_installed("withr") repos <- withr::with_options( list(repos = c(foo = "bar")), callr::r(function() getOption("repos")) diff --git a/tests/testthat/test-r-session-messages.R b/tests/testthat/test-r-session-messages.R index 110468fc..f8022897 100644 --- a/tests/testthat/test-r-session-messages.R +++ b/tests/testthat/test-r-session-messages.R @@ -34,6 +34,7 @@ test_that("callr_message, then error", { }) test_that("message handlers", { + skip_if_not_installed("withr") rs <- r_session$new() on.exit(rs$kill(), add = TRUE) @@ -58,6 +59,7 @@ test_that("message handlers", { }) test_that("large messages", { + skip_if_not_installed("withr") rs <- r_session$new() on.exit(rs$close(), add = TRUE) diff --git a/tests/testthat/test-r-session.R b/tests/testthat/test-r-session.R index 7c351093..fd6aa950 100644 --- a/tests/testthat/test-r-session.R +++ b/tests/testthat/test-r-session.R @@ -269,6 +269,7 @@ test_that("custom load hook", { }) test_that("traceback", { + skip_if_not_installed("withr") withr::local_options(callr.traceback = TRUE) rs <- r_session$new() on.exit(rs$kill(), add = TRUE) @@ -301,6 +302,7 @@ test_that("error in the load hook", { }) test_that("fds are not leaked", { + skip_if_not_installed("ps") rs <- r_session$new() on.exit(rs$kill(), add = TRUE) diff --git a/tests/testthat/test-rcmd.R b/tests/testthat/test-rcmd.R index aba6f1ee..9d05c47d 100644 --- a/tests/testthat/test-rcmd.R +++ b/tests/testthat/test-rcmd.R @@ -38,6 +38,7 @@ test_that("wd argument", { }) test_that("fail_on_status", { + skip_if_not_installed("withr") rand <- tempfile() expect_error( withr::with_dir( diff --git a/tests/testthat/test-spelling.R b/tests/testthat/test-spelling.R index 8c862864..8aa6195f 100644 --- a/tests/testthat/test-spelling.R +++ b/tests/testthat/test-spelling.R @@ -2,6 +2,7 @@ test_that("spell check", { skip_on_cran() skip_in_covr() + skip_if_not_installed("spelling") pkg_dir <- test_package_root() results <- spelling::spell_check_package(pkg_dir) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 1377d36d..31340727 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -1,5 +1,6 @@ test_that("is_complete_expression", { + skip_if_not_installed("withr") do_tests <- function() { expect_true(is_complete_expression("")) expect_true(is_complete_expression("1")) @@ -20,6 +21,7 @@ test_that("is_complete_expression", { }) test_that("default_repos", { + skip_if_not_installed("withr") def <- "https://cloud.r-project.org" withr::with_options(list(repos = NULL), From 701a98c336f05b777ea2645df1febd5fa82ea2e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Cs=C3=A1rdi?= Date: Fri, 22 Mar 2024 13:23:48 +0100 Subject: [PATCH 2/4] Skip test if withr is not installed --- tests/testthat/test-rcmd.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-rcmd.R b/tests/testthat/test-rcmd.R index 7d27631f..59ac6c73 100644 --- a/tests/testthat/test-rcmd.R +++ b/tests/testthat/test-rcmd.R @@ -11,6 +11,7 @@ test_that("rcmd show works", { }) test_that("rcmd echo works", { + skip_if_not_installed("withr") withr::local_options(width = 500) expect_output(rcmd("config", "CC", echo = TRUE), "config\\s+CC") gc() From 271e9a8dc4e931f58d84ec8788fe1aa9defea298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Cs=C3=A1rdi?= Date: Fri, 22 Mar 2024 15:23:50 +0100 Subject: [PATCH 3/4] Trying to fix GHA Need an older magick package on Windows && R < 4.0.0 --- .github/workflows/R-CMD-check.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index ee65ccb5..cea499f8 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -47,12 +47,20 @@ jobs: - uses: r-lib/actions/setup-pandoc@v2 - uses: r-lib/actions/setup-r@v2 + id: setup-r with: r-version: ${{ matrix.config.r }} http-user-agent: ${{ matrix.config.http-user-agent }} use-public-rspm: true - uses: r-lib/actions/setup-r-dependencies@v2 + if: ${{ startsWith(steps.setup-r.outputs.installed-r-version, '3') && runner.os == 'Windows' }} + with: + extra-packages: any::rcmdcheck, any::magick@2.8.2 + needs: check + + - uses: r-lib/actions/setup-r-dependencies@v2 + if: ${{ ! startsWith(steps.setup-r.outputs.installed-r-version, '3') || runner.os != 'Windows' }} with: extra-packages: any::rcmdcheck needs: check From bc1a5bdc08d76b57e77d5c83d6ae41dacaaa1083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Cs=C3=A1rdi?= Date: Fri, 22 Mar 2024 15:48:20 +0100 Subject: [PATCH 4/4] Give up on R 3.6.x on Windows :( Can't build an older version of magick, either. --- .github/workflows/R-CMD-check.yaml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index cea499f8..9f52973d 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -25,8 +25,6 @@ jobs: - {os: macos-latest, r: 'release'} - {os: windows-latest, r: 'release'} - # Use 3.6 to trigger usage of RTools35 - - {os: windows-latest, r: '3.6'} # use 4.1 to check with rtools40's older compiler - {os: windows-latest, r: '4.1'} @@ -54,13 +52,6 @@ jobs: use-public-rspm: true - uses: r-lib/actions/setup-r-dependencies@v2 - if: ${{ startsWith(steps.setup-r.outputs.installed-r-version, '3') && runner.os == 'Windows' }} - with: - extra-packages: any::rcmdcheck, any::magick@2.8.2 - needs: check - - - uses: r-lib/actions/setup-r-dependencies@v2 - if: ${{ ! startsWith(steps.setup-r.outputs.installed-r-version, '3') || runner.os != 'Windows' }} with: extra-packages: any::rcmdcheck needs: check