From b4cc9a0b3172195472af613130d553cb35dfa948 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 13 Aug 2025 11:29:31 -0500 Subject: [PATCH 1/5] In R CMD check, only notify about snapshots once Fixes #2207 --- R/expect-that.R | 12 +++++-- R/expectation.R | 16 +++++++-- R/reporter-check.R | 39 +++++++++++++++++++++ R/snapshot-file.R | 45 ++++++------------------- R/snapshot-github.R | 11 ------ R/snapshot.R | 13 ++++--- man/expectation.Rd | 2 +- man/fail.Rd | 5 ++- tests/testthat/_snaps/reporter-check.md | 29 ++++++++++++++++ tests/testthat/_snaps/snapshot-file.md | 32 ++---------------- tests/testthat/_snaps/snapshot.md | 4 +-- tests/testthat/test-reporter-check.R | 16 +++++++++ tests/testthat/test-snapshot-file.R | 36 ++------------------ tests/testthat/test-snapshot.R | 2 +- 14 files changed, 140 insertions(+), 122 deletions(-) diff --git a/R/expect-that.R b/R/expect-that.R index 3f48dde2f..a20751ca1 100644 --- a/R/expect-that.R +++ b/R/expect-that.R @@ -18,6 +18,7 @@ #' informative traceack for failures. You should only need to set this if #' you're calling `fail()` from a helper function; see #' `vignette("custom-expectation")` for details. +#' @param snapshot Is this a snapshot failure? #' @param trace An optional backtrace created by [rlang::trace_back()]. #' When supplied, the expectation is displayed with the backtrace. #' Expert use only. @@ -39,7 +40,8 @@ fail <- function( info = NULL, srcref = NULL, trace_env = caller_env(), - trace = NULL + trace = NULL, + snapshot = FALSE ) { if (is.null(trace)) { trace <- trace_back(top = getOption("testthat_topenv"), bottom = trace_env) @@ -50,7 +52,13 @@ fail <- function( } message <- paste(c(message, info), collapse = "\n") - expectation("failure", message, srcref = srcref, trace = trace) + expectation( + "failure", + message, + srcref = srcref, + trace = trace, + snapshot = snapshot + ) } #' @rdname fail diff --git a/R/expectation.R b/R/expectation.R index 05322fda5..e08325629 100644 --- a/R/expectation.R +++ b/R/expectation.R @@ -49,8 +49,20 @@ expect <- function( #' @keywords internal #' @inheritParams expect #' @export -expectation <- function(type, message, srcref = NULL, trace = NULL) { - exp <- new_expectation(type, message, srcref = srcref, trace = trace) +expectation <- function( + type, + message, + snapshot = FALSE, + srcref = NULL, + trace = NULL +) { + exp <- new_expectation( + type, + message, + snapshot = snapshot, + srcref = srcref, + trace = trace + ) exp_signal(exp) } #' @rdname expectation diff --git a/R/reporter-check.R b/R/reporter-check.R index 7772647a6..69cd45784 100644 --- a/R/reporter-check.R +++ b/R/reporter-check.R @@ -13,6 +13,7 @@ CheckReporter <- R6::R6Class( skips = NULL, warnings = NULL, n_ok = 0L, + has_snapshot_failure = FALSE, initialize = function(...) { self$capabilities$parallel_support <- TRUE @@ -64,6 +65,11 @@ CheckReporter <- R6::R6Class( self$rule("Failed tests", line = 2) self$cat_line(map_chr(problems, issue_summary, rule = TRUE)) self$cat_line() + + if (some(problems, \(x) isTRUE(attr(x, "snapshot")))) { + self$rule("To resolve snapshot failures", line = 1) + self$cat_line(snapshot_check_hint()) + } } else { # clean up unlink("testthat-problems.rds") @@ -105,3 +111,36 @@ summary_line <- function(n_fail, n_warn, n_skip, n_pass) { " ]" ) } + +snapshot_check_hint <- function() { + if (on_gh()) { + repository <- Sys.getenv("GITHUB_REPOSITORY") + run_id <- Sys.getenv("GITHUB_RUN_ID") + + call <- sprintf( + "testthat::snapshot_download_gh(\"%s\", \"%s\")", + repository, + run_id + ) + copy <- sprintf("* Run `%s` to download snapshots.", call) + } else { + copy <- c( + if (on_ci()) { + "* Download and unzip artifact." + } else { + "* Locate check directory." + }, + "* Copy 'tests/testthat/_snaps' to local package." + ) + } + + action <- c( + "* Run `testthat::snapshot_accept()` to accept all changes.", + "* Run `testthat::snapshot_review()` to review all changes." + ) + c(copy, action) +} + +run <- function(x) { + cli::format_inline(paste0("{.run testthat::", x, "}")) +} diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 890411797..1081a23c1 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -142,7 +142,11 @@ expect_snapshot_file <- function( return(equal) } - hint <- snapshot_review_hint(snapshotter$file, name) + if (in_rcmd_check()) { + hint <- "" + } else { + hint <- snapshot_review_hint(snapshotter$file, name) + } if (!equal) { msg <- sprintf( @@ -151,7 +155,7 @@ expect_snapshot_file <- function( paste0(snapshotter$file, "/", name), hint ) - return(fail(msg)) + return(fail(msg, snapshot = TRUE)) } pass(NULL) } @@ -167,45 +171,16 @@ announce_snapshot_file <- function(path, name = basename(path)) { } } -snapshot_review_hint <- function( - test, - name, - ci = on_ci(), - check = in_rcmd_check(), - reset_output = TRUE -) { +snapshot_review_hint <- function(test, name, reset_output = TRUE) { if (reset_output) { local_reporter_output() } - path <- paste0("tests/testthat/_snaps/", test, "/", new_name(name)) - - if (check) { - if (on_gh()) { - bullets <- snap_download_hint() - } else { - bullets <- c( - if (ci) "* Download and unzip run artifact\n", - if (!ci) "* Locate check directory\n", - paste0("* Copy '", path, "' to local test directory\n") - ) - } - } else { - bullets <- NULL - } - - paste0( - c( - bullets, - cli::format_inline( - "* Run {.run testthat::snapshot_review('{test}/')} to review changes" - ) - ), - collapse = "" + 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 @@ -254,7 +229,7 @@ snapshot_file_equal <- function( # We want to fail on CI since this suggests that the user has failed # to record the value locally if (fail_on_new) { - return(fail(message, trace_env = trace_env)) + return(fail(message, snapshot = TRUE, trace_env = trace_env)) } testthat_warn(message) TRUE diff --git a/R/snapshot-github.R b/R/snapshot-github.R index 56cf07448..74e278a11 100644 --- a/R/snapshot-github.R +++ b/R/snapshot-github.R @@ -40,17 +40,6 @@ snapshot_download_gh <- function(repository, run_id, dest_dir = ".") { dir_copy(src_snaps, dest_snaps) } -snap_download_hint <- function() { - repository <- Sys.getenv("GITHUB_REPOSITORY") - run_id <- Sys.getenv("GITHUB_RUN_ID") - - sprintf( - "* Call `snapshot_download_gh(\"%s\", \"%s\")` to download the snapshots from GitHub.\n", - repository, - run_id - ) -} - gh_find_job <- function(repository, run_id) { jobs_json <- gh::gh( "/repos/{repository}/actions/runs/{run_id}/jobs", diff --git a/R/snapshot.R b/R/snapshot.R index 87487acb5..b789d1f24 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -289,7 +289,7 @@ expect_snapshot_condition_ <- function( class ) } - return(fail(msg, trace_env = trace_env)) + return(fail(msg, snapshot = TRUE, trace_env = trace_env)) } expect_snapshot_helper( @@ -340,7 +340,11 @@ expect_snapshot_helper <- function( } else { variant_lab <- "" } - hint <- snapshot_accept_hint(variant, snapshotter$file) + if (in_rcmd_check()) { + hint <- "" + } else { + hint <- snapshot_accept_hint(variant, snapshotter$file) + } if (length(comp) != 0) { msg <- sprintf( @@ -350,7 +354,7 @@ expect_snapshot_helper <- function( paste0(comp, collapse = "\n\n"), hint ) - return(fail(msg, trace_env = trace_env)) + return(fail(msg, snapshot = TRUE, trace_env = trace_env)) } pass(NULL) @@ -368,13 +372,12 @@ snapshot_accept_hint <- function(variant, file, reset_output = TRUE) { } paste0( - if (on_gh()) snap_download_hint(), cli::format_inline( "* Run {.run testthat::snapshot_accept('{name}')} to accept the change." ), "\n", cli::format_inline( - "* Run {.run testthat::snapshot_review('{name}')} to interactively review the change." + "* Run {.run testthat::snapshot_review('{name}')} to review the change." ) ) } diff --git a/man/expectation.Rd b/man/expectation.Rd index c8509e9f1..a2301ddfb 100644 --- a/man/expectation.Rd +++ b/man/expectation.Rd @@ -7,7 +7,7 @@ \alias{is.expectation} \title{Construct an expectation object} \usage{ -expectation(type, message, srcref = NULL, trace = NULL) +expectation(type, message, snapshot = FALSE, srcref = NULL, trace = NULL) new_expectation( type, diff --git a/man/fail.Rd b/man/fail.Rd index 6bf16f5fc..99b24a8af 100644 --- a/man/fail.Rd +++ b/man/fail.Rd @@ -10,7 +10,8 @@ fail( info = NULL, srcref = NULL, trace_env = caller_env(), - trace = NULL + trace = NULL, + snapshot = FALSE ) pass(value) @@ -34,6 +35,8 @@ you're calling \code{fail()} from a helper function; see When supplied, the expectation is displayed with the backtrace. Expert use only.} +\item{snapshot}{Is this a snapshot failure?} + \item{value}{Value to return, typically the result of evaluating the \code{object} argument to the expectation.} } diff --git a/tests/testthat/_snaps/reporter-check.md b/tests/testthat/_snaps/reporter-check.md index a0198f5ce..9c9431da5 100644 --- a/tests/testthat/_snaps/reporter-check.md +++ b/tests/testthat/_snaps/reporter-check.md @@ -96,3 +96,32 @@ [ FAIL 4 | WARN 1 | SKIP 3 | PASS 1 ] +# generates informative snapshot hints + + Code + base::writeLines(snapshot_check_hint()) + Output + * Locate check directory. + * Copy 'tests/testthat/_snaps' to local package. + * Run `testthat::snapshot_accept()` to accept all changes. + * Run `testthat::snapshot_review()` to review all changes. + +--- + + Code + base::writeLines(snapshot_check_hint()) + Output + * Download and unzip artifact. + * Copy 'tests/testthat/_snaps' to local package. + * Run `testthat::snapshot_accept()` to accept all changes. + * Run `testthat::snapshot_review()` to review all changes. + +--- + + Code + base::writeLines(snapshot_check_hint()) + Output + * Run `testthat::snapshot_download_gh("r-lib/testthat", "123")` to download snapshots. + * Run `testthat::snapshot_accept()` to accept all changes. + * Run `testthat::snapshot_review()` to review all changes. + diff --git a/tests/testthat/_snaps/snapshot-file.md b/tests/testthat/_snaps/snapshot-file.md index 8e3cd3a98..ea65d1146 100644 --- a/tests/testthat/_snaps/snapshot-file.md +++ b/tests/testthat/_snaps/snapshot-file.md @@ -22,38 +22,12 @@ Error in `snapshot_file_equal_()`: ! 'doesnt-exist.txt' not found. -# snapshot_hint output differs in R CMD check +# generates informative hint Code - cat(snapshot_review_hint("lala", "foo.r", check = FALSE, ci = FALSE)) + cat(snapshot_review_hint("lala", "foo.r", reset_output = FALSE)) Output - * Run `testthat::snapshot_review('lala/')` to review changes - ---- - - Code - cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = FALSE)) - Output - * Locate check directory - * Copy 'tests/testthat/_snaps/lala/foo.new.r' to local test directory - * Run `testthat::snapshot_review('lala/')` to review changes - ---- - - Code - cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = TRUE)) - Output - * Download and unzip run artifact - * Copy 'tests/testthat/_snaps/lala/foo.new.r' to local test directory - * Run `testthat::snapshot_review('lala/')` to review changes - ---- - - Code - cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = TRUE)) - Output - * Call `snapshot_download_gh("r-lib/testthat", "123")` to download the snapshots from GitHub. - * Run `testthat::snapshot_review('lala/')` to review changes + * Run `testthat::snapshot_review('lala/foo.r')` to review the change. # expect_snapshot_file validates its inputs diff --git a/tests/testthat/_snaps/snapshot.md b/tests/testthat/_snaps/snapshot.md index defaf88d6..04c746a42 100644 --- a/tests/testthat/_snaps/snapshot.md +++ b/tests/testthat/_snaps/snapshot.md @@ -259,12 +259,12 @@ cat(snapshot_accept_hint("_default", "bar.R", reset_output = FALSE)) Output * Run `testthat::snapshot_accept('bar.R')` to accept the change. - * Run `testthat::snapshot_review('bar.R')` to interactively review the change. + * Run `testthat::snapshot_review('bar.R')` to review the change. Code cat(snapshot_accept_hint("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 interactively review the change. + * Run `testthat::snapshot_review('foo/bar.R')` to review the change. # expect_snapshot validates its inputs diff --git a/tests/testthat/test-reporter-check.R b/tests/testthat/test-reporter-check.R index 6602cceac..1c7cbbb4e 100644 --- a/tests/testthat/test-reporter-check.R +++ b/tests/testthat/test-reporter-check.R @@ -32,3 +32,19 @@ test_that("shows warnings when not on CRAN", { withr::local_options("NOT_CRAN" = "true") expect_snapshot_reporter(CheckReporter$new(), test_path("reporters/tests.R")) }) + + +test_that("generates informative snapshot hints", { + withr::local_envvar(GITHUB_ACTIONS = "false", CI = "false") + expect_snapshot(base::writeLines(snapshot_check_hint())) + + withr::local_envvar(CI = "true") + expect_snapshot(base::writeLines(snapshot_check_hint())) + + withr::local_envvar( + GITHUB_ACTIONS = "true", + GITHUB_REPOSITORY = "r-lib/testthat", + GITHUB_RUN_ID = "123" + ) + expect_snapshot(base::writeLines(snapshot_check_hint())) +}) diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index 5a786c2d8..8597c9e74 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -1,5 +1,5 @@ test_that("expect_snapshot_file works", { - path <- write_tmp_lines(letters) + path <- write_tmp_lines(letters[-1]) expect_snapshot_file(path, "foo.r", compare = compare_file_text) path <- withr::local_tempfile() @@ -157,41 +157,11 @@ test_that("split_path handles edge cases", { expect_equal(split_path("x/.b.c"), list(dir = "x", name = "", ext = "b.c")) }) -test_that("snapshot_hint output differs in R CMD check", { - snapshot_review_hint <- function(...) { - testthat:::snapshot_review_hint(..., reset_output = FALSE) - } - withr::local_envvar(GITHUB_ACTIONS = "false") - - expect_snapshot(cat(snapshot_review_hint( - "lala", - "foo.r", - check = FALSE, - ci = FALSE - ))) - expect_snapshot(cat(snapshot_review_hint( - "lala", - "foo.r", - check = TRUE, - ci = FALSE - ))) - expect_snapshot(cat(snapshot_review_hint( - "lala", - "foo.r", - check = TRUE, - ci = TRUE - ))) - - withr::local_envvar( - GITHUB_ACTIONS = "true", - GITHUB_REPOSITORY = "r-lib/testthat", - GITHUB_RUN_ID = "123" - ) +test_that("generates informative hint", { expect_snapshot(cat(snapshot_review_hint( "lala", "foo.r", - check = TRUE, - ci = TRUE + reset_output = FALSE ))) }) diff --git a/tests/testthat/test-snapshot.R b/tests/testthat/test-snapshot.R index c851e5437..eb61393cb 100644 --- a/tests/testthat/test-snapshot.R +++ b/tests/testthat/test-snapshot.R @@ -15,7 +15,7 @@ test_that("can snapshot everything", { print("1") message("2") warning("3") - stop("4") + stop("5") } expect_snapshot(f(), error = TRUE) }) From 6f9417bb02a97780e3fd5ac24bd3d6abb7623c05 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 13 Aug 2025 11:36:58 -0500 Subject: [PATCH 2/5] Remove unused field; revert snapshot changes --- R/reporter-check.R | 1 - tests/testthat/test-snapshot-file.R | 2 +- tests/testthat/test-snapshot.R | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/R/reporter-check.R b/R/reporter-check.R index 69cd45784..56dfd368c 100644 --- a/R/reporter-check.R +++ b/R/reporter-check.R @@ -13,7 +13,6 @@ CheckReporter <- R6::R6Class( skips = NULL, warnings = NULL, n_ok = 0L, - has_snapshot_failure = FALSE, initialize = function(...) { self$capabilities$parallel_support <- TRUE diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index 8597c9e74..bf56091f1 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -1,5 +1,5 @@ test_that("expect_snapshot_file works", { - path <- write_tmp_lines(letters[-1]) + path <- write_tmp_lines(letters) expect_snapshot_file(path, "foo.r", compare = compare_file_text) path <- withr::local_tempfile() diff --git a/tests/testthat/test-snapshot.R b/tests/testthat/test-snapshot.R index eb61393cb..c851e5437 100644 --- a/tests/testthat/test-snapshot.R +++ b/tests/testthat/test-snapshot.R @@ -15,7 +15,7 @@ test_that("can snapshot everything", { print("1") message("2") warning("3") - stop("5") + stop("4") } expect_snapshot(f(), error = TRUE) }) From 2978bc98d905cff39ea2e5a949522f7a759ebe86 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 13 Aug 2025 11:48:34 -0500 Subject: [PATCH 3/5] Make `snapshot_fail()` instead --- NEWS.md | 1 + R/expect-that.R | 32 ++++++++++++++++---------------- R/expectation.R | 16 ++-------------- R/snapshot-file.R | 4 ++-- R/snapshot.R | 4 ++-- man/expectation.Rd | 6 +++--- man/fail.Rd | 7 +++---- 7 files changed, 29 insertions(+), 41 deletions(-) diff --git a/NEWS.md b/NEWS.md index 65c52186c..e58881abc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* In `R CMD check`, snapshots now only advise on how to resolve failures once (#2207). * `snapshot_review()` includes a reject button and only displays the file navigation and the skip button if there are multiple files to review (#2025). * New `snapshot_download_gh()` makes it easy to get snapshots off GitHub and into your local package (#1779). * New `local_mocked_s3_method()`, `local_mocked_s4_method()`, and `local_mocked_r6_class()` allow you to mock S3 and S4 methods and R6 classes (#1892, #1916) diff --git a/R/expect-that.R b/R/expect-that.R index a20751ca1..94eb963e9 100644 --- a/R/expect-that.R +++ b/R/expect-that.R @@ -40,25 +40,25 @@ fail <- function( info = NULL, srcref = NULL, trace_env = caller_env(), - trace = NULL, - snapshot = FALSE + trace = NULL ) { - if (is.null(trace)) { - trace <- trace_back(top = getOption("testthat_topenv"), bottom = trace_env) - } - # Only show if there's at least one function apart from the expectation - if (trace_length(trace) <= 1) { + trace <- trace %||% capture_trace(trace_env) + message <- paste(c(message, info), collapse = "\n") + expectation("failure", message, srcref = srcref, trace = trace) +} + +snapshot_fail <- function(message, trace_env = caller_env()) { + trace <- capture_trace(trace_env) + expectation("failure", message, trace = trace, snapshot = TRUE) +} + +capture_trace <- function(trace_env) { + trace <- trace_back(top = getOption("testthat_topenv"), bottom = trace_env) + # Only include trace if there's at least one function apart from the expectation + if (!is.null(trace) && trace_length(trace) <= 1) { trace <- NULL } - - message <- paste(c(message, info), collapse = "\n") - expectation( - "failure", - message, - srcref = srcref, - trace = trace, - snapshot = snapshot - ) + trace } #' @rdname fail diff --git a/R/expectation.R b/R/expectation.R index e08325629..55f00845a 100644 --- a/R/expectation.R +++ b/R/expectation.R @@ -49,20 +49,8 @@ expect <- function( #' @keywords internal #' @inheritParams expect #' @export -expectation <- function( - type, - message, - snapshot = FALSE, - srcref = NULL, - trace = NULL -) { - exp <- new_expectation( - type, - message, - snapshot = snapshot, - srcref = srcref, - trace = trace - ) +expectation <- function(type, message, ..., srcref = NULL, trace = NULL) { + exp <- new_expectation(type, message, ..., srcref = srcref, trace = trace) exp_signal(exp) } #' @rdname expectation diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 1081a23c1..907349b68 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -155,7 +155,7 @@ expect_snapshot_file <- function( paste0(snapshotter$file, "/", name), hint ) - return(fail(msg, snapshot = TRUE)) + return(snapshot_fail(msg)) } pass(NULL) } @@ -229,7 +229,7 @@ snapshot_file_equal <- function( # We want to fail on CI since this suggests that the user has failed # to record the value locally if (fail_on_new) { - return(fail(message, snapshot = TRUE, trace_env = trace_env)) + return(snapshot_fail(message, trace_env = trace_env)) } testthat_warn(message) TRUE diff --git a/R/snapshot.R b/R/snapshot.R index b789d1f24..3fda104c6 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -289,7 +289,7 @@ expect_snapshot_condition_ <- function( class ) } - return(fail(msg, snapshot = TRUE, trace_env = trace_env)) + return(snapshot_fail(msg, trace_env = trace_env)) } expect_snapshot_helper( @@ -354,7 +354,7 @@ expect_snapshot_helper <- function( paste0(comp, collapse = "\n\n"), hint ) - return(fail(msg, snapshot = TRUE, trace_env = trace_env)) + return(snapshot_fail(msg, trace_env = trace_env)) } pass(NULL) diff --git a/man/expectation.Rd b/man/expectation.Rd index a2301ddfb..e4a23dda4 100644 --- a/man/expectation.Rd +++ b/man/expectation.Rd @@ -7,7 +7,7 @@ \alias{is.expectation} \title{Construct an expectation object} \usage{ -expectation(type, message, snapshot = FALSE, srcref = NULL, trace = NULL) +expectation(type, message, ..., srcref = NULL, trace = NULL) new_expectation( type, @@ -28,14 +28,14 @@ is.expectation(x) \item{message}{Message describing test failure} +\item{...}{Additional attributes for the expectation object.} + \item{srcref}{Optional \code{srcref} giving location of test.} \item{trace}{An optional backtrace created by \code{\link[rlang:trace_back]{rlang::trace_back()}}. When supplied, the expectation is displayed with the backtrace. Expert use only.} -\item{...}{Additional attributes for the expectation object.} - \item{.subclass}{An optional subclass for the expectation object.} \item{exp}{An expectation object, as created by diff --git a/man/fail.Rd b/man/fail.Rd index 99b24a8af..1f3842a0b 100644 --- a/man/fail.Rd +++ b/man/fail.Rd @@ -10,8 +10,7 @@ fail( info = NULL, srcref = NULL, trace_env = caller_env(), - trace = NULL, - snapshot = FALSE + trace = NULL ) pass(value) @@ -35,10 +34,10 @@ you're calling \code{fail()} from a helper function; see When supplied, the expectation is displayed with the backtrace. Expert use only.} -\item{snapshot}{Is this a snapshot failure?} - \item{value}{Value to return, typically the result of evaluating the \code{object} argument to the expectation.} + +\item{snapshot}{Is this a snapshot failure?} } \description{ These are the primitives that you can use to implement your own expectations. From 524586699b20865dabc17ac2a45c605caf3cfc93 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 13 Aug 2025 11:58:13 -0500 Subject: [PATCH 4/5] Record if we're in a check reporter --- R/expect-that.R | 1 - R/reporter-check.R | 8 ++++++++ R/snapshot-file.R | 2 +- R/snapshot.R | 2 +- R/testthat-package.R | 1 + R/utils.R | 4 ++-- man/fail.Rd | 2 -- 7 files changed, 13 insertions(+), 7 deletions(-) diff --git a/R/expect-that.R b/R/expect-that.R index 94eb963e9..b32cd7098 100644 --- a/R/expect-that.R +++ b/R/expect-that.R @@ -18,7 +18,6 @@ #' informative traceack for failures. You should only need to set this if #' you're calling `fail()` from a helper function; see #' `vignette("custom-expectation")` for details. -#' @param snapshot Is this a snapshot failure? #' @param trace An optional backtrace created by [rlang::trace_back()]. #' When supplied, the expectation is displayed with the backtrace. #' Expert use only. diff --git a/R/reporter-check.R b/R/reporter-check.R index 56dfd368c..6cf660de7 100644 --- a/R/reporter-check.R +++ b/R/reporter-check.R @@ -13,6 +13,7 @@ CheckReporter <- R6::R6Class( skips = NULL, warnings = NULL, n_ok = 0L, + old_in_check_reporter = NULL, initialize = function(...) { self$capabilities$parallel_support <- TRUE @@ -35,7 +36,14 @@ CheckReporter <- R6::R6Class( } }, + start_reporter = function() { + self$old_in_check_reporter <- in_check_reporter() + the$in_check_reporter <- TRUE + }, + end_reporter = function() { + the$in_check_reporter <- self$old_in_check_reporter + if (self$skips$size() || self$warnings$size() || self$problems$size()) { self$cat_line(summary_line( n_fail = self$problems$size(), diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 907349b68..dea4ce474 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -142,7 +142,7 @@ expect_snapshot_file <- function( return(equal) } - if (in_rcmd_check()) { + if (in_check_reporter()) { hint <- "" } else { hint <- snapshot_review_hint(snapshotter$file, name) diff --git a/R/snapshot.R b/R/snapshot.R index 3fda104c6..5030e1056 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -340,7 +340,7 @@ expect_snapshot_helper <- function( } else { variant_lab <- "" } - if (in_rcmd_check()) { + if (in_check_reporter()) { hint <- "" } else { hint <- snapshot_accept_hint(variant, snapshotter$file) diff --git a/R/testthat-package.R b/R/testthat-package.R index 799da070b..61fa9b316 100644 --- a/R/testthat-package.R +++ b/R/testthat-package.R @@ -22,6 +22,7 @@ the <- new.env(parent = emptyenv()) the$description <- character() the$top_level_test <- TRUE the$test_expectations <- 0 +the$in_check_reporter <- FALSE # The following block is used by usethis to automatically manage # roxygen namespace tags. Modify with care! diff --git a/R/utils.R b/R/utils.R index e3d7eb216..784a01f6b 100644 --- a/R/utils.R +++ b/R/utils.R @@ -51,8 +51,8 @@ first_upper <- function(x) { x } -in_rcmd_check <- function() { - nzchar(Sys.getenv("_R_CHECK_PACKAGE_NAME_", "")) +in_check_reporter <- function() { + isTRUE(the$in_check_reporter) } r_version <- function() paste0("R", getRversion()[, 1:2]) diff --git a/man/fail.Rd b/man/fail.Rd index 1f3842a0b..6bf16f5fc 100644 --- a/man/fail.Rd +++ b/man/fail.Rd @@ -36,8 +36,6 @@ Expert use only.} \item{value}{Value to return, typically the result of evaluating the \code{object} argument to the expectation.} - -\item{snapshot}{Is this a snapshot failure?} } \description{ These are the primitives that you can use to implement your own expectations. From ec4bfe3f721fa16a7b3a70592cfd9f616f7f8f22 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 13 Aug 2025 13:31:41 -0500 Subject: [PATCH 5/5] Tweak wording/framing --- R/reporter-check.R | 6 ++++-- tests/testthat/_snaps/reporter-check.md | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/R/reporter-check.R b/R/reporter-check.R index 6cf660de7..e70736ae0 100644 --- a/R/reporter-check.R +++ b/R/reporter-check.R @@ -74,7 +74,7 @@ CheckReporter <- R6::R6Class( self$cat_line() if (some(problems, \(x) isTRUE(attr(x, "snapshot")))) { - self$rule("To resolve snapshot failures", line = 1) + self$rule("Snapshots", line = 1) self$cat_line(snapshot_check_hint()) } } else { @@ -120,6 +120,8 @@ summary_line <- function(n_fail, n_warn, n_skip, n_pass) { } snapshot_check_hint <- function() { + intro <- "To review and process snapshots locally:" + if (on_gh()) { repository <- Sys.getenv("GITHUB_REPOSITORY") run_id <- Sys.getenv("GITHUB_RUN_ID") @@ -145,7 +147,7 @@ snapshot_check_hint <- function() { "* Run `testthat::snapshot_accept()` to accept all changes.", "* Run `testthat::snapshot_review()` to review all changes." ) - c(copy, action) + c(intro, copy, action) } run <- function(x) { diff --git a/tests/testthat/_snaps/reporter-check.md b/tests/testthat/_snaps/reporter-check.md index 9c9431da5..a6bab3c70 100644 --- a/tests/testthat/_snaps/reporter-check.md +++ b/tests/testthat/_snaps/reporter-check.md @@ -101,6 +101,7 @@ Code base::writeLines(snapshot_check_hint()) Output + To review and process snapshots locally: * Locate check directory. * Copy 'tests/testthat/_snaps' to local package. * Run `testthat::snapshot_accept()` to accept all changes. @@ -111,6 +112,7 @@ Code base::writeLines(snapshot_check_hint()) Output + To review and process snapshots locally: * Download and unzip artifact. * Copy 'tests/testthat/_snaps' to local package. * Run `testthat::snapshot_accept()` to accept all changes. @@ -121,6 +123,7 @@ Code base::writeLines(snapshot_check_hint()) Output + To review and process snapshots locally: * Run `testthat::snapshot_download_gh("r-lib/testthat", "123")` to download snapshots. * Run `testthat::snapshot_accept()` to accept all changes. * Run `testthat::snapshot_review()` to review all changes.