Always return failure message

And other minor refactorings

Fixes #836

Author: hadley

NAMESPACE

@@ -4,7 +4,6 @@ S3method(as.data.frame,testthat_results)
 S3method(as.expectation,default)
 S3method(as.expectation,error)
 S3method(as.expectation,expectation)
-S3method(as.expectation,logical)
 S3method(as.expectation,skip)
 S3method(as.expectation,warning)
 S3method(compare,POSIXt)

NEWS.md

@@ -1,5 +1,8 @@
 # testthat (development version)
 
+* Expectation object always contains the failure message, even when succesful 
+  (#836)
+
 * New `expect_vector()` is a wrapper around `vctrs::vec_assert()` making it
   easy to test against the vctrs definitions of prototype and size (#846).
 

R/expectation.R

@@ -1,7 +1,7 @@
 #' The building block of all `expect_` functions
 #'
 #' Use this if you are writing your own expectation. See
-#' `vignette("custom-expectation")` for details
+#' `vignette("custom-expectation")` for details.
 #'
 #' @param ok Was the expectation successful?
 #' @param failure_message What message should be shown if the expectation was
@@ -12,7 +12,9 @@
 #'   forward a srcref captured elsewhere.
 #' @export
 expect <- function(ok, failure_message, info = NULL, srcref = NULL) {
-  exp <- as.expectation.logical(ok, failure_message, info = info, srcref = srcref)
+  type <- if (ok) "success" else "failure"
+  message <- paste(c(failure_message, info), collapse = "\n")
+  exp <- expectation(type, message, srcref = srcref)
 
   withRestarts(
     if (expectation_broken(exp)) {
@@ -28,8 +30,14 @@ expect <- function(ok, failure_message, info = NULL, srcref = NULL) {
 
 #' Construct an expectation object
 #'
-#' For advanced use only.
+#' For advanced use only. If you are creating your own expectation, you should
+#' call `expect()` instead. See `vignette("custom-expectation")` for more
+#' details.
 #'
+#' @param type Expectation type. Must be one of "success", "failure", "error",
+#'   "skip", "warning".
+#' @param message Message describing test failure
+#' @param srcref Optional `srcref` giving location of test.
 #' @keywords internal
 #' @export
 expectation <- function(type, message, srcref = NULL) {
@@ -184,17 +192,6 @@ as.expectation.expectation <- function(x, ..., srcref = NULL) {
   x
 }
 
-#' @export
-as.expectation.logical <- function(x, message, ..., srcref = NULL, info = NULL) {
-  type <- if (x) "success" else "failure"
-  message <- if (x) "success" else add_info(message, info)
-  expectation(type, message, srcref = srcref)
-}
-
-add_info <- function(message, info = NULL) {
-  paste(c(message, info), collapse = "\n")
-}
-
 #' @export
 as.expectation.error <- function(x, ..., srcref = NULL) {
   error <- x$message
@@ -227,7 +224,9 @@ as.expectation.skip <- function(x, ..., srcref = NULL) {
 }
 
 #' @export
-print.expectation <- function(x, ...) cat(format(x), "\n")
+print.expectation <- function(x, ...) {
+  cat(format(x), "\n")
+}
 
 #' @export
 format.expectation_success <- function(x, ...) {

man/expect.Rd

@@ -0,0 +1,6 @@
+context("test-expectation")
+
+test_that("expectation contains failure message even when successful", {
+  e <- expect(TRUE, "I failed")
+  expect_equal(e$message, "I failed")
+})