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

[HttpClient] Add a mock client #30553

Open
wants to merge 10 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@GaryPEGEOT
Copy link
Contributor

GaryPEGEOT commented Mar 13, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR TODO

Add a new MockClient usable for test purpose:

/** @var ResponseInterface $response */
$response = $this->createMock(ResponseInterface::class);
$client = new MockClient();

// Add one or more predefined responses
$client->addResponse($response);

$client->request('GET', '/whatever', ['base_uri' => 'http://example.org']);

WDYT?

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 13, 2019

@nicolas-grekas nicolas-grekas referenced this pull request Mar 13, 2019

Open

[HttpClient] Next steps #30502

8 of 17 tasks complete

GaryPEGEOT added some commits Mar 13, 2019

@dunglas

This comment has been minimized.

Copy link
Member

dunglas commented Mar 14, 2019

Maybe could we also add a helper to create mock responses? Especially, implementing getInfo() requires some care (this method must return some mandatory values), and this could probably be automated. What I've in mind is something like:

new MockResponse(/* all args are optional */
    'content',
    200,
    ['X-Foo' => 'custom header, mandatory headers will be automatically populated'],
    ['x-info' => ['custom info, mandatory keys, including raw_headers will be automatically populated']
);
@GaryPEGEOT

This comment has been minimized.

Copy link
Contributor Author

GaryPEGEOT commented Mar 14, 2019

@dunglas just added the MockResponse, feedback welcome :)

@dunglas

This comment has been minimized.

Copy link
Member

dunglas commented Mar 14, 2019

It looks great!

@nicolas-grekas
Copy link
Member

nicolas-grekas left a comment

1st round :)

Show resolved Hide resolved src/Symfony/Component/HttpClient/MockClient.php Outdated
Show resolved Hide resolved src/Symfony/Component/HttpClient/MockClient.php Outdated
Show resolved Hide resolved src/Symfony/Component/HttpClient/MockClient.php Outdated
Show resolved Hide resolved src/Symfony/Component/HttpClient/MockClient.php Outdated
Show resolved Hide resolved src/Symfony/Component/HttpClient/MockClient.php Outdated
@@ -5,3 +5,4 @@ CHANGELOG
-----

* added the component
* added a mock client

This comment has been minimized.

@stof

stof Mar 18, 2019

Member

no need to add this in the changelog, as there is no component at all in previous version, so added the component describes the changes already.

}
/**
* Clear all predefined responses.

This comment has been minimized.

@stof

stof Mar 18, 2019

Member

Clears

{
use ResponseTrait;
protected const DEFAULT_INFO = [

This comment has been minimized.

@stof

stof Mar 18, 2019

Member

should be private rather than protected IMO, following our convention of not using protected without having a clear use case for inheritance.

This comment has been minimized.

@stof

stof Mar 18, 2019

Member

and change the access to self::, as using static:: to access a private thing is broken.

This comment has been minimized.

@GaryPEGEOT

GaryPEGEOT Mar 18, 2019

Author Contributor

Fixed

GaryPEGEOT added some commits Mar 18, 2019

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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.