Better handling of snapshot selection (#1546)

Fixes #1545

Author: hadley

NEWS.md

@@ -1,5 +1,9 @@
 # testthat (development version)
 
+* `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).
+
 * `expect_snapshot()` no longer inadvertently trims trailing new lines off
   of errors and messages (#1509).
 

R/snapshot-file.R

@@ -127,7 +127,7 @@ 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)
+  hint <- snapshot_review_hint(snapshotter$file, name)
 
   expect(
     equal,
@@ -150,15 +150,15 @@ 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")
   )
 }
 

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

man/snapshot_accept.Rd

@@ -1,25 +1,25 @@
 # snapshot_hint output differs in R CMD check
 
     Code
-      cat(snapshot_hint("lala", "foo.r", check = FALSE, ci = FALSE))
+      cat(snapshot_review_hint("lala", "foo.r", check = FALSE, ci = FALSE))
     Output
-      Run `testthat::snapshot_review('lala')` to review changes
+      Run `testthat::snapshot_review('lala/')` to review changes
 
 ---
 
     Code
-      cat(snapshot_hint("lala", "foo.r", check = TRUE, ci = FALSE))
+      cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = FALSE))
     Output
       * Locate check directory
       * Copy 'tests/testthat/_snaps/lala/foo.new.r' to local test directory
-      * Run `testthat::snapshot_review('lala')` to review changes
+      * Run `testthat::snapshot_review('lala/')` to review changes
 
 ---
 
     Code
-      cat(snapshot_hint("lala", "foo.r", check = TRUE, ci = TRUE))
+      cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = TRUE))
     Output
       * Download and unzip run artifact
       * Copy 'tests/testthat/_snaps/lala/foo.new.r' to local test directory
-      * Run `testthat::snapshot_review('lala')` to review changes
+      * Run `testthat::snapshot_review('lala/')` to review changes
 

tests/testthat/_snaps/snapshot-file.md

@@ -22,6 +22,22 @@
       Updating snapshots:
       * a.md
 
+---
+
+    Code
+      snapshot_accept("test/a.txt", path = path)
+    Message <message>
+      Updating snapshots:
+      * test/a.txt
+
+---
+
+    Code
+      snapshot_accept("test/", path = path)
+    Message <message>
+      Updating snapshots:
+      * test/a.txt
+
 # can work with variants
 
     Code

tests/testthat/_snaps/snapshot-manage.md

@@ -154,7 +154,7 @@ test_that("split_path handles edge cases", {
 })
 
 test_that("snapshot_hint output differs in R CMD check", {
-  expect_snapshot(cat(snapshot_hint("lala", "foo.r", check = FALSE, ci = FALSE)))
-  expect_snapshot(cat(snapshot_hint("lala", "foo.r", check = TRUE, ci = FALSE)))
-  expect_snapshot(cat(snapshot_hint("lala", "foo.r", check = TRUE, ci = TRUE)))
+  expect_snapshot(cat(snapshot_review_hint("lala", "foo.r", check = FALSE, ci = FALSE)))
+  expect_snapshot(cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = FALSE)))
+  expect_snapshot(cat(snapshot_review_hint("lala", "foo.r", check = TRUE, ci = TRUE)))
 })

tests/testthat/test-snapshot-file.R

@@ -11,6 +11,16 @@ test_that("can accept specific files", {
   path <- local_snapshot_dir(c("a.md", "a.new.md", "b.md", "b.new.md"))
   expect_snapshot(snapshot_accept("a", path = path))
   expect_equal(dir(file.path(path, "_snaps")), c("a.md", "b.md", "b.new.md"))
+
+  path <- local_snapshot_dir(c("test/a.txt", "test/a.new.txt"))
+  expect_snapshot(snapshot_accept("test/a.txt", path = path))
+  expect_equal(dir(file.path(path, "_snaps"), recursive = TRUE), "test/a.txt")
+
+  # or whole directory
+  path <- local_snapshot_dir(c("test/a.txt", "test/a.new.txt"))
+  expect_snapshot(snapshot_accept("test/", path = path))
+  expect_equal(dir(file.path(path, "_snaps"), recursive = TRUE), "test/a.txt")
+
 })
 
 test_that("can work with variants", {