Skip to content
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

Added method getReason() to SpotifyWebAPIException #156

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@mengidd
Copy link

commented May 31, 2019

When working with the Player API it's really handy to have the error reason string available.

I've not written much tests in my life, and I'm honestly not sure if this will do. If you want anything changed, just let me know and I will try to have it fixed.

Added method getReason() to SpotifyWebAPIException that returns the r…
…eason string returned by the player api if a request fails
@jwilsson
Copy link
Owner

left a comment

I think it looks good!

I've left one comment about another test that we could add.

throw new SpotifyWebAPIException($error->message, $error->status);
$exception = new SpotifyWebAPIException($error->message, $error->status);
if (isset($error->reason)) {

This comment has been minimized.

Copy link
@jwilsson

jwilsson Jun 1, 2019

Owner

I'm thinking we'd want a test for this if, we could mock a player error response similar to here:

public function testApi()
.

Let me know if you have any questions!

This comment has been minimized.

Copy link
@mengidd

mengidd Jun 14, 2019

Author

Due to the fact that the send method is mocked and parseBody (where the if-check is) is called from the send method I seem to have no way to reach the parseBody method as it's protected.

Like I've mentioned I'm relatively new to writing tests and I might be completely wrong here. Is the correct approach here to use ReflectionClass and test parseBody directly, or am I just dead wrong?

@mengidd

This comment has been minimized.

Copy link
Author

commented Jun 5, 2019

I will try to have this solved the coming weekend 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.