Merge commit '6ba4397f440014d3ee9a24441e2a913c2d06e8fe'

Author: hadley

NAMESPACE

@@ -106,8 +106,10 @@ export(expect_more_than)
 export(expect_named)
 export(expect_no_condition)
 export(expect_no_error)
+export(expect_no_failure)
 export(expect_no_match)
 export(expect_no_message)
+export(expect_no_success)
 export(expect_no_warning)
 export(expect_null)
 export(expect_output)
@@ -119,6 +121,7 @@ export(expect_setequal)
 export(expect_silent)
 export(expect_snapshot)
 export(expect_snapshot_error)
+export(expect_snapshot_failure)
 export(expect_snapshot_file)
 export(expect_snapshot_output)
 export(expect_snapshot_value)

NEWS.md

@@ -1,6 +1,9 @@
 # testthat (development version)
 
 * `expect_error()` and friends now error if you supply `...` but not `pattern` (#1932).
+* New `expect_no_failure()`, `expect_no_success()` and `expect_snapshot_failure()` provide more options for testing expectations.
+* `expect_error()` and friends no longer give an uninformative error if they fail inside a magrittr pipe (#1994).
+* `expect_setequal()` correctly identifies what is missing where (#1962).
 * `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

@@ -163,8 +163,7 @@ expect_warning <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label,
-      trace_env = caller_env()
+      label = label
     )
   } else {
     act <- quasi_capture(enquo(object), label, capture_warnings, ignore_deprecation = identical(regexp, NA))
@@ -196,8 +195,7 @@ expect_message <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label,
-      trace_env = caller_env()
+      label = label
     )
   } else {
     act <- quasi_capture(enquo(object), label, capture_messages)
@@ -225,8 +223,7 @@ expect_condition <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label,
-      trace_env = caller_env()
+      label = label
     )
   } else {
 
@@ -263,7 +260,7 @@ expect_condition_matching <- function(base_class,
     ...,
     inherit = inherit,
     ignore_deprecation = base_class == "warning" && identical(regexp, NA),
-    error_call = trace_env
+    error_call = error_call
   )
 
   act <- quasi_capture(

R/expect-no-condition.R

@@ -82,10 +82,10 @@ expect_no_condition <- function(object,
 
 
 expect_no_ <- function(base_class,
-                            object,
-                            regexp = NULL,
-                            class = NULL,
-                            error_call = caller_env()) {
+                       object,
+                       regexp = NULL,
+                       class = NULL,
+                       trace_env = caller_env()) {
 
   matcher <- cnd_matcher(
     base_class,
@@ -116,7 +116,7 @@ expect_no_ <- function(base_class,
           indent_lines(rlang::cnd_message(cnd))
         )
         message <- format_error_bullets(c(expected, i = actual))
-        fail(message, trace_env = error_call)
+        fail(message, trace_env = trace_env)
       }
     )
   }

R/expect-self-test.R

@@ -1,23 +1,56 @@
+capture_failure <- new_capture("expectation_failure")
+capture_success <- function(expr) {
+  cnd <- NULL
+
+  withCallingHandlers(
+    expr,
+    expectation_failure = function(cnd) {
+      invokeRestart("continue_test")
+    },
+    expectation_success = function(cnd) {
+      cnd <<- cnd
+    }
+  )
+  cnd
+}
+
+new_capture("expectation_success")
+
 #' Tools for testing expectations
 #'
-#' Use these expectations to test other expectations.
+#' @description
+#' * `expect_sucess()` and `expect_failure()` check that there's at least
+#'   one success or failure respectively.
+#' * `expect_snapshot_failure()` records the failure message so that you can
+#'   manually check that it is informative.
+#' * `expect_no_success()` and `expect_no_failure()` check that are no
+#'   successes or failures.
+#'
 #' Use `show_failure()` in examples to print the failure message without
 #' throwing an error.
 #'
-#' @param expr Expression that evaluates a single expectation.
+#' @param expr Code to evalute
 #' @param message Check that the failure message matches this regexp.
 #' @param ... Other arguments passed on to [expect_match()].
 #' @export
 expect_success <- function(expr) {
-  exp <- capture_expectation(expr)
+  exp <- capture_success(expr)
 
   if (is.null(exp)) {
-    fail("no expectation used.")
-  } else if (!expectation_success(exp)) {
-    fail(paste0(
-      "Expectation did not succeed:\n",
-      exp$message
-    ))
+    fail("Expectation did not succeed")
+  } else {
+    succeed()
+  }
+  invisible(NULL)
+}
+
+#' @export
+#' @rdname expect_success
+expect_no_success <- function(expr) {
+  exp <- capture_success(expr)
+
+  if (!is.null(exp)) {
+    fail("Expectation succeeded")
   } else {
     succeed()
   }
@@ -27,19 +60,31 @@ expect_success <- function(expr) {
 #' @export
 #' @rdname expect_success
 expect_failure <- function(expr, message = NULL, ...) {
-  exp <- capture_expectation(expr)
+  exp <- capture_failure(expr)
 
   if (is.null(exp)) {
-    fail("No expectation used")
-    return()
-  }
-  if (!expectation_failure(exp)) {
     fail("Expectation did not fail")
-    return()
+  } else if (!is.null(message)) {
+    expect_match(exp$message, message, ...)
+  } else {
+    succeed()
   }
+  invisible(NULL)
+}
 
-  if (!is.null(message)) {
-    expect_match(exp$message, message, ...)
+#' @export
+#' @rdname expect_success
+expect_snapshot_failure <- function(expr) {
+  expect_snapshot_error(expr, "expectation_failure")
+}
+
+#' @export
+#' @rdname expect_success
+expect_no_failure <- function(expr) {
+  exp <- capture_failure(expr)
+
+  if (!is.null(exp)) {
+    fail("Expectation failed")
   } else {
     succeed()
   }
@@ -67,10 +112,6 @@ show_failure <- function(expr) {
   invisible()
 }
 
-expect_snapshot_failure <- function(x) {
-  expect_snapshot_error(x, "expectation_failure")
-}
-
 expect_snapshot_reporter <- function(reporter, paths = test_path("reporters/tests.R")) {
   local_options(rlang_trace_format_srcrefs = FALSE)
   local_rng_version("3.3")

R/expect-setequal.R

@@ -35,16 +35,16 @@ expect_setequal <- function(object, expected) {
     warn("expect_setequal() ignores names")
   }
 
-  act_miss <- !act$val %in% exp$val
-  exp_miss <- !exp$val %in% act$val
+  act_miss <- setdiff(act$val, exp$val)
+  exp_miss <- setdiff(exp$val, act$val)
 
-  if (any(exp_miss) || any(act_miss)) {
+  if (length(exp_miss) || length(act_miss)) {
     fail(paste0(
       act$lab, " (`actual`) and ", exp$lab, " (`expected`) don't have the same values.\n",
-      if (any(act_miss))
-        paste0("* Only in `expected`: ",  values(act$val[act_miss]), "\n"),
-      if (any(exp_miss))
-        paste0("* Only in `actual`: ",  values(exp$val[exp_miss]), "\n")
+      if (length(act_miss))
+        paste0("* Only in `actual`: ", values(act_miss), "\n"),
+      if (length(exp_miss))
+        paste0("* Only in `expected`: ", values(exp_miss), "\n")
     ))
   } else {
     succeed()

man/expect_success.Rd

@@ -34,12 +34,12 @@
     Code
       expect_condition(stop("Hi!"), foo = "bar")
     Condition
-      Error:
+      Error in `expect_condition()`:
       ! `...` ignored when `pattern` is not set.
     Code
       expect_condition(stop("Hi!"), "x", foo = "bar")
     Condition
-      Error:
+      Error in `expect_condition()`:
       ! Failed to compare message to `pattern`.
       Caused by error in `grepl()`:
       ! unused argument (foo = "bar")

tests/testthat/_snaps/expect-condition.md

@@ -1,26 +1,33 @@
 # useful message on failure
 
+    "actual" (`actual`) and "expected" (`expected`) don't have the same values.
+    * Only in `actual`: "actual"
+    * Only in `expected`: "expected"
+    
+
+---
+
     1:2 (`actual`) and 2 (`expected`) don't have the same values.
-    * Only in `expected`: 1
+    * Only in `actual`: 1
 
 
 ---
 
     2 (`actual`) and 2:3 (`expected`) don't have the same values.
-    * Only in `actual`: 3
+    * Only in `expected`: 3
 
 
 ---
 
     1:2 (`actual`) and 2:3 (`expected`) don't have the same values.
-    * Only in `expected`: 1
-    * Only in `actual`: 3
+    * Only in `actual`: 1
+    * Only in `expected`: 3
 
 
 # truncates long vectors
 
     1:2 (`actual`) and 1:50 (`expected`) don't have the same values.
-    * Only in `actual`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ...
+    * Only in `expected`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ...
 
 
 # expect_contains() gives useful message on failure

tests/testthat/_snaps/expect-setequal.md

@@ -76,6 +76,12 @@ test_that("can capture Throwable conditions from rJava", {
   expect_error(throw("foo"), "foo", class = "Throwable")
 })
 
+test_that("capture correct trace_env (#1994)", {
+  # This should fail, not error
+  expect_failure(expect_error(stop("oops")) %>% expect_warning())
+  expect_failure(expect_warning(expect_error(stop("oops"))))
+})
+
 # expect_warning() ----------------------------------------------------------
 
 test_that("warnings are converted to errors when options('warn') >= 2", {