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

Fix array to string conversion in REST::sendRequest() #21

Merged

Conversation

mcrumm
Copy link
Contributor

@mcrumm mcrumm commented Mar 9, 2016

Minimum viable test for an array to string conversion error in REST::sendRequest().

Before:

$ vendor/bin/phpunit
PHPUnit 4.8.23 by Sebastian Bergmann and contributors.

........................E.............

Time: 178 ms, Memory: 8.00Mb

There was 1 error:

1) Google\Gcloud\Tests\Storage\Connection\RESTTest::testGetObjectReturnsResponseArray
Array to string conversion

/Users/mike/Code/mcrumm/gcloud-php/src/Storage/Connection/REST.php:263
/Users/mike/Code/mcrumm/gcloud-php/src/Storage/Connection/REST.php:152
/Users/mike/Code/mcrumm/gcloud-php/tests/Storage/Connection/RESTTest.php:39

FAILURES!
Tests: 38, Assertions: 40, Errors: 1.

FWIW, I found this repo last night while looking for a flysystem adapter for GCP. Not finding one that I liked, I set out to write one using gcloud-php as its base. This was the last of the issues I encountered while working on the initial implementation.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 9, 2016
@dwsupplee
Copy link
Contributor

Thanks again @mcrumm! Happy to merge this and excited to hear about your plans to use the library. :)

@mcrumm
Copy link
Contributor Author

mcrumm commented Mar 9, 2016

@dwsupplee I'm not sure what HHVM is complaining about, here. Any thoughts?

@dwsupplee
Copy link
Contributor

Yes, I believe it has to do with this line: https://github.com/GoogleCloudPlatform/gcloud-php/blob/master/src/Storage/Connection/REST.php#L228

Changing it to __DIR__ . '/ServiceDefinition/storage-v1.json' should do the trick. I will work quickly on getting my latest work up, it's got fixes for items like this as well as some restructuring to make tests easier to write.

@mcrumm
Copy link
Contributor Author

mcrumm commented Mar 9, 2016

Yep, that got hhvm to pass. I can roll that change back if it will conflict with your other work, and rebase when the time is right.

@dwsupplee
Copy link
Contributor

It's no problem, this is good to merge. Thanks again!

dwsupplee added a commit that referenced this pull request Mar 9, 2016
Fix array to string conversion in REST::sendRequest()
@dwsupplee dwsupplee merged commit ef69161 into googleapis:master Mar 9, 2016
@mcrumm mcrumm deleted the storage_connection_uri_expansion branch March 18, 2016 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants