Use Prophecy for mocking in "Api" class tests

[aik099]

Problems solved:

• the DocBlock of updateVersion and releaseVersion methods were saying that they return Result, but they never do
• the DocBlock of findVersionByName method was saying that it returns VersionId, but it actually returns all version fields
• tests for Api class were mocking Api class partially and that is considered as bad practice (changed to mock client class instead)
• several assertions per test were replaced with data provider usage (where appropriate)
• removed DocBlocks of test methods in favor of improved test names themselves
• added helper methods to be used for quick test writing for all other methods of Api class

P.S.
My apologies for 3 places, where PhpStorm removed trailing whitespaces. Not sure if it's worth redoing PR to exclude them.

[aik099]

@jpastoor , ready for review.

[aik099]

This test is replaced by testSetendpoint with data provider usage for cases, when trailing slash is present or not.

[jpastoor]

Yeah saw it, tried to remove the comment but you were too fast ;)

[aik099]

Then that's why I've got error from GitHub, when tried to reply to that comment on website. :) Decided to reply over email.

[jpastoor]

Agree on the change to mock the ClientInterface instead of Api::api.

No fan of the change from phpunit to prophesy with the passive aggressive title suggesting that this is THE way to do stuff like this. With extracting the expectClientCall you add a layer of indirection, and the assertFalses on the outputs suggest that is a meaningful route to test but we don't do anything with that state.

[jpastoor]

(OK for you to merge this one though since above is not important for me enough to be blocking or anything, just wanted to let you know the few things that crossed my mind :)

[aik099]

No fan of the change from phpunit to prophesy with the passive aggressive title suggesting that this is THE way to do stuff like this.

My apologies. I've renamed the PR. Actually this is THE way are thoughts of PHPUnit author (see https://thephp.cc/news/2015/02/phpunit-4-5-and-prophecy blogpost about why Prophecy is better mocking alternative as built-in PHPUnit-Mock library) .

With extracting the expectClientCall you add a layer of indirection ...

This is because of default parameter values. When specifying expectation you don't have any auto-complete based on parameters of mocked method. Since mocked method have ~6 parameters, that must be specified in each expectation and in most cases we only provide 3 of them, then extracting in separate method reduces chances of getting things wrong, when writing unit tests. Also, at least for me, having nice expect* method is better than copy-pasting long expectation creating code all over the place.

and the assertFalses on the outputs suggest that is a meaningful route to test but we don't do anything with that state.

This tests, that return value of method stays as it is currently. Currently API calls, that are just successful, but doesn't return anything useful just return false to end user and that's what I test.