3 ms·

There's a pretty significant problem here: with this approach, you lose test coverage of the code within your client wrapper. Namely, there's code like this whi

by btown 4y ago

There's a pretty significant problem here: with this approach, you lose test coverage of the code within your client wrapper. Namely, there's code like this which is not being tested: `.json()["repositories"]` Now, one might argue, those calls are so trivial that it doesn't need coverage, right? But what if you were trying to get the results from a nested key as a list and also check for an error status etc. etc.? Or what if this is XML deserialization with certain assumptions baked into the deserialization code? You could easily add complexity that you think your tests are covering, but they aren't. I prefer instead to use https://vcrpy.readthedocs.io/en/latest/ https://vcrpy.readthedocs.io/en/latest/ which helps you to create a fixture of what you don't own, and hooks every possible requests/http/urllib system to ensure you're not leaking a hit to a production service on any but your first test run. Tests even need to worry about mocks in most cases, they just need to be wrapped in a simple annotation. In a way, it's precisely the opposite of the OP's conclusion: mock what you don't own, at the lowest possible level to ensure all the code you wrote is covered. But use the right tool for the job!

oweiler 4y ago
If you have complex logic for extracting nested JSON, put it in a function and unit test the function.
hynek 4y ago
I’m my experience, you get most of that kind of coverage with integration test/user story tests/however-you-call-them. Solutions like vcr aren’t bullet-proof either and are a variation of in-process servers that the article mentions. To put it differently: the question is not whether vcr, but _where_ vcr.
theptip 4y ago
I was going to make the same observation, and suggest the same solution, though I’d advocate for doing both things. You definitely do need at least one test getting full coverage for the facade client simply because someone could accidentally insert white space that completely breaks the logic. Carving out a facade you own makes it really easy for all of the rest of your system to behave predictably in tests; for example what if you call an analytics API on every action? Probably easier to have UTs by default assemble your app with a MockAnalyticsClient with sensible noop mocks instead of actually wiring up the real mocks required for the library client. I’d call VCR “integration-ish” tests. I argue it’s ok to check in a blob of API response data because (ideally) your remote API doesn’t change that often. Ideally you do want to be able to run the integration tests periodically (nightly?) without the recorded response to make sure your assumptions about the remote api haven’t broken. That last bit is sometimes tricky to wire up. It’s also worth noting that VCR has some nice bells and whistles to do substitution (to elide secrets) and can also store the test data in say a storage bucket, if you don’t want to set up git-LFS or put the response data in your repo. (SOAP gets expensive as you may need to store the whole WSDL for example.) The key thing IMO is keeping the VCR out of all of your business logic tests, and confined just to integration tests targeted at the facade class. Biz logic tests should be as clean as possible and simply mocking the facade helps achieve that.
- theptip 4y ago
  After reading further in the linked docs, I see that “verified fakes” is another way of getting coverage of the facade class. The advantage is you never have to check in api results. The disadvantage is you can’t run them on every commit, so you learn about breaks to the facade class later. This might be a good trade-off for small clients that don’t change often. It might be a bad trade off for fairly thick/logic-heavy remote-API clients that change often or are under active development. (I’ve worked with one payments/forex API that would meet this description).
Dangeranger 4y ago
Since the example given in the article is an integration test, it does not prevent using an HTTP replay server to simulate making real calls to the service within end-to-end tests. I think the author even linked to some of his favorite examples for using an HTTP test server to replay responses to requests.

by btown 4y ago

oweiler 4y ago

If you have complex logic for extracting nested JSON, put it in a function and unit test the function.

hynek 4y ago

I’m my experience, you get most of that kind of coverage with integration test/user story tests/however-you-call-them. Solutions like vcr aren’t bullet-proof either and are a variation of in-process servers that the article mentions. To put it differently: the question is not whether vcr, but _where_ vcr.

theptip 4y ago

I was going to make the same observation, and suggest the same solution, though I’d advocate for doing both things. You definitely do need at least one test getting full coverage for the facade client simply because someone could accidentally insert white space that completely breaks the logic. Carving out a facade you own makes it really easy for all of the rest of your system to behave predictably in tests; for example what if you call an analytics API on every action? Probably easier to have UTs by default assemble your app with a MockAnalyticsClient with sensible noop mocks instead of actually wiring up the real mocks required for the library client. I’d call VCR “integration-ish” tests. I argue it’s ok to check in a blob of API response data because (ideally) your remote API doesn’t change that often. Ideally you do want to be able to run the integration tests periodically (nightly?) without the recorded response to make sure your assumptions about the remote api haven’t broken. That last bit is sometimes tricky to wire up. It’s also worth noting that VCR has some nice bells and whistles to do substitution (to elide secrets) and can also store the test data in say a storage bucket, if you don’t want to set up git-LFS or put the response data in your repo. (SOAP gets expensive as you may need to store the whole WSDL for example.) The key thing IMO is keeping the VCR out of all of your business logic tests, and confined just to integration tests targeted at the facade class. Biz logic tests should be as clean as possible and simply mocking the facade helps achieve that.

theptip 4y ago
After reading further in the linked docs, I see that “verified fakes” is another way of getting coverage of the facade class. The advantage is you never have to check in api results. The disadvantage is you can’t run them on every commit, so you learn about breaks to the facade class later. This might be a good trade-off for small clients that don’t change often. It might be a bad trade off for fairly thick/logic-heavy remote-API clients that change often or are under active development. (I’ve worked with one payments/forex API that would meet this description).

Dangeranger 4y ago

Since the example given in the article is an integration test, it does not prevent using an HTTP replay server to simulate making real calls to the service within end-to-end tests. I think the author even linked to some of his favorite examples for using an HTTP test server to replay responses to requests.