Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cassette config inheritance #153

Merged
merged 11 commits into from
Feb 18, 2020
Merged

Conversation

aaronwolen
Copy link
Member

@aaronwolen aaronwolen commented Feb 12, 2020

As discussed in #151. This changes the defaults for arguments in use_cassette()/insert_cassette() to NULL if they control parameters that are also configurable via VCRConfig. Before making this change I reformatted the code so there's only one argument per line to make the diff more readable.

Other changes:

  • Adds test helper vcr_test_configuration() to reset the current configuration and restore settings that should be in place for all tests. This could be called within a teardown() block at the top of every test file that includes modifications to the configuration. Currently it's only used in the new inheritance tests.
  • Adds tests to verify that cassettes inherit settings from vcr_configuration()

@aaronwolen aaronwolen linked an issue Feb 12, 2020 that may be closed by this pull request
@codecov-io
Copy link

codecov-io commented Feb 12, 2020

Codecov Report

Merging #153 into master will increase coverage by 1.6%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #153     +/-   ##
=========================================
+ Coverage    73.9%   75.51%   +1.6%     
=========================================
  Files          36       36             
  Lines        1506     1511      +5     
=========================================
+ Hits         1113     1141     +28     
+ Misses        393      370     -23
Impacted Files Coverage Δ
R/configuration.R 36.84% <ø> (+20.17%) ⬆️
R/use_cassette.R 100% <100%> (ø) ⬆️
R/insert_cassette.R 93.1% <100%> (+0.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 895b1f8...42309cf. Read the comment docs.

@sckott sckott added this to the v0.4.6 milestone Feb 13, 2020
@sckott
Copy link
Collaborator

sckott commented Feb 13, 2020

All looks good, except:

We should update the parameter docs for insert_cassette and use_cassette for those that are changed in this PR to say that their default values are set by vcr_configure - which only needs to be done in use_cassette i guess since insert_cassette inherits from use_cassette

@sckott
Copy link
Collaborator

sckott commented Feb 13, 2020

And document - make it explicit that any parameters set in insert_cassette/use_cassette override those set with vcr_configure

@aaronwolen
Copy link
Member Author

Cool, I'll work on updating the docs.

@aaronwolen
Copy link
Member Author

I took a stab at documenting the new inheritance behavior in
6d033a7 but I feel like it could be clearer. That was the best my brain could come up with late on a Friday afternoon 🙂. Let me know if you have any suggestions. Otherwise I'll make another pass on Monday.

@sckott
Copy link
Collaborator

sckott commented Feb 18, 2020

looks good to me - can you resolve the conflict, then i can merge

@aaronwolen
Copy link
Member Author

aaronwolen commented Feb 18, 2020

Thanks for looking it over. I made a few additional non-coding changes that include some minor rewording/reorgs in the docs. Suggesting looking over

  • 3108f76 to see if you agree with the rewording
  • cb23594, which reorganizes the configuration docs a bit to create a separate section for cassette options that can be configured both globally and locally—and added a brief description that attempts to explain how they relate to each other.

Also just rebased to sync up with master.

@sckott
Copy link
Collaborator

sckott commented Feb 18, 2020

Both look good, thanks.

@sckott sckott merged commit ffd1549 into ropensci:master Feb 18, 2020
aaronwolen added a commit to ropensci/osfr that referenced this pull request Oct 25, 2020
aaronwolen added a commit to ropensci/osfr that referenced this pull request Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cassette options in vcr configuration are ignored
3 participants