diff --git a/R/expect-condition.R b/R/expect-condition.R index 6929944aa..b292a1c64 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -280,6 +280,7 @@ expect_condition_matching <- function( trace_env = caller_env(), error_call = caller_env() ) { + check_condition_dots(regexp, ..., error_call = error_call) matcher <- cnd_matcher( base_class, class, @@ -320,21 +321,14 @@ expect_condition_matching <- function( cnd_matcher <- function( base_class, class = NULL, - pattern = NULL, + regexp = NULL, ..., inherit = TRUE, ignore_deprecation = FALSE, error_call = caller_env() ) { check_string(class, allow_null = TRUE, call = error_call) - check_string(pattern, allow_null = TRUE, allow_na = TRUE, call = error_call) - - if (is.null(pattern) && dots_n(...) > 0) { - cli::cli_abort( - "Can't specify {.arg ...} without {.arg pattern}.", - call = error_call - ) - } + check_string(regexp, allow_null = TRUE, allow_na = TRUE, call = error_call) function(cnd) { if (!inherit) { @@ -352,12 +346,12 @@ cnd_matcher <- function( if (!is.null(class) && !inherits(x, class)) { return(FALSE) } - if (!is.null(pattern) && !isNA(pattern)) { + if (!is.null(regexp) && !isNA(regexp)) { withCallingHandlers( - grepl(pattern, conditionMessage(x), ...), + grepl(regexp, conditionMessage(x), ...), error = function(e) { cli::cli_abort( - "Failed to compare {base_class} to {.arg pattern}.", + "Failed to compare {base_class} to {.arg regexp}.", parent = e, call = error_call ) @@ -582,3 +576,29 @@ cnd_message <- function(x) { withr::local_options(rlang_backtrace_on_error = "none") conditionMessage(x) } + +check_condition_dots <- function( + regexp = NULL, + ..., + error_call = caller_env() +) { + if (!is.null(regexp) || missing(...)) { + return() + } + + dot_names <- ...names() + if (is.null(dot_names)) { + dot_names <- rep("", ...length()) + } + unnamed <- dot_names == "" + dot_names[unnamed] <- paste0("..", seq_along(dot_names)[unnamed]) + + cli::cli_abort( + c( + "Can't supply {.arg ...} unless {.arg regexp} is set.", + "*" = "Unused arguments: {.arg {dot_names}}.", + i = "Did you mean to use {.arg regexp} so {.arg ...} is passed to {.fn grepl}?" + ), + call = error_call + ) +} diff --git a/R/expect-no-condition.R b/R/expect-no-condition.R index ee35e9eed..b4b82fec0 100644 --- a/R/expect-no-condition.R +++ b/R/expect-no-condition.R @@ -76,7 +76,7 @@ expect_no_ <- function( matcher <- cnd_matcher( base_class, class, - pattern = regexp, + regexp = regexp, ignore_deprecation = base_class == "warning" && is.null(regexp) && is.null(class) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 4f22799ce..0b398d382 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -143,7 +143,11 @@ run_cpp_tests <- function(package) { c(line, line, 1, 1) ) - exp <- new_expectation("error", exception_text, srcref = exception_srcref) + exp <- new_expectation( + "error", + exception_text, + srcref = exception_srcref + ) exp$test <- test_name get_reporter()$add_result( diff --git a/tests/testthat/_snaps/expect-condition.md b/tests/testthat/_snaps/expect-condition.md index c46f356ac..915b6cc98 100644 --- a/tests/testthat/_snaps/expect-condition.md +++ b/tests/testthat/_snaps/expect-condition.md @@ -12,7 +12,7 @@ expect_error(stop("!"), regexp = 1) Condition Error in `expect_error()`: - ! `pattern` must be a single string, `NA`, or `NULL`, not the number 1. + ! `regexp` must be a single string, `NA`, or `NULL`, not the number 1. Code expect_error(stop("!"), class = 1) Condition @@ -35,12 +35,35 @@ expect_condition(stop("Hi!"), foo = "bar") Condition Error in `expect_condition()`: - ! Can't specify `...` without `pattern`. + ! Can't supply `...` unless `regexp` is set. + * Unused arguments: `foo`. + i Did you mean to use `regexp` so `...` is passed to `grepl()`? + Code + expect_condition(stop("Hi!"), , , "bar") + Condition + Error in `expect_condition()`: + ! Can't supply `...` unless `regexp` is set. + * Unused arguments: `..1`. + i Did you mean to use `regexp` so `...` is passed to `grepl()`? + Code + expect_condition(stop("Hi!"), , , "bar", fixed = TRUE) + Condition + Error in `expect_condition()`: + ! Can't supply `...` unless `regexp` is set. + * Unused arguments: `..1` and `fixed`. + i Did you mean to use `regexp` so `...` is passed to `grepl()`? Code expect_condition(stop("Hi!"), "x", foo = "bar") Condition Error in `expect_condition()`: - ! Failed to compare condition to `pattern`. + ! Failed to compare condition to `regexp`. Caused by error in `grepl()`: ! unused argument (foo = "bar") + Code + expect_condition(stop("Hi!"), pattern = "bar", fixed = TRUE) + Condition + Error in `expect_condition()`: + ! Can't supply `...` unless `regexp` is set. + * Unused arguments: `pattern` and `fixed`. + i Did you mean to use `regexp` so `...` is passed to `grepl()`? diff --git a/tests/testthat/test-catch.R b/tests/testthat/test-catch.R index 97ae7f8ed..068c4e61f 100644 --- a/tests/testthat/test-catch.R +++ b/tests/testthat/test-catch.R @@ -7,4 +7,5 @@ test_that("get_routine() fails when no routine exists", { expect_error(get_routine("utils", "no_such_routine")) }) +skip_if_not_installed("xml2") run_cpp_tests("testthat") diff --git a/tests/testthat/test-compare.R b/tests/testthat/test-compare.R index bfa4f79a1..87dd9f30a 100644 --- a/tests/testthat/test-compare.R +++ b/tests/testthat/test-compare.R @@ -196,8 +196,14 @@ test_that("base lengths must be identical", { }) test_that("tzones must be identical", { - t1 <- ISOdatetime(2016, 2, 29, 12, 13, 14, "EST") - t2 <- ISOdatetime(2016, 2, 29, 12, 13, 14, "US/Eastern") + # skip on minimal setups + tryCatch( + { + t1 <- ISOdatetime(2016, 2, 29, 12, 13, 14, "EST") + t2 <- ISOdatetime(2016, 2, 29, 12, 13, 14, "US/Eastern") + }, + warning = function(w) skip(conditionMessage(w)) + ) expect_match(compare(t1, t2)$message, '"tzone": 1 string mismatch') }) diff --git a/tests/testthat/test-expect-condition.R b/tests/testthat/test-expect-condition.R index 4ac6f83d0..fb7a16a6a 100644 --- a/tests/testthat/test-expect-condition.R +++ b/tests/testthat/test-expect-condition.R @@ -243,7 +243,10 @@ test_that("can match parent conditions (#1493)", { test_that("unused arguments generate an error", { expect_snapshot(error = TRUE, { expect_condition(stop("Hi!"), foo = "bar") + expect_condition(stop("Hi!"), , , "bar") + expect_condition(stop("Hi!"), , , "bar", fixed = TRUE) expect_condition(stop("Hi!"), "x", foo = "bar") + expect_condition(stop("Hi!"), pattern = "bar", fixed = TRUE) }) }) diff --git a/tests/testthat/test-expect-inheritance.R b/tests/testthat/test-expect-inheritance.R index f24abc0c6..4375b495c 100644 --- a/tests/testthat/test-expect-inheritance.R +++ b/tests/testthat/test-expect-inheritance.R @@ -75,6 +75,7 @@ test_that("checks its inputs", { }) test_that("can check with actual class", { + skip_if_not_installed("S7") Foo <- S7::new_class("Foo", package = NULL) Bar <- S7::new_class("Bar", package = NULL) expect_success(expect_s7_class(Foo(), class = Foo)) diff --git a/tests/testthat/test-reporter-junit.R b/tests/testthat/test-reporter-junit.R index cb90efa91..85ca845de 100644 --- a/tests/testthat/test-reporter-junit.R +++ b/tests/testthat/test-reporter-junit.R @@ -1,4 +1,5 @@ test_that("reporter doesn't change without warning", { + skip_if_not_installed("xml2") expect_snapshot_reporter(JunitReporterMock$new()) })