Skip to content

Commit c4bdd9b

Browse files
committed
Display diffs when snapshotting text files
I haven't yet figured out how to test snapshots with snapshots, so you can run this to test interactively ```R test_that("expect_snapshot_file() show differences for text files", { path1 <- withr::local_tempfile(lines = sample(letters[1:10])) expect_snapshot_file(path1, "text-diff.txt") }) ``` Fixes #1593
1 parent 87d61c5 commit c4bdd9b

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# testthat (development version)
22

3+
* `expect_snapshot_file()` now considers `.json` to be a text file (#1593).
4+
* `expect_snapshot_file()` now shows differences for text files (#1593).
35
* `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)
46
* `test_dir()`, `test_file()`, `test_package()`, `test_check()`, `test_local()`, `source_file()` gain a `shuffle` argument uses `sample()` to randomly reorder the top-level expressions in each test file (#1942). This random reordering surfaces dependencies between tests and code outside of any test, as well as dependencies between tests. This helps you find and eliminate unintentional dependencies.
57
* `snapshot_accept(test)` now works when the test file name contains `.` (#1669).

R/snapshot-file.R

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ expect_snapshot_file <- function(
9494
transform = NULL,
9595
variant = NULL
9696
) {
97+
lab <- quo_label(enquo(path))
98+
9799
check_string(path)
98100
check_string(name)
99101
check_bool(cran)
@@ -111,6 +113,7 @@ expect_snapshot_file <- function(
111113
return(invisible())
112114
}
113115

116+
is_text <- is_text_file(name)
114117
if (!is_missing(binary)) {
115118
lifecycle::deprecate_soft(
116119
"3.0.3",
@@ -120,8 +123,6 @@ expect_snapshot_file <- function(
120123
compare <- if (binary) compare_file_binary else compare_file_text
121124
}
122125
if (is.null(compare)) {
123-
ext <- tools::file_ext(name)
124-
is_text <- ext %in% c("r", "R", "txt", "md", "Rmd")
125126
compare <- if (is_text) compare_file_text else compare_file_binary
126127
}
127128

@@ -131,7 +132,6 @@ expect_snapshot_file <- function(
131132
brio::write_lines(lines, path)
132133
}
133134

134-
lab <- quo_label(enquo(path))
135135
equal <- snapshotter$take_file_snapshot(
136136
name,
137137
path,
@@ -142,20 +142,42 @@ expect_snapshot_file <- function(
142142
return(equal)
143143
}
144144

145-
hint <- snapshot_review_hint(snapshotter$file, name)
145+
file <- snapshotter$file
146+
hint <- snapshot_review_hint(file, name, is_text = is_text)
146147

147148
if (!equal) {
148-
msg <- sprintf(
149-
"Snapshot of %s to '%s' has changed\n%s",
150-
lab,
151-
paste0(snapshotter$file, "/", name),
149+
if (is_text) {
150+
base <- paste0(c(snapshotter$snap_dir, file, variant), collapse = "/")
151+
old_path <- paste0(c(base, name), collapse = "/")
152+
new_path <- paste0(c(base, new_name(name)), collapse = "/")
153+
154+
comp <- waldo_compare(
155+
x = brio::read_lines(old_path),
156+
x_arg = "old",
157+
y = brio::read_lines(new_path),
158+
y_arg = "new",
159+
quote_strings = FALSE
160+
)
161+
comp <- c("Differences:", comp)
162+
} else {
163+
comp <- NULL
164+
}
165+
166+
msg <- c(
167+
sprintf("Snapshot of %s has changed.", lab),
168+
comp,
152169
hint
153170
)
154171
return(fail(msg))
155172
}
156173
pass(NULL)
157174
}
158175

176+
is_text_file <- function(path) {
177+
ext <- tools::file_ext(path)
178+
ext %in% c("r", "R", "txt", "md", "Rmd", "qmd", "json")
179+
}
180+
159181
#' @rdname expect_snapshot_file
160182
#' @export
161183
announce_snapshot_file <- function(path, name = basename(path)) {
@@ -172,6 +194,7 @@ snapshot_review_hint <- function(
172194
name,
173195
ci = on_ci(),
174196
check = in_rcmd_check(),
197+
is_text = FALSE,
175198
reset_output = TRUE
176199
) {
177200
if (reset_output) {
@@ -184,9 +207,13 @@ snapshot_review_hint <- function(
184207
if (check && ci) "* Download and unzip run artifact\n",
185208
if (check && !ci) "* Locate check directory\n",
186209
if (check) paste0("* Copy '", path, "' to local test directory\n"),
187-
if (check) "* ",
210+
if (is_text) {
211+
cli::format_inline(
212+
"* Run {.run testthat::snapshot_accept('{test}/')} to accept the change.\n"
213+
)
214+
},
188215
cli::format_inline(
189-
"Run {.run testthat::snapshot_review('{test}/')} to review changes"
216+
"* Run {.run testthat::snapshot_review('{test}/')} to review changes\n"
190217
)
191218
)
192219
}

tests/testthat/_snaps/snapshot-file.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
Code
2828
cat(snapshot_review_hint("lala", "foo.r", check = FALSE, ci = FALSE))
2929
Output
30-
Run `testthat::snapshot_review('lala/')` to review changes
30+
* Run `testthat::snapshot_review('lala/')` to review changes
3131

3232
---
3333

0 commit comments

Comments
 (0)