Snapshot .new files deleted for filenames with multiple dots #2326
Description
Summary
When using expect_snapshot_file() with snapshot filenames containing multiple dots (e.g., tc12.2_image_annotate.html), the generated .new files are incorrectly identified as "unused" and deleted during snapshot cleanup.
This prevents users from reviewing snapshot changes via snapshot_review() because the .new files no longer exist.
Reproducible Example
# Create package with testthat edition 3
usethis::create_package(file.path(tempdir(), "testpkg"))
usethis::use_testthat(3)
# Create test file
cat(file = testthat::test_path("test-snapshots.R"), '
test_that("multi_dot_filename", {
path <- tempfile(fileext = ".html")
writeLines("<html><body>New content</body></html>", path)
expect_snapshot_file(path, "tc12.2_image_annotate.html")
})
test_that("single_dot_control", {
path <- tempfile(fileext = ".html")
writeLines("<html><body>New content</body></html>", path)
expect_snapshot_file(path, "single_dot.html")
})
')
# Create existing snapshots with different content to trigger .new file creation
dir.create(testthat::test_path("_snaps/snapshots"), recursive = TRUE)
writeLines("<html><body>Old content</body></html>",
testthat::test_path("_snaps/snapshots/tc12.2_image_annotate.html"))
writeLines("<html><body>Old content</body></html>",
testthat::test_path("_snaps/snapshots/single_dot.html"))
# Run tests
devtools::test()Output:
Deleting unused snapshots: 'snapshots/tc12.new.2_image_annotate.html'
Result:
single_dot.new.html- preserved ✓tc12.new.2_image_annotate.html- deleted ✗
Root Cause
There is an inconsistency between how .new filenames are created vs how they are expected during cleanup:
File creation (R/snapshot-file.R, split_path())
split_path <- function(path) {
...
ext_loc <- regexpr(".", name, fixed = TRUE) # First dot
...
}Uses regexpr(".", ..., fixed = TRUE) which finds the first dot.
Cleanup expectation (R/snapshot-cleanup.R, snapshot_expected())
snap_files_seen_new <- paste0(
tools::file_path_sans_ext(snap_files_seen), # Last dot
".new.",
tools::file_ext(snap_files_seen) # Last dot
)Uses tools::file_path_sans_ext() and tools::file_ext() which split on the last dot.
For filename tc12.2_image_annotate.html:
| Function | Splits on | Result |
|---|---|---|
split_path() → new_name() |
First dot | Creates: tc12.new.2_image_annotate.html |
tools::file_path_sans_ext() |
Last dot | Expects: tc12.2_image_annotate.new.html |
The mismatch causes the actual .new file to be treated as orphaned and deleted.
Suggested Fix
Update snapshot_expected() in R/snapshot-cleanup.R to use split_path() instead of tools::file_path_sans_ext()/tools::file_ext():
# Current (inconsistent):
snap_files_seen_new <- paste0(
tools::file_path_sans_ext(snap_files_seen),
".new.",
tools::file_ext(snap_files_seen)
)
# Fixed (consistent with new_name()):
pieces <- split_path(snap_files_seen)
snap_files_seen_new <- paste0(
ifelse(pieces$dir == "", "", paste0(pieces$dir, "/")),
pieces$name, ".new.", pieces$ext
)Environment
- testthat version: 3.2.3 (also reproduced on latest dev)
- R version: 4.4.0+
Impact
This bug affects any package using expect_snapshot_file() with filenames containing multiple dots, which is common for versioned test cases (e.g., test1.2_feature.svg, plot.v2.png).