Overhaul expect_matches() (#2138)

Use `expect_snapshot_failure()` to look at more of the messaging, and then use that to make a bunch of minor improvements. Also polish docs and improve argument checking.

Author: hadley

NEWS.md

@@ -1,5 +1,6 @@
 # testthat (development version)
 
+* `expect_matches()` failures should be a little easier to read (#2135).
 * New `local_on_cran(TRUE)` allows you to simulate how your tests will run on CRAN (#2112).
 * `expect_no_*()` now executes the entire code block, rather than stopping at the first message or warning (#1991).
 * `expect_no_failures()` and `expect_no_successes()` are now deprecated as `expect_success()` now test for no failures and `expect_failure()` tests for no successes (#)

R/expect-match.R

@@ -1,10 +1,11 @@
 #' Does a string match a regular expression?
 #'
 #' @details
-#' `expect_match()` is a wrapper around [grepl()]. See its documentation for
-#' more detail about the individual arguments. `expect_no_match()` provides
-#' the complementary case, checking that a string *does not* match a regular
-#' expression.
+#' `expect_match()` checks if a character vector matches a regular expression,
+#' powered by [grepl()].
+#'
+#' `expect_no_match()` provides the complementary case, checking that a
+#' character vector *does not* match a regular expression.
 #'
 #' @inheritParams expect_that
 #' @inheritParams base::grepl
@@ -21,12 +22,11 @@
 #' expect_match("Testing is fun", "f.n")
 #' expect_no_match("Testing is fun", "horrible")
 #'
-#' \dontrun{
-#' expect_match("Testing is fun", "horrible")
+#' show_failure(expect_match("Testing is fun", "horrible"))
+#' show_failure(expect_match("Testing is fun", "horrible", fixed = TRUE))
 #'
 #' # Zero-length inputs always fail
-#' expect_match(character(), ".")
-#' }
+#' show_failure(expect_match(character(), "."))
 expect_match <- function(
   object,
   regexp,
@@ -37,11 +37,14 @@ expect_match <- function(
   info = NULL,
   label = NULL
 ) {
-  # Capture here to avoid environment-related messiness
   act <- quasi_label(enquo(object), label, arg = "object")
-  stopifnot(is.character(regexp), length(regexp) == 1)
 
-  stopifnot(is.character(act$val))
+  check_character(object)
+  check_string(regexp)
+  check_bool(perl)
+  check_bool(fixed)
+  check_bool(all)
+
   if (length(object) == 0) {
     return(fail(sprintf("%s is empty.", act$lab), info = info))
   }
@@ -75,11 +78,7 @@ expect_no_match <- function(
   # Capture here to avoid environment-related messiness
   act <- quasi_label(enquo(object), label, arg = "object")
   stopifnot(is.character(regexp), length(regexp) == 1)
-
   stopifnot(is.character(act$val))
-  if (length(object) == 0) {
-    return(fail(sprintf("%s is empty.", act$lab), info = info))
-  }
 
   expect_match_(
     act = act,
@@ -114,20 +113,17 @@ expect_match_ <- function(
     return(pass(act$val))
   }
 
-  escape <- if (fixed) identity else escape_regex
-
+  text <- encodeString(act$val)
   if (length(act$val) == 1) {
-    values <- paste0("Actual value: \"", escape(encodeString(act$val)), "\"")
+    values <- paste0('Text: "', text, '"')
   } else {
-    values <- paste0(
-      "Actual values:\n",
-      paste0("* ", escape(encodeString(act$val)), collapse = "\n")
-    )
+    values <- paste0("Text:\n", paste0("* ", text, collapse = "\n"))
   }
 
   msg <- sprintf(
-    if (negate) "%s does match %s.\n%s" else "%s does not match %s.\n%s",
-    escape(act$lab),
+    if (negate) "%s matches %s %s.\n%s" else "%s does not match %s %s.\n%s",
+    act$lab,
+    if (fixed) "string" else "regexp",
     encodeString(regexp, quote = '"'),
     values
   )

R/utils.R

@@ -2,31 +2,6 @@
 #' @export
 magrittr::`%>%`
 
-escape_regex <- function(x) {
-  chars <- c(
-    "*",
-    ".",
-    "?",
-    "^",
-    "+",
-    "$",
-    "|",
-    "(",
-    ")",
-    "[",
-    "]",
-    "{",
-    "}",
-    "\\"
-  )
-  gsub(
-    paste0("([\\", paste0(collapse = "\\", chars), "])"),
-    "\\\\\\1",
-    x,
-    perl = TRUE
-  )
-}
-
 can_entrace <- function(cnd) {
   !inherits(cnd, "Throwable")
 }

man/expect_match.Rd

@@ -0,0 +1,57 @@
+# generates useful failure messages
+
+    `zero` is empty.
+
+---
+
+    `one` does not match regexp "asdf".
+    Text: "bcde"
+
+---
+
+    `many` does not match regexp "asdf".
+    Text:
+    * a
+    * b
+    * c
+    * d
+    * e
+
+# checks its inputs
+
+    Code
+      expect_match(1)
+    Condition
+      Error in `expect_match()`:
+      ! `object` must be a character vector, not the number 1.
+    Code
+      expect_match("x", 1)
+    Condition
+      Error in `expect_match()`:
+      ! `regexp` must be a single string, not the number 1.
+    Code
+      expect_match("x", "x", fixed = 1)
+    Condition
+      Error in `expect_match()`:
+      ! `fixed` must be `TRUE` or `FALSE`, not the number 1.
+    Code
+      expect_match("x", "x", perl = 1)
+    Condition
+      Error in `expect_match()`:
+      ! `perl` must be `TRUE` or `FALSE`, not the number 1.
+    Code
+      expect_match("x", "x", all = 1)
+    Condition
+      Error in `expect_match()`:
+      ! `all` must be `TRUE` or `FALSE`, not the number 1.
+
+# expect_no_match works
+
+    `x` matches string "e*".
+    Text: "te*st"
+
+---
+
+    `x` matches regexp "TEST".
+    Text: "test"
+

tests/testthat/_snaps/expect-match.md

@@ -1,7 +1,7 @@
 # expect = string checks for match
 
-    `g\(\)` does not match "x".
-    Actual value: "!"
+    `g()` does not match regexp "x".
+    Text: "!"
 
 ---
 

tests/testthat/_snaps/expect-output.md

@@ -1,7 +1,7 @@
 # expect_failure() can optionally match message
 
-    Failure message does not match "banana".
-    Actual value: "apple"
+    Failure message does not match regexp "banana".
+    Text: "apple"
 
 # errors in expect_success bubble up
 

tests/testthat/_snaps/expect-self-test.md

@@ -1,47 +1,42 @@
-test_that("extra arguments to matches passed onto grepl", {
-  expect_success(expect_match("te*st", "e*", fixed = TRUE))
-  expect_success(expect_match("test", "TEST", ignore.case = TRUE))
-})
+test_that("generates useful failure messages", {
+  zero <- character(0)
+  expect_snapshot_failure(expect_match(zero, 'asdf'))
 
-test_that("special regex characters are escaped in output", {
-  error <- tryCatch(
-    expect_match("f() test", "f() test"),
-    expectation = function(e) e$message
-  )
-  expect_equal(
-    error,
-    "\"f\\(\\) test\" does not match \"f() test\".\nActual value: \"f\\(\\) test\""
-  )
-})
+  one <- "bcde"
+  expect_snapshot_failure(expect_match(one, 'asdf'))
 
-test_that("correct reporting of expected label", {
-  expect_failure(expect_match("[a]", "[b]"), escape_regex("[a]"), fixed = TRUE)
-  expect_failure(expect_match("[a]", "[b]", fixed = TRUE), "[a]", fixed = TRUE)
+  many <- letters[1:5]
+  expect_snapshot_failure(expect_match(many, 'asdf'))
 })
 
-test_that("errors if obj is empty str", {
-  x <- character(0)
-  err <- expect_error(
-    expect_match(x, 'asdf'),
-    class = "expectation_failure"
-  )
-  expect_match(err$message, 'is empty')
+test_that("checks its inputs", {
+  expect_snapshot(error = TRUE, {
+    expect_match(1)
+    expect_match("x", 1)
+    expect_match("x", "x", fixed = 1)
+    expect_match("x", "x", perl = 1)
+    expect_match("x", "x", all = 1)
+  })
 })
 
-test_that("prints multiple unmatched values", {
-  err <- expect_error(
-    expect_match(letters[1:10], 'asdf'),
-    class = "expectation_failure"
-  )
-  expect_match(err$message, "does not match")
+test_that("extra arguments passed onto grepl", {
+  expect_failure(expect_match("\\s", "\\s"))
+  expect_success(expect_match("\\s", "\\s", fixed = TRUE))
+
+  expect_failure(expect_match("test", "TEST"))
+  expect_success(expect_match("test", "TEST", ignore.case = TRUE))
 })
 
 test_that("expect_no_match works", {
   expect_success(expect_no_match("[a]", "[b]"))
   expect_success(expect_no_match("[a]", "[b]", fixed = TRUE))
-  expect_failure(
-    expect_no_match("te*st", "e*", fixed = TRUE),
-    escape_regex("te*st")
-  )
-  expect_failure(expect_no_match("test", "TEST", ignore.case = TRUE), "test")
+
+  x <- "te*st"
+  expect_snapshot_failure(expect_no_match(x, "e*", fixed = TRUE))
+  x <- "test"
+  expect_snapshot_failure(expect_no_match(x, "TEST", ignore.case = TRUE))
+})
+
+test_that("empty string is never a match", {
+  expect_success(expect_no_match(character(), "x"))
 })