From 2b36f1b94a8475d6064b1f1eb524432b5457ad5e Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 29 Oct 2024 14:17:49 -0500
Subject: [PATCH] Make interactive snapshot output a bit more obvious

Testing this change revealed a bug in `test_code()`: specifying the default reporter happened _after_ `local_test_context()` meaning that it failed to capture the actual user settings. I fixed this by now requiring a reporter (rather than a backup reporter).

Fixes #1992
---
 R/describe.R      |  3 ++-
 R/snapshot-file.R |  2 +-
 R/snapshot.R      | 12 +++++++-----
 R/source.R        |  2 +-
 R/test-example.R  |  2 +-
 R/test-that.R     |  8 +++++---
 6 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/R/describe.R b/R/describe.R
index 09a5e5e8d..94b662bf6 100644
--- a/R/describe.R
+++ b/R/describe.R
@@ -76,13 +76,14 @@ describe <- function(description, code) {
 }
 
 describe_it <- function(description, code, env = parent.frame()) {
+  reporter <- get_reporter() %||% local_interactive_reporter()
   local_test_context()
 
   test_code(
     description,
     code,
     env = env,
-    default_reporter = local_interactive_reporter(),
+    reporter = reporter,
     skip_on_empty = FALSE
   )
 }
diff --git a/R/snapshot-file.R b/R/snapshot-file.R
index 154302e68..3d5f0b5af 100644
--- a/R/snapshot-file.R
+++ b/R/snapshot-file.R
@@ -101,7 +101,7 @@ expect_snapshot_file <- function(path,
 
   snapshotter <- get_snapshotter()
   if (is.null(snapshotter)) {
-    snapshot_not_available(paste0("New path: ", path))
+    snapshot_not_available("New path", path)
     return(invisible())
   }
 
diff --git a/R/snapshot.R b/R/snapshot.R
index 421933f59..6b2f3e656 100644
--- a/R/snapshot.R
+++ b/R/snapshot.R
@@ -272,7 +272,7 @@ expect_snapshot_helper <- function(lab, val,
 
   snapshotter <- get_snapshotter()
   if (is.null(snapshotter)) {
-    snapshot_not_available(paste0("Current value:\n", save(val)))
+    snapshot_not_available("Current snapshot", save(val))
     return(invisible())
   }
 
@@ -322,12 +322,15 @@ snapshot_accept_hint <- function(variant, file, reset_output = TRUE) {
   )
 }
 
-snapshot_not_available <- function(message) {
+snapshot_not_available <- function(header, message) {
+  local_reporter_output()
+
   cli::cli_inform(c(
-    "{.strong Can't compare snapshot to reference when testing interactively.}",
-    i = "Run {.run devtools::test()} or {.code testthat::test_file()} to see changes."
+    i = "Can't create snapshot or compare to reference when testing interactively."
   ))
+  cat(cli::rule(header), "\n", sep = "")
   cat(message, "\n", sep = "")
+  cat(cli::rule(), "\n", sep = "")
 }
 
 local_snapshot_dir <- function(snap_names, .env = parent.frame()) {
@@ -362,4 +365,3 @@ with_is_snapshotting <- function(code) {
   withr::local_envvar(TESTTHAT_IS_SNAPSHOT = "true")
   code
 }
-
diff --git a/R/source.R b/R/source.R
index 82cffcac5..14a44f4c0 100644
--- a/R/source.R
+++ b/R/source.R
@@ -44,7 +44,7 @@ source_file <- function(path,
       test = NULL,
       code = exprs,
       env = env,
-      default_reporter = StopReporter$new()
+      reporter = get_reporter() %||% StopReporter$new()
     ))
   } else {
     withCallingHandlers(
diff --git a/R/test-example.R b/R/test-example.R
index c4e20eb25..f8625bb71 100644
--- a/R/test-example.R
+++ b/R/test-example.R
@@ -65,7 +65,7 @@ test_example <- function(path, title = path) {
     test = title,
     code = parse(ex_path, encoding = "UTF-8"),
     env = env,
-    default_reporter = StopReporter$new(),
+    reporter = get_reporter() %||% StopReporter$new(),
     skip_on_empty = FALSE
   )
   if (ok) succeed(path)
diff --git a/R/test-that.R b/R/test-that.R
index 090a9f5c7..8b3ab2fc7 100644
--- a/R/test-that.R
+++ b/R/test-that.R
@@ -46,21 +46,23 @@ test_that <- function(desc, code) {
     }
   }
 
+  # Must initialise interactive reporter before local_test_context()
+  reporter <- get_reporter() %||% local_interactive_reporter()
   local_test_context()
+
   test_code(
     desc,
     code,
     env = parent.frame(),
-    default_reporter = local_interactive_reporter()
+    reporter = reporter
   )
 }
 
 # Access error fields with `[[` rather than `$` because the
 # `$.Throwable` from the rJava package throws with unknown fields
-test_code <- function(test, code, env, default_reporter, skip_on_empty = TRUE) {
+test_code <- function(test, code, env, reporter, skip_on_empty = TRUE) {
 
   frame <- caller_env()
-  reporter <- get_reporter() %||% default_reporter
 
   if (!is.null(test)) {
     reporter$start_test(context = reporter$.context, test = test)