-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
AbstractRequesterTest: Add a test #53
base: master
Are you sure you want to change the base?
AbstractRequesterTest: Add a test #53
Conversation
4693a7d
to
208ebd3
Compare
You're completely right. The class dont have a test case for query 🤦 I'd love to implement this feature, but as you know I have a upcoming version Take a look specifically here: I can see two options:
What do you think? |
208ebd3
to
05ccd9f
Compare
I'll rebase it on 3.1, shouldn't be too big of a challenge. My main goal was to establish a base for further tests. If you happened to do that already, I'm perfectly fine with it. BTW: The tests for my feature branch failed for PHP 7.4 due to a deprecation notice. Reason was an incompatibility between 7.4 and the PHPUnit installed there. It would help if you cherry-picked 05ccd9f, just to get this already out of the way. Happy hacking and muito obrigado! :) |
OK, I cherry-picked the commit you indicate into 3.1 branch. Muito obrigado for your help here :) |
I think this can be closed, as the tests you wrote are even more than the single one I wrote. I do think that you complicated this a bit though. PHPUnit comes with a whole suite of tools for mocking things. All you need to do is create a mock for I also noticed an actual error in your code. Check out |
First of all, thanks for the considerations you did. I really appreciate. Now, let's divide in two parts your comments. The first one is "Why I am not using the PHPUnit Mock?" The answer is because there are differents tests to be tested :) If I am just testing the
Since I already have the MockRequester as an example to fake a request I prefered to use it in the tests. However it doest'n exclude use the PHPUnit Mock for test the component and it is welcome. About the immutable implementation you are 100% right and I just miss it. Thank you for the heads up. I followed your recommendation and implemented the |
@UlrichEckhardt |
This is a very basic unit test of the
AbstractRequesterClass
. I've been experimenting with the handling of query parameters (see master branch here for a work in progress snapshot) and found that no such test exists. All other tests always use this class in a much wider context, but not in isolation.Also, there's an oddity which I stumbled across: The path segment
/v1
is neither present in the code making the request nor is it in the code handling the request. Still, things seem to work fine.