From 7698c6c9ad749cdc9c5bec52d609ff73636def64 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 28 Jul 2025 08:47:52 -0500 Subject: [PATCH 1/7] Improve self-testing capabilities `expect_success()` and `expect_failure()` are now stricter, testing the full set of assumptions (i.e. exactly one success/failure and zero failures/successes). This means that `expect_no_successes()` and `expect_no_failures()` can be deprecated. Fixes #2110 --- NEWS.md | 1 + R/expect-self-test.R | 120 ++++++++++++++-------- R/try-again.R | 2 +- man/expect_no_success.Rd | 19 ++++ man/expect_success.Rd | 16 +-- tests/testthat/_snaps/expect-self-test.md | 23 +++++ tests/testthat/test-expect-known.R | 27 ++--- tests/testthat/test-expect-no-condition.R | 2 +- tests/testthat/test-expect-self-test.R | 74 ++++++++----- tests/testthat/test-try-again.R | 6 +- vignettes/.gitignore | 2 + vignettes/custom-expectation.Rmd | 22 ++++ 12 files changed, 211 insertions(+), 103 deletions(-) create mode 100644 man/expect_no_success.Rd create mode 100644 tests/testthat/_snaps/expect-self-test.md diff --git a/NEWS.md b/NEWS.md index 66eba8104..b64d5c79d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `expect_no_failures()` and `expect_no_succeses()` are now deprecated as `expect_success()` now test for no failures and `expect_failure()` tests for no successes (#) * `local_edition()` now gives a useful error for bad values (#1547). * testthat now requires R 4.1. * `expect_s4_class()` now supports unquoting (@stibu81, #2064). diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 74e220e9d..65194af49 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -1,30 +1,39 @@ -capture_failure <- new_capture("expectation_failure") -capture_success <- function(expr) { +capture_success_failure <- function(expr) { cnd <- NULL - withCallingHandlers( - expr, - expectation_failure = function(cnd) { - invokeRestart("continue_test") - }, - expectation_success = function(cnd) { - cnd <<- cnd - } + n_success <- 0 + n_failure <- 0 + + last_failure <- NULL + + withRestarts( + withCallingHandlers( + expr, + expectation_failure = function(cnd) { + last_failure <<- cnd + n_failure <<- n_failure + 1 + # Finish the test without bubbling up + invokeRestart("failed") + }, + expectation_success = function(cnd) { + n_success <<- n_success + 1 + # Don't bubble up to any other handlers + invokeRestart("continue_test") + } + ), + failed = function() {} ) - cnd -} -new_capture("expectation_success") + list(n_success = n_success, n_failure = n_failure, last_failure = last_failure) +} #' Tools for testing expectations #' #' @description -#' * `expect_success()` and `expect_failure()` check that there's at least -#' one success or failure respectively. -#' * `expect_snapshot_failure()` records the failure message so that you can -#' manually check that it is informative. -#' * `expect_no_success()` and `expect_no_failure()` check that are no -#' successes or failures. +#' `expect_success()` checks that there's exactly one success and no failures; +#' `expect_failure()` checks that there's exactly one failure and no successes. +#' `expect_snapshot_failure()` records the failure message so that you can +#' manually check that it is informative. #' #' Use `show_failure()` in examples to print the failure message without #' throwing an error. @@ -34,41 +43,40 @@ new_capture("expectation_success") #' @param ... Other arguments passed on to [expect_match()]. #' @export expect_success <- function(expr) { - exp <- capture_success(expr) + status <- capture_success_failure(expr) - if (is.null(exp)) { - fail("Expectation did not succeed") - } else { + if (status$n_success == 1 && status$n_failure == 0) { succeed() + } else if (status$n_success == 0) { + fail("Expectation did not succeed") + } else if (status$n_success > 1) { + fail(sprintf("Expectation succeeded %i times, instead of once", status$n_success)) + } else if (status$n_failure > 0) { + fail(sprintf("Expectation failed %i times, instead of zero", status$n_failure)) } - invisible(NULL) -} -#' @export -#' @rdname expect_success -expect_no_success <- function(expr) { - exp <- capture_success(expr) - - if (!is.null(exp)) { - fail("Expectation succeeded") - } else { - succeed() - } invisible(NULL) } #' @export #' @rdname expect_success expect_failure <- function(expr, message = NULL, ...) { - exp <- capture_failure(expr) + status <- capture_success_failure(expr) - if (is.null(exp)) { + if (status$n_failure == 1 && status$n_success == 0) { + if (!is.null(message)) { + return(expect_match(status$last_failure$message, message, ...)) + } + } else if (status$n_failure == 0) { fail("Expectation did not fail") - } else if (!is.null(message)) { - expect_match(exp$message, message, ...) - } else { - succeed() + } else if (status$n_failure > 1) { + # This should be impossible, but including for completeness + fail("Expectation failed more than once") + } else if (status$n_success != 0) { + fail(sprintf("Expectation succeeded %i times, instead of never", status$n_success)) } + + succeed() invisible(NULL) } @@ -78,12 +86,36 @@ expect_snapshot_failure <- function(expr) { expect_snapshot_error(expr, "expectation_failure") } +#' Test for absence of success or failure +#' +#' @description +#' `r lifecycle::badge("deprecated")` +#' +#' These functions are deprecated because [expect_success()] and +#' [expect_failure()] now test for exactly one success or no failures, and +#' exactly one failure and no successes. +#' +#' @keywords internal #' @export -#' @rdname expect_success +expect_no_success <- function(expr) { + lifecycle::deprecate_warn("3.3.0", "expect_no_success()", "expect_failure()") + status <- capture_success_failure(expr) + + if (status$n_success > 0) { + fail("Expectation succeeded") + } else { + succeed() + } + invisible(NULL) +} + +#' @export +#' @rdname expect_no_success expect_no_failure <- function(expr) { - exp <- capture_failure(expr) + lifecycle::deprecate_warn("3.3.0", "expect_no_failure()", "expect_success()") + status <- capture_success_failure(expr) - if (!is.null(exp)) { + if (status$n_failure > 0) { fail("Expectation failed") } else { succeed() diff --git a/R/try-again.R b/R/try-again.R index 6df4e4898..adebc4f75 100644 --- a/R/try-again.R +++ b/R/try-again.R @@ -42,5 +42,5 @@ try_again <- function(times, code) { times <- times - 1L } - stop(e) + exp_signal(e) } diff --git a/man/expect_no_success.Rd b/man/expect_no_success.Rd new file mode 100644 index 000000000..437ce6091 --- /dev/null +++ b/man/expect_no_success.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/expect-self-test.R +\name{expect_no_success} +\alias{expect_no_success} +\alias{expect_no_failure} +\title{Test for absence of success or failure} +\usage{ +expect_no_success(expr) + +expect_no_failure(expr) +} +\description{ +\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} + +These functions are deprecated because \code{\link[=expect_success]{expect_success()}} and +\code{\link[=expect_failure]{expect_failure()}} now test for exactly one success or no failures, and +exactly one failure and no successes. +} +\keyword{internal} diff --git a/man/expect_success.Rd b/man/expect_success.Rd index a917e28e1..4acc0a435 100644 --- a/man/expect_success.Rd +++ b/man/expect_success.Rd @@ -2,23 +2,17 @@ % Please edit documentation in R/expect-self-test.R \name{expect_success} \alias{expect_success} -\alias{expect_no_success} \alias{expect_failure} \alias{expect_snapshot_failure} -\alias{expect_no_failure} \alias{show_failure} \title{Tools for testing expectations} \usage{ expect_success(expr) -expect_no_success(expr) - expect_failure(expr, message = NULL, ...) expect_snapshot_failure(expr) -expect_no_failure(expr) - show_failure(expr) } \arguments{ @@ -29,14 +23,10 @@ show_failure(expr) \item{...}{Other arguments passed on to \code{\link[=expect_match]{expect_match()}}.} } \description{ -\itemize{ -\item \code{expect_success()} and \code{expect_failure()} check that there's at least -one success or failure respectively. -\item \code{expect_snapshot_failure()} records the failure message so that you can +\code{expect_success()} checks that there's exactly one success and no failures; +\code{expect_failure()} checks that there's exactly one failure and no successes. +\code{expect_snapshot_failure()} records the failure message so that you can manually check that it is informative. -\item \code{expect_no_success()} and \code{expect_no_failure()} check that are no -successes or failures. -} Use \code{show_failure()} in examples to print the failure message without throwing an error. diff --git a/tests/testthat/_snaps/expect-self-test.md b/tests/testthat/_snaps/expect-self-test.md new file mode 100644 index 000000000..9b55dfa1f --- /dev/null +++ b/tests/testthat/_snaps/expect-self-test.md @@ -0,0 +1,23 @@ +# errors in expect_success bubble up + + Code + expect_success(abort("error")) + Condition + Error: + ! error + +# expect_no are deprecated + + Code + expect_no_failure(succeed()) + Condition + Warning: + `expect_no_failure()` was deprecated in testthat 3.3.0. + i Please use `expect_success()` instead. + Code + expect_no_success(fail()) + Condition + Warning: + `expect_no_success()` was deprecated in testthat 3.3.0. + i Please use `expect_failure()` instead. + diff --git a/tests/testthat/test-expect-known.R b/tests/testthat/test-expect-known.R index e37e247bb..6d28f7c1e 100644 --- a/tests/testthat/test-expect-known.R +++ b/tests/testthat/test-expect-known.R @@ -12,12 +12,9 @@ test_that("uses specified width", { test_that("creates file on first run", { file <- withr::local_tempfile() - expect_success( - expect_warning( - expect_known_output(cat("ok!\n"), file), - "Creating reference" - ) - ) + expect_known_output(cat("ok!\n"), file) |> + expect_success() |> + expect_warning("Creating reference") expect_true(file.exists(file)) }) @@ -77,12 +74,9 @@ test_that("correctly matches to a file", { }) test_that("first run is successful", { - expect_success( - expect_warning( - expect_known_value(2, "two.rds"), - "Creating reference" - ) - ) + expect_known_value(2, "two.rds") |> + expect_success() |> + expect_warning("Creating reference") unlink("two.rds") }) @@ -131,12 +125,9 @@ test_that("version 3 is possible", { # expect_known_hash ------------------------------------------------------- test_that("empty hash succeeds with warning", { - expect_success( - expect_warning( - expect_known_hash(1:10), - "No recorded hash" - ) - ) + expect_known_hash(1:10) |> + expect_success() |> + expect_warning("No recorded hash") }) test_that("only succeeds if hash is correct", { diff --git a/tests/testthat/test-expect-no-condition.R b/tests/testthat/test-expect-no-condition.R index 22a59fbbc..1b278d72a 100644 --- a/tests/testthat/test-expect-no-condition.R +++ b/tests/testthat/test-expect-no-condition.R @@ -22,7 +22,7 @@ test_that("expect_no_* pass with pure code", { }) test_that("expect_no_* don't emit success when they fail", { - expect_no_success(expect_no_error(stop("!"))) + expect_failure(expect_no_error(stop("!"))) }) test_that("capture correct trace_env (#1994)", { diff --git a/tests/testthat/test-expect-self-test.R b/tests/testthat/test-expect-self-test.R index 305bb54c6..f0a1254e4 100644 --- a/tests/testthat/test-expect-self-test.R +++ b/tests/testthat/test-expect-self-test.R @@ -1,26 +1,27 @@ -test_that("fail always fails", { - expect_failure(fail()) - expect_failure(fail("abc"), "abc") -}) - -test_that("succeed always succeeds", { - expect_success(succeed()) -}) +test_that("expect_failure() requires 1 failure and zero successes", { + expect_success(expect_failure(fail())) -test_that("expect_success errors if null", { - expect_error(expect_success(NULL)) + expect_failure(expect_failure({})) + expect_failure(expect_failure(succeed())) + expect_failure(expect_failure({succeed(); fail()})) }) -test_that("expect_success errors with msg", { - expect_error(expect_success(stop("asdf")), 'asdf') +test_that("expect_failure() can optionally match message", { + expect_success(expect_failure(fail("apple"), "apple")) + expect_failure(expect_failure(fail("apple"), "banana")) }) -test_that("expect_failure errors if null", { - expect_error(expect_failure(NULL)) +test_that("expect_success() requires 1 success and zero failures", { + expect_success(expect_success(succeed())) + + expect_failure(expect_success({})) + expect_failure(expect_success(fail())) + expect_failure(expect_success({succeed(); fail()})) + expect_failure(expect_success({succeed(); succeed()})) }) -test_that("expect_failure errors if no failure", { - expect_error(expect_failure(TRUE)) +test_that("errors in expect_success bubble up", { + expect_snapshot(expect_success(abort("error")), error = TRUE) }) test_that("show_failure", { @@ -28,18 +29,41 @@ test_that("show_failure", { expect_output(show_failure(expect_true(FALSE)), "FALSE is not TRUE") }) -test_that("can test for presence and absense of failure", { - expect_success(expect_failure(fail())) - expect_success(expect_no_failure(succeed())) +test_that("can count successes and failures", { + status <- capture_success_failure({}) + expect_equal(status$n_success, 0) + expect_equal(status$n_failure, 0) - expect_failure(expect_failure(succeed())) - expect_failure(expect_no_failure(fail())) + status <- capture_success_failure({ + succeed() + succeed() + fail() + }) + expect_equal(status$n_success, 2) + expect_equal(status$n_failure, 1) + + # No code run after first fail + status <- capture_success_failure({ + succeed() + fail() + succeed() + fail() + }) + expect_equal(status$n_success, 1) + expect_equal(status$n_failure, 1) }) -test_that("can test for presence and absense of success", { - expect_success(expect_success(succeed())) - expect_success(expect_no_success(fail())) +test_that("expect_no are deprecated", { + expect_snapshot({ + expect_no_failure(succeed()) + expect_no_success(fail()) + }) +}) - expect_failure(expect_success(fail())) +test_that("expect_no still work", { + withr::local_options(lifecycle_verbosity = "quiet") + expect_success(expect_no_failure(succeed())) + expect_failure(expect_no_failure(fail())) + expect_success(expect_no_success(fail())) expect_failure(expect_no_success(succeed())) }) diff --git a/tests/testthat/test-try-again.R b/tests/testthat/test-try-again.R index 68bfee79f..d76faf4a5 100644 --- a/tests/testthat/test-try-again.R +++ b/tests/testthat/test-try-again.R @@ -1,7 +1,11 @@ succeed_after <- function(i) { function() { i <<- i - 1 - if (i > 0) fail(paste0("i is ", i)) + if (i > 0) { + fail(paste0("i is ", i)) + } else { + succeed() + } } } diff --git a/vignettes/.gitignore b/vignettes/.gitignore index 097b24163..9e2bd63c1 100644 --- a/vignettes/.gitignore +++ b/vignettes/.gitignore @@ -1,2 +1,4 @@ *.html *.R + +/.quarto/ diff --git a/vignettes/custom-expectation.Rmd b/vignettes/custom-expectation.Rmd index 8bc1a4d7a..a2d8e47ef 100644 --- a/vignettes/custom-expectation.Rmd +++ b/vignettes/custom-expectation.Rmd @@ -10,6 +10,10 @@ vignette: > ```{r setup, include = FALSE} library(testthat) knitr::opts_chunk$set(collapse = TRUE, comment = "#>") + +# Pretend we're snapsotting +snapper <- local_snapshotter() +snapper$start_file("snapshotting.Rmd", "test") ``` This vignette shows you how to create custom expectations that work identically to the built-in `expect_` functions. Since these functions will need to be loaded in order for your tests to work, we recommend putting them in an appropriately named helper file, i.e. `tests/testthat/helper-expectations.R`. @@ -86,3 +90,21 @@ expect_length <- function(object, n) { fail(message) } ``` + +## Testing your expectations + +testthat comes with three expectations designed specifically to test expectations: `expect_success()` and `expect_failure()`: + +* `expect_success()` checks that your expectation emits exactly one success and zero failures. +* `expect_failure()` checks that your expectation emits exactly one failure and zero successes. +* `expect_failure_snapshot()` captures the failure message in a snapshot, making it easier to review if it's useful or not. + +```{r} +test_that("expect_length works as expected", { + x <- 1:10 + expect_success(expect_length(x, 10)) + expect_failure(expect_length(x, 11)) + + expect_snapshot_failure(expect_length(x, 11)) +}) +``` From 87e6a9ff0d2fc5fe24e0aabf494f57129d50e86c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 28 Jul 2025 09:14:22 -0500 Subject: [PATCH 2/7] Fix failing test --- tests/testthat/test-expect-condition.R | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-expect-condition.R b/tests/testthat/test-expect-condition.R index 858528819..f0e940415 100644 --- a/tests/testthat/test-expect-condition.R +++ b/tests/testthat/test-expect-condition.R @@ -78,8 +78,17 @@ test_that("can capture Throwable conditions from rJava", { test_that("capture correct trace_env (#1994)", { # This should fail, not error - expect_failure(expect_error(stop("oops")) |> expect_warning()) - expect_failure(expect_warning(expect_error(stop("oops")))) + status <- capture_success_failure({ + stop("oops") |> expect_error() |> expect_warning() + }) + expect_equal(status$n_success, 1) # from expect_error() + expect_equal(status$n_failure, 1) # from expect_warning() + + status <- capture_success_failure({ + stop("oops") %>% expect_error() %>% expect_warning() + }) + expect_equal(status$n_success, 1) # from expect_error() + expect_equal(status$n_failure, 1) # from expect_warning() }) # expect_warning() ---------------------------------------------------------- From 217cb1c2bef68c51ab2be2b8af1632eac064ac05 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 28 Jul 2025 09:14:39 -0500 Subject: [PATCH 3/7] Reformat --- R/expect-self-test.R | 29 +++++++++++++++++++------- tests/testthat/test-expect-known.R | 12 +++++------ tests/testthat/test-expect-self-test.R | 17 +++++++++++---- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 0f501ae30..2a290d12c 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -24,7 +24,11 @@ capture_success_failure <- function(expr) { failed = function() {} ) - list(n_success = n_success, n_failure = n_failure, last_failure = last_failure) + list( + n_success = n_success, + n_failure = n_failure, + last_failure = last_failure + ) } #' Tools for testing expectations @@ -50,9 +54,15 @@ expect_success <- function(expr) { } else if (status$n_success == 0) { fail("Expectation did not succeed") } else if (status$n_success > 1) { - fail(sprintf("Expectation succeeded %i times, instead of once", status$n_success)) + fail(sprintf( + "Expectation succeeded %i times, instead of once", + status$n_success + )) } else if (status$n_failure > 0) { - fail(sprintf("Expectation failed %i times, instead of zero", status$n_failure)) + fail(sprintf( + "Expectation failed %i times, instead of zero", + status$n_failure + )) } invisible(NULL) @@ -73,7 +83,10 @@ expect_failure <- function(expr, message = NULL, ...) { # This should be impossible, but including for completeness fail("Expectation failed more than once") } else if (status$n_success != 0) { - fail(sprintf("Expectation succeeded %i times, instead of never", status$n_success)) + fail(sprintf( + "Expectation succeeded %i times, instead of never", + status$n_success + )) } succeed() @@ -87,14 +100,14 @@ expect_snapshot_failure <- function(expr) { } #' Test for absence of success or failure -#' +#' #' @description #' `r lifecycle::badge("deprecated")` -#' -#' These functions are deprecated because [expect_success()] and +#' +#' These functions are deprecated because [expect_success()] and #' [expect_failure()] now test for exactly one success or no failures, and #' exactly one failure and no successes. -#' +#' #' @keywords internal #' @export expect_no_success <- function(expr) { diff --git a/tests/testthat/test-expect-known.R b/tests/testthat/test-expect-known.R index 4536cbbff..5518b8200 100644 --- a/tests/testthat/test-expect-known.R +++ b/tests/testthat/test-expect-known.R @@ -12,8 +12,8 @@ test_that("uses specified width", { test_that("creates file on first run", { file <- withr::local_tempfile() - expect_known_output(cat("ok!\n"), file) |> - expect_success() |> + expect_known_output(cat("ok!\n"), file) |> + expect_success() |> expect_warning("Creating reference") expect_true(file.exists(file)) @@ -73,8 +73,8 @@ test_that("correctly matches to a file", { }) test_that("first run is successful", { - expect_known_value(2, "two.rds") |> - expect_success() |> + expect_known_value(2, "two.rds") |> + expect_success() |> expect_warning("Creating reference") unlink("two.rds") }) @@ -124,8 +124,8 @@ test_that("version 3 is possible", { # expect_known_hash ------------------------------------------------------- test_that("empty hash succeeds with warning", { - expect_known_hash(1:10) |> - expect_success() |> + expect_known_hash(1:10) |> + expect_success() |> expect_warning("No recorded hash") }) diff --git a/tests/testthat/test-expect-self-test.R b/tests/testthat/test-expect-self-test.R index f0a1254e4..3e5214295 100644 --- a/tests/testthat/test-expect-self-test.R +++ b/tests/testthat/test-expect-self-test.R @@ -3,7 +3,10 @@ test_that("expect_failure() requires 1 failure and zero successes", { expect_failure(expect_failure({})) expect_failure(expect_failure(succeed())) - expect_failure(expect_failure({succeed(); fail()})) + expect_failure(expect_failure({ + succeed() + fail() + })) }) test_that("expect_failure() can optionally match message", { @@ -13,11 +16,17 @@ test_that("expect_failure() can optionally match message", { test_that("expect_success() requires 1 success and zero failures", { expect_success(expect_success(succeed())) - + expect_failure(expect_success({})) expect_failure(expect_success(fail())) - expect_failure(expect_success({succeed(); fail()})) - expect_failure(expect_success({succeed(); succeed()})) + expect_failure(expect_success({ + succeed() + fail() + })) + expect_failure(expect_success({ + succeed() + succeed() + })) }) test_that("errors in expect_success bubble up", { From 066ab5964a4c7ba809aa5fcd73643a140b77c20d Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 28 Jul 2025 09:16:23 -0500 Subject: [PATCH 4/7] Make code structure more parallel --- R/expect-self-test.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 2a290d12c..da22b08bd 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -77,6 +77,7 @@ expect_failure <- function(expr, message = NULL, ...) { if (!is.null(message)) { return(expect_match(status$last_failure$message, message, ...)) } + succeed() } else if (status$n_failure == 0) { fail("Expectation did not fail") } else if (status$n_failure > 1) { @@ -89,7 +90,6 @@ expect_failure <- function(expr, message = NULL, ...) { )) } - succeed() invisible(NULL) } From 056f7bce86435097f18361c3c76badd43e183206 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 28 Jul 2025 15:42:06 -0500 Subject: [PATCH 5/7] Use `pass()`; rearrange for easier comprehension --- R/expect-self-test.R | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 226e18d2f..a914f248c 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -49,23 +49,23 @@ capture_success_failure <- function(expr) { expect_success <- function(expr) { status <- capture_success_failure(expr) - if (status$n_success == 1 && status$n_failure == 0) { - succeed() - } else if (status$n_success == 0) { + if (status$n_success == 0) { fail("Expectation did not succeed") } else if (status$n_success > 1) { fail(sprintf( "Expectation succeeded %i times, instead of once", status$n_success )) - } else if (status$n_failure > 0) { + } + + if (status$n_failure > 0) { fail(sprintf( "Expectation failed %i times, instead of zero", status$n_failure )) } - invisible(NULL) + pass(NULL) } #' @export @@ -73,24 +73,24 @@ expect_success <- function(expr) { expect_failure <- function(expr, message = NULL, ...) { status <- capture_success_failure(expr) - if (status$n_failure == 1 && status$n_success == 0) { - if (!is.null(message)) { - return(expect_match(status$last_failure$message, message, ...)) - } - succeed() - } else if (status$n_failure == 0) { + if (status$n_failure == 0) { fail("Expectation did not fail") } else if (status$n_failure > 1) { # This should be impossible, but including for completeness fail("Expectation failed more than once") - } else if (status$n_success != 0) { + } + + if (status$n_success != 0) { fail(sprintf( "Expectation succeeded %i times, instead of never", status$n_success )) } - invisible(NULL) + if (!is.null(message)) { + return(expect_match(status$last_failure$message, message, ...)) + } + pass(NULL) } #' @export From f79d7cc4dd397ccc8c2459adf8cffa2a4626b31b Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 29 Jul 2025 07:50:46 -0500 Subject: [PATCH 6/7] Update NEWS.md Co-authored-by: Lionel Henry --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index e6ba9a805..8e4f43a32 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # testthat (development version) -* `expect_no_failures()` and `expect_no_succeses()` are now deprecated as `expect_success()` now test for no failures and `expect_failure()` tests for no successes (#) +* `expect_no_failures()` and `expect_no_successes()` are now deprecated as `expect_success()` now test for no failures and `expect_failure()` tests for no successes (#) * New `pass()` function to use in place of `succeed()` (#2113). * `expectation()` is now a combination of `new_expectation()` and `exp_signal()` (#2125). * `is_null()`/`matches()` deprecated in 2.0.0 (2017-12-19) and `is_true()`/`is_false()` deprecated in 2.1.0 (2019-04-23) have been removed (#2109). From 464251f8ee46fd01790e83db29347cc8783c852c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 29 Jul 2025 07:53:20 -0500 Subject: [PATCH 7/7] One more test --- tests/testthat/test-expect-self-test.R | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/testthat/test-expect-self-test.R b/tests/testthat/test-expect-self-test.R index 3e5214295..2dc664f82 100644 --- a/tests/testthat/test-expect-self-test.R +++ b/tests/testthat/test-expect-self-test.R @@ -7,6 +7,13 @@ test_that("expect_failure() requires 1 failure and zero successes", { succeed() fail() })) + + expect_success(expect_failure({ + fail() + # Following succeed/fail are never reached + succeed() + fail() + })) }) test_that("expect_failure() can optionally match message", {