Entrace unexpected errors in snapshots (#2283)

This ensures that their backtraces point to the source of the error, not the internals of `expect_snapshot()`

Fixes #2277

Author: hadley

NEWS.md

@@ -46,6 +46,7 @@
 * `expect_snapshot()` on reports how to resolve failures once when running inside `R CMD check`.
 * `expect_snapshot()` no longer skips on CRAN, as that skips the rest of the test. Instead it just returns, neither succeeding nor failing (#1585).
 * `expect_snapshot()` and `expect_snapshot_file()` hints now include the path to the package, if it's not the current working directory (#1577).
+* `expect_snapshot()` gives a more informative backtrace when the code inside the snapshot errors (#2277).
 * `expect_snapshot_file()` now clearly errors if the `path` doesn't exist (#2191), or has been used alreaday (#1592). It now considers `.json` to be a text file (#1593), and shows differences for text files in the console (#1593).
 * `expect_snapshot_value()` can now handle expressions that generate `-` (#1678) or zero length atomic vectors (#2042).
 * `expect_vector()` fails, instead of erroring, if `object` is not a vector (@plietar, #2224).

R/verify-output.R

@@ -84,7 +84,10 @@ verify_exec <- function(expr, env = caller_env(), replay = output_replay) {
   })
   source <- unlist(exprs, recursive = FALSE)
 
-  handler <- evaluate::new_output_handler(value = testthat_print)
+  handler <- evaluate::new_output_handler(
+    value = testthat_print,
+    calling_handlers = list(error = function(cnd) rlang::entrace(cnd))
+  )
   results <- evaluate::evaluate(
     source,
     envir = env,

tests/testthat/_snaps/reporter-progress.md

@@ -192,7 +192,8 @@
     - | 7 1      1 | reporters/backtraces                                           
     \ | 8 1      1 | reporters/backtraces                                           
     | | 9 1      1 | reporters/backtraces                                           
-    x | 9 1      1 | reporters/backtraces
+    / | 10 1      1 | reporters/backtraces                                          
+    x | 10 1      1 | reporters/backtraces
     --------------------------------------------------------------------------------
     Error ('reporters/backtraces.R:3:8'): errors thrown at block level are entraced
     Error in `g()`: foo
@@ -294,7 +295,15 @@
      25.                                                 \-f(x - 1)
      26.                                                   \-f(x - 1)
 
-    Failure ('reporters/backtraces.R:62:6'): (code run outside of `test_that()`)
+    Error ('reporters/backtraces.R:64:3'): errors in snapshots get useful backtraces
+    Error in `h()`: !
+    Backtrace:
+        x
+     1. \-f()
+     2.   \-g()
+     3.     \-h()
+    
+    Failure ('reporters/backtraces.R:70:6'): (code run outside of `test_that()`)
     Expected FALSE to be TRUE.
     Differences:
     `actual`:   FALSE
@@ -307,7 +316,7 @@
      3.     \-h()
      4.       \-testthat::expect_true(FALSE)
 
-    Failure ('reporters/backtraces.R:67:3'): nested expectations get backtraces
+    Failure ('reporters/backtraces.R:75:3'): nested expectations get backtraces
     Expected FALSE to be TRUE.
     Differences:
     `actual`:   FALSE
@@ -416,7 +425,15 @@
      25.                                                 \-f(x - 1)
      26.                                                   \-f(x - 1)
 
-    Failure ('reporters/backtraces.R:62:6'): (code run outside of `test_that()`)
+    Error ('reporters/backtraces.R:64:3'): errors in snapshots get useful backtraces
+    Error in `h()`: !
+    Backtrace:
+        x
+     1. \-f()
+     2.   \-g()
+     3.     \-h()
+    
+    Failure ('reporters/backtraces.R:70:6'): (code run outside of `test_that()`)
     Expected FALSE to be TRUE.
     Differences:
     `actual`:   FALSE
@@ -429,7 +446,7 @@
      3.     \-h()
      4.       \-testthat::expect_true(FALSE)
 
-    Failure ('reporters/backtraces.R:67:3'): nested expectations get backtraces
+    Failure ('reporters/backtraces.R:75:3'): nested expectations get backtraces
     Expected FALSE to be TRUE.
     Differences:
     `actual`:   FALSE
@@ -442,7 +459,7 @@
      3.     \-h()
      4.       \-testthat::expect_true(FALSE)
 
-    [ FAIL 9 | WARN 1 | SKIP 0 | PASS 1 ]
+    [ FAIL 10 | WARN 1 | SKIP 0 | PASS 1 ]
 
     No one gets it right on their first try
 

tests/testthat/_snaps/snapshot-reporter.md

@@ -3,5 +3,5 @@
     Code
       err$message
     Output
-      [1] "Expected NULL to throw a error."
+      [1] "Error: Expected NULL to throw a error."
 

tests/testthat/_snaps/snapshot.md

@@ -243,7 +243,7 @@
     Condition <simpleWarning>
       Warning in `f()`:
       bar
-    Condition <simpleError>
+    Condition <rlang_error>
       Error in `f()`:
       ! baz
 

tests/testthat/reporters/backtraces.R

@@ -56,6 +56,14 @@ test_that("deep stacks are shown", {
   f(25)
 })
 
+test_that("errors in snapshots get useful backtraces", {
+  f <- function() g()
+  g <- function() h()
+  h <- function() stop("!")
+
+  expect_snapshot(f())
+})
+
 # Expectations ----------------------------------------------------------------
 f <- function() g()
 g <- function() h()

tests/testthat/test-reporter-progress.R

@@ -43,6 +43,7 @@ test_that("can fully suppress incremental updates", {
 })
 
 test_that("reports backtraces", {
+  withr::local_envvar(TESTTHAT_MAX_FAILS = Inf)
   expect_snapshot_reporter(
     ProgressReporter$new(update_interval = 0, min_time = Inf),
     test_path("reporters/backtraces.R")