8000 Ensure that PHPUnit's error handler is still working in isolated tests by alexpott · Pull Request #24575 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Ensure that PHPUnit's error handler is still working in isolated tests #24575

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Ensure that PHPUnit's error handler is still working in isolated tests #24575

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fb8ba79
Ensure that PHPUnit's error handler is still working in isolated tests
alexpott Oct 16, 2017
d985e80
Coding standards
alexpott Oct 16, 2017
d56b8b9
Fix some of the tests
alexpott Oct 16, 2017
cdcacce
Fix autoloading of bootstrap.php in tests under isolation
alexpott Oct 16, 2017
a1e028f
Remove unused use
alexpott Oct 16, 2017
810b131
Some clean ups
alexpott Oct 16, 2017
07ca35a
Remove unnecessary changes
alexpott Oct 18, 2017
1fb801a
Add missing word.
alexpott Oct 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix autoloading of bootstrap.php in tests under isolation
  • Loading branch information
@alexpott
alexpott committed Oct 24, 2017
commit cdcacce031137746082b77f5911a7c47aa158221
1 change: 1 addition & 0 deletions src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
16 changes: 6 additions & 10 deletions src/Symfony/Bridge/PhpUnit/Tests/ProcessIsolationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be enough to reset the autoloaded state then?

Copy link
Contributor
@paul-m paul-m Oct 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the use case of deprecating an interface (and other scenarios), you'd add @trigger_error() after the namespace declaration. That means the error will be triggered on discovery and then never repeated, for a false positive.

Disabling @preserveGlobalState here (on a test) implies to me that for some process-isolated tests to reflect all deprecations, they should have this same annotation, so a documentation update would be needed, and/or the test listener needs to set this value on isolated tests.

Alternately, merge deprecations from discovery and parent process with the ones from the child process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh I'm not sure when we'd get a chance to do that? We need bootstrap.php to be loaded before the test is run in isolation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8000

Worked out something about the failing tests on travis for 7.2 - it's because of something in src/Symfony/Bridge/PhpUnit/bin/simple-phpunit - i've downloaded the travis docker image and replayed the build script. Running phpunit via that script it fails. Running vanilla phpunit it passes.

*/
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
Expand Down
7 changes: 2 additions & 5 deletions src/Symfony/Bridge/PhpUnit/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
0