Improve ... checking logic for 3e expect_error() (and friends)

Fixes #1932

NEWS.md

@@ -1,5 +1,6 @@
 # testthat (development version)
 
+* `expect_error()` and friends now error if you supply `...` but not `pattern` (#1932).
 * `expect_true()` and `expect_false()` give better errors if `actual` isn't a vector (#1996).
 * `expect_no_*()` expectations no longer incorrectly emit a passing test result if they in fact fail (#1997).
 * Require the latest version of waldo (0.6.0) in order to get the latest goodies (#1955).

R/expect-condition.R

@@ -256,10 +256,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,
@@ -301,6 +297,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(
+      "{.arg ...} ignored when {.arg pattern} is not set.",
+      call = error_call
+    )
+  }
+
   function(cnd) {
     if (!inherit) {
       cnd$parent <- NULL
@@ -318,7 +321,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 message to {.arg pattern}.",
+              parent = e,
+              call = error_call
+            )
+          }
+        )
+
       } else {
         TRUE
       }

tests/testthat/_snaps/expect-condition.md

@@ -29,14 +29,18 @@
 
     `f1()` did not throw a condition with class <bar>.
 
-# 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:
+      ! `...` ignored when `pattern` is not set.
+    Code
+      expect_condition(stop("Hi!"), "x", foo = "bar")
+    Condition
+      Error:
+      ! Failed to compare message to `pattern`.
+      Caused by error in `grepl()`:
+      ! unused argument (foo = "bar")
 

tests/testthat/test-expect-condition.R

@@ -217,8 +217,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")
+  })
 })