From e8fb98f4af7d335fb15be6343794519538a9efd7 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 6 Oct 2025 08:50:38 -0500 Subject: [PATCH 1/3] Check `expect()` inputs Fixes #2247 --- R/expectation.R | 3 +++ tests/testthat/_snaps/expectation.md | 13 +++++++++++++ tests/testthat/test-expectation.R | 7 +++++++ 3 files changed, 23 insertions(+) create mode 100644 tests/testthat/_snaps/expectation.md diff --git a/R/expectation.R b/R/expectation.R index 55f00845a..69783d257 100644 --- a/R/expectation.R +++ b/R/expectation.R @@ -20,6 +20,9 @@ expect <- function( trace = NULL, trace_env = caller_env() ) { + check_bool(ok) + check_character(failure_message) + if (!ok) { return(fail( failure_message, diff --git a/tests/testthat/_snaps/expectation.md b/tests/testthat/_snaps/expectation.md new file mode 100644 index 000000000..977def5f4 --- /dev/null +++ b/tests/testthat/_snaps/expectation.md @@ -0,0 +1,13 @@ +# validates key inputs + + Code + expect(1) + Condition + Error in `expect()`: + ! `ok` must be `TRUE` or `FALSE`, not the number 1. + Code + expect(TRUE, 1) + Condition + Error in `expect()`: + ! `failure_message` must be a character vector, not the number 1. + diff --git a/tests/testthat/test-expectation.R b/tests/testthat/test-expectation.R index 3e63e2ca2..39d02f07f 100644 --- a/tests/testthat/test-expectation.R +++ b/tests/testthat/test-expectation.R @@ -7,6 +7,13 @@ test_that("info only evaluated on failure", { expect_no_error(expect(TRUE, "fail", info = stop("!"))) }) +test_that("validates key inputs", { + expect_snapshot(error = TRUE, { + expect(1) + expect(TRUE, 1) + }) +}) + test_that("can subclass expectation", { exp <- new_expectation( "failure", From c8daa5c7558f55ac04384131bb37dccba9e78a9b Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 6 Oct 2025 08:58:29 -0500 Subject: [PATCH 2/3] Check `fail()` too; and clarify docs --- R/expect-that.R | 8 ++++++-- R/expectation.R | 4 +++- man/expect.Rd | 4 +++- man/fail.Rd | 5 +++-- man/succeed.Rd | 5 +++-- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/R/expect-that.R b/R/expect-that.R index 730f3b2fb..e814212c1 100644 --- a/R/expect-that.R +++ b/R/expect-that.R @@ -8,8 +8,9 @@ #' Learn more about creating your own expectations in #' `vignette("custom-expectation")`. #' -#' @param message Failure message to send to the user. It's best practice to -#' describe both what is expected and what was actually received. +#' @param message A character vector describing the failure. The +#' first element should describe the expected value, and the second (and +#' optionally subsequence) elements should describe what was actually seen. #' @param info Character vector continuing additional information. Included #' for backward compatibility only and new expectations should not use it. #' @param srcref Location of the failure. Should only needed to be explicitly @@ -41,6 +42,9 @@ fail <- function( trace_env = caller_env(), trace = NULL ) { + check_character(message) + check_character(info, all_null = TRUE) + trace <- trace %||% capture_trace(trace_env) message <- paste(c(message, info), collapse = "\n") expectation("failure", message, srcref = srcref, trace = trace) diff --git a/R/expectation.R b/R/expectation.R index 69783d257..a155a3430 100644 --- a/R/expectation.R +++ b/R/expectation.R @@ -5,7 +5,9 @@ #' `vignette("custom-expectation")` for details. #' #' @param ok `TRUE` or `FALSE` indicating if the expectation was successful. -#' @param failure_message Message to show if the expectation failed. +#' @param failure_message A character vector describing the failure. The +#' first element should describe the expected value, and the second (and +#' optionally subsequence) elements should describe what was actually seen. #' @inheritParams fail #' @return An expectation object from either `succeed()` or `fail()`. #' with a `continue_test` restart. diff --git a/man/expect.Rd b/man/expect.Rd index b1da702e3..2b0c29fab 100644 --- a/man/expect.Rd +++ b/man/expect.Rd @@ -16,7 +16,9 @@ expect( \arguments{ \item{ok}{\code{TRUE} or \code{FALSE} indicating if the expectation was successful.} -\item{failure_message}{Message to show if the expectation failed.} +\item{failure_message}{A character vector describing the failure. The +first element should describe the expected value, and the second (and +optionally subsequence) elements should describe what was actually seen.} \item{info}{Character vector continuing additional information. Included for backward compatibility only and new expectations should not use it.} diff --git a/man/fail.Rd b/man/fail.Rd index 09d9d4350..04d4cc446 100644 --- a/man/fail.Rd +++ b/man/fail.Rd @@ -16,8 +16,9 @@ fail( pass(value) } \arguments{ -\item{message}{Failure message to send to the user. It's best practice to -describe both what is expected and what was actually received.} +\item{message}{A character vector describing the failure. The +first element should describe the expected value, and the second (and +optionally subsequence) elements should describe what was actually seen.} \item{info}{Character vector continuing additional information. Included for backward compatibility only and new expectations should not use it.} diff --git a/man/succeed.Rd b/man/succeed.Rd index c05398609..772e19db4 100644 --- a/man/succeed.Rd +++ b/man/succeed.Rd @@ -7,8 +7,9 @@ succeed(message = "Success has been forced", info = NULL) } \arguments{ -\item{message}{Failure message to send to the user. It's best practice to -describe both what is expected and what was actually received.} +\item{message}{A character vector describing the failure. The +first element should describe the expected value, and the second (and +optionally subsequence) elements should describe what was actually seen.} \item{info}{Character vector continuing additional information. Included for backward compatibility only and new expectations should not use it.} From e73580c29460b869c1db4f20979587e53fbb0702 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 6 Oct 2025 09:02:41 -0500 Subject: [PATCH 3/3] Fix typo --- R/expect-that.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/expect-that.R b/R/expect-that.R index e814212c1..2c09edb5f 100644 --- a/R/expect-that.R +++ b/R/expect-that.R @@ -43,7 +43,7 @@ fail <- function( trace = NULL ) { check_character(message) - check_character(info, all_null = TRUE) + check_character(info, allow_null = TRUE) trace <- trace %||% capture_trace(trace_env) message <- paste(c(message, info), collapse = "\n")