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

Contributing advice template? #145

Open
maelle opened this issue Feb 6, 2020 · 8 comments
Open

Contributing advice template? #145

maelle opened this issue Feb 6, 2020 · 8 comments

Comments

@maelle
Copy link
Member

maelle commented Feb 6, 2020

Packages that use vcr for testing have the need for similar guidelines regarding testing (at least as long as the use of cassettes isn't widespread).

  • Could a template live somewhere in this package?
  • It might include a part that says "fill this in with workflow regarding setup/teardown" cf How to make tests work for external PRs in a repo #137
  • It might give a link to the book
  • Could it be mentioned by use_vcr(), "add the following lines to your contributing guide".
  • If use_vcr() ends up editing the gitattributes to hide cassette diffs, the contributing guide should mention that as well.

How such a template could look like.

This package uses [`vcr`](https://docs.ropensci.org/vcr/) on top of `testthat` for testing: HTTP requests and responses are saved so that when running tests, the cached things are used instead of repeated calls to web services.

If you're new to such a testing setup, read LINK.

In practice, it means 

* **For new tests** you should add `vcr::use_cassette()` around your function calls that use HTTP, or around whole tests. Check out existing test files, in particular <add the path of one of your tests>. Running a new test the first time will create cassettes in tests/fixtures/ i.e. YAML files containing HTTP requests and response. <add the path to the cassette(s) corresponding to the test you linked> Please commit the cassette with your test.

* If you edit the HTTP request a function or a test is making, delete the corresponding cassette before running the tests.

<Add lines regarding how your package handles secrets in testing>
@sckott
Copy link
Collaborator

sckott commented Feb 6, 2020

Good idea.

Could a template live somewhere in this package?

probably yes

Maybe use_vcr could mention that the user can run another function to get this template? as it might be too much to have the info that use_vcr prints plus all of this.

@maelle
Copy link
Member Author

maelle commented Mar 4, 2020

For packages with authentication, the template should also include a line about what the vcr config of the package does to protect API keys.

@maelle
Copy link
Member Author

maelle commented Mar 4, 2020

In the template, mention how vcr works, but also basic troubleshooting advice (or a link to basic troubleshooting advice) such as "Have you tried re-recording the fixture(s)?".

@sckott
Copy link
Collaborator

sckott commented Mar 4, 2020

include a line about what the vcr config of the package does to protect API keys

good idea

also basic troubleshooting advice

in this chapter? https://books.ropensci.org/http-testing/gotchas.html

@sckott
Copy link
Collaborator

sckott commented Mar 16, 2020

an eg PR ropensci/taxize#805

I think we need some tooling for contributors to help them manipulate cassettes (and files on disk if applicable) - e.g,. we could have a fxn that takes as input the exported package fxn name the user is working on testing, then delete all cassettes associated with that fxn. I guess ideally it'd not be greedy, so conservatively deleting cassette files.

@maelle
Copy link
Member Author

maelle commented Mar 24, 2020

I think we need some tooling for contributors to help them manipulate cassettes (and files on disk if applicable) - e.g,. we could have a fxn that takes as input the exported package fxn name the user is working on testing, then delete all cassettes associated with that fxn. I guess ideally it'd not be greedy, so conservatively deleting cassette files.

That sounds like an excellent idea! How would you guess which cassettes are associated with the function?

  • If the test file is named correctly, then all cassettes created in the test file corresponding to the function.

  • But what about cassettes created by other test files?

Should you open another issue about this function?

And maybe I'll wait on the contributing advice template, if the workflow is meant to get smoother with such helpers.

@sckott
Copy link
Collaborator

sckott commented Mar 24, 2020

How would you guess which cassettes are associated with the function?

I don't know yet. leveraging whatever covr does perhaps

this issue: #103 which is waiting on #152 to be done first.

@maelle
Copy link
Member Author

maelle commented Dec 17, 2020

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

No branches or pull requests

2 participants