subugoe / metacheck

Automatically check metadata compliance for hybrid open access (OA).
http://subugoe.github.io/metacheck/
GNU Affero General Public License v3.0
8 stars 1 forks source link

isolate tests from live APIs #77

Open maxheld83 opened 3 years ago

maxheld83 commented 3 years ago

tests will currently ocassionally invalidate when cr data changes, and some of these changes percolate through. that is causing noise.

maxheld83 commented 3 years ago

I thought and read a little bit about testing against (live) APIs, also including the recent ropensci book on the topic: https://books.ropensci.org/http-testing/ Some comments on that in this PR and related issue: https://github.com/ropensci-books/http-testing/pull/91

I think generally we should:

njahn82 commented 3 years ago

To test, if a compliance check works, we would record the HTTP calls and responses just once and re-use them, right? Like https://github.com/ropensci/vcr

maxheld83 commented 3 years ago

Mmh, I am not 💯sure yet, but I don't think we should be mocking anything in metacheck (as would be done via vcr).

There's basically two sets of tests relevant here:

  1. unit testing: Those should only test the transformations, plots, etc. -- all the metacheck-specific stuff, but never anything dependent on APIs, mocked or otherwise. It's the API wrapper packages job to unit test their handling of the API, not metacheck's job. I'm doing this for biblids in https://github.com/subugoe/biblids/issues/69. Not sure yet how / if this is done in rcrossref, might be related to https://github.com/subugoe/metacheck/issues/183.
  2. integration tests: We should test whether metacheck works with other services, in this case, the current crossref (and doi) API. This should be a very sparse test (i.e. maybe happen only on main, and should be only one snapshotted test). Being an integration test, this should be against the live API, because that's the point.

So in summary, I don't think there's any need for mocking in metacheck. Mocking/faking also always adds a level of complexity that needs to be justified, and I don't think it is here.

This issue should still stay open until: