Make succeed()/failure() primary

Instead of `expect()`. I think this makes the reasoning a little easier to follow, and will support the transition away from `expect()`.

Author: hadley

R/expect-that.R

@@ -53,15 +53,28 @@ expect_that <- function(object, condition, info = NULL, label = NULL) {
 fail <- function(
   message = "Failure has been forced",
   info = NULL,
-  trace_env = caller_env()
+  srcref = NULL,
+  trace_env = caller_env(),
+  trace = NULL
 ) {
-  expect(FALSE, message, info = info, trace_env = trace_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
+  }
+
+  message <- paste(c(message, info), collapse = "\n")
+  expectation("failure", message, srcref = srcref, trace = trace)
 }
 
 #' @rdname fail
 #' @export
 succeed <- function(message = "Success has been forced", info = NULL) {
-  expect(TRUE, message, info = info)
+  message <- paste(c(message, info), collapse = "\n")
+
+  expectation("success", message)
 }
 
 #' Negate an expectation

R/expectation.R

@@ -43,49 +43,28 @@ expect <- function(
   trace = NULL,
   trace_env = caller_env()
 ) {
-  type <- if (ok) "success" else "failure"
-
-  # Preserve existing API which appear to be used in package test code
-  # Can remove in next major release
-  if (missing(failure_message)) {
-    warn("`failure_message` is missing, with no default.")
-    message <- "unknown failure"
+  if (ok) {
+    succeed(failure_message)
   } else {
-    # A few packages include code in info that errors on evaluation
-    if (ok) {
-      message <- paste(failure_message, collapse = "\n")
-    } else {
-      message <- paste(c(failure_message, info), collapse = "\n")
-    }
-  }
-
-  if (!ok) {
-    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
-    }
+    fail(
+      failure_message,
+      info,
+      srcref = srcref,
+      trace = trace,
+      trace_env = trace_env
+    )
   }
-
-  exp <- expectation(type, message, srcref = srcref, trace = trace)
-  exp_signal(exp)
 }
 
-
 #' Construct an expectation object
 #'
+#' @description
 #' For advanced use only. If you are creating your own expectation, you should
 #' call [expect()] instead. See `vignette("custom-expectation")` for more
 #' details.
 #'
-#' Create an expectation with `expectation()` or `new_expectation()`
-#' and signal it with `exp_signal()`.
+#' `new_expectation()` creates an expectation object and `exp_signal()` signals
+#' it. `expectation()` does both.
 #'
 #' @param type Expectation type. Must be one of "success", "failure", "error",
 #'   "skip", "warning".
@@ -94,7 +73,8 @@ expect <- function(
 #' @inheritParams expect
 #' @export
 expectation <- function(type, message, srcref = NULL, trace = NULL) {
-  new_expectation(type, message, srcref = srcref, trace = trace)
+  exp <- new_expectation(type, message, srcref = srcref, trace = trace)
+  exp_signal(exp)
 }
 #' @rdname expectation
 #' @param ... Additional attributes for the expectation object.
@@ -207,7 +187,7 @@ as.expectation.error <- function(x, srcref = NULL) {
     cnd_message(x)
   )
 
-  expectation("error", msg, srcref, trace = x[["trace"]])
+  new_expectation("error", msg, srcref = srcref, trace = x[["trace"]])
 }
 
 
@@ -217,12 +197,17 @@ is_simple_error <- function(x) {
 
 #' @export
 as.expectation.warning <- function(x, srcref = NULL) {
-  expectation("warning", cnd_message(x), srcref, trace = x[["trace"]])
+  new_expectation(
+    "warning",
+    cnd_message(x),
+    srcref = srcref,
+    trace = x[["trace"]]
+  )
 }
 
 #' @export
 as.expectation.skip <- function(x, ..., srcref = NULL) {
-  expectation("skip", cnd_message(x), srcref, trace = x[["trace"]])
+  new_expectation("skip", cnd_message(x), srcref = srcref, trace = x[["trace"]])
 }
 
 #' @export

R/test-compiled-code.R

@@ -60,7 +60,7 @@ run_cpp_tests <- function(package) {
       reporter$add_result(
         context = "Catch",
         test = "Catch",
-        result = expectation("failure", e$message)
+        result = new_expectation("failure", e$message)
       )
       reporter$end_test(context = "Catch", test = "Catch")
     }
@@ -89,7 +89,7 @@ run_cpp_tests <- function(package) {
       get_reporter()$start_test(context = context_name, test = test_name)
 
       for (i in seq_len(successes)) {
-        exp <- expectation("success", "")
+        exp <- new_expectation("success", "")
         exp$test <- test_name
         get_reporter()$add_result(
           context = context_name,
@@ -122,7 +122,7 @@ run_cpp_tests <- function(package) {
           c(line, line, 1, 1)
         )
 
-        exp <- expectation("failure", org_text, srcref = failure_srcref)
+        exp <- new_expectation("failure", org_text, srcref = failure_srcref)
         exp$test <- test_name
 
         get_reporter()$add_result(
@@ -143,7 +143,7 @@ run_cpp_tests <- function(package) {
           c(line, line, 1, 1)
         )
 
-        exp <- expectation("error", exception_text, srcref = exception_srcref)
+        exp <- new_expectation("error", exception_text, srcref = exception_srcref)
         exp$test <- test_name
 
         get_reporter()$add_result(

R/test-that.R

@@ -141,7 +141,7 @@ test_code <- function(test, code, env, reporter, skip_on_empty = TRUE) {
   }
   handle_expectation <- function(e) {
     handled <<- TRUE
-    register_expectation(e, 6)
+    register_expectation(e, 7)
     invokeRestart("continue_test")
   }
   handle_warning <- function(e) {

man/expectation.Rd

@@ -3,11 +3,13 @@
     1: expect_true(FALSE)
     2: expect_waldo_constant(act, TRUE, info = info, ignore_attr = TRUE)
     3: expect(length(comp) == 0, sprintf("%s is not %s\n\n%s", act$lab, deparse(co
+    4: fail(failure_message, info, srcref = srcref, trace = trace, trace_env = tra
 
     1: f()
     2: expect_true(FALSE)
     3: expect_waldo_constant(act, TRUE, info = info, ignore_attr = TRUE)
     4: expect(length(comp) == 0, sprintf("%s is not %s\n\n%s", act$lab, deparse(co
+    5: fail(failure_message, info, srcref = srcref, trace = trace, trace_env = tra
 
     1: stop("stop")
 

man/fail.Rd

@@ -3,10 +3,6 @@ test_that("expectation contains failure message even when successful", {
   expect_equal(e$message, "I failed")
 })
 
-test_that("expect warns if no `failure_message`", {
-  expect_warning(expect(TRUE), "missing, with no default")
-})
-
 test_that("info only evaluated on failure", {
   expect_error(expect(TRUE, "fail", info = stop("!")), NA)
 })