diff --git a/NEWS.md b/NEWS.md index b7c476749..bf297564e 100644 --- a/NEWS.md +++ b/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 (#) diff --git a/R/expect-match.R b/R/expect-match.R index c57e1aefd..888858613 100644 --- a/R/expect-match.R +++ b/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 ) diff --git a/R/utils.R b/R/utils.R index d2eeb0a99..52eaeef6e 100644 --- a/R/utils.R +++ b/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") } diff --git a/man/expect_match.Rd b/man/expect_match.Rd index 7c44e680a..163857a6f 100644 --- a/man/expect_match.Rd +++ b/man/expect_match.Rd @@ -79,12 +79,11 @@ expect_match("Testing is fun", "fun") 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(), ".")) } \seealso{ Other expectations: diff --git a/tests/testthat/_snaps/expect-match.md b/tests/testthat/_snaps/expect-match.md new file mode 100644 index 000000000..07b6533a4 --- /dev/null +++ b/tests/testthat/_snaps/expect-match.md @@ -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" + diff --git a/tests/testthat/_snaps/expect-output.md b/tests/testthat/_snaps/expect-output.md index 286a9acd0..6374bc930 100644 --- a/tests/testthat/_snaps/expect-output.md +++ b/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: "!" --- diff --git a/tests/testthat/_snaps/expect-self-test.md b/tests/testthat/_snaps/expect-self-test.md index 5b559af52..06a684c74 100644 --- a/tests/testthat/_snaps/expect-self-test.md +++ b/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 diff --git a/tests/testthat/test-expect-match.R b/tests/testthat/test-expect-match.R index 2a28d3911..447a5b15b 100644 --- a/tests/testthat/test-expect-match.R +++ b/tests/testthat/test-expect-match.R @@ -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")) })