Merge branch 'main' into hyperlinks

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: testthat
 Title: Unit Testing for R
-Version: 3.1.1.9000
+Version: 3.1.2.9000
 Authors@R: 
     c(person(given = "Hadley",
              family = "Wickham",
@@ -40,7 +40,7 @@ Imports:
     R6 (>= 2.2.0),
     rlang (>= 0.4.9),
     utils,
-    waldo (>= 0.2.4),
+    waldo (>= 0.3.1),
     withr (>= 2.4.3)
 Suggests: 
     covr,
@@ -56,6 +56,8 @@ Suggests:
     xml2
 VignetteBuilder: 
     knitr
+Remotes:
+    r-lib/rlang
 Config/testthat/edition: 3
 Config/testthat/start-first: watcher, parallel*
 Config/Needs/website: tidyverse/tidytemplate

NEWS.md

@@ -1,53 +1,62 @@
 # testthat (development version)
 
-* `expect_snapshot()` no longer inadvertently trims trailing new lines off
-  of errors and messages (#1509).
+* Package that explicitly depend on rlang in their description file
+  are now opting into a new snapshot display for errors, warnings, and
+  messages. This display will eventually become the default for all
+  packages.
 
-* `test_that()` no longer inappropriately skips when calling `expect_equal()`
-  when you've temporarily set the locale to non-UTF-8 (#1285).
 
-* When a snapshot changes the hint also mentions that you can use 
-  `snapshot_review()` (#1500, @DanChaltiel).
+# testthat 3.1.2
 
-* `snapshot_accept()` now works better with variants (#1508).
+* testthat now uses brio for all reading and writing (#1120). This
+  ensures that snapshots always use "\n" to separate lines (#1516).
 
-* `expect_snapshot()` now only adds a `.new` file for the variants that 
-  actually changed, not all variants.
+* `expect_snapshot()` no longer inadvertently trims trailing new lines off
+  of errors and messages (#1509).
 
-* If a snapshot has a different value, but compares as equal (e.g. because 
-  you've set a numeric tolerance), the saved values are no longer changed if
-  there's another difference in the file.
+* If `expect_snapshot()` generates a snapshot with different value but
+  still compares as equal (e.g. because you've set a numeric tolerance), the 
+  saved values no longer update if another snapshot in the same file changes.
 
-* New `expect_snapshot_warning()` to match `expect_snapshot_error()` (#1532).
+* `expect_snapshot()` now only adds a `.new` file for the variants that 
+  actually changed, not all variants, while `expect_snapshot_file()` with
+  variant with no longer immediately deletes `.new` files (#1468).
 
 * `expect_snapshot_file()` gains a `transform` argument to match 
   `expect_snapshot()` (#1474). `compare` now defaults to `NULL`, automatically 
   guessing the comparison type based on the extension.
 
-* Snapshot cleanup also removes all empty directories (#1457).
+* `expect_snapshot_file()` now errors if the file being snapshot does not exist; 
+  `SnapshotReporter` also now treats the file directory as an absolute path 
+  (#1476, @malcolmbarrett)
+
+* New `expect_snapshot_warning()` to match `expect_snapshot_error()` (#1532).
+
+* `JUnitReporter` now includes skip messages/reasons (@rfineman, #1507)
 
-* `.new` file snapshots variants are no longer immediately deleted after
-  creation (#1468).
-
 * `local_reproducible_output()` gains a `lang` argument so that you can 
   optionally override the language used to translate error messages (#1483).
+  It also sets the global option `cli.num_colors` in addition to 
+  `crayon.enabled`.
 
-* `expect_snapshot_file()` now errors if the file being snapshot does not exist; 
-  `SnapshotReporter` also now treats the file directory as an absolute path 
-  (#1476, @malcolmbarrett)
+* `test_that()` no longer inappropriately skips when calling `expect_equal()`
+  when you've temporarily set the locale to non-UTF-8 (#1285).
 
 * `skip_if_offline()` now automatically calls `skip_on_cran()` (#1479).
 
-* testthat now uses brio for all reading and writing (#1120). This
-  ensures that snapshots always use "\n" to separate lines (#1516).
-
-* `local_reproducible_output()` now sets the global option
-  `cli.num_colors` in addition to `crayon.enabled`.
+* `snapshot_accept()` and `snapshot_review()` now work with exactly the same
+  file specification which can be a snapshot name, a file name, or a directory
+  (#1546). They both work better with variants (#1508). Snapshot cleanup also 
+  removes all empty directories (#1457).
 
 * JUnit reporter now includes skip messages/reasons (@rfineman, #1507). 
 
 * Test results show hyperlinks to failed expectation when supported (#1544). 
 
+* When a snapshot changes the hint also mentions that you can use 
+  `snapshot_review()` (#1500, @DanChaltiel) and the message tells you what 
+  variant is active (#1540).
+
 # testthat 3.1.1
 
 * Condition expectations like `expect_error()` now match across the

R/edition.R

@@ -103,8 +103,11 @@ find_dep_version <- function(name, path, package = NULL) {
 
   dep[[2]]
 }
+has_dep <- function(name, path, package = NULL) {
+  !is.null(find_dep_version(name, path, package = package))
+}
 
 use_rlang_1_0 <- function() {
-  ver <- peek_option("testthat:::rlang_dep")
-  is_string(ver) && package_version(ver) >= "0.99.0.9001"
+  rlang::is_true(peek_option("testthat:::rlang_dep")) &&
+    is_installed("rlang", version = "0.99.0.9001")
 }

R/expect-condition.R

@@ -153,7 +153,8 @@ expect_warning <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label
+      label = label,
+      trace_env = caller_env()
     )
   } else {
     act <- quasi_capture(enquo(object), label, capture_warnings)
@@ -185,7 +186,8 @@ expect_message <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label
+      label = label,
+      trace_env = caller_env()
     )
   } else {
     act <- quasi_capture(enquo(object), label, capture_messages)
@@ -213,7 +215,8 @@ expect_condition <- function(object,
       ...,
       inherit = inherit,
       info = info,
-      label = label
+      label = label,
+      trace_env = caller_env()
     )
   } else {
     act <- quasi_capture(enquo(object), label, capture_condition, entrace = TRUE)
@@ -239,7 +242,8 @@ expect_condition_matching <- function(base_class,
                                       ...,
                                       inherit = TRUE,
                                       info = NULL,
-                                      label = NULL) {
+                                      label = NULL,
+                                      trace_env = caller_env()) {
   ellipsis::check_dots_used(action = warn)
 
   matcher <- cnd_matcher(
@@ -261,7 +265,7 @@ expect_condition_matching <- function(base_class,
 
   # Access error fields with `[[` rather than `$` because the
   # `$.Throwable` from the rJava package throws with unknown fields
-  expect(is.null(msg), msg, info = info, trace = act$cap[["trace"]])
+  expect(is.null(msg), msg, info = info, trace = act$cap[["trace"]], trace_env = trace_env)
 
   # If a condition was expected, return it. Otherwise return the value
   # of the expression.

R/local.R

@@ -152,10 +152,8 @@ local_test_directory <- function(path, package = NULL, .env = parent.frame()) {
   # Set edition before changing working directory in case path is relative
   local_edition(find_edition(path, package), .env = .env)
 
-  rlang_dep <- find_dep_version("rlang", path, package)
-
   withr::local_options(
-    "testthat:::rlang_dep" = rlang_dep,
+    "testthat:::rlang_dep" = has_dep("rlang", path, package),
     .local_envir = .env
   )
   withr::local_dir(

R/reporter-check.R

@@ -35,6 +35,16 @@ CheckReporter <- R6::R6Class("CheckReporter",
     },
 
     end_reporter = function() {
+      if (self$skips$size() || self$warnings$size() || self$problems$size()) {
+        self$cat_line(summary_line(
+          n_fail = self$problems$size(),
+          n_warn = self$warnings$size(),
+          n_skip = self$skips$size(),
+          n_pass = self$n_ok
+        ))
+        self$cat_line()
+      }
+
       if (self$skips$size() > 0) {
         self$rule("Skipped tests", line = 2)
         self$cat_line(skip_bullets(self$skips$as_list()))

R/snapshot-file.R

@@ -126,8 +126,12 @@ expect_snapshot_file <- function(path,
   }
 
   lab <- quo_label(enquo(path))
-  equal <- snapshotter$take_file_snapshot(name, path, compare, variant = variant)
-  hint <- snapshot_hint(snapshotter$file, name)
+  equal <- snapshotter$take_file_snapshot(name, path,
+    file_equal = compare,
+    variant = variant,
+    trace_env = caller_env()
+  )
+  hint <- snapshot_review_hint(snapshotter$file, name)
 
   expect(
     equal,
@@ -150,19 +154,19 @@ announce_snapshot_file <- function(path, name = basename(path)) {
   }
 }
 
-snapshot_hint <- function(test, name, ci = on_ci(), check = in_rcmd_check()) {
+snapshot_review_hint <- function(test, name, ci = on_ci(), check = in_rcmd_check()) {
   path <- paste0("tests/testthat/_snaps/", test, "/", new_name(name))
 
   paste0(
     if (check && ci) "* Download and unzip run artifact\n",
     if (check && !ci) "* Locate check directory\n",
     if (check) paste0("* Copy '", path, "' to local test directory\n"),
     if (check) "* ",
-    paste0("Run `testthat::snapshot_review('", test, "')` to review changes")
+    paste0("Run `testthat::snapshot_review('", test, "/')` to review changes")
   )
 }
 
-snapshot_file_equal <- function(snap_test_dir, snap_name, path, file_equal = compare_file_binary) {
+snapshot_file_equal <- function(snap_test_dir, snap_name, path, file_equal = compare_file_binary, fail_on_new = FALSE, trace_env = NULL) {
   if (!file.exists(path)) {
     abort(paste0("`", path, "` not found"))
   }
@@ -182,7 +186,14 @@ snapshot_file_equal <- function(snap_test_dir, snap_name, path, file_equal = com
   } else {
     dir.create(snap_test_dir, showWarnings = FALSE, recursive = TRUE)
     file.copy(path, cur_path)
-    testthat_warn(paste0("Adding new file snapshot: '", cur_path, "'"))
+
+    message <- paste0("Adding new file snapshot: 'tests/testhat/_snaps/", snap_name, "'")
+    if (fail_on_new) {
+      fail(message, trace_env = trace_env)
+    } else {
+      testthat_warn(message)
+    }
+
     TRUE
   }
 }

R/snapshot-manage.R

@@ -5,9 +5,10 @@
 #'    modified snapshot. This is particularly useful for whole file snapshots
 #'    created by `expect_snapshot_file()`.
 #'
-#' @param files Optionally, filter affects to snapshots from specified test
-#'   files. Can be full path to test (`tests/testthat/test-foo.R`), file name
-#'   (`test-foo.R`), or test name (`foo`).
+#' @param files Optionally, filter effects to snapshots from specified files.
+#'   This can be a snapshot name (e.g. `foo` or `foo.md`), a snapshot file name
+#'   (e.g. `testfile/foo.txt`), or a snapshot file directory (e.g. `testfile/`).
+#'
 #' @param path Path to tests.
 #' @export
 snapshot_accept <- function(files = NULL, path = "tests/testthat") {
@@ -156,7 +157,14 @@ snapshot_meta <- function(files = NULL, path = "tests/testthat") {
   rownames(out) <- NULL
 
   if (!is.null(files)) {
-    out <- out[out$name %in% paste0(files, ".md"), , drop = FALSE]
+    is_dir <- substr(files, nchar(files), nchar(files)) == "/"
+    dirs <- files[is_dir]
+    files <- files[!is_dir]
+
+    dirs <- substr(dirs, 1, nchar(dirs) - 1)
+    files <- ifelse(tools::file_ext(files) == "", paste0(files, ".md"), files)
+
+    out <- out[out$name %in% files | out$test %in% dirs, , drop = FALSE]
   }
 
   out

R/snapshot-reporter.R

@@ -8,13 +8,15 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
     test_file_seen = character(),
     snap_file_seen = character(),
     variants_changed = FALSE,
+    fail_on_new = FALSE,
 
     old_snaps = NULL,
     cur_snaps = NULL,
     new_snaps = NULL,
 
-    initialize = function(snap_dir = "_snaps") {
+    initialize = function(snap_dir = "_snaps", fail_on_new = FALSE) {
       self$snap_dir <- normalizePath(snap_dir, mustWork = FALSE)
+      self$fail_on_new <- fail_on_new
     },
 
     start_file = function(path, test = NULL) {
@@ -42,7 +44,8 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
                              load = identity,
                              ...,
                              tolerance = testthat_tolerance(),
-                             variant = NULL) {
+                             variant = NULL,
+                             trace_env = NULL) {
       i <- self$new_snaps$append(self$test, variant, save(value))
 
       old_raw <- self$old_snaps$get(self$test, variant, i)
@@ -70,26 +73,41 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
         check_roundtrip(value, load(value_enc), ..., tolerance = tolerance)
 
         self$cur_snaps$append(self$test, variant, value_enc)
-        testthat_warn(paste0(
+
+        message <- paste0(
           "Adding new snapshot",
-          if (variant != "_default") paste0(" to variant '", variant, "'"),
+          if (variant != "_default") paste0(" for variant '", variant, "'"),
+          if (self$fail_on_new) " in CI",
           ":\n",
           value_enc
-        ))
+        )
+        if (self$fail_on_new) {
+          fail(message, trace_env = trace_env)
+        } else {
+          testthat_warn(message)
+        }
         character()
       }
     },
 
-    take_file_snapshot = function(name, path, file_equal, variant = NULL) {
+    take_file_snapshot = function(name, path, file_equal, variant = NULL, trace_env = NULL) {
       self$announce_file_snapshot(name)
 
       if (is.null(variant)) {
         snap_dir <- file.path(self$snap_dir, self$file)
       } else {
         snap_dir <- file.path(self$snap_dir, variant, self$file)
       }
-      snapshot_file_equal(snap_dir, name, path, file_equal)
+      snapshot_file_equal(
+        snap_test_dir = snap_dir,
+        snap_name = name,
+        path = path,
+        file_equal = file_equal,
+        fail_on_new = self$fail_on_new,
+        trace_env = trace_env
+      )
     },
+
     # Also called from announce_snapshot_file()
     announce_file_snapshot = function(name) {
       self$snap_file_seen <- c(self$snap_file_seen, file.path(self$file, name))
@@ -122,7 +140,7 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
     end_reporter = function() {
       # clean up if we've seen all files
       tests <- context_name(find_test_scripts(".", full.names = FALSE))
-      if (all(tests %in% self$test_file_seen)) {
+      if (!on_ci() && all(tests %in% self$test_file_seen)) {
         snapshot_cleanup(self$snap_dir,
           test_files_seen = self$test_file_seen,
           snap_files_seen = self$snap_file_seen
@@ -173,9 +191,9 @@ get_snapshotter <- function() {
 #'
 #' @export
 #' @keywords internal
-local_snapshotter <- function(snap_dir = NULL, cleanup = FALSE, .env = parent.frame()) {
+local_snapshotter <- function(snap_dir = NULL, cleanup = FALSE, fail_on_new = FALSE, .env = parent.frame()) {
   snap_dir <- snap_dir %||% withr::local_tempdir(.local_envir = .env)
-  reporter <- SnapshotReporter$new(snap_dir = snap_dir)
+  reporter <- SnapshotReporter$new(snap_dir = snap_dir, fail_on_new = fail_on_new)
   if (!identical(cleanup, FALSE)) {
     warn("`cleanup` is deprecated")
   }