Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce that snapshot files should be unique #1592

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

* Test results show hyperlinks to failed expectation when supported (#1544).

* `expect_snapshot_file(name=)` must have a unique file path. If a snapshot file attempts to be
saved with a duplicate `name`, an error will be thrown. (#1592)

# testthat 3.1.2

Expand Down
15 changes: 15 additions & 0 deletions R/snapshot-reporter.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
test = NULL,
test_file_seen = character(),
snap_file_seen = character(),
snap_file_saved = character(),
variants_changed = FALSE,
fail_on_new = FALSE,

Expand All @@ -22,6 +23,7 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
start_file = function(path, test = NULL) {
self$file <- context_name(path)
self$test_file_seen <- c(self$test_file_seen, self$file)
self$snap_file_saved <- character()

self$variants_changed <- character()

Expand Down Expand Up @@ -98,6 +100,19 @@ SnapshotReporter <- R6::R6Class("SnapshotReporter",
} else {
snap_dir <- file.path(self$snap_dir, variant, self$file)
}

save_path <- file.path(snap_dir, name)
if (save_path %in% self$snap_file_saved) {
fail(
paste0(
"Snapshot file '", name, "' has already been saved.",
" Please provide a unique snapshot file name"
),
trace_env = trace_env
)
}
self$snap_file_saved <- c(self$snap_file_saved, save_path)

snapshot_file_equal(
snap_test_dir = snap_dir,
snap_name = name,
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/R4.0/snapshot-file/nickname.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Shake and Throw
R4.0
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/R4.0/snapshot.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# can access nickname

Code
version$nickname
r_version()
Output
[1] "Shake and Throw"
[1] "R4.0"

2 changes: 1 addition & 1 deletion tests/testthat/_snaps/R4.1/snapshot-file/nickname.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Bird Hippie
R4.1
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/R4.1/snapshot.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# can access nickname

Code
version$nickname
r_version()
Output
[1] "Bird Hippie"
[1] "R4.1"

33 changes: 33 additions & 0 deletions tests/testthat/_snaps/snapshot-file/foo-binary.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"","mpg","cyl","disp","hp","drat","wt","qsec","vs","am","gear","carb"
"Mazda RX4",21,6,160,110,3.9,2.62,16.46,0,1,4,4
"Mazda RX4 Wag",21,6,160,110,3.9,2.875,17.02,0,1,4,4
"Datsun 710",22.8,4,108,93,3.85,2.32,18.61,1,1,4,1
"Hornet 4 Drive",21.4,6,258,110,3.08,3.215,19.44,1,0,3,1
"Hornet Sportabout",18.7,8,360,175,3.15,3.44,17.02,0,0,3,2
"Valiant",18.1,6,225,105,2.76,3.46,20.22,1,0,3,1
"Duster 360",14.3,8,360,245,3.21,3.57,15.84,0,0,3,4
"Merc 240D",24.4,4,146.7,62,3.69,3.19,20,1,0,4,2
"Merc 230",22.8,4,140.8,95,3.92,3.15,22.9,1,0,4,2
"Merc 280",19.2,6,167.6,123,3.92,3.44,18.3,1,0,4,4
"Merc 280C",17.8,6,167.6,123,3.92,3.44,18.9,1,0,4,4
"Merc 450SE",16.4,8,275.8,180,3.07,4.07,17.4,0,0,3,3
"Merc 450SL",17.3,8,275.8,180,3.07,3.73,17.6,0,0,3,3
"Merc 450SLC",15.2,8,275.8,180,3.07,3.78,18,0,0,3,3
"Cadillac Fleetwood",10.4,8,472,205,2.93,5.25,17.98,0,0,3,4
"Lincoln Continental",10.4,8,460,215,3,5.424,17.82,0,0,3,4
"Chrysler Imperial",14.7,8,440,230,3.23,5.345,17.42,0,0,3,4
"Fiat 128",32.4,4,78.7,66,4.08,2.2,19.47,1,1,4,1
"Honda Civic",30.4,4,75.7,52,4.93,1.615,18.52,1,1,4,2
"Toyota Corolla",33.9,4,71.1,65,4.22,1.835,19.9,1,1,4,1
"Toyota Corona",21.5,4,120.1,97,3.7,2.465,20.01,1,0,3,1
"Dodge Challenger",15.5,8,318,150,2.76,3.52,16.87,0,0,3,2
"AMC Javelin",15.2,8,304,150,3.15,3.435,17.3,0,0,3,2
"Camaro Z28",13.3,8,350,245,3.73,3.84,15.41,0,0,3,4
"Pontiac Firebird",19.2,8,400,175,3.08,3.845,17.05,0,0,3,2
"Fiat X1-9",27.3,4,79,66,4.08,1.935,18.9,1,1,4,1
"Porsche 914-2",26,4,120.3,91,4.43,2.14,16.7,0,1,5,2
"Lotus Europa",30.4,4,95.1,113,3.77,1.513,16.9,1,1,5,2
"Ford Pantera L",15.8,8,351,264,4.22,3.17,14.5,0,1,5,4
"Ferrari Dino",19.7,6,145,175,3.62,2.77,15.5,0,1,5,6
"Maserati Bora",15,8,301,335,3.54,3.57,14.6,0,1,5,8
"Volvo 142E",21.4,4,121,109,4.11,2.78,18.6,1,1,4,2
17 changes: 15 additions & 2 deletions tests/testthat/test-snapshot-file.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ test_that("expect_snapshot_file works", {
withr::local_options(lifecycle_verbosity = "quiet")
expect_snapshot_file(
path,
"foo.csv",
"foo-binary.csv",
binary = FALSE
)
})
Expand All @@ -48,13 +48,26 @@ test_that("expect_snapshot_file works in a different directory", {

test_that("expect_snapshot_file works with variant", {
expect_snapshot_file(
write_tmp_lines(version$nickname),
write_tmp_lines(r_version()),
"nickname.txt",
compare = compare_file_text,
variant = r_version()
)
})

test_that("expect_snapshot_file finds duplicate snapshot files", {
# Save to the same file as in previous test
expect_error(
expect_snapshot_file(
write_tmp_lines(r_version()),
"nickname.txt",
compare = compare_file_text,
variant = r_version()
),
"provide a unique snapshot file name"
)
})

test_that("basic workflow", {
snapper <- local_snapshotter()

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-snapshot.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("can access nickname", {
expect_snapshot(version$nickname, variant = r_version())
expect_snapshot(r_version(), variant = r_version())
})

test_that("can snapshot output", {
Expand Down