fhdsl / metricminer

R package that digs up data that matters from APIs, making it dashboard-ready
https://hutchdatascience.org/metricminer/
MIT License
2 stars 0 forks source link

What do you want to test? #39

Open sckott opened 10 months ago

sckott commented 10 months ago

Following up on our call today regarding testing real vs. fake http requests ...

I wouldn't say there's a one size fits all solution for this, but when you are making http requests to the internet in a package in a test, you aren't just testing your code, you're also testing the remote API. So some folks to better isolate and only test their own code (and not whether a remote API is up or down or changed) they use something (fakes, mocks, etc.) to prevent real http requests from being made.

There's a variety of options in the book. I'm most familiar with vcr and webmockr since I maintain them, and can help with those if you go that way.

Even if you do most of you tests without running real http requests, it's a good idea to have some real http requests in case the remote API changes - one way to do that with vcr by changing one env var VCR_TURN_OFF, which you could do on github actions or locally to ignore the fixtures and do real http requests

One caveat with this route is that you should be sure you're not including secrets in your test fixtures.

You could decide you do want real http requests for all your tests, and no changes needed! happy to chat more about this

howardbaik commented 10 months ago

Thanks for this helpful explanation @sckott. @cansavvy I think we should definitely have some real http requests using vcr, and I can work on this down the road.

cansavvy commented 10 months ago

Yes for sure! I wanna make all the testing more specific and robust! So this is great. Thanks for writing this down, @sckott !

sckott commented 10 months ago

@howardbaek

have some real http requests using vcr

note that vcr helps you avoid real HTTP requests, but using that env var thing mentioned you can turn it off to make real HTTP requests. just clarifying in case there's any confusion.

howardbaik commented 10 months ago

Thanks @sckott . I'll let you know if I have any further questions once I drive deeper.