ropensci / vcr

Record HTTP calls and replay them
https://docs.ropensci.org/vcr
Other
77 stars 12 forks source link

Should vcr_configure() respect your existing configuration? #136

Closed aaronwolen closed 4 years ago

aaronwolen commented 4 years ago

Yes, I think so. 😁

As currently implemented every call to vcr_configure() reverts all settings back to their defaults, e.g.

library(vcr)
vcr_configure(log = TRUE)
#> <vcr configuration>
#>   Cassette Dir: .
#>   Record: once
#>   URI Parser: crul::url_parse
#>   Match Requests on: method, uri
#>   Preserve Bytes?: FALSE
#>   Logging?: TRUE (vcr.log)
#>   ignored hosts: 
#>   ignore localhost?: FALSE
#>   Write disk path:

# logging is TRUE

# switch to a new cassette directory disables logging
vcr_configure(dir = "fixtures/test1")
#> <vcr configuration>
#>   Cassette Dir: fixtures/test1
#>   Record: once
#>   URI Parser: crul::url_parse
#>   Match Requests on: method, uri
#>   Preserve Bytes?: FALSE
#>   Logging?: FALSE
#>   ignored hosts: 
#>   ignore localhost?: FALSE
#>   Write disk path:

# logging is FALSE again

Created on 2020-01-22 by the reprex package (v0.3.0)

It seems like vcr_configure() should only update settings specified as arguments. This behavior would be especially convenient for tests, so that global settings could be defined once and test-specific parameters could be updated as needed.

Thoughts?

sckott commented 4 years ago

Sure, that makes sense I think. I imagine the stuff in helper-pkgname.R gets run before any tests, so that vcr_configure will be the first to get run.

hmm, not sure how this will play together with default parameter values though. it's nice to set defaults for users, those values that make the most sense. So if vcr_configure only sets configuration options if specified by the user, I guess we have to internally keep track of default options (if user just runs vcr_configure() all options are whatever the default is) vs. those set by an argument used by the user

aaronwolen commented 4 years ago

I imagine the stuff in helper-pkgname.R gets run before any tests, so that vcr_configure will be the first to get run.

That's how I was planning to set up osfr: configure vcr's log location, record mode, and matching params in helper-osfr.R, then specify a new cassette dir at the top of each individual test file.

Not sure how best to implement this yet but I'll think about it.

sckott commented 4 years ago

sounds good.

not at all judging, just curious about the motivation for different directory for each test file?

aaronwolen commented 4 years ago

I sense judging... 🙂

I perform a lot of the same setup steps in each osfr test file (generate a project, add a directory, etc) in order to start with blank slate on OSF, so organizing cassettes into directories means I don't need to worry about creating unique names, e.g.:

tests/testthat
├── cassettes
│  └── uploading
│     ├── create-dir.yml
│     ├── create-proj1.yml
│  └── directories
│     ├── create-dir.yml
│     ├── create-proj1.yml

Seemed like a clean solution but I'm open to other suggested best practices.

There are few minor issues with request matching that have prevented me from recording multiple requests to a cassette, so for the moment every test gets its own cassette. I'll add an issue or PR soon but have been focused on getting osfr to CRAN pre-rstudio::conf.

sckott commented 4 years ago

Makes sense. There may be best practices that have arisen in other language communities that I don't know about - can take a look at some point - a nice advantage of porting code from a long standing community

sckott commented 4 years ago

@aaronwolen does #141 fully address this issue?

aaronwolen commented 4 years ago

Yes 👍