expanding tests for snapshot-manage.R

[mcol]

Following the discussion with @hadley started in #2029, I'm generating new tests in snapshot-manage.R. Specifically, there are no tests in which the snapshot files have different extensions. So before continuing with #2029, I'd like to add tests to the main branch against which I can continue developing that PR.

This test passes as I had expected (by the way the use of expect_message is just for ease of exposition here, the real tests use expect_snapshot):

path <- local_snapshot_dir(c("a.md", "a.new.md", "b.txt", "b.new.txt"))
expect_message(snapshot_accept("a", path = path), "Updating snapshots:")
expect_equal(dir(file.path(path, "_snaps")), c("a.md", "b.new.txt", "b.txt"))

I'm trying to understand what should the behaviour be in this case, as my expectations below is not satisfied:

path <- local_snapshot_dir(c("a.md", "a.new.md", "b.txt", "b.new.txt"))
expect_message(snapshot_accept("b", path = path), "Updating snapshots:")
## Error: `snapshot_accept("b", path = path)` did not throw the expected message.

expect_equal(dir(file.path(path, "_snaps")), c("a.md", "a.new.md", "b.txt"))
## Error: dir(file.path(path, "_snaps")) (`actual`) not equal to c("a.md", "a.new.md", "b.txt") (`expected`).
## `actual`:   "a.md" "a.new.md" "b.new.txt" "b.txt"
## `expected`: "a.md" "a.new.md"             "b.txt"

This behaviour is due to this line, which hardcodes the .md extension:

testthat/R/snapshot-manage.R

Line 134 in 654e9fd

snap_test <- ifelse(snap_file, basename(dirname(cur)), gsub("\\.md$", "", basename(cur)))

So is the existing code at fault (and no user has ever noticed)? Or effectively only snapshots with .md extension can be accepted and updated? Can someone advise?

[mcol]

I've just realised that this passes:

path <- local_snapshot_dir(c("a.md", "a.new.md", "b.txt", "b.new.txt"))
expect_message(snapshot_accept("b.txt", path = path), "Updating snapshots:")
expect_equal(dir(file.path(path, "_snaps")), c("a.md", "a.new.md", "b.txt"))

So perhaps it's expected that for non .md files, the complete filename with extension should be specified? Then it may be simply a documentation issue.

[hadley]

I think you can probably consider all of the "name" matching stuff there for historical reasons; now the majority of folks will click on the link emitted by testthat which always includes the extension.

[hadley]

Just to close the loop from the PR, this case doesn't matter because testthat only creates snapshots with extension .md in the root _snaps directory.

path <- local_snapshot_dir(c("a.md", "a.new.md", "b.txt", "b.new.txt"))
expect_message(snapshot_accept("b", path = path), "Updating snapshots:")