empty cassettes on interactive tests

[dpprdan]

vcr creates empty cassettes on interactive tests, cf. #225

Steps to reproduce:

1. clone https://github.com/maelle/exemplighratia/ locally
2. checkout vcrtest branch
3. devtools::load_all(".")
4. open tests/testthat/test-api-status.R and run test locally (or just the vcr::use_cassette(...) bit).

This will give you the following:

> devtools::load_all(".")
i Loading exemplighratia
> test_that("gh_api_status() works", {
+   vcr::use_cassette("gh_api_status", {
+     status <- gh_api_status()
+   })
+   testthat::expect_type(status, "character")
+ })
-- Warning (Line 2): gh_api_status() works -------------------------------------
Empty cassette (gh_api_status) deleted; consider the following:
 - If an error occurred resolve that first, then check:
 - vcr only supports crul & httr; requests w/ curl, download.file, etc. are not supported
 - If you are using crul/httr, are you sure you made an HTTP request?

Backtrace:
 1. vcr::use_cassette(...)
 2. cassette$eject()
 3. private$remove_empty_cassette()

AFAICT there are two things happening:

1. vcr tries to create a (new) cassette in the project root, i.e. the current working dir (Note that the package contains a cassette for abovementioned test in tests/fixtures). Arguably, this is not even a bug, because everything happens as specified. The vcr setup is in tests/testthat/setup-examplighratia.R (like the vcr docs state). setup-* files are not loaded when test files are run interactively (see table in https://blog.r-hub.io/2020/11/18/testthat-utility-belt/). So this all works as specified. Maybe we have to think about changing the recommendation to put vcr setup code into setup-*.R. It might be sufficient to state that tests have to be run through testthat::test_file() (cf. the Run tests button in RStudio), though, and cannot be run interactively. (Maybe this is stated in the docs somewhere and I missed it?) (You can see the new cassette in the package root, when you try to run the test without an internet connection, BTW).
2. The new cassette is empty and therefore deleted right away. Now why the cassette is empty, I don't know. Possibly because vcr's defaults prohibit the response to reach the cassette?

(BTW, since this looks a lot like #225, my hunch is that it wasn't really about vcr_test_path(), but about this.)

cc @maelle

Session Info

```r > sessioninfo::session_info() - Session info ------------------------------------------------------------------------------------------------------------ setting value version R version 4.1.2 (2021-11-01) os Windows 10 x64 (build 19043) system x86_64, mingw32 ui RStudio language en collate German_Germany.1252 ctype German_Germany.1252 tz Europe/Berlin date 2021-12-20 rstudio 2021.09.0+351 Ghost Orchid (desktop) pandoc 2.16.2 @ C:\\PROGRA~1\\Pandoc\\pandoc.exe - Packages ---------------------------------------------------------------------------------------------------------------- ! package * version date (UTC) lib source backports 1.4.1 2021-12-13 [1] CRAN (R 4.1.2) base64enc 0.1-3 2015-07-28 [1] CRAN (R 4.1.0) cachem 1.0.6 2021-08-19 [1] CRAN (R 4.1.2) callr 3.7.0 2021-04-20 [1] CRAN (R 4.1.0) cli 3.1.0 2021-10-27 [1] CRAN (R 4.1.1) crayon 1.4.2 2021-10-29 [1] CRAN (R 4.1.1) crul 1.2.0 2021-11-22 [1] CRAN (R 4.1.2) curl 4.3.2 2021-06-23 [1] CRAN (R 4.1.0) desc 1.4.0 2021-09-28 [1] CRAN (R 4.1.1) devtools 2.4.3 2021-11-30 [1] CRAN (R 4.1.2) digest 0.6.29 2021-12-01 [1] CRAN (R 4.1.2) ellipsis 0.3.2 2021-04-29 [1] CRAN (R 4.1.0) R exemplighratia * 0.0.0.9000 [?] fansi 0.5.0 2021-05-25 [1] CRAN (R 4.1.0) fastmap 1.1.0 2021-01-25 [1] CRAN (R 4.1.0) fauxpas 0.5.0 2020-04-13 [1] CRAN (R 4.1.0) fs 1.5.2 2021-12-08 [1] CRAN (R 4.1.2) glue 1.5.1 2021-11-30 [1] CRAN (R 4.1.2) httpcode 0.3.0 2020-04-10 [1] CRAN (R 4.1.0) httr 1.4.2 2020-07-20 [1] CRAN (R 4.1.0) jsonlite 1.7.2 2020-12-09 [1] CRAN (R 4.1.0) lifecycle 1.0.1 2021-09-24 [1] CRAN (R 4.1.1) magrittr 2.0.1 2020-11-17 [1] CRAN (R 4.1.0) memoise 2.0.1 2021-11-26 [1] CRAN (R 4.1.2) pacman * 0.5.1 2019-03-11 [1] CRAN (R 4.1.0) pillar 1.6.4 2021-10-18 [1] CRAN (R 4.1.1) pkgbuild 1.3.0 2021-12-09 [1] CRAN (R 4.1.2) pkgconfig 2.0.3 2019-09-22 [1] CRAN (R 4.1.0) pkgload 1.2.4 2021-11-30 [1] CRAN (R 4.1.2) prettyunits 1.1.1 2020-01-24 [1] CRAN (R 4.1.0) processx 3.5.2 2021-04-30 [1] CRAN (R 4.1.0) ps 1.6.0 2021-02-28 [1] CRAN (R 4.1.0) purrr 0.3.4 2020-04-17 [1] CRAN (R 4.1.0) R.cache 0.15.0 2021-04-30 [1] CRAN (R 4.1.0) R.methodsS3 1.8.1 2020-08-26 [1] CRAN (R 4.1.0) R.oo 1.24.0 2020-08-26 [1] CRAN (R 4.1.0) R.utils 2.11.0 2021-09-26 [1] CRAN (R 4.1.1) R6 2.5.1 2021-08-19 [1] CRAN (R 4.1.1) Rcpp 1.0.7 2021-07-07 [1] CRAN (R 4.1.0) remotes 2.4.2 2021-11-30 [1] CRAN (R 4.1.2) reprex 2.0.1 2021-08-05 [1] CRAN (R 4.1.0) rex 1.2.1 2021-11-26 [1] CRAN (R 4.1.2) rlang 0.4.12 2021-10-18 [1] CRAN (R 4.1.1) rprojroot 2.0.2 2020-11-15 [1] CRAN (R 4.1.0) rstudioapi 0.13 2020-11-12 [1] CRAN (R 4.1.0) sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.1.2) styler 1.6.2 2021-09-23 [1] CRAN (R 4.1.1) testthat * 3.1.1 2021-12-03 [1] CRAN (R 4.1.2) tibble 3.1.6 2021-11-07 [1] CRAN (R 4.1.2) todor 0.1.2 2021-05-24 [1] CRAN (R 4.1.0) triebeard 0.3.0 2016-08-04 [1] CRAN (R 4.1.0) urltools 1.7.3 2019-04-14 [1] CRAN (R 4.1.0) usethis * 2.1.5 2021-12-09 [1] CRAN (R 4.1.2) utf8 1.2.2 2021-07-24 [1] CRAN (R 4.1.0) vcr 1.0.2 2021-12-20 [1] Github (ropensci/vcr@79d2f88) vctrs 0.3.8 2021-04-29 [1] CRAN (R 4.1.0) webmockr 0.8.0 2021-03-14 [1] CRAN (R 4.1.0) whisker 0.4 2019-08-28 [1] CRAN (R 4.1.0) withr 2.4.3 2021-11-30 [1] CRAN (R 4.1.2) yaml 2.2.1 2020-02-01 [1] CRAN (R 4.1.0) [1] C:/Users/Daniel.AK-HAMBURG/Documents/R/win-library/4.1 [2] C:/Program Files/R/R-4.1.2/library R -- Package was removed from disk. ```

[sckott]

Thanks! I'll have a look

[sckott]

Looked at this.

I think I hadn't run into this b/c most (all?) of my pkgs that use vcr have a helper-*.R file instead of setup-*.R - which is loaded during load_all

As far as i can tell there's sort of two options

1. with setup-*.R on interactive tests make sure to load vcr before running any tests that use vcr (that works for me, does it work for you?)
   • caveat: cassettes will need to be cleaned up since they'll be created in the root dir (assuming that's where tests are run).
2. with helper-*.R on interactive tests there's nothing to do - your vcr setup will be loaded with load_all

As for the empty cassette, I think it's b/c vcr is not loaded yet, and important bits in https://github.com/ropensci/vcr/blob/master/R/onLoad.R are loaded upon library(vcr)

[dpprdan]

LGTM

Re 1. Instead of just loading vcr it's probably better to run the whole setup-vcr.R file before testing interactively, so vcr is loaded with all custom options, including [vcr_]dir. That way there is no need to clean up cassettes afterwards.

I think 2. (helper-*) is preferable, because it causes less mental load/things that can go wrong. IIUC, the only difference between setup-* and helper-* is that code in the former is not loaded by devtools::load_all() while the latter is. When working with vcr-enabled tests I'd generally want the latter. Or am I overlooking any problems with helper-*, vcr-specific or otherwise?

A few things to consider should we want to switch to recommend helper, however:

Testthat docs no longer recommend using helper-*.R files:

Helper files start with helper and are executed before tests are /run. They're also loaded by devtools::load_all(), so there's no real point to them and you should just put your helper code in R/.

So maybe we should inform the testthat team that we recommend the use of a helper file, even though Hadley already said that they aren't going away?

In addition, use_vcr() currently creates a setup-vcr.R, so that would have to change if we want to go down this road.

Finally regarding a third option to place the vcr setup in R/ (as prompted by the testthat quote): I don't know how to get this to work reliably, at least I couldn't when I quickly tried it with requireNamespace() etc., which would be necessary so that vcr doesn't have to be in Imports. But generally, I think it is better to separate the test (setup) code from the main code.

[maelle]

IIUC, the only difference between setup-* and helper-* is that code in the former is not loaded by devtools::load_all() while the latter is.

I also think that's the only difference between the two. https://blog.r-hub.io/2020/11/18/testthat-utility-belt/

[sckott]

Can someone refresh my memory on this? What's the next step here, if any?

[dpprdan]

I'd recommend the helper-* approach, because AFAICT that is the only one that automatically loads the vcr setup when developing/testing interactively (which I'd want as a developer) and does not load vcr when using a package (which I wouldn't want).

If you agree, there are two things to do:

1. change use_vcr() so that it creates a helper-vcr.R instead of setup-vcr.R
2. Update all documentation to use helper-vcr.R, including the http-testing book.

I could draft PRs to that effect if you want.

[maelle]

Yeah when I recommended using setup I had misunderstood recommendations. :see_no_evil: https://blog.r-hub.io/2020/11/18/testthat-utility-belt/#code-called-in-your-tests

[maelle]

Thanks @dpprdan

[sckott]

Thanks @dpprdan - Yeah, lets change to helper-vcr.R - that'd be great if you could draft PRs

[dpprdan]

Testthat docs no longer recommend using helper-*.R files:

Just found this https://github.com/r-lib/testthat/pull/1626

1. gotta love Jenny's commit messages/PR titles
2. Seems like Hadley agrees with us now.

[dpprdan]

use_vcr() creates a setup-pkgname.R (as documented here, where pkgname is the name of the package using vcr) whereas some of the documentation refers to setup-vcr.R, (EDIT) e.g. here

https://github.com/ropensci/vcr/blob/4579c52b5113009b2272b9316c63adfbf02261c0/man/rmdhunks/setup.Rmd#L20

I think I would like to change this all to helper-vcr.R, because that makes "This file contains the vcr setup code" more clear. Or should all helper code be located in one file? I guess that if there are more than one helper-*.R file, the order in which they are loaded could matter and the helper-vcr.R file is (more) likely to be the last one to load? But that seems very edge-casey...?

What do you think @sckott @maelle? Am I missing something?

[maelle]

What an awesome find!

I think it's fine, in a worst case scenario an user could rename it?

[sckott]

@dpprdan So the idea is that a package (e.g., tidytags) that uses vcr would have potentially many helper files, e.g., helper-tidytags.R and helper-vcr.R. They could in theory put the code in helper-vcr.R into helper-tidytags.R, yes? But the idea perhaps is that you can have many helper- files to separate out different test concerns?

[dpprdan]

Yes, that is correct. You can put all setup code into one helper*.R file or into separate files.

At present, use_vcr() would create a helper-tidytags.R (if not present already) and add the vcr setup code in that file.