Ensure that expect_snapshot() registers restart (#2271)

Renamed `continue_test` to `muffle_expectation` to make it more clear what the purporse of this restart is — it's to allow us to opt-out of further handling.

Author: hadley

NEWS.md

@@ -850,7 +850,7 @@ This release mostly focusses on an overhaul of how testthat works with condition
 
 * New `exp_signal()` function is a condition signaller that
   implements the testthat protocol (signal with `stop()` if the
-  expectation is broken, with a `continue_test` restart).
+  expectation is broken, with a `muffle_expectation` restart).
 
 * Existence of restarts is first checked before invocation. This makes
   it possible to signal warnings or messages with a different

R/expect-self-test.R

@@ -11,13 +11,11 @@ capture_success_failure <- function(expr) {
     expectation_failure = function(cnd) {
       last_failure <<- cnd
       n_failure <<- n_failure + 1
-      # Don't bubble up to any other handlers
-      invokeRestart("continue_test")
+      invokeRestart("muffle_expectation")
     },
     expectation_success = function(cnd) {
       n_success <<- n_success + 1
-      # Don't bubble up to any other handlers
-      invokeRestart("continue_test")
+      invokeRestart("muffle_expectation")
     }
   )
 

R/expectation.R

@@ -10,7 +10,7 @@
 #'   optionally subsequence) elements should describe what was actually seen.
 #' @inheritParams fail
 #' @return An expectation object from either `succeed()` or `fail()`.
-#'   with a `continue_test` restart.
+#'   with a `muffle_expectation` restart.
 #' @seealso [exp_signal()]
 #' @keywords internal
 #' @export
@@ -104,7 +104,7 @@ exp_signal <- function(exp) {
     } else {
       signalCondition(exp)
     },
-    continue_test = function(e) NULL
+    muffle_expectation = function(e) NULL
   )
 
   invisible(exp)

R/snapshot.R

@@ -131,7 +131,12 @@ expect_snapshot_ <- function(
     if (error) {
       fail(msg, trace = state$error[["trace"]])
     } else {
-      cnd_signal(state$error)
+      # This might be a failed expectation, so we need to make sure
+      # that we can muffle it
+      withRestarts(
+        cnd_signal(state$error),
+        muffle_expectation = function() NULL
+      )
     }
     return()
   }

R/test-that.R

@@ -111,8 +111,7 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) {
   handle_expectation <- function(e) {
     the$test_expectations <- the$test_expectations + 1L
     register_expectation(e, 7)
-    # Don't bubble up to any other handlers
-    invokeRestart("continue_test")
+    invokeRestart("muffle_expectation")
   }
   handle_warning <- function(e) {
     # When options(warn) < 0, warnings are expected to be ignored.

man/expect.Rd

@@ -104,6 +104,14 @@
       Error:
       ! Expected `print("!")` to throw a error.
 
+# snapshots of failures fail
+
+    Code
+      expect_snapshot(fail())
+    Condition
+      Error:
+      ! Failure has been forced
+
 # can capture error/warning messages
 
     This is an error

tests/testthat/_snaps/snapshot.md

@@ -69,6 +69,10 @@ test_that("always checks error status", {
   expect_snapshot_failure(expect_snapshot(print("!"), error = TRUE))
 })
 
+test_that("snapshots of failures fail", {
+  expect_snapshot_failure(expect_snapshot(fail()))
+})
+
 test_that("can capture error/warning messages", {
   expect_snapshot_error(stop("This is an error"))
   expect_snapshot_warning(warning("This is a warning"))