Combine hint functions

Author: hadley

NAMESPACE

@@ -24,6 +24,7 @@ S3method(print,comparison)
 S3method(print,expectation)
 S3method(print,mismatch_character)
 S3method(print,mismatch_numeric)
+S3method(print,testthat_hint)
 S3method(print,testthat_results)
 S3method(snapshot_replay,character)
 S3method(snapshot_replay,condition)

R/snapshot-file.R

@@ -157,11 +157,6 @@ expect_snapshot_file <- function(
   }
 
   file <- snapshotter$file
-  if (in_check_reporter()) {
-    hint <- ""
-  } else {
-    hint <- snapshot_review_hint(file, name, is_text = is_text)
-  }
 
   if (!equal) {
     if (is_text) {
@@ -181,6 +176,8 @@ expect_snapshot_file <- function(
       comp <- NULL
     }
 
+    hint <- snapshot_hint(file, variant, file, show_accept = is_text)
+
     msg <- c(
       sprintf("Snapshot of %s has changed.", lab),
       comp,
@@ -207,28 +204,6 @@ announce_snapshot_file <- function(path, name = basename(path)) {
   }
 }
 
-snapshot_review_hint <- function(
-  test,
-  name,
-  is_text = FALSE,
-  reset_output = TRUE
-) {
-  if (reset_output) {
-    local_reporter_output()
-  }
-
-  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."
-    )
-  )
-}
-
 snapshot_file_equal <- function(
   snap_dir, # _snaps/
   snap_test, # test file name

R/snapshot.R

@@ -367,18 +367,12 @@ expect_snapshot_helper <- function(
   } else {
     variant_lab <- ""
   }
-  if (in_check_reporter()) {
-    hint <- ""
-  } else {
-    hint <- snapshot_accept_hint(variant, snapshotter$file)
-  }
 
   if (length(comp) != 0) {
-    msg <- sprintf(
-      "Snapshot of %s has changed%s:\n%s\n\n%s",
-      lab,
-      variant_lab,
-      paste0(comp, collapse = "\n\n"),
+    hint <- snapshot_hint(NULL, variant, snapshotter$file)
+    msg <- c(
+      sprintf("Snapshot of %s has changed%s:", lab, variant_lab),
+      comp,
       hint
     )
     return(snapshot_fail(msg, trace_env = trace_env))
@@ -387,18 +381,25 @@ expect_snapshot_helper <- function(
   pass(NULL)
 }
 
-snapshot_accept_hint <- function(variant, file, reset_output = TRUE) {
+snapshot_hint <- function(
+  file,
+  variant,
+  name,
+  show_accept = TRUE,
+  reset_output = TRUE
+) {
+  if (in_check_reporter()) {
+    return("")
+  }
+
   if (reset_output) {
     local_reporter_output()
   }
 
-  if (is.null(variant) || variant == "_default") {
-    name <- file
-  } else {
-    name <- file.path(variant, file)
-  }
+  id <- c(file, if (!identical(variant, "_default")) variant, name)
+  full_name <- paste0(id, collapse = "/")
 
-  args <- encodeString(name, quote = '"')
+  args <- encodeString(full_name, quote = '"')
   # Include path argument if we're in a different working directory
   wd <- Sys.getenv("TESTTHAT_WD", unset = "")
   if (wd != "") {
@@ -408,17 +409,23 @@ snapshot_accept_hint <- function(variant, file, reset_output = TRUE) {
     }
   }
 
-  paste0(
-    cli::format_inline(
-      "* Run {.run testthat::snapshot_accept({args})} to accept the change."
-    ),
-    "\n",
-    cli::format_inline(
-      "* Run {.run testthat::snapshot_review({args})} to review the change."
-    )
+  accept_link <- cli::format_inline("{.run testthat::snapshot_accept({args})}")
+  review_link <- cli::format_inline("{.run testthat::snapshot_review({args})}")
+
+  out <- c(
+    if (show_accept) sprintf("* Run %s to accept the change.", accept_link),
+    sprintf("* Run %s to review the change.", review_link)
   )
+  structure(out, class = "testthat_hint")
 }
 
+#' @export
+print.testthat_hint <- function(x, ...) {
+  cat(paste0(x, "\n", collapse = ""))
+  invisible(x)
+}
+
+
 snapshot_not_available <- function(message) {
   local_reporter_output()
 

tests/testthat/_snaps/snapshot-file.md

@@ -25,18 +25,17 @@
 # generates informative hint
 
     Code
-      base::writeLines(snapshot_review_hint("lala", "foo.R", reset_output = FALSE))
+      snapshot_hint("lala", NULL, "foo.R", show_accept = FALSE, 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))
+      snapshot_hint("lala", NULL, "foo.R", show_accept = 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.
+      * 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/_snaps/snapshot.md

@@ -264,12 +264,12 @@
 # hint is informative
 
     Code
-      cat(snapshot_accept_hint("_default", "bar.R", reset_output = FALSE))
+      snapshot_hint(NULL, "_default", "bar.R", reset_output = FALSE)
     Output
       * Run `testthat::snapshot_accept("bar.R")` to accept the change.
       * Run `testthat::snapshot_review("bar.R")` to review the change.
     Code
-      cat(snapshot_accept_hint("foo", "bar.R", reset_output = FALSE))
+      snapshot_hint(NULL, "foo", "bar.R", reset_output = FALSE)
     Output
       * Run `testthat::snapshot_accept("foo/bar.R")` to accept the change.
       * Run `testthat::snapshot_review("foo/bar.R")` to review the change.

tests/testthat/test-snapshot-file.R

@@ -163,18 +163,24 @@ test_that("split_path handles edge cases", {
 })
 
 test_that("generates informative hint", {
-  expect_snapshot(base::writeLines(snapshot_review_hint(
+  local_mocked_bindings(in_check_reporter = function() FALSE)
+  withr::local_envvar(GITHUB_ACTIONS = "false", TESTTHAT_WD = NA)
+
+  expect_snapshot(snapshot_hint(
     "lala",
+    NULL,
     "foo.R",
+    show_accept = FALSE,
     reset_output = FALSE
-  )))
+  ))
 
-  expect_snapshot(base::writeLines(snapshot_review_hint(
+  expect_snapshot(snapshot_hint(
     "lala",
+    NULL,
     "foo.R",
-    is_text = TRUE,
+    show_accept = TRUE,
     reset_output = FALSE
-  )))
+  ))
 })
 
 test_that("expect_snapshot_file validates its inputs", {

tests/testthat/test-snapshot.R

@@ -166,21 +166,28 @@ test_that("errors and warnings are folded", {
 # })
 
 test_that("hint is informative", {
+  local_mocked_bindings(in_check_reporter = function() FALSE)
   withr::local_envvar(GITHUB_ACTIONS = "false", TESTTHAT_WD = NA)
 
   expect_snapshot({
-    cat(snapshot_accept_hint("_default", "bar.R", reset_output = FALSE))
-    cat(snapshot_accept_hint("foo", "bar.R", reset_output = FALSE))
+    snapshot_hint(NULL, "_default", "bar.R", reset_output = FALSE)
+    snapshot_hint(NULL, "foo", "bar.R", reset_output = FALSE)
   })
 })
 
 test_that("hint includes path when WD is different", {
+  local_mocked_bindings(in_check_reporter = function() FALSE)
   withr::local_envvar(TESTTHAT_WD = "..")
 
-  hint <- snapshot_accept_hint("_default", "bar.R", reset_output = FALSE)
+  hint <- snapshot_hint(NULL, "_default", "bar.R", reset_output = FALSE)
   hint <- gsub(getwd(), "<WD>", hint)
   # Can't use snapshot here because its hint will get the wrong path
-  expect_match(hint, 'snapshot_accept("bar.R", "<WD>")', fixed = TRUE)
+  expect_match(
+    hint,
+    'snapshot_accept("bar.R", "<WD>")',
+    fixed = TRUE,
+    all = FALSE
+  )
 })
 
 test_that("expect_snapshot requires a non-empty test label", {