[BUGFIX] Dependencies between unit tests 35/50435/5
authorChristian Kuhn <lolli@schwarzbu.ch>
Sat, 29 Oct 2016 20:42:44 +0000 (22:42 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Sun, 30 Oct 2016 18:13:20 +0000 (19:13 +0100)
Our unit tests have an annoying history of dependencies between
each other: There are tests that rely on global state changes from
other tests. Those are hard and time consuming to track down and
lead to erratic test results if not fixed.

The patch resolves the hopefully last occurences of those cross
dependencies, all of these issues were recently introduced:

* A bug in extbase DebuggerUtility lets a test in core DebugUtilityTest fail
* SchemaColumnDefinitionListenerTest is changed to cope with a possible
  global change by a different test that we can not prevent otherwise
* felogin FrontendLoginControllerTest properly handles registered
  global objects to prevent side effects on TemplateServiceTest

To prevent such issues in the future, the new require-dev dependency
"phpunit-randomizer" is added. It allows running tests in random order
to reveal cross dependencies:

./bin/phpunit-randomizer -c typo3/sysext/core/Build/UnitTests.xml --order rand

Each run echos out a specific seed that can be used to replay this specific
order of tests:

./bin/phpunit-randomizer -c typo3/sysext/core/Build/UnitTests.xml --order rand:1234

The randomizer will be added to the pre-merge tests with a couple of
runs to find those cross dependencies more quickly.

Change-Id: Id08eda19df98f1a8adf25259f852928020acd810
Resolves: #78499
Releases: master
Reviewed-on: https://review.typo3.org/50435
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Daniel Lorenz <daniel.lorenz@extco.de>
Tested-by: Daniel Lorenz <daniel.lorenz@extco.de>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
composer.json
composer.lock
typo3/sysext/core/Tests/Unit/Database/Schema/EventListener/SchemaColumnDefinitionListenerTest.php
typo3/sysext/core/Tests/Unit/FormProtection/FormProtectionFactoryTest.php
typo3/sysext/core/Tests/Unit/Utility/DebugUtilityTest.php
typo3/sysext/extbase/Classes/Utility/DebuggerUtility.php
typo3/sysext/extbase/Tests/Unit/Utility/DebuggerUtilityTest.php
typo3/sysext/felogin/Tests/Unit/Controller/FrontendLoginControllerTest.php

index dd1bdce..1f4ad12 100644 (file)
@@ -56,7 +56,8 @@
                "codeception/codeception": "^2.2",
                "se/selenium-server-standalone": "~2.53",
                "7elix/styleguide": "~8.0.0",
-               "friendsofphp/php-cs-fixer": "^1.12"
+               "friendsofphp/php-cs-fixer": "^1.12",
+               "fiunchinho/phpunit-randomizer": "~2.0.3"
        },
        "suggest": {
                "ext-gd": "GDlib/Freetype is required for building images with text (GIFBUILDER) and can also be used to scale images",
index 7b14851..0030bd6 100644 (file)
@@ -4,8 +4,8 @@
         "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
         "This file is @generated automatically"
     ],
-    "hash": "003eade60234f3d5d8f2a088304fb744",
-    "content-hash": "ababc5ccfce6144c72465f283d78d7db",
+    "hash": "038c18ebffccc69e1744eb2700cc7665",
+    "content-hash": "df9342718f5bf608aa509f610d98f69f",
     "packages": [
         {
             "name": "cogpowered/finediff",
             "time": "2016-10-14 15:16:51"
         },
         {
+            "name": "fiunchinho/phpunit-randomizer",
+            "version": "v2.0.3",
+            "source": {
+                "type": "git",
+                "url": "https://github.com/fiunchinho/phpunit-randomizer.git",
+                "reference": "74d656b7991b90278593c6218878336bdb3c6a96"
+            },
+            "dist": {
+                "type": "zip",
+                "url": "https://api.github.com/repos/fiunchinho/phpunit-randomizer/zipball/74d656b7991b90278593c6218878336bdb3c6a96",
+                "reference": "74d656b7991b90278593c6218878336bdb3c6a96",
+                "shasum": ""
+            },
+            "require-dev": {
+                "phpunit/phpunit": "^4.2 || ^5.0"
+            },
+            "bin": [
+                "bin/phpunit-randomizer"
+            ],
+            "type": "library",
+            "extra": {
+                "branch-alias": {
+                    "dev-master": "1.0.x-dev"
+                }
+            },
+            "autoload": {
+                "psr-0": {
+                    "PHPUnitRandomizer": "src/"
+                }
+            },
+            "notification-url": "https://packagist.org/downloads/",
+            "authors": [
+                {
+                    "name": "Jose Manuel Armesto",
+                    "email": "jose@armesto.net"
+                }
+            ],
+            "description": "Execute your test cases in random order, so you can check if they have hidden dependencies",
+            "homepage": "http://armesto.net/",
+            "keywords": [
+                "phpunit",
+                "testing",
+                "unit testing"
+            ],
+            "time": "2016-05-09 09:58:08"
+        },
+        {
             "name": "friendsofphp/php-cs-fixer",
             "version": "v1.12.2",
             "source": {
index 672dc66..b76063b 100644 (file)
@@ -75,7 +75,11 @@ class SchemaColumnDefinitionListenerTest extends UnitTestCase
      */
     public function buildsColumnForEnumDataType()
     {
-        Type::addType('enum', EnumType::class);
+        if (Type::hasType('enum')) {
+            Type::overrideType('enum', EnumType::class);
+        } else {
+            Type::addType('enum', EnumType::class);
+        }
         $databasePlatformProphet = $this->prophesize(AbstractPlatform::class);
         $databasePlatformProphet->getDoctrineTypeMapping('enum')->willReturn('enum');
         $this->connectionProphet->getDatabasePlatform()->willReturn($databasePlatformProphet->reveal());
@@ -99,7 +103,11 @@ class SchemaColumnDefinitionListenerTest extends UnitTestCase
      */
     public function buildsColumnForSetDataType()
     {
-        Type::addType('set', SetType::class);
+        if (Type::hasType('set')) {
+            Type::overrideType('set', SetType::class);
+        } else {
+            Type::addType('set', SetType::class);
+        }
         $databasePlatformProphet = $this->prophesize(AbstractPlatform::class);
         $databasePlatformProphet->getDoctrineTypeMapping('set')->willReturn('set');
         $this->connectionProphet->getDatabasePlatform()->willReturn($databasePlatformProphet->reveal());
index 514010f..c8441d4 100644 (file)
@@ -22,10 +22,6 @@ use TYPO3\CMS\Core\Registry;
  */
 class FormProtectionFactoryTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
 {
-    protected function setUp()
-    {
-    }
-
     protected function tearDown()
     {
         FormProtectionFactory::purgeInstances();
index 396514a..e8aef2d 100644 (file)
@@ -90,17 +90,17 @@ class DebugUtilityTest extends UnitTestCase
                     ],
                 ],
                 'array(3 items)' . PHP_EOL
-. '   0 => "foo" (3 chars)' . PHP_EOL
-. '   1 => "bar" (3 chars)' . PHP_EOL
-. '   baz => array(1 item)' . PHP_EOL
-. '      0 => 42 (integer)',
+                    . '   0 => "foo" (3 chars)' . PHP_EOL
+                    . '   1 => "bar" (3 chars)' . PHP_EOL
+                    . '   baz => array(1 item)' . PHP_EOL
+                    . '      0 => 42 (integer)',
             ],
             'Debug object' => [
                 $object,
                 'stdClass prototype object' . PHP_EOL
-. '   foo => public 42 (integer)' . PHP_EOL
-. '   bar => public array(1 item)' . PHP_EOL
-. '      0 => "baz" (3 chars)'
+                    . '   foo => public 42 (integer)' . PHP_EOL
+                    . '   bar => public array(1 item)' . PHP_EOL
+                    . '      0 => "baz" (3 chars)'
             ],
         ];
     }
index 87d8611..558051a 100644 (file)
@@ -513,9 +513,11 @@ class DebuggerUtility
         if ($ansiColors === true) {
             $title = '\e[1m' . $title . '\e[0m';
         }
+        $backupBlacklistedClassNames = self::$blacklistedClassNames;
         if (is_array($blacklistedClassNames)) {
             self::$blacklistedClassNames = $blacklistedClassNames;
         }
+        $backupBlacklistedPropertyNames = self::$blacklistedPropertyNames;
         if (is_array($blacklistedPropertyNames)) {
             self::$blacklistedPropertyNames = $blacklistedPropertyNames;
         }
@@ -563,6 +565,8 @@ class DebuggerUtility
                        </div>
                        ';
         }
+        self::$blacklistedClassNames = $backupBlacklistedClassNames;
+        self::$blacklistedPropertyNames = $backupBlacklistedPropertyNames;
         if ($return === true) {
             return $css . $output;
         } else {
index f13afa9..75b93cd 100644 (file)
@@ -22,16 +22,6 @@ use TYPO3\CMS\Extbase\Utility\DebuggerUtility;
 class DebuggerUtilityTest extends UnitTestCase
 {
     /**
-     * @var \TYPO3\CMS\Extbase\Utility\DebuggerUtility
-     */
-    protected $debugger;
-
-    protected function setUp()
-    {
-        $this->debugger = $this->getAccessibleMock(\TYPO3\CMS\Extbase\Utility\DebuggerUtility::class, ['dummy']);
-    }
-
-    /**
      * @test
      */
     public function debuggerRewindsInstancesOfIterator()
@@ -45,7 +35,7 @@ class DebuggerUtilityTest extends UnitTestCase
             $obj->property = $i;
             $objectStorage->attach($obj);
         }
-        $this->debugger->var_dump($objectStorage, null, 8, true, false, true);
+        DebuggerUtility::var_dump($objectStorage, null, 8, true, false, true);
         $this->assertTrue($objectStorage->valid());
     }
 
@@ -56,7 +46,7 @@ class DebuggerUtilityTest extends UnitTestCase
     {
         $testObject = new \stdClass();
         $testObject->foo = 'bar';
-        $result = $this->debugger->var_dump($testObject, null, 8, true, false, true);
+        $result = DebuggerUtility::var_dump($testObject, null, 8, true, false, true);
         $this->assertRegExp('/foo.*bar/', $result);
     }
 
index 74ec80e..bd78c11 100644 (file)
@@ -50,10 +50,16 @@ class FrontendLoginControllerTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
     protected $testTableName;
 
     /**
+     * @var array
+     */
+    protected $singletonInstances;
+
+    /**
      * Set up
      */
     protected function setUp()
     {
+        $this->singletonInstances = GeneralUtility::getSingletonInstances();
         $GLOBALS['TSFE'] = new \stdClass();
         $GLOBALS['TSFE']->gr_list = '0,-1';
         $this->testTableName = 'sys_domain';
@@ -66,6 +72,19 @@ class FrontendLoginControllerTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
     }
 
     /**
+     * Tear down
+     */
+    protected function tearDown()
+    {
+        // setUpDatabaseMock() prepares some instances via addInstance(), but not all
+        // tests use that instance. purgeInstances() removes left overs and resets known
+        // singletons afterwards
+        GeneralUtility::purgeInstances();
+        GeneralUtility::resetSingletonInstances($this->singletonInstances);
+        parent::tearDown();
+    }
+
+    /**
      * Set up a fake site path and host
      */
     protected function setUpFakeSitePathAndHost()