Wrong path with "Run `testthat::snapshot_review('')` to review changes"

[MatthieuStigler]

When running testthat::test_file() on an external file (i.e. not linked to a package). a change in the snaphot will trigger a error message:

Snapshot of testcase to 'external_file/base-histogram.svg' has changed Run testthat::snapshot_review('external_file/') to review changes

The path indicated in testthat::snapshot_review('external_file/') is however non-informative, leading to No snapshots to update. This does not seem to be just a path issue: adding the correct full path still lead to the same error. It seems on the other hand that using the argument path will solve the problem?

Reproducible code ran with testthat 3.1.2, not reprexable though:

f <-tempdir()
file_ext <- paste0(f, "/external_file.R")

cat("disp_hist_base <- function() hist(mtcars$disp, breaks=20)\n
    test_that('base_histo',{
  vdiffr::expect_doppelganger('base histogram', disp_hist_base)})",
    file = file_ext)

## run twice
testthat::test_file(file_ext)
testthat::test_file(file_ext)

## now change doc
cat("disp_hist_base <- function() hist(mtcars$disp, breaks=10)\n
    test_that('base_histo',{
  vdiffr::expect_doppelganger('base histogram', disp_hist_base)})",
    file = file_ext)

testthat::test_file(file_ext)

testthat::snapshot_review('external_file/')
testthat::snapshot_review(paste(f, '_snaps/external_file/', sep="/"))

testthat::snapshot_review(path=paste(f))

reprex::reprex()

Example output:

> f <-tempdir()
> file_ext <- paste0(f, "/external_file.R")
> 
> cat("disp_hist_base <- function() hist(mtcars$disp, breaks=20)\n
+     test_that('base_histo',{
+   vdiffr::expect_doppelganger('base histogram', disp_hist_base)})",
+     file = file_ext)
> 
> ## run twice
> testthat::test_file(file_ext)

══ Testing external_file.R ═════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 1 ]

── Warning (external_file.R:4:3): base_histo ───────────────────────────────────
Adding new file snapshot: 'tests/testhat/_snaps/base-histogram.svg'

[ FAIL 0 | WARN 1 | SKIP 0 | PASS 1 ]
> testthat::test_file(file_ext)

══ Testing external_file.R ═════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 1 ] Done!
> 
> ## now change doc
> cat("disp_hist_base <- function() hist(mtcars$disp, breaks=10)\n
+     test_that('base_histo',{
+   vdiffr::expect_doppelganger('base histogram', disp_hist_base)})",
+     file = file_ext)
> 
> testthat::test_file(file_ext)

══ Testing external_file.R ═════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]

── Failure (external_file.R:4:3): base_histo ───────────────────────────────────
Snapshot of `testcase` to 'external_file/base-histogram.svg' has changed
Run `testthat::snapshot_review('external_file/')` to review changes
Backtrace:
 1. vdiffr::expect_doppelganger("base histogram", disp_hist_base)
      at external_file.R:4:2
 3. testthat::expect_snapshot_file(...)

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]
> 
> testthat::snapshot_review('external_file/')
No snapshots to update
> testthat::snapshot_review(paste(f, '_snaps/external_file/', sep="/"))
No snapshots to update
> 
> testthat::snapshot_review(path=paste(f))
Starting Shiny app for snapshot review
ℹ Use Ctrl + C to quit

[jonathan-g]

There are two parts to this problem:

1. The path is wrong. It's relative to the test directory, which may not be the user's current working directory, so it would be better to convert it to an absolute path or prepend the relative path from the user's cwd to the test directory.
2. The first argument to testthat::snapshot_review() is the file argument, and this is the path argument, so the message should name the argument: "Run testthat::snapshot_review(path='tests/testthat/_snaps/external_file/') to review changes"

[MLopez-Ibanez]

2. The first argument to `testthat::snapshot_review()` is the `file` argument, and this is the path argument, so the message should name the argument: "Run testthat::snapshot_review(path='tests/testthat/_snaps/external_file/') to review changes"

Looking at the code, this is also wrong. The path must be the absolute path to "tests/testthat/" and the code will append "_snaps".

The function is meant to be used from the root directory of the package, so the default path is "tests/testthat". If used in some other way, the path to "_snaps" has to be specified. The function snapshot_meta does not report a non-existent path and simply reports no files.

In the example above, the correct invokation is:

testthat::snapshot_review('external_file/', path=".")

or

testthat::snapshot_review('external_file/', path=f)

I think the error should report:

Snapshot of `testcase` to 'external_file/base-histogram.svg' has changed
Run `testthat::snapshot_review('external_file/', path = '/tmp/XXXXX')` to review changes

where '/tmp/XXXXX' is the dirname of the failing testcase removing "_snaps/external_file/base-histogram.svg". The path argument in the error could be dropped if it matches file.path(getwd(), "tests/testthat") as that is the default.