Injecting services and using mocks in tests after deprecation #23772
Comments
|
What about your requirements and environment makes test-specific config files a bad solution? Your original message doesn't explain why the supported manner of handling this (using test-specific config) doesn't work for you. |
|
Could you provide some code examples showing how you are mocking the services you are talking about? From #23311 (comment)
Can you develop? If you mean replacing/removing compiler passes and/or bundle extensions or redo their jobs differently, that's possible, but what would it solve? If you are properly replacing a service, it should receive the same treatment and be wired as before, except for the changes you make.
Configuration can be different for each single test. What you need today is a testing kernel class and a testcase extending KernelTestCase that takes the filename of the config(s) to load from the options passed to As far as I can tell (from the information you given and from what I know about functional testing in Symfony) I think that the main issue is the lack of documentation on the subject, which appears only now that Again, I would be curious to see one of your tests. |
|
I can't agree that CachePoolClearCommandTest is inspiring, rather I find it very complex, somehow messy and, say that, awkward. Just count, how many code is there and what it is doing, for example take a look to WebTestCase (this another WebTestCase class, this is very confusing and smells), or take a look to that app/AppKernel.php lying right here in the test folder. And… most of this code is just to inject service? No way. |
|
Thanks everyone for your quick responses! Here's an example of what I'm doing. It will not run -- I've edited it down to hopefully make a more concise example. use GuzzleHttp\Client as GuzzleClient;
use GuzzleHttp\Handler\MockHandler;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Psr7\Response;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\Middleware;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\DependencyInjection\Container;
class SomeAPITest extends WebTestCase
{
public function setUp(){
# ...
}
public function testGetToken(): void
{
// before we get here setUp() has been called and booted the kernel
// and set $this->container
// --------------------------------------------------------------------
// setup -- must swap in replacement services early
$history = [];
$token_response = [
"info" => [],
"status" => [],
"result" => [
"accessToken" => "deadbeefdeadbeefdeadbeef12345678"
]
];
# this test only adds a single response to a single mock, but other tests
# might add dozens of responses to multiple mocks. Microservices, YMMV.
$this->mockSomeGuzzleService('guzzle.client.some_api', [
[200, [], json_encode($token_response)],
], $history);
// --------------------------------------------------------------------
# The service "my_app.some_api_client" depends on "guzzle.client.some_api"
# which we have replaced above.
$my_api_service = $this->container->get('my_app.some_api_client');
# This is the actual call being tested. In this example we are testing a service
# directly but the same methodology would apply to testing an endpoint.
$token = $my_api_service->getAccessToken();
// --------------------------------------------------------------------
# validate returned value
$this->assertEquals(
'deadbeefdeadbeefdeadbeef12345678',
$token
);
// --------------------------------------------------------------------
# validate the number and details of requests that were made
$requests = $this->getMockHistory($history, 'simple');
$this->assertEquals(
[
[
'method' => 'GET',
'uri' => '/oauth/access_token?username=USERNAME&password=PASSWORD'
],
],
$requests
);
}
/**
* Replaces a guzzle service with a mock. In real code this lives in a base class.
*/
protected function mockSomeGuzzleService(
string $service_name,
array $responses,
array &$history_container): void
{
$guzzle_responses = [];
foreach ($responses as $r) {
if (is_array($r)) {
$reflect = new \ReflectionClass('GuzzleHttp\Psr7\Response');
$guzzle_responses[] = $reflect->newInstanceArgs($r);
} else {
$guzzle_responses[] = new Response($r);
}
}
$mockHandler = new MockHandler($guzzle_responses);
$stackHandler = HandlerStack::create($mockHandler);
if (false !== $history_container) {
$history = Middleware::history($history_container);
$stackHandler->push($history);
}
$mock = new GuzzleClient(['handler' => $stackHandler]);
// the magic, why must it go away?
$this->container->set($service_name, $mock);
}The Guzzle service that I am replacing is being provided by the Symfony bundle 8p/GuzzleBundle, which is awesome. Being able to see api calls in the profiler has floored more than one Cake guy around here. As you can see, the actual application code doesn't have to know anything about what the testing is doing. My clients are built with just a few lines in config.yml and the heavy lifting of building special mock clients only happens in the tests. Thanks for reading! |
|
@robfrawley The services that I am replacing are not mine and the authors did not provide a way for me to configure them in the way that I would need. |
|
@scott-r-lindsey The problem lies in the test to me, it seems to be a mix of unit and functional. Asserting that class For functionally testing an endpoint that returns a token, predictable data should be available and the test should actually hit this endpoint (with a testing http client such as the one provided by Testing that a service returned by the container is properly wired does not make sense in this context, wiring it is the symfony job that is well tested already. Better test the corresponding class unitarily. I'd expect something like this for your functional test: use Symfony\Bundle\FrameworkBundle\Test\WebTestCase
class SomeAPITest extends WebTestCase
{
protected function setUp()
{
static::bootKernel();
// eventually load some testing fixtures if you have not replaced your api client beforehand to make it uses a class that returns hardcoded data
}
public function testGetAccessToken()
{
$client = static::createClient();
$client->request('GET', '/oauth/access_token?username=USERNAME&password=PASSWORD');
$response = $client->getResponse();
$this->assertSame(Response::HTTP_OK, $response->getStatusCode());
$this->assertStringMatchesFormat('{"access_token":"%s"}', $response->getContent());
}
}and something like this as unit test: use PHPUnit\Framework\TestCase;
use Guzzle\Client;
class SomeApiClientTest extends TestCase
{
public function testGetToken()
{
$httpClient = $this->getMockBuilder(Client::class)->getMock();
$httpClient
->expects($this->once())
->method('request')
->with('GET', '/oauth/access_token', ['query' => ['username' => 'foo', 'password' => 'bar']])
->willReturn('foo_access_token');
$apiClient = new ApiClient($guzzle);
$this->assertSame($apiClient->getAccessToken(), 'foo_access_token');
}
}(of course examples miss some details of your api that I'm not aware of like how the current user is known by |
|
I'm sorry, I must not have explained correctly. The "/oauth/access_token" path can not be tested with static::createClient() because it is an external URL provided by another system. Also, we are not trying to validate the output from that endpoint. I'll try to show what I am doing with a new example. I will be testing my endpoints per the Symfony Docs here: Your First Functional Test So perhaps I'll write a test like so: namespace Tests\AppBundle\Controller;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
class PostControllerTest extends WebTestCase
{
public function testCreateProject()
{
$client = static::createClient();
$client->request(
'POST',
'/project',
array('color' => 'blue'),
array()
);
$response = $client->getResponse();
# validations ...
}
}The application is acting as a broker to an external API. The particular endpoint of my system, POST /project will trigger two calls to this external API. They will be:
The actual http calls are being produced by Guzzle clients deep in my application. I must replace the Guzzle Clients in order to:
The Guzzle clients are being provided by a handy bundle which does not have the ability to configure Guzzle clients to the desired states, but that's ok because I can replace them in testing only with Guzzle clients configured for this purpose. Hopefully my problem is more clear now. Thanks for reading. |
|
(bump) |
|
@scott-r-lindsey Ok got it, thanks for explaining and sorry for the delay. |
|
It seem that if configuration alone were a solution, that would put me on the hook to create a configuration file per test, which would be unpleasant. Guzzle clients are immutable once created (per PSR-7). |
But your mock handler is mutable, isn't it? I mean something like Then // before
$mockHandler = new MockHandler($guzzle_responses);
$stackHandler = HandlerStack::create($mockHandler);
if (false !== $history_container) {
$history = Middleware::history($history_container);
$stackHandler->push($history);
}
$mock = new GuzzleClient(['handler' => $stackHandler]);
$this->container->set($service_name, $mock);
// after
$mockHandler = $this->container->get('guzzle.mock_handler');
foreach ($guzzle_responses as $response) {
$mockHandler->append($response);
}Wouldn't that work? |
|
I suppose I can try it. Thank you for looking into it. |
|
Again, I'm not saying that this is good enough nor that we shouldn't do anything for improving the situation. Just want to dig into the existing alternatives, even verbose, to see if we can improve the current provided test cases and how. Thanks for trying. |
|
The big advantage of the solution suggested by @chalasr above is that it is much more reliable. Overwriting an existing service may not work well if this service got inlined somewhere or a service using it was already instantiated earlier than setting your mock. And with the changes about how private services are handled in 4.0, it would become even more complex. |
|
@stof "reliability" is not a convincing argument. The service container is a leaky abstraction, and I'm ok with that. Also, what are my tests for, if not to fail when in surprising ways when changes are made? If the functionality must be removed in service of other goals so be it, but it was an excellent and useful function that solved problems for me. |
|
@scott-r-lindsey not only for you. I think we are quite a few to use service override in tests to inject mock instead of the real service. Guzzle mock is a really good example. |
|
I used to have a different configuration files for different service mocks, but I found it too cumbersome and inconvenient. I would like to have proper way to configure kernel's container. Should I keep using such approach as shown below? class SomeControllerTest extends \Symfony\Bundle\FrameworkBundle\Test\WebTestCase
{
protected static function createKernel(array $opt = [])
{
return new class('test', true) extends \AppKernel
{
public function registerContainerConfiguration(
\Symfony\Component\Config\Loader\LoaderInterface $loader
) {
parent::registerContainerConfiguration($loader);
// mocked service have effect only for this test
$container = new \Symfony\Component\DependencyInjection\ContainerBuilder();
$container->register(
'AppBundle\\Service\\SomeApiClientService',
MockApiClientService::class)->setAutowired(true);
return $container;
}
protected function getContainerClass()
{
// how should I do that properly?
return 'SomeControllerTest_' . parent::getContainerClass();
}
};
}
public function testSomething()
{
$client = static::createClient();
$crawler = $this->client->request('GET', '/some-page-with-access-to-external-API');
// ...do the test
}
} |
|
@scott-r-lindsey what I mean about reliability is that depending on which optimizations are performed and which calls are made on the container, your tests in Symfony 3.2 may have used the actual guzzle client instead of the mock. So tests may have failed if your mock has an assertion on it (much less likely if it is used as a stub), but actual network requests could have been performed. |
|
@stof I understand what you meant -- you just have to replace the client before you use it. I actually do the replace twice; once in setUp() and again at the top of each test so that my tests will default to "mock queue is empty" errors if I forget to replace the client. Regardless, if a more reliable method were on offer I would use it. |
|
@scott-r-lindsey adding |
|
@chalasr I am afraid that this did not do the trick. The handler argument is set in the constructor for the guzzle client and it must be provided within an array. I do not think Symfony configuration supports this manner of initializing an object. guzzle.client.some_api:
class: GuzzleHttp\Client
arguments: ['@guzzle.handler_stack']Result: So I tried this: guzzle.client.someapi:
class: GuzzleHttp\Client
arguments:
handler: '@guzzle.handler_stack'Result: At this point, I think I have to create a proxy object and load it instead? I don't know enough about Symfony internals to know if that will work. |
|
@scott-r-lindsey sorry for the mistake, array of objects are supported: guzzle.client.habitat:
class: GuzzleHttp\Client
arguments:
- {handler: '@guzzle.handler_stack'} |
|
@chalasr I think I'm all set, thank you for digging into this. I appended this to config_test.yml: services:
guzzle.mock_handler.someapi:
class: GuzzleHttp\Handler\MockHandler
guzzle.handler_stack.someapi:
class: GuzzleHttp\HandlerStack
arguments: ['@guzzle.mock_handler.someapi']
guzzle.client.someapi:
class: GuzzleHttp\Client
arguments:
- {handler: '@guzzle.handler_stack.someapi'}And this is in my tests' base class: protected function mockSomeAPIGuzzle(array $responses): void
{
$mock_handler = $this->container->get('guzzle.mock_handler.someapi');
$handler_stack = $this->container->get('guzzle.handler_stack.someapi');
$client = $this->container->get('guzzle.client.someapi');
foreach ($responses as $r) {
if (is_array($r)) {
$mock_handler->append(new Response(...$r));
} else {
$mock_handler->append(new Response($r));
}
}
$history = Middleware::history($this->someapi_history_container);
$handler_stack->push($history);
}And one of my tests looks like this: class SomeTest extends BaseTestCase
{
public function testGetProjectDetails()
{
$mock_token = [
# ...
];
$mock_project_details = [
# ...
];
$this->mockSomeAPIGuzzle([
[200, [], json_encode($mock_token)],
[200, [], json_encode($mock_project_details)],
]);
# do the test....
}
}I still suspect that there may be edge cases where you might want to replace a service in one test but not another, and for that case the lack of flexibility will hurt. But for my use case, this is solved. Thank you! |
|
@scott-r-lindsey Thanks for using your project as the guinea pig, that will help to solve the documentation issue for sure. For anyone running into this but for a different use case which you think cannot be solved with the solution provided here, please consider opening a new discussion with code examples. |
|
See #24418 |
…colas-grekas) This PR was merged into the 3.3 branch. Discussion ---------- [DI] Allow setting any public non-initialized services | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23311, #23772 | License | MIT | Doc PR | - I think we went a but too far with this deprecation and that we should relax the rule: setting a pre-defined service is OK *if* it has not already been initialized. This will allow setting them in test cases, as reported several times (see linked issues). Commits ------- d314b1f [DI] Allow setting any public non-initialized services
I open this issue, more or less a dupe of issue 23311 hopefully create some more discussion around the deprecation of Container::set() for non-synthetic services (#19192). I can't imagine a use case for Container::set() outside of testing, but in the context of testing it is very, very helpful.
My use case is testing a large application which interacts with a number of APIs via Guzzle clients (configured as services) provided by 8p/GuzzleBundle.
My test suite replaces the various Guzzle services with Guzzle MocksHandlers immediately after booting the Kernel. This lets me boot and exercise the entire application, validate outgoing requests, and creates hard errors when unexpected requests are attempted.
It seems that I'm a year late to object to this feature being removed, but for those of us that we using it it was invaluable. Is there an objection to @emil-nasso's proposed solution? Alternate configuration by itself doesn't get us very far.
I can't think of a solution to this that is close to as clean as the old way of doing it, so please correct me if I'm missing something.
The text was updated successfully, but these errors were encountered: