ropensci / webchem

Chemical Information from the Web
https://docs.ropensci.org/webchem
Other
161 stars 41 forks source link

Use `httptest` for API mocking #388

Open Aariq opened 1 year ago

Aariq commented 1 year ago

I've played around with httptest2 in the azmetr package and it's relatively easy to get set up. I'm going to close #306 and this issue will replace it. I'd love to be able to carve out some time to work on this, but not sure when that'll happen. Maybe I can start things off with a small PR that just does one of the APIs as an example. What's the slowest API in terms of testing?

stitam commented 1 year ago

Hi @Aariq, thanks for looking into this, if you start working on API mocking I'll join in.

What I'm wondering about is the advantage of httptest2 over vcr. I have some experience with vcr but no experience with httptest2. I know vcr is an rOpensci package, I don't know about httptest2. Cranlogs counts 11k downloads per month for vcr, and 1.5k for httptest2. I'm not against httptest2 but I do remember liking vcr. Maybe we could discuss this a bit more or implement both for a single webservice each and see which one is easier to maintain over time?

Selecting the API: PubChem and ChemSpider are the slowest (many tests) and for ChemSpider there is also the monthly limit. I think ChemSpider would be the best candidate.

Aariq commented 1 year ago

The major downside of httrtest is that it (by default) creates long folder paths (same as API URL) to store the API snapshots, which CRAN doesn't like. There is additional setup to make it not do that. vcr just stores all the snapshots in folders that you decide the name of.

Aariq commented 1 year ago

First, httptest is the testing package that goes with the httr package and httptest2 goes with httr2. httptest and vcr have a very similar interface and functionality—with vcr, you wrap tests or functions inside of tests with use_cassette() and with httptest you wrap them with with_mock_dir(). Learning httptest was @maelle's suggestion (https://github.com/ropensci/webchem/pull/306#issuecomment-1301935111).

maelle commented 1 year ago

if you use httptest and then later switch from httr to httr2, it'll be very easy to switch to httptest2.

Aariq commented 1 year ago

Played around with httptest a bit (#410) and it quickly became clear that dealing with the long file paths was going to be a pain. Just sticking with vcr is going to be loads easier I think.

maelle commented 1 year ago

@Aariq it's a reasonable choice but if the beginning of each file paths it's the same, you can edit the configuration. (you'd need httptest latest version as the former one had a bug https://github.com/nealrichardson/httptest/issues/82)

Happy to help if needed, and absolutely no pressure to switch to httptest.

Aariq commented 1 year ago

Thanks @maelle. I did follow the guide to fix non-portable file paths and it just didn't go super smoothly. https://github.com/ropensci/webchem/pull/410/files#diff-a74e30985354d9f5007abd4f0d76acf8927bbfc2abbfe69a4fc80e11dcbc54d9

There are a lot of different API endpoints that webchem hits, so I was finding it to be a lot of work. vcr won't work for all of the functions in webchem, but it'll be easier to set up. Still deciding which is best.