Overhaul expect_matches()

Use `expect_snapshot_failure()` to look at more of the messaging, and then use that to make a bunch of minor improvements.

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

@@ -75,11 +75,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 +110,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 does match %s %s.\n%s" else "%s does not match %s %s.\n%s",
+    act$lab,
+    if (fixed) "string" else "regexp",
     encodeString(regexp, quote = '"'),
     values
   )

tests/testthat/_snaps/expect-match.md

@@ -0,0 +1,29 @@
+# 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
+
+# expect_no_match works
+
+    `x` does match string "e*".
+    Text: "te*st"
+
+---
+
+    `x` does match regexp "TEST".
+    Text: "test"
+

tests/testthat/_snaps/expect-output.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-self-test.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/test-expect-match.R

@@ -1,47 +1,32 @@
-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("extra arguments passed onto grepl", {
+  expect_failure(expect_match("\\s", "\\s"))
+  expect_success(expect_match("\\s", "\\s", fixed = TRUE))
 
-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")
+  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"))
 })