[FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var #22668

ogizanagi · 2017-05-08T08:03:17Z

Q	A
Branch?	3.3
Bug fix?	no
New feature?	yes, but must-have for the new project structure when using flex
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#22661
License	MIT
Doc PR	todo in https://symfony.com/doc/current/testing.html#your-first-functional-test

So, when using flex, the new phpunit.xml.dist will be:

<?xml version="1.0" encoding="UTF-8"?>

<!-- https://phpunit.de/manual/current/en/appendixes.configuration.html -->
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/6.1/phpunit.xsd"
         backupGlobals="false"
         colors="true"
         bootstrap="vendor/autoload.php"
>
    <php>
        <ini name="error_reporting" value="-1" />
-        <server name="KERNEL_DIR" value="etc/" />
+        <server name="KERNEL_CLASS" value="App\Kernel" />
    </php>

    <testsuites>
        <testsuite name="Project Test Suite">
            <directory>tests/</directory>
        </testsuite>
    </testsuites>
</phpunit>

As it may cause issues when refactoring, I added a class_exists check with an appropriate exception to indicate the class is either not found or not autoloadable.

javiereguiluz · 2017-05-08T08:31:37Z

src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php

@@ -106,6 +106,14 @@ private static function getPhpUnitCliConfigArgument()
     */
    protected static function getKernelClass()
    {
+        if (isset($_SERVER['KERNEL_CLASS'])) {
+            if (!class_exists($class = $_SERVER['KERNEL_CLASS'])) {
+                throw new \RuntimeException(sprintf('Class "%s" not found or cannot be autoloaded. Please check the KERNEL_CLASS value in your phpunit.xml matches the fully-qualified class name of your Kernel or override the WebTestCase::createKernel() method.', $class));


I don't like the use of "Please" in error messages. I'm proposing changing this:

Class "%s" not found or cannot be autoloaded. Please check the KERNEL_CLASS value in your phpunit.xml matches the fully-qualified class name of your Kernel or override the WebTestCase::createKernel() method.

to this:

Class "%s" doesn't exist or cannot be autoloaded. Check that the KERNEL_CLASS value in phpunit.xml matches the fully-qualified class name of your Kernel or override the WebTestCase::createKernel() method.

Updated. Thank you :)

I also updated the WebTestCase::createKernel() part to use static::class instead of WebTestCase, so the message will indicate the final class. Anyway it was wrong, as we're in KernelTestCase, so I might propose a fix on lower branches too for the other exception a few lines below.

…ith a var

fabpot · 2017-05-08T13:59:32Z

Thank you @ogizanagi.

… kernel class with a var (ogizanagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | yes, but must-have for the new project structure when using flex | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22661 | License | MIT | Doc PR | todo in https://symfony.com/doc/current/testing.html#your-first-functional-test So, when using flex, the new `phpunit.xml.dist` will be: ```diff <?xml version="1.0" encoding="UTF-8"?>  <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/6.1/phpunit.xsd" backupGlobals="false" colors="true" bootstrap="vendor/autoload.php" > <php> <ini name="error_reporting" value="-1" /> - <server name="KERNEL_DIR" value="etc/" /> + <server name="KERNEL_CLASS" value="App\Kernel" /> </php> <testsuites> <testsuite name="Project Test Suite"> <directory>tests/</directory> </testsuite> </testsuites> </phpunit> ``` As it may cause issues when refactoring, I added a `class_exists` check with an appropriate exception to indicate the class is either not found or not autoloadable. Commits ------- 4f68912 [FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var

…izanagi) This PR was merged into the master branch. Discussion ---------- [PhpUnit] Use the new KERNEL_CLASS var for KernelTestCase Once symfony/symfony#22668 is merged. Commits ------- c32f3e5 [PhpUnit] Use the new KERNEL_CLASS var for KernelTestCase

fabpot · 2017-05-08T14:01:24Z

What about deprecating KERNEL_DIR in 3.4?

ogizanagi · 2017-05-08T15:06:09Z

I agree about deprecating it in 3.4. I don't see any good reason to keep it appart from the ability to use a non-autoloadable kernel (still doable if needed by overriding the method, as done by the SecurityBundle WebTestCase for instance).
Actually, it'll deprecate the entire getKernelClass() previous implementation, so not setting KERNEL_CLASS will throw the deprecation in 3.4, and throw an exception on 4.0.

ogizanagi · 2017-05-09T07:26:24Z

I've opened #22675 about the deprecation in 3.4.

…KERNEL_CLASS (ogizanagi) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #22668 (comment) | License | MIT | Doc PR | N/A Commits ------- d102fc0 [FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS

…KERNEL_CLASS (ogizanagi) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony/symfony#22668 (comment) | License | MIT | Doc PR | N/A Commits ------- d102fc08e4 [FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS

PHPUnit 8.x version required PHP ^7.2, so I'm setting 7.x version to support PHP 7.1. There is new way to specify Kernel class, related PR: symfony/symfony#22668

* Set PHP 7.1.3 required version I've tried to specify ^7.0 version at first, but main package which is symfony/framework-bundle@v4.4.8 requires PHP ^7.1.3. * Bump Symfony FrameworkBundle to ^4.4.8 Current Symfony Framework stable version is v5.0.8, but I guess it requires significant codebase upgrade, so I've sticked with 4.4.8 which shouldn't cause any breaking changes. Old requirement was ^3.3|^4.1 which compatible with 4.4.8. * Bump PHPUnit version to ^7.0 PHPUnit 8.x version required PHP ^7.2, so I'm setting 7.x version to support PHP 7.1. There is new way to specify Kernel class, related PR: symfony/symfony#22668 * Bump PHP CS Fixer version to ^2.16.3 Configuration and all renamed rules fixed. Config file renamed to .php_cs.dist as recommended in migration guide. Migration guide from 1.x to 2.x: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/master/UPGRADE.md#config-file * Remove PHP_CodeSniffer package Second linter doesn't make sense. I think Symfony user would prefer PHP CS Fixer over PHP_CodeSniffer because first one maintained by Symfony members. * Remove satooshi/php-coveralls package from Composer This package is abandoned and Coveralls recommends to install it directly in Travis-CI task script. * Update Travic-CI config I've changed test versions to PHP 7.1.3 and 7.2. PHPUnit generates coverage report in report/logs/clover.xml file. Then PHP CS Fixer runs with --dry-run option to not override anything just to show coding style errors. * Add basic Coveralls config This is basic recommended config for a PHP based project. * Add symfony/yaml package This package was part of satooshi/php-coveralls, now it should be defined as dev dependency. * Do not commit composer.lock I think committed composer.lock can cause CI errors while tests on fresh installs are better. * Remove confusing Ruby comment

…izanagi) This PR was merged into the master branch. Discussion ---------- [PhpUnit] Use the new KERNEL_CLASS var for KernelTestCase Once symfony/symfony#22668 is merged. Commits ------- c32f3e5 [PhpUnit] Use the new KERNEL_CLASS var for KernelTestCase

This was referenced May 8, 2017

Symfony Flex + phpunit #22661

Closed

[PhpUnit] Use the new KERNEL_CLASS var for KernelTestCase symfony/recipes#59

Merged

javiereguiluz reviewed May 8, 2017

View reviewed changes

[FrameworkBundle] KernelTestCase: allow to provide the kernel class w…

4f68912

…ith a var

linaori approved these changes May 8, 2017

View reviewed changes

fabpot merged commit 4f68912 into symfony:master May 8, 2017

ogizanagi deleted the feature/3.3/fwb/kernel_test_case_kernel_class branch May 8, 2017 14:58

ogizanagi mentioned this pull request May 9, 2017

[FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS #22675

Merged

fabpot mentioned this pull request May 17, 2017

Release v3.3.0-RC1 #22731

Merged

ogizanagi mentioned this pull request Aug 10, 2017

Fix tests in local machine (OS platform issue) EasyCorp/EasyAdminBundle#1752

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var #22668

[FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var #22668

ogizanagi commented May 8, 2017 •

edited

Loading

javiereguiluz May 8, 2017

ogizanagi May 8, 2017

fabpot commented May 8, 2017

fabpot commented May 8, 2017

ogizanagi commented May 8, 2017 •

edited

Loading

ogizanagi commented May 9, 2017

[FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var #22668

[FrameworkBundle] KernelTestCase: allow to provide the kernel class with a var #22668

Conversation

ogizanagi commented May 8, 2017 • edited Loading

javiereguiluz May 8, 2017

Choose a reason for hiding this comment

ogizanagi May 8, 2017

Choose a reason for hiding this comment

fabpot commented May 8, 2017

fabpot commented May 8, 2017

ogizanagi commented May 8, 2017 • edited Loading

ogizanagi commented May 9, 2017

ogizanagi commented May 8, 2017 •

edited

Loading

ogizanagi commented May 8, 2017 •

edited

Loading