Merged origin/main into oo-mocking-enhancement

Author: hadley

NEWS.md

@@ -1,6 +1,9 @@
 # testthat 3.3.1
 
 * `local_mocked_s3_method()` and `local_mocked_s4_method()` can now mock methods that don't already exist, and can use `definition = NULL` to temporarily remove a method. `local_mocked_s4_method()` now also works when the generic is defined in another package (#2302).
+* `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).
 
 * testthat now emits OpenTelemetry traces for tests when tracing is enabled. Requires the otel and otelsdk packages (#2282).

R/expect-comparison.R

@@ -74,7 +74,6 @@ failure_compare <- function(act, exp, operator) {
         actual_op
       )
     }
-
   } else {
     msg_act <- sprintf(
       "Actual comparison: \"%s\" %s \"%s\"",

R/expect-constant.R

@@ -19,7 +19,7 @@
 #' a <- 1:4
 #' show_failure(expect_true(length(a) == 3))
 #' show_failure(expect_equal(length(a), 3))
-#' 
+#'
 #' x <- c(TRUE, TRUE, FALSE, TRUE)
 #' show_failure(expect_true(all(x)))
 #' show_failure(expect_all_true(x))

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()
 }
 

R/reporter-check.R

@@ -123,7 +123,7 @@ summary_line <- function(n_fail, n_warn, n_skip, n_pass) {
 snapshot_check_hint <- function() {
   intro <- "To review and process snapshots locally:"
 
-  if (on_gh()) {
+  if (on_gh() && Sys.getenv("GITHUB_JOB") == "R-CMD-check") {
     repository <- Sys.getenv("GITHUB_REPOSITORY")
     run_id <- Sys.getenv("GITHUB_RUN_ID")
 

R/snapshot-github.R

@@ -7,11 +7,13 @@
 #' take the artifacts from.
 #'
 #' Note that you should not generally need to use this function manually;
-#' instead copy and paste from the hint emitted on GitHub.
+#' instead copy and paste from the hint emitted on GitHub. This hint is only
+#' emitted when running in a job named "R-CMD-check", since that's where the
+#' testthat artifact is typically uploaded.
 #'
 #' @param repository Repository owner/name, e.g. `"r-lib/testthat"`.
 #' @param run_id Run ID, e.g. `"47905180716"`. You can find this in the action url.
-#' @param dest_dir Directory to download to. Defaults to the current directory.
+#' @param dest_dir Package root directory. Defaults to the current directory.
 #' @export
 snapshot_download_gh <- function(repository, run_id, dest_dir = ".") {
   check_string(repository)

R/snapshot-reporter.R

@@ -135,7 +135,7 @@ SnapshotReporter <- R6::R6Class(
       save_path <- paste0(c(self$file, variant, name), collapse = "/")
       if (save_path %in% self$snap_file_saved) {
         cli::cli_abort(
-          "Snapshot file names must be unique. {.arg name} has already been used.",
+          "Snapshot file names must be unique. {.val {name}} has already been used.",
           call = trace_env
         )
       }

R/test-files.R

@@ -149,7 +149,6 @@ test_files <- function(
   shuffle = FALSE,
   error_call = caller_env()
 ) {
-
   # Must keep these two blocks in sync
   if (parallel) {
     test_files_parallel(

man/snapshot_download_gh.Rd

@@ -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