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 9bc6670e6..c9028b2b7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ # testthat (development version) * `expect_error()` and friends now error if you supply `...` but not `pattern` (#1932). +* 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). * Require the latest version of waldo (0.6.0) in order to get the latest goodies (#1955). diff --git a/R/expect-condition.R b/R/expect-condition.R index dbbcb62b2..3381407f4 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 { @@ -263,7 +260,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/R/expect-setequal.R b/R/expect-setequal.R index d710a1ac7..cfca84654 100644 --- a/R/expect-setequal.R +++ b/R/expect-setequal.R @@ -35,16 +35,16 @@ expect_setequal <- function(object, expected) { warn("expect_setequal() ignores names") } - act_miss <- !act$val %in% exp$val - exp_miss <- !exp$val %in% act$val + act_miss <- setdiff(act$val, exp$val) + exp_miss <- setdiff(exp$val, act$val) - if (any(exp_miss) || any(act_miss)) { + if (length(exp_miss) || length(act_miss)) { fail(paste0( act$lab, " (`actual`) and ", exp$lab, " (`expected`) don't have the same values.\n", - if (any(act_miss)) - paste0("* Only in `expected`: ", values(act$val[act_miss]), "\n"), - if (any(exp_miss)) - paste0("* Only in `actual`: ", values(exp$val[exp_miss]), "\n") + if (length(act_miss)) + paste0("* Only in `actual`: ", values(act_miss), "\n"), + if (length(exp_miss)) + paste0("* Only in `expected`: ", values(exp_miss), "\n") )) } else { succeed() 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/_snaps/expect-condition.md b/tests/testthat/_snaps/expect-condition.md index a39d112a9..f74a1d8fa 100644 --- a/tests/testthat/_snaps/expect-condition.md +++ b/tests/testthat/_snaps/expect-condition.md @@ -34,12 +34,12 @@ Code expect_condition(stop("Hi!"), foo = "bar") Condition - Error: + Error in `expect_condition()`: ! `...` ignored when `pattern` is not set. Code expect_condition(stop("Hi!"), "x", foo = "bar") Condition - Error: + Error in `expect_condition()`: ! Failed to compare message to `pattern`. Caused by error in `grepl()`: ! unused argument (foo = "bar") diff --git a/tests/testthat/_snaps/expect-setequal.md b/tests/testthat/_snaps/expect-setequal.md index 1225d9914..3ea8247ad 100644 --- a/tests/testthat/_snaps/expect-setequal.md +++ b/tests/testthat/_snaps/expect-setequal.md @@ -1,26 +1,33 @@ # useful message on failure + "actual" (`actual`) and "expected" (`expected`) don't have the same values. + * Only in `actual`: "actual" + * Only in `expected`: "expected" + + +--- + 1:2 (`actual`) and 2 (`expected`) don't have the same values. - * Only in `expected`: 1 + * Only in `actual`: 1 --- 2 (`actual`) and 2:3 (`expected`) don't have the same values. - * Only in `actual`: 3 + * Only in `expected`: 3 --- 1:2 (`actual`) and 2:3 (`expected`) don't have the same values. - * Only in `expected`: 1 - * Only in `actual`: 3 + * Only in `actual`: 1 + * Only in `expected`: 3 # truncates long vectors 1:2 (`actual`) and 1:50 (`expected`) don't have the same values. - * Only in `actual`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ... + * Only in `expected`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ... # expect_contains() gives useful message on failure diff --git a/tests/testthat/test-expect-condition.R b/tests/testthat/test-expect-condition.R index 53d892862..33e3d1711 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())) +}) diff --git a/tests/testthat/test-expect-setequal.R b/tests/testthat/test-expect-setequal.R index b4de30800..6966822ec 100644 --- a/tests/testthat/test-expect-setequal.R +++ b/tests/testthat/test-expect-setequal.R @@ -24,6 +24,8 @@ test_that("error for non-vectors", { }) test_that("useful message on failure", { + expect_snapshot_failure(expect_setequal("actual", "expected")) + expect_snapshot_failure(expect_setequal(1:2, 2)) expect_snapshot_failure(expect_setequal(2, 2:3)) expect_snapshot_failure(expect_setequal(1:2, 2:3)) @@ -112,4 +114,3 @@ test_that("expect_in() gives useful message on failure", { expect_snapshot_failure(expect_in(x1, x2)) expect_snapshot_failure(expect_in(x1, x3)) }) -