Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Fix getting the container from a non-booted kernel in KernelBrowser#45581
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedFeb 28, 2022
fancyweb commentedFeb 28, 2022
It's possible when you use several clients, the pseudo-code is something like this: $client =self::createClient();$client->request();self::ensureKernelShutdown();$otherClient =self::createClient();$otherClient->request();// Now fails$client->request(); |
jderusse commentedFeb 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Had the same in a personal project. Simple reproducer: $client =self::createClient();$client->request('GET','/user');$client->request('GET','/user'); <== reboot the containerstatic::getContainer(); <= throws Could not find service"test.service_container".... |
nicolas-grekas commentedFeb 28, 2022
Can you figure out a realistic test case ? |
nicolas-grekas commentedFeb 28, 2022
Does calling disableReboot() work around the issue btw? |
fancyweb commentedFeb 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@jderusse I don't think the proposed fix works for your case, isn't it? It looks like you are impacted by the reset itself, not just getting the container.
Do you mean to write a non-regression test in this PR with my scenario?
Yes but I hope I don't have to call it everywhere after a patch release 😬 Another workaround is to boot the kernel manually btw but then it shutdowns it and then reboots it again 😓 $client->getKernel()->boot() |
jderusse commentedFeb 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Here is more context about the failing test: $client =self::createClient();$crawler =$this->request('GET','/user');$client->submit($crawler->selectButton('Save')->form(['profile[email]' =>'foo@bar.com',]));$user =static::getContainer()->get(UserRepository::class)->findOneBy(['email' =>'foo@bar.com']);static::assertNotNull($user);
Yes, but this is not what I want. Other workarounds:
|
kbond commentedFeb 28, 2022
I'm seeing the same thing, another workaround: $client =self::createClient();$client->request('GET','/user');$client->request('GET','/user'); <== reboot the containerstatic::ensureKernelShutdown();// fixstatic::getContainer(); |
Pixelshaped commentedFeb 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All of this is due to the merge ofhttps://github.com/symfony/framework-bundle/blame/5.4/KernelBrowser.php#L175 Container is now reset between requests, whereas it wasn't before. Not sure why we need to reset the Container between each and every request. |
fancyweb commentedFeb 28, 2022
This PR is only linked to the reset problem. It won't fix it. This PR only fixes getting the container from a non-booted kernel. |
kbond commentedFeb 28, 2022
@fancyweb, you're right, there appears to be two separate issues here. |
fancyweb commentedFeb 28, 2022
Yes but OFC if the fix for the reset issue ends up removing getting the container here, this issue will also be fixed 😃 |
Matth-- commentedFeb 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This issue also occurs when following a redirect and trying to fetch a service to do some assertions publicfunctiontestRedirect():void{$client =self::createClient();$client->request('GET','/redirect');// works$service =self::getContainer()->get(Service::class);$service->method();// Follow redirect$crawler =$client->followRedirect();// does not work$service =self::getContainer()->get(Service::class);$service->method();$this->assertStringContainsString('OK',$crawler->text());} |
Pixelshaped commentedMar 1, 2022
What's odd to me is that TestContainer overrides the |
bde797b to70e07ceComparenicolas-grekas commentedMar 1, 2022
Replaced by#45595, thanks for the test case :) |
…icolas-grekas)This PR was merged into the 4.4 branch.Discussion----------[FrameworkBundle] Fix resetting container between tests| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#40965,fix#45580| License | MIT| Doc PR | -Replaces#45479 and#45581, related to#34078.Calling `boot()` on an already booted kernel does reset the container, so we don't need to care anymore about the state of kernel.#45580 is fixed by removing `private static $kernelContainer`, which can be out of sync with `static::$kernel->getContainer()` since `KernelBrowser` creates new containers when shutting down the kernel between requests.Commits-------4453bdb [FrameworkBundle] Fix resetting container between tests
Uh oh!
There was an error while loading.Please reload this page.
#45479 is causing a regression on some tests.
The kernel might not be booted and with Symfony > 4.4, an exception is thrown.