diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 056d0e5e2..dda23e068 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -188,11 +188,13 @@ snapshot_review_hint <- function( ) } + snapshot_file_equal <- function( - snap_test_dir, - snap_name, - snap_variant, - path, + snap_dir, # _snaps/ + snap_test, # test file name + snap_name, # snapshot file name + snap_variant, # variant (optional) + path, # path to new file file_equal = compare_file_binary, fail_on_new = FALSE, trace_env = caller_env() @@ -201,8 +203,14 @@ snapshot_file_equal <- function( abort(paste0("`", path, "` not found")) } + if (is.null(snap_variant)) { + snap_test_dir <- file.path(snap_dir, snap_test) + } else { + snap_test_dir <- file.path(snap_dir, snap_variant, snap_test) + } + cur_path <- file.path(snap_test_dir, snap_name) - new_path <- new_name(cur_path) + new_path <- file.path(snap_test_dir, new_name(snap_name)) if (file.exists(cur_path)) { eq <- file_equal(cur_path, path) @@ -217,10 +225,10 @@ snapshot_file_equal <- function( dir.create(snap_test_dir, showWarnings = FALSE, recursive = TRUE) file.copy(path, cur_path) - message <- paste0( + message <- paste_c( "Adding new file snapshot: 'tests/testthat/_snaps/", - snap_variant, - if (!is.null(snap_variant)) "/", + c(snap_variant, if (!is.null(snap_variant)) "/"), + c(snap_test, "/"), snap_name, "'" ) diff --git a/R/snapshot-reporter.R b/R/snapshot-reporter.R index 24356852d..cd1b3e673 100644 --- a/R/snapshot-reporter.R +++ b/R/snapshot-reporter.R @@ -108,13 +108,9 @@ SnapshotReporter <- R6::R6Class( ) { 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_test_dir = snap_dir, + snap_dir = self$snap_dir, + snap_test = self$file, snap_name = name, snap_variant = variant, path = path, diff --git a/R/utils.R b/R/utils.R index dadd0ba2f..868c46574 100644 --- a/R/utils.R +++ b/R/utils.R @@ -56,3 +56,7 @@ in_rcmd_check <- function() { } r_version <- function() paste0("R", getRversion()[, 1:2]) + +paste_c <- function(...) { + paste0(c(...), collapse = "") +} diff --git a/tests/testthat/_snaps/snapshot-file.md b/tests/testthat/_snaps/snapshot-file.md index a97a546e2..e36419766 100644 --- a/tests/testthat/_snaps/snapshot-file.md +++ b/tests/testthat/_snaps/snapshot-file.md @@ -1,15 +1,15 @@ # warns on first creation Code - out <- snapshot_file_equal(tempdir(), "test.txt", NULL, path) + out <- snapshot_file_equal_(path) Condition Warning: - Adding new file snapshot: 'tests/testthat/_snaps/test.txt' + Adding new file snapshot: 'tests/testthat/_snaps/my-test/test.txt' --- Code - expect_true(snapshot_file_equal(tempdir(), "test.txt", NULL, "doesnt-exist.txt")) + snapshot_file_equal_("doesnt-exist.txt") Condition Error in `snapshot_file_equal()`: ! `doesnt-exist.txt` not found diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index 193761087..a21b49775 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -1,9 +1,6 @@ test_that("expect_snapshot_file works", { - expect_snapshot_file( - write_tmp_lines(letters), - "foo.r", - compare = compare_file_text - ) + path <- write_tmp_lines(letters) + expect_snapshot_file(path, "foo.r", compare = compare_file_text) path <- withr::local_tempfile() png(path, width = 300, height = 300, type = "cairo") @@ -15,19 +12,11 @@ test_that("expect_snapshot_file works", { mtcars2 <- mtcars # mtcars2$wt[10] <- NA write.csv(mtcars2, path) - expect_snapshot_file( - path, - "foo.csv", - compare = compare_file_text - ) + expect_snapshot_file(path, "foo.csv", compare = compare_file_text) # Deprecated `binary` argument still works withr::local_options(lifecycle_verbosity = "quiet") - expect_snapshot_file( - path, - "foo.csv", - binary = FALSE - ) + expect_snapshot_file(path, "foo.csv", binary = FALSE) }) @@ -53,25 +42,21 @@ test_that("expect_snapshot_file works with variant", { test_that("basic workflow", { snapper <- local_snapshotter(fail_on_new = FALSE) + path <- write_tmp_lines(letters) # warns on first run snapper$start_file("snapshot-6", "test") - expect_warning( - expect_snapshot_file(write_tmp_lines(letters), "letters.txt"), - "Adding new" - ) + expect_warning(expect_snapshot_file(path, "letters.txt"), "Adding new") snapper$end_file() # succeeds if unchanged snapper$start_file("snapshot-6", "test") - expect_success(expect_snapshot_file(write_tmp_lines(letters), "letters.txt")) + expect_success(expect_snapshot_file(path, "letters.txt")) snapper$end_file() # fails if changed snapper$start_file("snapshot-6", "test") - expect_failure(expect_snapshot_file( - write_tmp_lines(letters[-1]), - "letters.txt" - )) + path2 <- write_tmp_lines(letters[-1]) + expect_failure(expect_snapshot_file(path2, "letters.txt")) snapper$end_file() }) @@ -93,42 +78,48 @@ test_that("can transform snapshot contents", { test_that("warns on first creation", { path <- write_tmp_lines("a") - withr::defer(unlink(file.path(tempdir(), "test.txt"))) + snap_dir <- withr::local_tempdir() + + snapshot_file_equal_ <- function(path) { + snapshot_file_equal( + snap_dir = snap_dir, + snap_test = "my-test", + snap_name = "test.txt", + snap_variant = NULL, + path = path + ) + } # Warns on first run - expect_snapshot(out <- snapshot_file_equal(tempdir(), "test.txt", NULL, path)) + expect_snapshot(out <- snapshot_file_equal_(path)) expect_true(out) # Errors on non-existing file - expect_snapshot(error = TRUE, { - expect_true(snapshot_file_equal( - tempdir(), - "test.txt", - NULL, - "doesnt-exist.txt" - )) - }) + expect_snapshot(snapshot_file_equal_("doesnt-exist.txt"), error = TRUE) # Unchanged returns TRUE - expect_true(snapshot_file_equal(tempdir(), "test.txt", NULL, path)) - expect_true(file.exists(file.path(tempdir(), "test.txt"))) - expect_false(file.exists(file.path(tempdir(), "test.new.txt"))) + expect_true(snapshot_file_equal_(path)) + expect_true(file.exists(file.path(snap_dir, "my-test/test.txt"))) + expect_false(file.exists(file.path(snap_dir, "my-test/test.new.txt"))) # Changed returns FALSE path2 <- write_tmp_lines("b") - expect_false(snapshot_file_equal(tempdir(), "test.txt", NULL, path2)) - expect_true(file.exists(file.path(tempdir(), "test.txt"))) - expect_true(file.exists(file.path(tempdir(), "test.new.txt"))) + expect_false(snapshot_file_equal_(path2)) + expect_true(file.exists(file.path(snap_dir, "my-test/test.txt"))) + expect_true(file.exists(file.path(snap_dir, "my-test/test.new.txt"))) # Changing again overwrites path2 <- write_tmp_lines("c") - expect_false(snapshot_file_equal(tempdir(), "test.txt", NULL, path2)) - expect_equal(brio::read_lines(file.path(tempdir(), "test.new.txt")), "c") + expect_false(snapshot_file_equal_(path2)) + expect_equal( + brio::read_lines(file.path(snap_dir, "my-test/test.new.txt")), + "c" + ) # Unchanged cleans up - expect_true(snapshot_file_equal(tempdir(), "test.txt", NULL, path)) - expect_true(file.exists(file.path(tempdir(), "test.txt"))) - expect_false(file.exists(file.path(tempdir(), "test.new.txt"))) + expect_true(snapshot_file_equal_(path)) + expect_true(file.exists(file.path(snap_dir, "my-test/test.txt"))) + expect_false(file.exists(file.path(snap_dir, "my-test/test.new.txt"))) }) # helpers -----------------------------------------------------------------