diff --git a/NAMESPACE b/NAMESPACE index e13d30231..deb5f2010 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -106,8 +106,10 @@ export(expect_more_than) export(expect_named) export(expect_no_condition) export(expect_no_error) +export(expect_no_failure) export(expect_no_match) export(expect_no_message) +export(expect_no_success) export(expect_no_warning) export(expect_null) export(expect_output) @@ -119,6 +121,7 @@ export(expect_setequal) export(expect_silent) export(expect_snapshot) export(expect_snapshot_error) +export(expect_snapshot_failure) export(expect_snapshot_file) export(expect_snapshot_output) export(expect_snapshot_value) diff --git a/NEWS.md b/NEWS.md index fa4c464cc..c1877aff1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # testthat (development version) +* New `expect_no_failure()`, `expect_no_success()` and `expect_snapshot_failure()` provide more options for testing expectations. +* `expect_error()` and friends no longer give an uninformative error if they fail inside a magrittr pipe (#1994). * `expect_setequal()` correctly identifies what is missing where (#1962). * `expect_true()` and `expect_false()` give better errors if `actual` isn't a vector (#1996). * `expect_no_*()` expectations no longer incorrectly emit a passing test result if they in fact fail (#1997). diff --git a/R/expect-condition.R b/R/expect-condition.R index 6f299cc59..ce5c1d4a6 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -163,8 +163,7 @@ expect_warning <- function(object, ..., inherit = inherit, info = info, - label = label, - trace_env = caller_env() + label = label ) } else { act <- quasi_capture(enquo(object), label, capture_warnings, ignore_deprecation = identical(regexp, NA)) @@ -196,8 +195,7 @@ expect_message <- function(object, ..., inherit = inherit, info = info, - label = label, - trace_env = caller_env() + label = label ) } else { act <- quasi_capture(enquo(object), label, capture_messages) @@ -225,8 +223,7 @@ expect_condition <- function(object, ..., inherit = inherit, info = info, - label = label, - trace_env = caller_env() + label = label ) } else { @@ -256,6 +253,7 @@ expect_condition_matching <- function(base_class, label = NULL, trace_env = caller_env(), error_call = caller_env()) { + check_dots_used(error = function(cnd) { warn(conditionMessage(cnd), call = error_call) }) @@ -267,7 +265,7 @@ expect_condition_matching <- function(base_class, ..., inherit = inherit, ignore_deprecation = base_class == "warning" && identical(regexp, NA), - error_call = trace_env + error_call = error_call ) act <- quasi_capture( diff --git a/R/expect-no-condition.R b/R/expect-no-condition.R index ac79e922c..a3f8b40d9 100644 --- a/R/expect-no-condition.R +++ b/R/expect-no-condition.R @@ -82,10 +82,10 @@ expect_no_condition <- function(object, expect_no_ <- function(base_class, - object, - regexp = NULL, - class = NULL, - error_call = caller_env()) { + object, + regexp = NULL, + class = NULL, + trace_env = caller_env()) { matcher <- cnd_matcher( base_class, @@ -116,7 +116,7 @@ expect_no_ <- function(base_class, indent_lines(rlang::cnd_message(cnd)) ) message <- format_error_bullets(c(expected, i = actual)) - fail(message, trace_env = error_call) + fail(message, trace_env = trace_env) } ) } diff --git a/R/expect-self-test.R b/R/expect-self-test.R index 112018800..6ff00d5a4 100644 --- a/R/expect-self-test.R +++ b/R/expect-self-test.R @@ -1,23 +1,56 @@ +capture_failure <- new_capture("expectation_failure") +capture_success <- function(expr) { + cnd <- NULL + + withCallingHandlers( + expr, + expectation_failure = function(cnd) { + invokeRestart("continue_test") + }, + expectation_success = function(cnd) { + cnd <<- cnd + } + ) + cnd +} + +new_capture("expectation_success") + #' Tools for testing expectations #' -#' Use these expectations to test other expectations. +#' @description +#' * `expect_sucess()` 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. +#' #' Use `show_failure()` in examples to print the failure message without #' throwing an error. #' -#' @param expr Expression that evaluates a single expectation. +#' @param expr Code to evalute #' @param message Check that the failure message matches this regexp. #' @param ... Other arguments passed on to [expect_match()]. #' @export expect_success <- function(expr) { - exp <- capture_expectation(expr) + exp <- capture_success(expr) if (is.null(exp)) { - fail("no expectation used.") - } else if (!expectation_success(exp)) { - fail(paste0( - "Expectation did not succeed:\n", - exp$message - )) + fail("Expectation did not succeed") + } else { + succeed() + } + invisible(NULL) +} + +#' @export +#' @rdname expect_success +expect_no_success <- function(expr) { + exp <- capture_success(expr) + + if (!is.null(exp)) { + fail("Expectation succeeded") } else { succeed() } @@ -27,19 +60,31 @@ expect_success <- function(expr) { #' @export #' @rdname expect_success expect_failure <- function(expr, message = NULL, ...) { - exp <- capture_expectation(expr) + exp <- capture_failure(expr) if (is.null(exp)) { - fail("No expectation used") - return() - } - if (!expectation_failure(exp)) { fail("Expectation did not fail") - return() + } else if (!is.null(message)) { + expect_match(exp$message, message, ...) + } else { + succeed() } + invisible(NULL) +} - if (!is.null(message)) { - expect_match(exp$message, message, ...) +#' @export +#' @rdname expect_success +expect_snapshot_failure <- function(expr) { + expect_snapshot_error(expr, "expectation_failure") +} + +#' @export +#' @rdname expect_success +expect_no_failure <- function(expr) { + exp <- capture_failure(expr) + + if (!is.null(exp)) { + fail("Expectation failed") } else { succeed() } @@ -67,10 +112,6 @@ show_failure <- function(expr) { invisible() } -expect_snapshot_failure <- function(x) { - expect_snapshot_error(x, "expectation_failure") -} - expect_snapshot_reporter <- function(reporter, paths = test_path("reporters/tests.R")) { local_options(rlang_trace_format_srcrefs = FALSE) local_rng_version("3.3") diff --git a/man/expect_success.Rd b/man/expect_success.Rd index e4d6304fa..b533e0239 100644 --- a/man/expect_success.Rd +++ b/man/expect_success.Rd @@ -2,25 +2,42 @@ % 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{ -\item{expr}{Expression that evaluates a single expectation.} +\item{expr}{Code to evalute} \item{message}{Check that the failure message matches this regexp.} \item{...}{Other arguments passed on to \code{\link[=expect_match]{expect_match()}}.} } \description{ -Use these expectations to test other expectations. +\itemize{ +\item \code{expect_sucess()} 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 +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/test-expect-condition.R b/tests/testthat/test-expect-condition.R index d5cff5b02..4ee1a9234 100644 --- a/tests/testthat/test-expect-condition.R +++ b/tests/testthat/test-expect-condition.R @@ -76,6 +76,12 @@ test_that("can capture Throwable conditions from rJava", { expect_error(throw("foo"), "foo", class = "Throwable") }) +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")))) +}) + # expect_warning() ---------------------------------------------------------- test_that("warnings are converted to errors when options('warn') >= 2", { diff --git a/tests/testthat/test-expect-no-condition.R b/tests/testthat/test-expect-no-condition.R index f5c2c0d43..a52649abc 100644 --- a/tests/testthat/test-expect-no-condition.R +++ b/tests/testthat/test-expect-no-condition.R @@ -14,20 +14,13 @@ test_that("expect_no_* conditions behave as expected", { }) test_that("expect_no_* don't emit success when they fail", { + expect_no_success(expect_no_error(stop("!"))) +}) - catch_cnds <- function(code) { - cnds <- list() - - withCallingHandlers(code, condition = function(cnd) { - cnds[[length(cnds) + 1]] <<- cnd - invokeRestart("continue_test") - }) - cnds - } - - cnds <- catch_cnds(expect_no_error(stop("!"))) - expect_length(cnds, 1) - expect_s3_class(cnds[[1]], "expectation_failure") +test_that("capture correct trace_env (#1994)", { + # This should fail, not error + expect_failure(expect_message({message("a"); warn("b")}) %>% expect_no_warning()) + expect_failure(expect_no_message({message("a"); warn("b")}) %>% expect_warning()) }) test_that("unmatched conditions bubble up", { diff --git a/tests/testthat/test-expect-self-test.R b/tests/testthat/test-expect-self-test.R index 20b2b0f12..305bb54c6 100644 --- a/tests/testthat/test-expect-self-test.R +++ b/tests/testthat/test-expect-self-test.R @@ -27,3 +27,19 @@ test_that("show_failure", { expect_null(show_failure(NULL)) 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())) + + expect_failure(expect_failure(succeed())) + expect_failure(expect_no_failure(fail())) +}) + +test_that("can test for presence and absense of success", { + expect_success(expect_success(succeed())) + expect_success(expect_no_success(fail())) + + expect_failure(expect_success(fail())) + expect_failure(expect_no_success(succeed())) +})