Display diffs when snapshotting text files (#2205)

I haven't yet figured out how to test snapshots with snapshots, so you can run this to test interactively

```R
test_that("expect_snapshot_file() show differences for text files", {
  path1 <- withr::local_tempfile(lines = sample(letters[1:10]))
  expect_snapshot_file(path1, "text-diff.txt")
})
```

Fixes #1593

Author: hadley

NEWS.md

@@ -1,5 +1,7 @@
 # testthat (development version)
 
+* `expect_snapshot_file()` now considers `.json` to be a text file (#1593).
+* `expect_snapshot_file()` now shows differences for text files (#1593).
 * The failure messages for all `expect_` functions have been rewritten to first state what was expected and then what was actually received (#2142).
 * `test_file(desc = ...)` no longer loses snapshot results (#2066).
 * In `R CMD check`, snapshots now only advise on how to resolve failures once (#2207).

R/expect-that.R

@@ -48,6 +48,7 @@ fail <- function(
 
 snapshot_fail <- function(message, trace_env = caller_env()) {
   trace <- capture_trace(trace_env)
+  message <- paste(message, collapse = "\n")
   expectation("failure", message, trace = trace, snapshot = TRUE)
 }
 

R/snapshot-file.R

@@ -106,6 +106,8 @@ expect_snapshot_file <- function(
   transform = NULL,
   variant = NULL
 ) {
+  lab <- quo_label(enquo(path))
+
   check_string(path)
   check_string(name)
   check_bool(cran)
@@ -125,6 +127,7 @@ expect_snapshot_file <- function(
     return(invisible())
   }
 
+  is_text <- is_text_file(name)
   if (!is_missing(binary)) {
     lifecycle::deprecate_soft(
       "3.0.3",
@@ -134,8 +137,6 @@ expect_snapshot_file <- function(
     compare <- if (binary) compare_file_binary else compare_file_text
   }
   if (is.null(compare)) {
-    ext <- tools::file_ext(name)
-    is_text <- ext %in% c("r", "R", "txt", "md", "Rmd")
     compare <- if (is_text) compare_file_text else compare_file_binary
   }
 
@@ -145,7 +146,6 @@ expect_snapshot_file <- function(
     brio::write_lines(lines, path)
   }
 
-  lab <- quo_label(enquo(path))
   equal <- snapshotter$take_file_snapshot(
     name,
     path,
@@ -156,24 +156,46 @@ expect_snapshot_file <- function(
     return(equal)
   }
 
+  file <- snapshotter$file
   if (in_check_reporter()) {
     hint <- ""
   } else {
-    hint <- snapshot_review_hint(snapshotter$file, name)
+    hint <- snapshot_review_hint(file, name, is_text = is_text)
   }
 
   if (!equal) {
-    msg <- sprintf(
-      "Snapshot of %s to '%s' has changed\n%s",
-      lab,
-      paste0(snapshotter$file, "/", name),
+    if (is_text) {
+      base <- paste0(c(snapshotter$snap_dir, file, variant), collapse = "/")
+      old_path <- paste0(c(base, name), collapse = "/")
+      new_path <- paste0(c(base, new_name(name)), collapse = "/")
+
+      comp <- waldo_compare(
+        x = brio::read_lines(old_path),
+        x_arg = "old",
+        y = brio::read_lines(new_path),
+        y_arg = "new",
+        quote_strings = FALSE
+      )
+      comp <- c("Differences:", comp)
+    } else {
+      comp <- NULL
+    }
+
+    msg <- c(
+      sprintf("Snapshot of %s has changed.", lab),
+      comp,
       hint
     )
     return(snapshot_fail(msg))
   }
   pass(NULL)
 }
 
+is_text_file <- function(path) {
+  ext <- tools::file_ext(path)
+  ext %in% c("r", "R", "txt", "md", "Rmd", "qmd", "json")
+}
+
 #' @rdname expect_snapshot_file
 #' @export
 announce_snapshot_file <- function(path, name = basename(path)) {
@@ -185,13 +207,25 @@ announce_snapshot_file <- function(path, name = basename(path)) {
   }
 }
 
-snapshot_review_hint <- function(test, name, reset_output = TRUE) {
+snapshot_review_hint <- function(
+  test,
+  name,
+  is_text = FALSE,
+  reset_output = TRUE
+) {
   if (reset_output) {
     local_reporter_output()
   }
 
-  cli::format_inline(
-    "* Run {.run testthat::snapshot_review('{test}/{name}')} to review the change."
+  c(
+    if (is_text) {
+      cli::format_inline(
+        "* Run {.run testthat::snapshot_accept('{test}/{name}')} to accept the change."
+      )
+    },
+    cli::format_inline(
+      "* Run {.run testthat::snapshot_review('{test}/{name}')} to review the change."
+    )
   )
 }
 

tests/testthat/_snaps/snapshot-file.md

@@ -25,9 +25,18 @@
 # generates informative hint
 
     Code
-      cat(snapshot_review_hint("lala", "foo.r", reset_output = FALSE))
+      base::writeLines(snapshot_review_hint("lala", "foo.R", reset_output = FALSE))
     Output
-      * Run `testthat::snapshot_review('lala/foo.r')` to review the change.
+      * Run `testthat::snapshot_review('lala/foo.R')` to review the change.
+
+---
+
+    Code
+      base::writeLines(snapshot_review_hint("lala", "foo.R", is_text = TRUE,
+        reset_output = FALSE))
+    Output
+      * Run `testthat::snapshot_accept('lala/foo.R')` to accept the change.
+      * Run `testthat::snapshot_review('lala/foo.R')` to review the change.
 
 # expect_snapshot_file validates its inputs
 

tests/testthat/test-snapshot-file.R

@@ -163,9 +163,16 @@ test_that("split_path handles edge cases", {
 })
 
 test_that("generates informative hint", {
-  expect_snapshot(cat(snapshot_review_hint(
+  expect_snapshot(base::writeLines(snapshot_review_hint(
     "lala",
-    "foo.r",
+    "foo.R",
+    reset_output = FALSE
+  )))
+
+  expect_snapshot(base::writeLines(snapshot_review_hint(
+    "lala",
+    "foo.R",
+    is_text = TRUE,
     reset_output = FALSE
   )))
 })