diff --git a/NEWS.md b/NEWS.md index 569632b0d..8e4f43a32 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `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). diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 2dae2deab..a914f248c 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -1,30 +1,43 @@ -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,42 +47,50 @@ 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)) { + if (status$n_success == 0) { fail("Expectation did not succeed") - } else { - succeed() + } else if (status$n_success > 1) { + fail(sprintf( + "Expectation succeeded %i times, instead of once", + status$n_success + )) } - invisible(NULL) -} - -#' @export -#' @rdname expect_success -expect_no_success <- function(expr) { - exp <- capture_success(expr) - if (!is.null(exp)) { - fail("Expectation succeeded") - } else { - succeed() + if (status$n_failure > 0) { + fail(sprintf( + "Expectation failed %i times, instead of zero", + status$n_failure + )) } - invisible(NULL) + + pass(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 == 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") } - invisible(NULL) + + if (status$n_success != 0) { + fail(sprintf( + "Expectation succeeded %i times, instead of never", + status$n_success + )) + } + + if (!is.null(message)) { + return(expect_match(status$last_failure$message, message, ...)) + } + pass(NULL) } #' @export @@ -78,12 +99,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 df68f1d89..67b19f5f3 100644 --- a/R/try-again.R +++ b/R/try-again.R @@ -44,5 +44,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-condition.R b/tests/testthat/test-expect-condition.R index 4ac6f83d0..5209f45d4 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() ---------------------------------------------------------- diff --git a/tests/testthat/test-expect-known.R b/tests/testthat/test-expect-known.R index 3f11e801e..5518b8200 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)) }) @@ -76,12 +73,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") }) @@ -130,12 +124,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 8d39676e9..c05b248f8 100644 --- a/tests/testthat/test-expect-no-condition.R +++ b/tests/testthat/test-expect-no-condition.R @@ -20,7 +20,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..2dc664f82 100644 --- a/tests/testthat/test-expect-self-test.R +++ b/tests/testthat/test-expect-self-test.R @@ -1,26 +1,43 @@ -test_that("fail always fails", { - expect_failure(fail()) - expect_failure(fail("abc"), "abc") -}) +test_that("expect_failure() requires 1 failure and zero successes", { + expect_success(expect_failure(fail())) -test_that("succeed always succeeds", { - expect_success(succeed()) -}) + expect_failure(expect_failure({})) + expect_failure(expect_failure(succeed())) + expect_failure(expect_failure({ + succeed() + fail() + })) -test_that("expect_success errors if null", { - expect_error(expect_success(NULL)) + expect_success(expect_failure({ + fail() + # Following succeed/fail are never reached + 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 +45,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/custom-expectation.Rmd b/vignettes/custom-expectation.Rmd index 4b3a9a853..ed8fd1dae 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 write your expectations that work identically to the built-in `expect_` functions. @@ -78,3 +82,21 @@ mtcars |> expect_s3_class("data.frame") |> expect_length(11) ``` + +## 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)) +}) +```