diff --git a/NEWS.md b/NEWS.md index c1877aff1..c9028b2b7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `expect_error()` and friends now error if you supply `...` but not `pattern` (#1932). * New `expect_no_failure()`, `expect_no_success()` and `expect_snapshot_failure()` provide more options for testing expectations. * `expect_error()` and friends no longer give an uninformative error if they fail inside a magrittr pipe (#1994). * `expect_setequal()` correctly identifies what is missing where (#1962). diff --git a/R/expect-condition.R b/R/expect-condition.R index ce5c1d4a6..f032b5573 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -253,11 +253,6 @@ expect_condition_matching <- function(base_class, label = NULL, trace_env = caller_env(), error_call = caller_env()) { - - check_dots_used(error = function(cnd) { - warn(conditionMessage(cnd), call = error_call) - }) - matcher <- cnd_matcher( base_class, class, @@ -299,6 +294,13 @@ cnd_matcher <- function(base_class, 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 + ) + } + function(cnd) { if (!inherit) { cnd$parent <- NULL @@ -316,7 +318,17 @@ cnd_matcher <- function(base_class, return(FALSE) } if (!is.null(pattern) && !isNA(pattern)) { - grepl(pattern, conditionMessage(x), ...) + withCallingHandlers( + grepl(pattern, conditionMessage(x), ...), + error = function(e) { + cli::cli_abort( + "Failed to compare {base_class} to {.arg pattern}.", + parent = e, + call = error_call + ) + } + ) + } else { TRUE } diff --git a/tests/testthat/_snaps/expect-condition.md b/tests/testthat/_snaps/expect-condition.md index 7afc7df7d..c46f356ac 100644 --- a/tests/testthat/_snaps/expect-condition.md +++ b/tests/testthat/_snaps/expect-condition.md @@ -29,14 +29,18 @@ `f1()` did not throw a condition with class . -# unused arguments generate a warning +# unused arguments generate an error Code expect_condition(stop("Hi!"), foo = "bar") Condition - Warning in `expect_condition()`: - Arguments in `...` must be used. - x Problematic argument: - * foo = "bar" - i Did you misspell an argument name? + Error in `expect_condition()`: + ! Can't specify `...` without `pattern`. + Code + expect_condition(stop("Hi!"), "x", foo = "bar") + Condition + Error in `expect_condition()`: + ! Failed to compare condition to `pattern`. + Caused by error in `grepl()`: + ! unused argument (foo = "bar") diff --git a/tests/testthat/test-expect-condition.R b/tests/testthat/test-expect-condition.R index 4ee1a9234..33e3d1711 100644 --- a/tests/testthat/test-expect-condition.R +++ b/tests/testthat/test-expect-condition.R @@ -223,8 +223,11 @@ test_that("can match parent conditions (#1493)", { expect_error(expect_error(f(), "Parent message.", inherit = FALSE)) }) -test_that("unused arguments generate a warning", { - expect_snapshot(expect_condition(stop("Hi!"), foo = "bar")) +test_that("unused arguments generate an error", { + expect_snapshot(error = TRUE, { + expect_condition(stop("Hi!"), foo = "bar") + expect_condition(stop("Hi!"), "x", foo = "bar") + }) })