From 81a454f761f214c719aa0998aef6442e18db4852 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 08:34:09 -0500 Subject: [PATCH 01/12] debug expect snapshot --- R/snapshot-file.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 0106f4bb2..869e9c952 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -160,6 +160,7 @@ expect_snapshot_file <- function( file_equal = compare, variant = variant, ) + str(list(testthat_equal = equal, name = name, path = path, variant = variant, compare = compare)) if (inherits(equal, "expectation_failure")) { return(equal) } From a4ea7ea5e885d5449ea3c3eb0ebdb9660cf09dcc Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 09:25:53 -0500 Subject: [PATCH 02/12] Add legacy fix for shinytest2 While we don't support `continue_test` directly, it has the same intent as `muffle_expectation` as it was the previous name. So support it in the handler. --- R/expectation.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/expectation.R b/R/expectation.R index c16b8a1b2..0fc276a65 100644 --- a/R/expectation.R +++ b/R/expectation.R @@ -104,7 +104,10 @@ exp_signal <- function(exp) { } else { signalCondition(exp) }, - muffle_expectation = function(e) NULL + muffle_expectation = function(e) NULL, + # Legacy support for shinytest2 + # https://github.com/r-lib/testthat/pull/2271#discussion_r2528722708 + continue_test = function(e) NULL ) invisible(exp) From 6a2db9370a9c3d09a0c28069d5fe3372d23e5484 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 09:53:38 -0500 Subject: [PATCH 03/12] Remove debug --- R/snapshot-file.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 869e9c952..ef0af47ae 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -160,7 +160,7 @@ expect_snapshot_file <- function( file_equal = compare, variant = variant, ) - str(list(testthat_equal = equal, name = name, path = path, variant = variant, compare = compare)) + if (inherits(equal, "expectation_failure")) { return(equal) } From 7319769fe4b61a00fda6d859804689924f3d761c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 10:02:28 -0500 Subject: [PATCH 04/12] If `snapshot_fail()` is muffled, continue with a new snapshot warning and return `TRUE` --- R/snapshot-file.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/snapshot-file.R b/R/snapshot-file.R index ef0af47ae..b55e882f1 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -262,8 +262,11 @@ 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(snapshot_fail(message, trace_env = trace_env)) + # Do not manually call return here. + # If snapshot_fail is silenced by `muffle_expectation`, let it behave like `fail_on_new = FALSE` + snapshot_fail(message, trace_env = trace_env) } + testthat_warn(message) TRUE } From b09f0ddd637aef12401a95efbf7af6300b739e8c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 10:02:44 -0500 Subject: [PATCH 05/12] test new behavior --- tests/testthat/_snaps/snapshot-file.md | 8 ++++++++ tests/testthat/test-snapshot-file.R | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/tests/testthat/_snaps/snapshot-file.md b/tests/testthat/_snaps/snapshot-file.md index 10e62fac0..157af2cfd 100644 --- a/tests/testthat/_snaps/snapshot-file.md +++ b/tests/testthat/_snaps/snapshot-file.md @@ -22,6 +22,14 @@ Error in `snapshot_file_equal_()`: ! 'doesnt-exist.txt' not found. +--- + + Code + out <- snapshot_file_equal_w_muffle(path_muffle) + Condition + Warning: + Adding new file snapshot: 'tests/testthat/_snaps/muffle-test/test.txt' + # generates informative hint Code diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index 5151674b2..10028e529 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -136,6 +136,29 @@ test_that("warns on first creation", { expect_true(snapshot_file_equal_(path)) expect_true(file.exists(file.path(snap_dir, "my-test/test.txt"))) expect_false(file.exists(file.path(snap_dir, "my-test/test.new.txt"))) + + # Similar to shinytest2::AppDriver$expect_values() screenshot snapshot behavior + path_muffle <- write_tmp_lines("muffle-a") + snapshot_file_equal_w_muffle <- function(path) { + withCallingHandlers( + { + snapshot_file_equal( + snap_dir = snap_dir, + snap_test = "muffle-test", + snap_name = "test.txt", + snap_variant = NULL, + path = path, + fail_on_new = TRUE + ) + }, + expectation = function(e) { + invokeRestart("muffle_expectation") + } + ) + } + # Warns on first run: Should fail on new, however it is muffled + expect_snapshot(out <- snapshot_file_equal_w_muffle(path_muffle)) + expect_true(out) }) # helpers ----------------------------------------------------------------- From 5479838430e81c630c4c70764b86c1e7b56388e8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 10:05:28 -0500 Subject: [PATCH 06/12] Update NEWS.md --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 3845257a3..cf5a04b04 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # testthat (development version) +* Fixed support for `shinytest2::AppDriver$expect_values()` screenshot snapshot failing on CI (#2293, #2288). + # testthat 3.3.0 ## Lifecycle changes From 2bdadd042d5901fde29cca83deba10daf1cec206 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 21 Nov 2025 11:46:12 -0500 Subject: [PATCH 07/12] Update snap with latest `fail()` stack trace --- tests/testthat/_snaps/reporter-debug.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/testthat/_snaps/reporter-debug.md b/tests/testthat/_snaps/reporter-debug.md index 4c639d157..1d834c847 100644 --- a/tests/testthat/_snaps/reporter-debug.md +++ b/tests/testthat/_snaps/reporter-debug.md @@ -3,11 +3,25 @@ 1: expect_true(x) 2: expect_waldo_constant_(act, exp, info = info, ignore_attr = TRUE) 3: fail(msg, info = info, trace_env = trace_env) + 4: expectation("failure", message, srcref = srcref, trace = trace) + 5: exp_signal(exp) + 6: withRestarts(if (expectation_broken(exp)) { + stop(exp) + } else { + signalC + 7: withRestartList(expr, restarts) 1: f() 2: expect_true(FALSE) 3: expect_waldo_constant_(act, exp, info = info, ignore_attr = TRUE) 4: fail(msg, info = info, trace_env = trace_env) + 5: expectation("failure", message, srcref = srcref, trace = trace) + 6: exp_signal(exp) + 7: withRestarts(if (expectation_broken(exp)) { + stop(exp) + } else { + signalC + 8: withRestartList(expr, restarts) 1: stop("stop") From 664c7c92e149962ab7e16c39633f643f8009fccb Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 24 Nov 2025 11:34:52 +0200 Subject: [PATCH 08/12] Simplify implementation/tests --- R/snapshot-file.R | 5 ++--- R/test-that.R | 2 +- tests/testthat/_snaps/reporter-debug.md | 18 +++++------------- tests/testthat/_snaps/snapshot-file.md | 8 -------- tests/testthat/test-snapshot-file.R | 23 ----------------------- 5 files changed, 8 insertions(+), 48 deletions(-) diff --git a/R/snapshot-file.R b/R/snapshot-file.R index b55e882f1..562b387e0 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -262,12 +262,11 @@ 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) { - # Do not manually call return here. - # If snapshot_fail is silenced by `muffle_expectation`, let it behave like `fail_on_new = FALSE` snapshot_fail(message, trace_env = trace_env) + } else { + testthat_warn(message) } - testthat_warn(message) TRUE } } diff --git a/R/test-that.R b/R/test-that.R index 4e7fbae92..db5f15394 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -77,7 +77,7 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { if (debug_end >= 0) { start <- eval_bare(quote(base::sys.nframe()), test_env) + 1L e$start_frame <- start - e$end_frame <- sys.nframe() - debug_end - 1L + e$end_frame <- sys.nframe() - debug_end - 3L } e$test <- test %||% "(code run outside of `test_that()`)" diff --git a/tests/testthat/_snaps/reporter-debug.md b/tests/testthat/_snaps/reporter-debug.md index 1d834c847..0cb917a20 100644 --- a/tests/testthat/_snaps/reporter-debug.md +++ b/tests/testthat/_snaps/reporter-debug.md @@ -5,11 +5,6 @@ 3: fail(msg, info = info, trace_env = trace_env) 4: expectation("failure", message, srcref = srcref, trace = trace) 5: exp_signal(exp) - 6: withRestarts(if (expectation_broken(exp)) { - stop(exp) - } else { - signalC - 7: withRestartList(expr, restarts) 1: f() 2: expect_true(FALSE) @@ -17,22 +12,19 @@ 4: fail(msg, info = info, trace_env = trace_env) 5: expectation("failure", message, srcref = srcref, trace = trace) 6: exp_signal(exp) - 7: withRestarts(if (expectation_broken(exp)) { - stop(exp) - } else { - signalC - 8: withRestartList(expr, restarts) 1: stop("stop") + 2: eval(code, test_env) + 3: eval(code, test_env) 1: f() 2: g() - 3: h() - 4: stop("!") 1: skip("skip") + 2: eval(code, test_env) + 3: eval(code, test_env) 1: f() - 2: warning("def") + 2: eval(code, test_env) diff --git a/tests/testthat/_snaps/snapshot-file.md b/tests/testthat/_snaps/snapshot-file.md index 157af2cfd..10e62fac0 100644 --- a/tests/testthat/_snaps/snapshot-file.md +++ b/tests/testthat/_snaps/snapshot-file.md @@ -22,14 +22,6 @@ Error in `snapshot_file_equal_()`: ! 'doesnt-exist.txt' not found. ---- - - Code - out <- snapshot_file_equal_w_muffle(path_muffle) - Condition - Warning: - Adding new file snapshot: 'tests/testthat/_snaps/muffle-test/test.txt' - # generates informative hint Code diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index 10028e529..5151674b2 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -136,29 +136,6 @@ test_that("warns on first creation", { expect_true(snapshot_file_equal_(path)) expect_true(file.exists(file.path(snap_dir, "my-test/test.txt"))) expect_false(file.exists(file.path(snap_dir, "my-test/test.new.txt"))) - - # Similar to shinytest2::AppDriver$expect_values() screenshot snapshot behavior - path_muffle <- write_tmp_lines("muffle-a") - snapshot_file_equal_w_muffle <- function(path) { - withCallingHandlers( - { - snapshot_file_equal( - snap_dir = snap_dir, - snap_test = "muffle-test", - snap_name = "test.txt", - snap_variant = NULL, - path = path, - fail_on_new = TRUE - ) - }, - expectation = function(e) { - invokeRestart("muffle_expectation") - } - ) - } - # Warns on first run: Should fail on new, however it is muffled - expect_snapshot(out <- snapshot_file_equal_w_muffle(path_muffle)) - expect_true(out) }) # helpers ----------------------------------------------------------------- From e19648c95b8cd5be55a62282a75adb5c1d2d428c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 24 Nov 2025 11:38:06 +0200 Subject: [PATCH 09/12] Wrong place --- R/test-that.R | 2 +- tests/testthat/_snaps/reporter-debug.md | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/R/test-that.R b/R/test-that.R index db5f15394..4e7fbae92 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -77,7 +77,7 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { if (debug_end >= 0) { start <- eval_bare(quote(base::sys.nframe()), test_env) + 1L e$start_frame <- start - e$end_frame <- sys.nframe() - debug_end - 3L + e$end_frame <- sys.nframe() - debug_end - 1L } e$test <- test %||% "(code run outside of `test_that()`)" diff --git a/tests/testthat/_snaps/reporter-debug.md b/tests/testthat/_snaps/reporter-debug.md index 0cb917a20..1d834c847 100644 --- a/tests/testthat/_snaps/reporter-debug.md +++ b/tests/testthat/_snaps/reporter-debug.md @@ -5,6 +5,11 @@ 3: fail(msg, info = info, trace_env = trace_env) 4: expectation("failure", message, srcref = srcref, trace = trace) 5: exp_signal(exp) + 6: withRestarts(if (expectation_broken(exp)) { + stop(exp) + } else { + signalC + 7: withRestartList(expr, restarts) 1: f() 2: expect_true(FALSE) @@ -12,19 +17,22 @@ 4: fail(msg, info = info, trace_env = trace_env) 5: expectation("failure", message, srcref = srcref, trace = trace) 6: exp_signal(exp) + 7: withRestarts(if (expectation_broken(exp)) { + stop(exp) + } else { + signalC + 8: withRestartList(expr, restarts) 1: stop("stop") - 2: eval(code, test_env) - 3: eval(code, test_env) 1: f() 2: g() + 3: h() + 4: stop("!") 1: skip("skip") - 2: eval(code, test_env) - 3: eval(code, test_env) 1: f() - 2: eval(code, test_env) + 2: warning("def") From b1c460bc92773f22ab78ee4c9f51fa041ff3d973 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 24 Nov 2025 11:39:23 +0200 Subject: [PATCH 10/12] Correct place --- R/test-that.R | 2 +- tests/testthat/_snaps/reporter-debug.md | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/R/test-that.R b/R/test-that.R index 4e7fbae92..21474f5f7 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -110,7 +110,7 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { } handle_expectation <- function(e) { the$test_expectations <- the$test_expectations + 1L - register_expectation(e, 7) + register_expectation(e, 9) invokeRestart("muffle_expectation") } handle_warning <- function(e) { diff --git a/tests/testthat/_snaps/reporter-debug.md b/tests/testthat/_snaps/reporter-debug.md index 1d834c847..824d64b0d 100644 --- a/tests/testthat/_snaps/reporter-debug.md +++ b/tests/testthat/_snaps/reporter-debug.md @@ -5,11 +5,6 @@ 3: fail(msg, info = info, trace_env = trace_env) 4: expectation("failure", message, srcref = srcref, trace = trace) 5: exp_signal(exp) - 6: withRestarts(if (expectation_broken(exp)) { - stop(exp) - } else { - signalC - 7: withRestartList(expr, restarts) 1: f() 2: expect_true(FALSE) @@ -17,11 +12,6 @@ 4: fail(msg, info = info, trace_env = trace_env) 5: expectation("failure", message, srcref = srcref, trace = trace) 6: exp_signal(exp) - 7: withRestarts(if (expectation_broken(exp)) { - stop(exp) - } else { - signalC - 8: withRestartList(expr, restarts) 1: stop("stop") From 71973d69c3c4bb57ce3a66cdbe53931922acb9d1 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 24 Nov 2025 11:39:47 +0200 Subject: [PATCH 11/12] Two more --- R/test-that.R | 2 +- tests/testthat/_snaps/reporter-debug.md | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/R/test-that.R b/R/test-that.R index 21474f5f7..05f7fb0cd 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -110,7 +110,7 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { } handle_expectation <- function(e) { the$test_expectations <- the$test_expectations + 1L - register_expectation(e, 9) + register_expectation(e, 11) invokeRestart("muffle_expectation") } handle_warning <- function(e) { diff --git a/tests/testthat/_snaps/reporter-debug.md b/tests/testthat/_snaps/reporter-debug.md index 824d64b0d..4c639d157 100644 --- a/tests/testthat/_snaps/reporter-debug.md +++ b/tests/testthat/_snaps/reporter-debug.md @@ -3,15 +3,11 @@ 1: expect_true(x) 2: expect_waldo_constant_(act, exp, info = info, ignore_attr = TRUE) 3: fail(msg, info = info, trace_env = trace_env) - 4: expectation("failure", message, srcref = srcref, trace = trace) - 5: exp_signal(exp) 1: f() 2: expect_true(FALSE) 3: expect_waldo_constant_(act, exp, info = info, ignore_attr = TRUE) 4: fail(msg, info = info, trace_env = trace_env) - 5: expectation("failure", message, srcref = srcref, trace = trace) - 6: exp_signal(exp) 1: stop("stop") From af2f9ad4929b1f3f7de6f610918f782f5a8cfdae Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 24 Nov 2025 11:40:36 +0200 Subject: [PATCH 12/12] WS --- R/snapshot-file.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 562b387e0..eb9b28719 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -160,7 +160,6 @@ expect_snapshot_file <- function( file_equal = compare, variant = variant, ) - if (inherits(equal, "expectation_failure")) { return(equal) }