Fix undefined variable error in HttpRequestWrapper

[mcrumm]

Fixes #19

[mcrumm]

Oops... I missed the part where my application default credentials were getting loaded in my local environment.

@dwsupplee Any suggestions for providing some credentials that could work for this test? Or would it be better to create mocks for token generation flow?

I'll admit, the test is a little heavy-handed for the amount of code that changed, but I didn't want to submit anything without some form of test to back it.

[dwsupplee]

Thanks for the contribution. It looks like there were some issues with the build:

Google\Gcloud\Tests\HttpRequestWrapperTest::testSignRequestCanCheckTheExistingCredentialsExpiry
DomainException: Could not load the default credentials. Browse to https://developers.google.com/accounts/docs/application-default-credentials for more information

If you've got the time to address this, that would be fantastic. If not, my upcoming PR should address this for you in a few days.

[mcrumm]

Sure! Let me see what I can do.

[dwsupplee]

@mcrumm

Apologies for the timing of my response earlier, I didn't see your question before I had posted.

You should be able to provide a mock keyFile and mock response from the authHttpHandler, something along the lines of the following:

use GuzzleHttp\Psr7\Response;

$keyFile = json_encode([
    'type' => 'authorized_user',
    'client_id' => '[email protected]',
    'client_secret' => 'example',
    'refresh_token' => 'abc'
]);

$authHttpHandler = function($request) {
    return new Response(
        200,
        [],
        json_encode(['access_token' => 'abc'])
    );
};

$wrapper = new HttpRequestWrapper(
    $keyFile,
    null,
    [],
    null,
    $authHttpHandler
);

Let me know if this helps! :)

[mcrumm]

@dwsupplee OK, I've got a much simpler approach to the test, and the build is passing again.

[mcrumm]

@dwsupplee Sorry for all the nonsense. This is "passing", but that's because I broke the test :-(

I'm fixing it up, now.

[dwsupplee]

@mcrumm No nonsense at all! I think it's awesome you're taking the time to help and contribute.

What do you think about using the mocks as I outlined above? It should help us up the actual line coverage.

[mcrumm]

@dwsupplee I didn't see your comment until after I had started down the path of using Reflection to open up the private HttpRequestWrapper::$credentials variable in order to skip the OAuth2 flow, but still hit the expiry check.

Let me know if you're satisfied with the structure of this test. If not, I can take another swing at it using the approach you mentioned.

[dwsupplee]

👍 We can go ahead and run with this for now. My upcoming commit has a few changes to this class and will be providing more coverage, when that happens I may make some changes to match the structure of the rest of the tests.

[mcrumm]

Sounds good! Thanks for the merge.

Any chance we'll see a (pre-)release on packagist after your next refactor?

[dwsupplee]

Yes, we're planning to get a release on packagist and docs published by the end of the month.

[mcrumm]

👍 Looking forward to it!