search

chrismou / phergie-irc-plugin-react-google

Phergie plugin for returning Google search results and estimated result counts (https://github.com/phergie/phergie-irc-bot-react)
BSD 2-Clause "Simplified" License
1 stars 1 forks source link

Fixes empty API responses when used with phergie/phergie-irc-plugin-… #3

Closed svpernova09 closed 8 years ago

svpernova09 commented 8 years ago

Fixes empty API responses when used with phergie/phergie-irc-plugin-react-url version 2+

Not sure how to get around the test failure since you're actually using mocked up JSON results.

Tests will currently fail because your tests are using string JSON instead of actual mocked Guzzle responses.

PHPUnit 4.8.18 by Sebastian Bergmann and contributors.

Runtime:        PHP 5.6.16
Configuration:  /Users/halo/PhpstormProjects/phergie-irc-plugin-react-google/phpunit.xml

....PHP Fatal error:  Call to a member function getBody() on string in /phergie-irc-plugin-react-google/src/Provider/GoogleSearch.php on line 68

Fatal error: Call to a member function getBody() on string in /phergie-irc-plugin-react-google/src/Provider/GoogleSearch.php on line 68
chrismou commented 8 years ago

@svpernova09 Wowzer, it's been a while since I've used phake. Had to remind myself how it works. Mockery ftw.

So it's currently passing a json string to doResolveTest (ie https://github.com/chrismou/phergie-irc-plugin-react-google/blob/master/tests/PluginTest.php#L117) whereas now it'll need a phake object passed in, with a return value being set. Something like:

$this->apiResponse = Phake::mock('GuzzleHttp\Message\Response');

in setUp(), then something like:

$data = file_get_contents(__DIR__ . '/_data/webSearchResults.json');
Phake::when($this->apiResponse)->getBody()->thenReturn($data);

everywhere where doResolveTest is called (with the relevant json being loaded in)

Then the docblocks will need to be updated, because scrutinzer is beefing about the declared parameter types being incorrect now

Or alternatively, you could change https://github.com/chrismou/phergie-irc-plugin-react-google/commit/1d6b37355caadf676cd6e52c2fdca6ae16ae10fa#diff-f7fcf01b75ab0236c970de527853c3abR136 to pass in $response->getBody(). That'll probably still require some changes, but they'll be more centralised.

TBH, I could buy into either approach, though part of me feels like passing the json to getSuccessLines makes more sense, as there's no need for that method to concern itself with what http client is being used. It should just be able to take a json array and process it.

svpernova09 commented 8 years ago

@chrismou Your changes in master look good. Should be good to tag a version. Thanks!

chrismou commented 8 years ago

:+1: Cheers. I'll get this and the other's all sorted and tagged tomorrow