Make error from finding arguments in '...' in expect_condition (et al) more informative

R/expect-condition.R

@@ -286,17 +286,28 @@ expect_condition_matching <- function(base_class,
 
 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) {
+  check_string(regexp, allow_null = TRUE, allow_na = TRUE, call = error_call)
+
+  if (is.null(regexp) && dots_n(...) > 0) {
+    # pretty adversarial: expect_error(foo(), NULL, "error", 1)
+    dots_names <- ...names()
+    if (is.null(dots_names)) {
+      cli::cli_abort(
+        "Found unnamed arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}.",
+        call = error_call
+      )
+    }
     cli::cli_abort(
-      "Can't specify {.arg ...} without {.arg pattern}.",
+      c("Found arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}.",
+        "*" = "First problematic argument:",
+        "i" = dots_names[which(nzchar(dots_names))[1L]]
+      ),
       call = error_call
     )
   }
@@ -317,12 +328,12 @@ cnd_matcher <- function(base_class,
       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
             )

R/expect-no-condition.R

@@ -90,7 +90,7 @@ expect_no_ <- function(base_class,
   matcher <- cnd_matcher(
     base_class,
     class,
-    pattern = regexp,
+    regexp = regexp,
     ignore_deprecation = base_class == "warning" && is.null(regexp) && is.null(class)
   )
 

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,14 @@
       expect_condition(stop("Hi!"), foo = "bar")
     Condition
       Error in `expect_condition()`:
-      ! Can't specify `...` without `pattern`.
+      ! Found arguments in `...` to be passed to `grepl()`, but no `regexp`.
+      * First problematic argument:
+      i foo
     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")
 

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")

tests/testthat/test-compare.R

@@ -194,8 +194,11 @@ 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')
 })

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))

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())
 })