Issues with HttpPlug mock client

[KDederichs]

Hey,

I'm having issues with the HttpPlug Mock client: https://docs.php-http.org/en/latest/integrations/symfony-bundle.html#usage-for-reusable-bundles

I can't seem to set request matches, the conditionalResults in the Mock client is always empty.

I tried setting it using: self::getContainer() and $this->browser()->getContainer() but both seem to be the wrong ones.

Anyone know the right container the request is made against?

[kbond]

Hey @KDederichs!

What about $this->browser()->client()->getContainer()?

[KDederichs]

Nope also doesn't do it.

I put in some logging at different places in the code (Mock Client) and so far the the on method of the Mock Client is called correctly but then later the sendRequest (also Mock Client) just jumps to the default success response with an empty conditionalResults array. So I'd assume it's making that request against a different instance of the container which I can't seem to find.

[KDederichs]

It seems to work if I directly do that in the constructor of the KernelBrowser (https://github.com/zenstruck/browser/blob/ea84ec6b593a2607d0aa754dbde9bf17c7602f13/src/Browser/KernelBrowser.php#L33) though.

If you call $client->getContainer() there and register the matcher it's getting used.

So I'd say that's the client I need but apparently not the one I seem to get.

[KDederichs]

Ok found it:

Turns out $this->browser() always gives you a new, different instance so you have to save it and then operate on that.

Might be good to place that a bit more prominent in the docs (cause I didn't see it till explicitly looking for if it's mentionen)

[kbond]

Ah ok, glad you got it sorted. This is the same behavior of static::createClient()->getContainer().

Might be good to place that a bit more prominent in the docs (cause I didn't see it till explicitly looking for if it's mentionen)

Where in the docs do you think this should be mentioned? Would you be up to making a PR?

[kbond]

Could $browser->disableReboot() solve this? Or maybe take a look at https://github.com/Happyr/service-mocking.

[KDederichs]

I don't think disabling reboot does anything tbh since each $this->browser() boots its own kernel I think. So everything you do to that kernel will always ever be valid for that instance of the browser.

Like I said it's not an issue IF you know it does it. But the fact that it does boot up a new browser every time is not obvious at first glance and you're inclined to just use $this->browser() over and over when you need it.

I'll look into where one might put it in the docs soon, maybe as comment or text in/above the first examples would do alreayd. Just something to make people aware of it from the get go.

[kbond]

Ah, right ok, disableReboot() would just prevent the container from being reset between requests for the current instance of $browser.

[nikophil]

shouldn't we provide a ->mockInContainer($serviceId, $mockedService) (or maybe another name: replaceInContainer()) and document this?

[nikophil]

beside of this, do we really need to call ensureKernelShutdown() when browser() is called?

But the fact that it does boot up a new browser every time is not obvious at first glance and you're inclined to just use $this->browser() over and over when you need it.

I quite agree with this statement, at first glance, it could be really confusing this kind of code does not work:

self::getContainer()->set('http.client', new MockHttpClient());

$this->browser()->doSomeStuffWithMockclient();

[kbond]

This feels out of scope of this package to me. https://github.com/Happyr/service-mocking is the solution imo.

self::getContainer()->set('http.client', new MockHttpClient());

Related issue: https://github.com/symfony/symfony/issues/49930

[kbond]

beside of this, do we really need to call ensureKernelShutdown() when browser() is called?

We do when using WebTestCase because of https://github.com/symfony/symfony/blob/0d9562ff6cdda11c71f0eb2bce0076f0d3a8ea9f/src/Symfony/Bundle/FrameworkBundle/Test/WebTestCase.php#L41

[nikophil]

Thanks to pointing this issue. This would actually be the silver bullet for this problem

Nevertheless, I'm still questioning the fact that we actually always reset the kernel before even the first request.

As soon as we call self::getContainer(), the kernel gets booted. So, can't we check if the kernel is booted before calling ensureKernelShutdown()? this would at least solve all use cases where we only want to perform one request with a modified container. IMO this would clearly mitigate the "WTF effect" produced by the kernel reset before the first request.