Snapshot file root directory issues

[malcolmbarrett]

Perhaps it's my mental model of expect_snapshot_file() that is broken, but I'm having numerous issues related to directory paths. I created a reprex package here: https://github.com/malcolmbarrett/file.snapshot with the relevant test file here: https://github.com/malcolmbarrett/file.snapshot/blob/main/tests/testthat/test-file-snapshot.R

Here's a summary of the issues I'm having:

• testthat tells me that a snapshot is getting created even if a file doesn't exist. It seems like testthat just doesn't write an empty file even though it says it does, but it also seems to apply this logic to non-existant files.
• testthat is working from the testthat directory rather than the package root. I believe that's by testthat's design, and normally this probably makes sense, but I think this either should be addressed or better explained in the docs for this type of test.
• If I change the working directory, the snapshots are not created even if I provide absolute paths

I suspect this is a mixture of intentional and unintentional behavior (and possibly my own misunderstanding!) but unfortunately, I'm not sure which is which.

[malcolmbarrett]

I now suspect the third issue has to do with these lines:

testthat/R/snapshot-reporter.R

Lines 77 to 78 in 8821548

	snap_dir <- file.path(self$snap_dir, self$file)
	self$snap_file_seen <- c(file.path(self$file, name), self$snap_file_seen)

I'm assuming that self here has some memory of where the test is located.

[hadley]

What are you trying to do? Those tests don't look anything like what I'd expect. Generally you should be calling expect_snapshot_file() from a wrapper which would generally save the file you generated into a temp file.

[malcolmbarrett]

Oh, sorry, I should have clarified that they didn't necessarily represent my use case. It was just convenient to snapshot local files to show the circumstances under which it was and wasn't working.

I bumped into this issue while testing project setup code, similar to what usethis does, e.g. create a project in a temporary directory and work within that directory for the duration of the test. I was trying to snapshot local files created in that process. While I did not necessarily wrap it all in a function, I think the idea was the same: it was a temporary file, and I provided the path to expect_snapshot_file(). Notably, the wrapper example in the docs also fails when you set a different directory from the actual test location. I notice that expect_snapshot() writes files fine under those conditions (and currently I'm just working around this by reading and cat()ing files into that snapshot).

I think the main issues here for me are

1. expect_snapshot_file() claims it's writing a file when it's not (either because the file doesn't exist or because the file is empty)
2. expect_snapshot_file() seems to write to whatever directory you're in rather than the original test directory.

The fact that the local directory at the start of the test is in the testthat folder is probably not really an issue, especially if it expects an absolute path to a temp file.

[hadley]

Ok, I think you're saying that you're changing the working directory in test_that(), and then that causes expect_snapshot() to write to the wrong directory?

[malcolmbarrett]

Yes, except that expect_snapshot() seems to work fine, while expect_snapshot_file() does not

[hadley]

Hmmm, that's probably because snapshots generated by expect_snapshot() are written at the end of the file, after the working directory change has reverted. expect_snapshot_file() is written as it goes, so I think ensuring that SnapshotReporter$snap_dir is an absolute path rather than a relative path should fix the problem. Do you want to have a go at a PR to see if that fixes your problem? If my intuition is correct, I think you just need to call normalizePath() in SnapshotReporter$initialize().

[malcolmbarrett]

Yes, I'll take a crack at that as soon as I am back from vacation. Thanks!

[maelle]

@malcolmbarrett I've encountered the exact same issue and this thread (and your PR) made me very happy. 😁