Always report success/failure count (#2298)

Author: MichaelChirico

NEWS.md

@@ -1,5 +1,6 @@
 # testthat 3.3.1
 
+* `expect_success()` and `expect_failure()` are more clear about what the expectation actually did (#2297).
 * The hint to use `snapshot_download_gh()` is now only emitted when running in a job named "R-CMD-check" (#2300).
 * `expect_snapshot_file()` correctly reports file name if duplicated (@MichaelChirico, #2296).
 * Fixed support for `shinytest2::AppDriver$expect_values()` screenshot snapshot failing on CI (#2293, #2288).

R/expect-self-test.R

@@ -26,6 +26,36 @@ capture_success_failure <- function(expr) {
   )
 }
 
+format_success_failure <- function(status, exp_n_success, exp_n_failure) {
+  pluralise <- function(n, singular, plural) {
+    paste(n, ngettext(n, singular, plural))
+  }
+
+  tick <- cli::col_green(cli::symbol$tick)
+  cross <- cli::col_red(cli::symbol$cross)
+
+  success_ok <- status$n_success == exp_n_success
+  failure_ok <- status$n_failure == exp_n_failure
+
+  c(
+    sprintf(
+      "Expected %s and %s.",
+      pluralise(exp_n_success, "success", "successes"),
+      pluralise(exp_n_failure, "failure", "failures")
+    ),
+    sprintf(
+      "%s Observed %s.",
+      if (success_ok) tick else cross,
+      pluralise(status$n_success, "success", "successes")
+    ),
+    sprintf(
+      "%s Observed %s.",
+      if (failure_ok) tick else cross,
+      pluralise(status$n_failure, "failure", "failures")
+    )
+  )
+}
+
 #' Test your custom expectations
 #'
 #' @description
@@ -44,17 +74,11 @@ capture_success_failure <- function(expr) {
 expect_success <- function(expr) {
   status <- capture_success_failure(expr)
 
-  expected <- "Expected exactly one success and no failures."
-  if (status$n_success != 1) {
-    actual <- sprintf("Actually succeeded %i times", status$n_success)
-    fail(c(expected, actual))
-  } else if (status$n_failure > 0) {
-    actual <- sprintf("Actually failed %i times", status$n_failure)
-    fail(c(expected, actual))
-  } else {
+  if (status$n_success == 1 && status$n_failure == 0) {
     pass()
+    return(invisible())
   }
-
+  fail(format_success_failure(status, exp_n_success = 1, exp_n_failure = 0))
   invisible()
 }
 
@@ -63,21 +87,17 @@ expect_success <- function(expr) {
 expect_failure <- function(expr, message = NULL, ...) {
   status <- capture_success_failure(expr)
 
-  expected <- "Expected exactly one failure and no successes."
-  if (status$n_failure != 1) {
-    actual <- sprintf("Actually failed %i times", status$n_failure)
-    fail(c(expected, actual))
-  } else if (status$n_success != 0) {
-    actual <- sprintf("Actually succeeded %i times", status$n_success)
-    fail(c(expected, actual))
-  } else {
+  if (status$n_failure == 1 && status$n_success == 0) {
     if (is.null(message)) {
       pass()
     } else {
       act <- labelled_value(status$last_failure$message, "failure message")
       expect_match_(act, message, ..., title = "message")
     }
+    return(invisible())
   }
+
+  fail(format_success_failure(status, exp_n_success = 0, exp_n_failure = 1))
   invisible()
 }
 

tests/testthat/_snaps/expect-self-test.md

@@ -4,20 +4,30 @@
       expect_failure(expect_no_failure())
     Condition
       Error:
-      ! Expected exactly one failure and no successes.
-      Actually failed 0 times
+      ! Expected 0 successes and 1 failure.
+      v Observed 0 successes.
+      x Observed 0 failures.
     Code
       expect_failure(expect_many_failures())
     Condition
       Error:
-      ! Expected exactly one failure and no successes.
-      Actually failed 2 times
+      ! Expected 0 successes and 1 failure.
+      v Observed 0 successes.
+      x Observed 2 failures.
     Code
       expect_failure(expect_has_success())
     Condition
       Error:
-      ! Expected exactly one failure and no successes.
-      Actually succeeded 1 times
+      ! Expected 0 successes and 1 failure.
+      x Observed 1 success.
+      v Observed 1 failure.
+    Code
+      expect_failure(expect_both_wrong())
+    Condition
+      Error:
+      ! Expected 0 successes and 1 failure.
+      x Observed 1 success.
+      x Observed 0 failures.
     Code
       expect_failure(expect_failure_foo(), "bar")
     Condition
@@ -52,20 +62,30 @@
       expect_success(expect_no_success())
     Condition
       Error:
-      ! Expected exactly one success and no failures.
-      Actually succeeded 0 times
+      ! Expected 1 success and 0 failures.
+      x Observed 0 successes.
+      v Observed 0 failures.
     Code
       expect_success(expect_many_successes())
     Condition
       Error:
-      ! Expected exactly one success and no failures.
-      Actually succeeded 2 times
+      ! Expected 1 success and 0 failures.
+      x Observed 2 successes.
+      v Observed 0 failures.
     Code
       expect_success(expect_has_failure())
     Condition
       Error:
-      ! Expected exactly one success and no failures.
-      Actually failed 1 times
+      ! Expected 1 success and 0 failures.
+      v Observed 1 success.
+      x Observed 1 failure.
+    Code
+      expect_success(expect_both_wrong())
+    Condition
+      Error:
+      ! Expected 1 success and 0 failures.
+      x Observed 0 successes.
+      x Observed 1 failure.
 
 # expect_no are deprecated
 

tests/testthat/test-expect-self-test.R

@@ -31,12 +31,16 @@ test_that("expect_failure() generates a useful error messages", {
     fail()
     pass()
   }
+  expect_both_wrong <- function() {
+    pass()
+  }
   expect_failure_foo <- function() fail("foo")
 
   expect_snapshot_failure({
     expect_failure(expect_no_failure())
     expect_failure(expect_many_failures())
     expect_failure(expect_has_success())
+    expect_failure(expect_both_wrong())
     expect_failure(expect_failure_foo(), "bar")
   })
 })
@@ -76,11 +80,15 @@ test_that("expect_success() generates a useful error messages", {
     fail()
     pass()
   }
+  expect_both_wrong <- function() {
+    fail()
+  }
 
   expect_snapshot_failure({
     expect_success(expect_no_success())
     expect_success(expect_many_successes())
     expect_success(expect_has_failure())
+    expect_success(expect_both_wrong())
   })
 })