vcr_test_path() might need an update

[maelle]

Cc @adamhsparks @dpprdan

It seems tests sometimes are run in

:/RCompile/CRANguest/R-release/packagename.Rcheck/tests_i386/testthat

So tests_i386 not tests.

[steffilazerte]

@maelle asked me to share my experiences with this problem here.

I've had the same problems with weathercan on winbuilder since June but wasn't sure why (I do use vcr but didn't realize it might be a vcr issue).

Windows devel worked fine (and still does), but not release nor old release. I eventually just ignored them and reported the rhub windows tests when submitting to CRAN in November :sweat_smile:

The problem I see is that on winbuilder the tests were run twice, once i386 and once for 64, and the test folders were renamed to tests_i386 and tests_64 (or something like that). Then I'd get the following error:

* checking for unstated dependencies in 'tests' ... OK
* checking tests ...
** running tests for arch 'i386' ... [3s] ERROR
  Running 'testthat.R' [2s]
Running the tests in 'tests/testthat.R' failed.
Complete output:
  > library(testthat)
  > library(weathercan)
  weathercan v0.6.1
  The included data `stations` has been deprecated in favour of the function `stations()`.
  See ?stations for more details.
  > 
  > Sys.setenv("R_TESTS" = "")
  > 
  > test_check("weathercan")
  Error: No root directory found in d:/RCompile/CRANguest/R-release/weathercan.Rcheck/tests_i386/testthat or its parent directories. Root criterion: contains a directory "tests"
  Execution halted

[maelle]

@sckott I am starting to think that function was not such a good suggestion :grin:

I see rprojtools has rprojroot::has_file_pattern() but not rprojroot::has_dir_pattern()

[sckott]

I see rprojtools has rprojroot::has_file_pattern() but not rprojroot::has_dir_pattern()

@maelle that's to suggest what exactly? i see we use rprojroot::has_dir, but not sure the relationship to those other 2 functions

[sckott]

thx for the deets @steffilazerte - and that error you showed is with using vcr_test_path(), correct?

[maelle]

@maelle that's to suggest what exactly? i see we use rprojroot::has_dir, but not sure the relationship to those other 2 functions

Well we try to find a directory called "tests" but it could be a directory with "tests" in its name e.g. "tests_i386". Now, we also want the exact match to have priority. :thinking:

[sckott]

Okay, thanks. Do we know how testthat handles this when the dir isn't exactly tests?

[maelle]

testthat looks for the testthat folder I think? i.e. one level down compared to vcr

[steffilazerte]

thx for the deets @steffilazerte - and that error you showed is with using vcr_test_path(), correct?

I use vcr_test_path("fixtures") in the setup file yes, but I honestly have no idea if that's related to the error.

[dpprdan]

(Sorry for the verbosity, just trying to dissect/document for me/posterity, should we have to return to this).

It seems to me that most problems we've had with vcr_test_path() so far can be rooted to false assumptions about which working dir the tests are performed in at a given time. Kirill sums up nicely when and where we want ({vcr}) files be found in the rprojroot vignette:

1. During package development (working directory: package root)
2. When testing with devtools::test() (working directory: tests/testthat)
3. When running R CMD check (working directory: a renamed recursive copy of tests)

3. is formally true, i.e. R CMD check changes the working dir to a renamed copy of tests. From Writing R Extensions:

The directory tests is copied to the check area, and the tests are run with the copy as the working directory [...]

However, {testthat} then changes the working directory to testthat within that folder (via test_dir() and local_test_directory()), so this is effectively the working directory for {vcr} then. The main difference between 2. and 3. is that in the case of 2. those tests/testthat folders are within the package directory, while they are outside the package dir in case of 3.

E.g. after R CMD check is run (R 4.1.2 on Windows), the folder structure of exemplighratia.Rcheck looks like this:

+-- 00check.log
+-- 00install.out
+-- 00_pkg_src
|   \-- exemplighratia
+-- exemplighratia
|   +-- DESCRIPTION
|   +-- help
|   +-- html
|   +-- INDEX
|   +-- LICENSE
|   +-- Meta
|   +-- NAMESPACE
|   \-- R
+-- exemplighratia-Ex.pdf
+-- exemplighratia-Ex.R
+-- exemplighratia-Ex.Rout
+-- exemplighratia-Ex.timings
\-- tests
    +-- fixtures
    +-- startup.Rs
    +-- testthat
    +-- testthat.R
    \-- testthat.Rout

Now our current problem, AFAIU it, is that instead of a single tests dir, R-devel's R CMD check now effectively creates two tests dirs, tests_i386 and (possibly) tests_64. Since vcr_test_path() expects a root dir with the name tests (matched exactly), it obviously fails. This also means that we cannot safely rely on the name of the working dir during R CMD check tests.

Do we know how testthat handles this when the dir isn't exactly tests?

testthat::test_path() sets relative paths to tests/testthat in case of 1. or to ".", i.e. the working dir, in case of 2. and 3. (note that {testthat} controls the working dir in those cases as well - which {vcr} doesn't/can't).

Long story short, I think the best option right now is to track the testthat directory, move up one level and work from there. I'll open a PR to that effect shortly.

I am starting to think that function was not such a good suggestion

The function was an excellent suggestion IMO, @maelle, we all just weren't aware of the complexities :point_up: involved. :grimacing: :sweat_smile: