Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony 3.4/4.0 support : KERNEL_DIR is replaced by KERNEL_CLASS #343

Closed
gnutix opened this issue Nov 8, 2017 · 11 comments
Closed

Symfony 3.4/4.0 support : KERNEL_DIR is replaced by KERNEL_CLASS #343

gnutix opened this issue Nov 8, 2017 · 11 comments
Milestone

Comments

@gnutix
Copy link
Contributor

gnutix commented Nov 8, 2017

According to : https://github.com/symfony/symfony/blob/master/UPGRADE-3.4.md#frameworkbundle

Using the KERNEL_DIR environment variable or the automatic guessing based on the phpunit.xml / phpunit.xml.dist file location is deprecated since 3.4. Set the KERNEL_CLASS environment variable to the fully-qualified class name of your Kernel instead. Not setting the KERNEL_CLASS environment variable will throw an exception on 4.0 unless you override the KernelTestCase::createKernel() or KernelTestCase::getKernelClass() method.

If I remove the KERNEL_DIR env variable from phpunit.xml.dist, I get the following deprecation warning :

The Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::getPhpUnitCliConfigArgument() method is deprecated since 3.4 and will be removed in 4.0.
The Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::getPhpUnitXmlDir()
method is deprecated since 3.4 and will be removed in 4.0

Which, I think, are caused by this line : https://github.com/liip/LiipFunctionalTestBundle/blob/master/Test/WebTestCase.php#L77

I'm not sure what getKernelClass should do exactly if KERNEL_CLASS is available (autoloading or not for example?).

@Jean85
Copy link
Contributor

Jean85 commented Nov 13, 2017

I'm also getting an other deprecation due to this bundle in SF 3.4, which is this one:

User Deprecated: Auto-registration of the command "Liip\FunctionalTestBundle\Command\TestCommand" is deprecated since Symfony 3.4 and won't be supported in 4.0. Use PSR-4 based service discovery instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Auto-registration of the command "Liip\FunctionalTestBundle\Command\TestCommand" is deprecated since Symfony 3.4 and won't be supported in 4.0. Use PSR-4 based service discovery instead.

This is a lot easier to fix, BTW.

I don't think that a bundle should support more than 2 major versions of Symfony at once, so we need a new major ASAP! Should we push forward #332 ad #338 ?

@felipeparaujo
Copy link

+1 for this. Any news on wether the bundle is getting updated?

@alexislefebvre
Copy link
Collaborator

Thanks for your comments, see #346 and #351 for the support of Symfony 3.4 (and 4.0 that will require some work).

@alexislefebvre alexislefebvre added this to the 2.0 milestone Jan 11, 2018
@soullivaneuh
Copy link
Contributor

Requiring SF 3.4 as minimum on the stable branch is not breaking according to semver.

@soullivaneuh
Copy link
Contributor

BTW, why do we need to override the getKernelClass? I don't get it.

@alexislefebvre
Copy link
Collaborator

According to #217 (comment):

its used in KernelTestCase::createKernel()

The intention of this is to enable more flexibility in the structuring of the app dir .. specifically to support multiple kernels.

So, do we still need it? What would be the use case for this?

@soullivaneuh
Copy link
Contributor

So, do we still need it? What would be the use case for this?

This is exactly what I was wondering. :trollface:

@alexislefebvre
Copy link
Collaborator

Ping @lsmith77 I quoted your explanation in a previous comment: #343 (comment) Could you please give your opinion about the need of getKernelClass now that we are working on the 2.x branch?

@lsmith77
Copy link
Contributor

I think the multiple kernel approach is pretty dead at this point, so imho we can drop it.

@Jean85
Copy link
Contributor

Jean85 commented Jan 16, 2018

Yep. It was used in the tests, but I think that @soullivaneuh solved that too in his latest PRs.

@soullivaneuh
Copy link
Contributor

@Jean85 @lsmith77 yeah, it was already done on #379. The issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants