From fb8ba79799ea851a58eab6c8820acc946c3b2a3f Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 14:30:39 +0100 Subject: [PATCH 1/8] Ensure that PHPUnit's error handler is still working in isolated tests --- .../PhpUnit/DeprecationErrorHandler.php | 6 +++++ .../PhpUnit/Tests/ProcessIsolationTest.php | 24 ++++++++++++++++++- src/Symfony/Bridge/PhpUnit/phpunit.xml.dist | 5 ++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php index 011c28e948f2e..c8a220a40fcf5 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php @@ -255,6 +255,12 @@ public static function collectDeprecations($outputFile) } $deprecations[] = array(error_reporting(), $msg); }); + // This can registered before the PHPUnit error handler. + if (!$previousErrorHandler) { + $UtilPrefix = class_exists('PHPUnit_Util_ErrorHandler') ? 'PHPUnit_Util_' : 'PHPUnit\Util\\'; + $previousErrorHandler = $UtilPrefix . 'ErrorHandler::handleError'; + } + register_shutdown_function(function () use ($outputFile, &$deprecations) { file_put_contents($outputFile, serialize($deprecations)); }); diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 0cfbe515ee9e4..84ce7ea67f5cf 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -3,6 +3,7 @@ namespace Symfony\Bridge\PhpUnit\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\DeprecationErrorHandler; /** * Don't remove this test case, it tests the legacy group. @@ -13,12 +14,33 @@ */ class ProcessIsolationTest extends TestCase { + /** + * {@inheritdoc} + */ + protected function setUp() + { + // Ensure we are using the deprecation error handler. Unfortunately the code in bootstrap.php does not appear to + // be working. + DeprecationErrorHandler::collectDeprecations(getenv('SYMFONY_DEPRECATIONS_SERIALIZE')); + parent::setUp(); + } /** * @expectedDeprecation Test abc */ public function testIsolation() { @trigger_error('Test abc', E_USER_DEPRECATED); - $this->addToAssertionCount(1); + } + + public function testCallingOtherErrorHandler() + { + if (method_exists($this, 'expectException')) { + $this->expectException('PHPUnit\Framework\Error\Warning'); + $this->expectExceptionMessage('Test that PHPUnit\'s error handler fires.'); + } else { + $this->setExpectedException('PHPUnit_Framework_Error_Warning', 'Test that PHPUnit\'s error handler fires.'); + } + + trigger_error('Test that PHPUnit\'s error handler fires.', E_USER_WARNING); } } diff --git a/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist b/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist index 816cfe4927ed3..6dfa49bb1c5d7 100644 --- a/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist +++ b/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist @@ -19,6 +19,11 @@ + + + + + ./ From d985e800ab9d1aa49866c03a9d31d62e68738812 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 14:35:33 +0100 Subject: [PATCH 2/8] Coding standards --- src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php | 2 +- src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php index c8a220a40fcf5..bcd28be06bab8 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php @@ -258,7 +258,7 @@ public static function collectDeprecations($outputFile) // This can registered before the PHPUnit error handler. if (!$previousErrorHandler) { $UtilPrefix = class_exists('PHPUnit_Util_ErrorHandler') ? 'PHPUnit_Util_' : 'PHPUnit\Util\\'; - $previousErrorHandler = $UtilPrefix . 'ErrorHandler::handleError'; + $previousErrorHandler = $UtilPrefix.'ErrorHandler::handleError'; } register_shutdown_function(function () use ($outputFile, &$deprecations) { diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 84ce7ea67f5cf..6396fdf08788c 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -24,6 +24,7 @@ protected function setUp() DeprecationErrorHandler::collectDeprecations(getenv('SYMFONY_DEPRECATIONS_SERIALIZE')); parent::setUp(); } + /** * @expectedDeprecation Test abc */ From d56b8b90cf132747afc03151b94d4ac06fec63c5 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 14:56:45 +0100 Subject: [PATCH 3/8] Fix some of the tests --- src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 6396fdf08788c..026e23b9a8c49 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -31,15 +31,17 @@ protected function setUp() public function testIsolation() { @trigger_error('Test abc', E_USER_DEPRECATED); + $this->addToAssertionCount(1); } public function testCallingOtherErrorHandler() { + $class = class_exists('PHPUnit\Framework\Exception') ? 'PHPUnit\Framework\Exception' : 'PHPUnit_Framework_Exception'; if (method_exists($this, 'expectException')) { - $this->expectException('PHPUnit\Framework\Error\Warning'); + $this->expectException($class); $this->expectExceptionMessage('Test that PHPUnit\'s error handler fires.'); } else { - $this->setExpectedException('PHPUnit_Framework_Error_Warning', 'Test that PHPUnit\'s error handler fires.'); + $this->setExpectedException($class, 'Test that PHPUnit\'s error handler fires.'); } trigger_error('Test that PHPUnit\'s error handler fires.', E_USER_WARNING); From cdcacce031137746082b77f5911a7c47aa158221 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 17:02:56 +0100 Subject: [PATCH 4/8] Fix autoloading of bootstrap.php in tests under isolation --- .../PhpUnit/Legacy/SymfonyTestsListenerTrait.php | 1 + .../PhpUnit/Tests/ProcessIsolationTest.php | 16 ++++++---------- src/Symfony/Bridge/PhpUnit/bootstrap.php | 7 ++----- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php index 532ce202af194..d00d723190d59 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php @@ -317,6 +317,7 @@ public function endTest($test, $time) $result->addWarning($test, new $Warning('Using the "Legacy" prefix to mark all tests of a class as legacy is deprecated since version 3.3 and will be removed in 4.0. Use the "@group legacy" notation instead to add the test to the legacy group.'), $time); } } + putenv('SYMFONY_DEPRECATIONS_SERIALIZE='); } public function handleError($type, $msg, $file, $line, $context = array()) diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 026e23b9a8c49..485efc37bb625 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -11,19 +11,15 @@ * @group legacy * * @runTestsInSeparateProcesses + * @preserveGlobalState disabled + * + * Note that for the deprecation handler to work in a separate process we need to disable the preservation of global + * state. This is because composer's autoloader stores which files have been autoloaded in the global + * '__composer_autoload_files'. If this is preserved then bootstrap.php will not run again meaning that deprecations + * won't be collected. */ class ProcessIsolationTest extends TestCase { - /** - * {@inheritdoc} - */ - protected function setUp() - { - // Ensure we are using the deprecation error handler. Unfortunately the code in bootstrap.php does not appear to - // be working. - DeprecationErrorHandler::collectDeprecations(getenv('SYMFONY_DEPRECATIONS_SERIALIZE')); - parent::setUp(); - } /** * @expectedDeprecation Test abc diff --git a/src/Symfony/Bridge/PhpUnit/bootstrap.php b/src/Symfony/Bridge/PhpUnit/bootstrap.php index 2803caf02bcae..bff446ad9841e 100644 --- a/src/Symfony/Bridge/PhpUnit/bootstrap.php +++ b/src/Symfony/Bridge/PhpUnit/bootstrap.php @@ -13,11 +13,8 @@ use Symfony\Bridge\PhpUnit\DeprecationErrorHandler; // Detect if we're loaded by an actual run of phpunit -if (!defined('PHPUNIT_COMPOSER_INSTALL') && !class_exists('PHPUnit_TextUI_Command', false) && !class_exists('PHPUnit\TextUI\Command', false)) { - if ($ser = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) { - DeprecationErrorHandler::collectDeprecations($ser); - } - +if ($ser = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) { + DeprecationErrorHandler::collectDeprecations($ser); return; } From a1e028f25bef913a0bf5cf3ecd34ed8984ce2432 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 17:08:11 +0100 Subject: [PATCH 5/8] Remove unused use --- src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 485efc37bb625..967ab75a54f48 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -3,7 +3,6 @@ namespace Symfony\Bridge\PhpUnit\Tests; use PHPUnit\Framework\TestCase; -use Symfony\Bridge\PhpUnit\DeprecationErrorHandler; /** * Don't remove this test case, it tests the legacy group. From 810b131506d1192c624f88d24de046139399cffd Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 16 Oct 2017 17:10:58 +0100 Subject: [PATCH 6/8] Some clean ups --- src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php | 2 +- src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php | 1 - src/Symfony/Bridge/PhpUnit/bootstrap.php | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php index d00d723190d59..db35c22a87f7d 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php @@ -270,6 +270,7 @@ public function endTest($test, $time) } } $this->runsInSeparateProcess = false; + putenv('SYMFONY_DEPRECATIONS_SERIALIZE='); } if ($this->expectedDeprecations) { @@ -317,7 +318,6 @@ public function endTest($test, $time) $result->addWarning($test, new $Warning('Using the "Legacy" prefix to mark all tests of a class as legacy is deprecated since version 3.3 and will be removed in 4.0. Use the "@group legacy" notation instead to add the test to the legacy group.'), $time); } } - putenv('SYMFONY_DEPRECATIONS_SERIALIZE='); } public function handleError($type, $msg, $file, $line, $context = array()) diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 967ab75a54f48..6845cf8f4bdb8 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -19,7 +19,6 @@ */ class ProcessIsolationTest extends TestCase { - /** * @expectedDeprecation Test abc */ diff --git a/src/Symfony/Bridge/PhpUnit/bootstrap.php b/src/Symfony/Bridge/PhpUnit/bootstrap.php index bff446ad9841e..99d202708ae01 100644 --- a/src/Symfony/Bridge/PhpUnit/bootstrap.php +++ b/src/Symfony/Bridge/PhpUnit/bootstrap.php @@ -15,6 +15,7 @@ // Detect if we're loaded by an actual run of phpunit if ($ser = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) { DeprecationErrorHandler::collectDeprecations($ser); + return; } From 07ca35a5a1477840ddd10f8847d51d225f8c99e8 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Wed, 18 Oct 2017 14:19:05 +0100 Subject: [PATCH 7/8] Remove unnecessary changes --- .../Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php | 1 - src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php | 6 ------ src/Symfony/Bridge/PhpUnit/bootstrap.php | 6 ++++-- src/Symfony/Bridge/PhpUnit/phpunit.xml.dist | 5 ----- 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php index db35c22a87f7d..532ce202af194 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php @@ -270,7 +270,6 @@ public function endTest($test, $time) } } $this->runsInSeparateProcess = false; - putenv('SYMFONY_DEPRECATIONS_SERIALIZE='); } if ($this->expectedDeprecations) { diff --git a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php index 6845cf8f4bdb8..ec8f124a5f2c1 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php @@ -10,12 +10,6 @@ * @group legacy * * @runTestsInSeparateProcesses - * @preserveGlobalState disabled - * - * Note that for the deprecation handler to work in a separate process we need to disable the preservation of global - * state. This is because composer's autoloader stores which files have been autoloaded in the global - * '__composer_autoload_files'. If this is preserved then bootstrap.php will not run again meaning that deprecations - * won't be collected. */ class ProcessIsolationTest extends TestCase { diff --git a/src/Symfony/Bridge/PhpUnit/bootstrap.php b/src/Symfony/Bridge/PhpUnit/bootstrap.php index 99d202708ae01..2803caf02bcae 100644 --- a/src/Symfony/Bridge/PhpUnit/bootstrap.php +++ b/src/Symfony/Bridge/PhpUnit/bootstrap.php @@ -13,8 +13,10 @@ use Symfony\Bridge\PhpUnit\DeprecationErrorHandler; // Detect if we're loaded by an actual run of phpunit -if ($ser = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) { - DeprecationErrorHandler::collectDeprecations($ser); +if (!defined('PHPUNIT_COMPOSER_INSTALL') && !class_exists('PHPUnit_TextUI_Command', false) && !class_exists('PHPUnit\TextUI\Command', false)) { + if ($ser = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) { + DeprecationErrorHandler::collectDeprecations($ser); + } return; } diff --git a/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist b/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist index 6dfa49bb1c5d7..816cfe4927ed3 100644 --- a/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist +++ b/src/Symfony/Bridge/PhpUnit/phpunit.xml.dist @@ -19,11 +19,6 @@ - - - - - ./ From 1fb801aa73faaf143f12b734afe5feeb359541d5 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 24 Oct 2017 17:46:44 +0100 Subject: [PATCH 8/8] Add missing word. --- src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php index bcd28be06bab8..9397af1f22799 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php @@ -255,7 +255,7 @@ public static function collectDeprecations($outputFile) } $deprecations[] = array(error_reporting(), $msg); }); - // This can registered before the PHPUnit error handler. + // This can be registered before the PHPUnit error handler. if (!$previousErrorHandler) { $UtilPrefix = class_exists('PHPUnit_Util_ErrorHandler') ? 'PHPUnit_Util_' : 'PHPUnit\Util\\'; $previousErrorHandler = $UtilPrefix.'ErrorHandler::handleError';