Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Closed

Conversation

@fancyweb
Copy link
Contributor

@fancywebfancyweb commentedFeb 28, 2022
edited
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

#45479 is causing a regression on some tests.

The kernel might not be booted and with Symfony > 4.4, an exception is thrown.

@nicolas-grekas
Copy link
Member

I don't think having a not-booted kernel is possible. I saw this failure when working on#45479, and the issue was a badly mock. This is what should be fixed on newly failing tests. See patch on#45479.

I'd prefer not blindly ignoring an exception.

@fancyweb
Copy link
ContributorAuthor

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
Copy link
Member

jderusse commentedFeb 28, 2022
edited
Loading

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
Copy link
Member

Can you figure out a realistic test case ?

@nicolas-grekas
Copy link
Member

Does calling disableReboot() work around the issue btw?

@fancyweb
Copy link
ContributorAuthor

fancyweb commentedFeb 28, 2022
edited
Loading

@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.

Can you figure out a realistic test case ?

Do you mean to write a non-regression test in this PR with my scenario?

Does calling disableReboot() work around the issue btw?

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
Copy link
Member

jderusse commentedFeb 28, 2022
edited
Loading

Can you figure out a realistic test case ?

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);

Does calling disableReboot() work around the issue btw?

Yes, but this is not what I want.

Other workarounds:

  • replacestatic::getContainer() by$client->getContainer()
  • adding$container = static::getContainer(); before the second request and then replacestatic::getContainer() by$container

@kbond
Copy link
Member

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
Copy link

Pixelshaped commentedFeb 28, 2022
edited
Loading

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
Copy link
ContributorAuthor

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
Copy link
Member

@fancyweb, you're right, there appears to be two separate issues here.

@fancyweb
Copy link
ContributorAuthor

Yes but OFC if the fix for the reset issue ends up removing getting the container here, this issue will also be fixed 😃

@Matth--
Copy link
Contributor

Matth-- commentedFeb 28, 2022
edited
Loading

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
Copy link

What's odd to me is that TestContainer overrides thereset() method so that it has no effect (see:https://github.com/symfony/symfony/blob/6.1/src/Symfony/Bundle/FrameworkBundle/Test/TestContainer.php#L123). So it means the instance of Container that KernelBrowser handles is not a test container?

@nicolas-grekas
Copy link
Member

Replaced by#45595, thanks for the test case :)

@fancywebfancyweb deleted the fwb/fix-kernel-browser-reset branchMarch 1, 2022 12:35
nicolas-grekas added a commit that referenced this pull requestMar 1, 2022
…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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@fancyweb@nicolas-grekas@jderusse@kbond@Pixelshaped@Matth--@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp