-
Notifications
You must be signed in to change notification settings - Fork 14
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
Unit tests and a base for easier testing endpoints #10
base: master
Are you sure you want to change the base?
Conversation
…ndpoint Added tests for Fixture endpoint class Added mocks for fixture/{id} endpoint
Looks good, so the JSON file must match the name of the parameter? Am i reading that correctly? |
Yeah, pretty much it the same. Instead of having an API where you call /resource/id, you have in your mocks a id.json in the resource folder. That way you don't need to waste much time creating mocks for the responses. |
I’m not sure if that would work with all of the resources, and some allow additional params (which I think are parsed into the query string). We could potentially do it differently, making the filename protected, then for each test case, override the filename? |
I understand, that's why I'm just saying that this is just a base endpoint test case. I'm not very familiar with the sportmonks API but in any case, we could just rearrange the way we store the mocks to support also query parameters. That way we just need to modify the callback in the Endpoint test case class to map the $url variable to the correct file in the mocks folder. The way I see it, the only thing we should test in the endpoint classes is the $url variable that is being passed to the request method. What happens in the request method doesn't belong to the project responsibilities, that's why the project uses symfony's package to handle the call to the API. |
Basically you really don't want to make real requests to 3rd party API's within your tests - that makes no sense. eg. if that 3rd party service is down your tests will fail - and you cannot control that. Another point is that usually those 3rd services have somekind of rate limit within those API requests, so you will hit those limits quite soon too. And basically if/when you're using those 3rd api's in your tests you're testing that 3rd party api, not your application. And how to fix this? There is PHP-VCR library to mock those 3rd party API calls totally, which basically solves all the problems that I described earlier. |
I think the OP is using HTTP client to mock the requests? |
I'm actually using Symfony\Component\HttpClient\MockHttpClient, which does no http calls whatsoever. This class is already in the symfony package the project was already using and it's used for testing purposes, to mock the responses without using any external dependency. I just took a quick look at the PHP-VCR package and it seems like it's doing pretty much the same thing I've done. I wouldn't use it just because I try to use the less dependencies I can, and I believe that having the mocks coded by yourself makes you more in control of your own project and it's also a great way for you to develop your testing skills. I'm not saying the package is bad, I just believe it's an overkill for a simple project. |
And in my opinion it's not "secure" to mock the client itself - what if the client itself changes some basic behaviour (and yes I know that Symfony has BC promise, but that won't still cover that case) on that component itself. That is the main reason I mentioned that PHP-VCR library. |
also note that those mocks are created automatically on first request where you're trying to use those - so there isn't really manual work at all. |
When you use dependencies, you're always allowing some kind of upgrade where the dependency changes and you're left with a broken project, that's why I try to use the least amount of dependencies possible. |
Sure, I'm just saying my point of view of this - so seems like we just need to agree to disagree with this - and that is just fine. |
This is to fix #9 (or at least, create a base so that it's easier to test the "endpoint" classes)
Added:
Any feedback is appreciated !