-
Notifications
You must be signed in to change notification settings - Fork 341
Description
I think we need to move away from expect() and towards explicitly calling fail() and (new) pass() in each test. Rough/draft implementations of pass() and fail() below β the idea is to make these the core functions so that you don't need to follow a long sequence of calls to understand what's happening. We need to introduce a new pass() so that it can invisibly return a value, which is what should always happen during success.
fail <- function(
message,
info = NULL,
srcref = NULL,
trace = NULL,
trace_env = caller_env()
) {
if (is.null(trace)) {
trace <- trace_back(top = getOption("testthat_topenv"), bottom = trace_env)
}
# Only show if there's at least one function apart from the expectation
if (trace_length(trace) <= 1) {
trace <- NULL
}
exp <- new_expectation("failure", message, srcref = srcref, trace = trace)
withRestarts(
stop(exp),
continue_test = function(e) NULL
)
}
pass <- function(value) {
signalCondition(new_expectation("success", ""))
invisible(value)
}Once we have these two functions, there's a clearer principle for correctly defining expect_ functions: every terminal branch of a function should either be pass() or fail() (or an error about invalid inputs)
Then we soft-deprecate expect() and succeed() and rewrite all testthat's expectations, e.g.
# OLD
expect(
length(comp) == 0,
sprintf(
"%s is not %s\n\n%s",
act$lab,
deparse(constant),
paste0(comp, collapse = "\n\n")
),
info = info,
trace_env = caller_env()
)
invisible(act$val)
# NEW
if (length(comp) != 0) {
msg <- sprintf(
"%s is not %s\n\n%s",
act$lab,
deparse(constant),
paste0(comp, collapse = "\n\n")
)
fail(msg, info = info, trace_env = caller_env())
}
pass(act$val)(Once this is done I think we can also consider soft deprecating expectation() and as_expectation())