contao / test-case

[READ-ONLY] Contao Test Case
6 stars 4 forks source link

Correctly boot the kernel if it hasn't been booted #8

Closed aschempp closed 3 years ago

aschempp commented 4 years ago

Fixes failing functional tests in https://github.com/contao/contao/pull/1516/checks?check_run_id=845742533

leofeyer commented 4 years ago

I don't think this is correct. You have to take care of booting the kernel in your test instead of relying on the test case to do so.

aschempp commented 4 years ago

why is that? If the loadFixture method uses self::$container->get(), it should make sure the kernel is booted? Same as every method is responsible for initializing the Contao framework, regardless of that the routing likely already does?

leofeyer commented 4 years ago

That is because Symfony wants it this way:

https://github.com/symfony/symfony/blob/6c9a25cccc145f623a6a6e152c94d9ee789f43a3/src/Symfony/Bundle/FrameworkBundle/Test/WebTestCase.php#L44-L46

Booting the kernel before calling $this->createClient() triggers a deprecation warning in Symfony 4.4 and throws an exception in Symfony 5. I have adjusted our unit tests accordingly in https://github.com/contao/contao/pull/1872.

aschempp commented 4 years ago

so we're supposed to call createClient in every test?

aschempp commented 4 years ago

also, shouldn't our fixtures then check if the kernel is booted and throw an exception otherwise?

leofeyer commented 4 years ago

so we're supposed to call createClient in every test?

I think so. Not 100% sure though.

also, shouldn't our fixtures then check if the kernel is booted and throw an exception otherwise?

Well, either way there will be an exception. It could be our own or the "trying to call get() on null" exception. 🤷‍♂️

aschempp commented 4 years ago

Yeah, but the „get() on null“ is confusing and the source is kinda unknown. But I updated the PR to add an exception. 😊

aschempp commented 3 years ago

do we still need this or should we close it?