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.

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

Jump to bottom

[PhpUnitBridge] Fix some errors when using serialized deprecations #33820

Merged
merged 1 commit into from Feb 4, 2020
Merged

[PhpUnitBridge] Fix some errors when using serialized deprecations #33820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php
View file
Open in desktop
Expand Up @@ -104,7 +104,19 @@ public static function collectDeprecations($outputFile)
return \call_user_func(self::getPhpUnitErrorHandler(), $type, $msg, $file, $line, $context);
}

$deprecations[] = [error_reporting(), $msg, $file];
$trace = debug_backtrace();
$filesStack = [];
foreach ($trace as $line) {
if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
continue;
}

if (isset($line['file'])) {
$filesStack[] = $line['file'];
}
}

$deprecations[] = [error_reporting(), $msg, $file, $filesStack];

return null;
});
Expand Down
31 changes: 23 additions & 8 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php
View file
Open in desktop
Expand Up @@ -44,6 +44,8 @@ class Deprecation
*/
private static $internalPaths = [];

private $originalFilesStack;

/**
* @param string $message
* @param string $file
Expand All @@ -64,6 +66,7 @@ public function __construct($message, array $trace, $file)
$this->message = $parsedMsg['deprecation'];
$this->originClass = $parsedMsg['class'];
$this->originMethod = $parsedMsg['method'];
$this->originalFilesStack = $parsedMsg['files_stack'];
// If the deprecation has been triggered via
// \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest()
// then we need to use the serialized information to determine
Expand Down Expand Up @@ -162,6 +165,24 @@ public function isMuted()
return false !== strpos($this->triggeringFile, \DIRECTORY_SEPARATOR.'vendor'.\DIRECTORY_SEPARATOR.'phpunit'.\DIRECTORY_SEPARATOR);
}

private function getOriginalFilesStack(): array
{
if (null === $this->originalFilesStack) {
$this->originalFilesStack = [];
foreach ($this->trace as $line) {
if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
continue;
}
if (!isset($line['file'])) {
continue;
}
$this->originalFilesStack[] = $line['file'];
}
}

return $this->originalFilesStack;
}

/**
* Tells whether both the calling package and the called package are vendor
* packages.
Expand All @@ -178,14 +199,8 @@ public function getType()
return self::TYPE_UNDETERMINED;
}
$erroringFile = $erroringPackage = null;
foreach ($this->trace as $line) {
if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
continue;
}
if (!isset($line['file'])) {
continue;
}
$file = $line['file'];

foreach ($this->getOriginalFilesStack() as $file) {
if ('-' === $file || 'Standard input code' === $file || !realpath($file)) {
continue;
}
Expand Down
145 changes: 144 additions & 1 deletion src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php
View file
Open in desktop
Expand Up @@ -11,12 +11,32 @@

namespace Symfony\Bridge\PhpUnit\Tests\DeprecationErrorHandler;

use Composer\Autoload\ClassLoader;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\DeprecationErrorHandler;
use Symfony\Bridge\PhpUnit\DeprecationErrorHandler\Deprecation;
use Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerForV5;

class DeprecationTest extends TestCase
{
public static function setUpBeforeClass(): void
{
$vendorDir = self::getVendorDir();

mkdir($vendorDir.'/myfakevendor/myfakepackage1', 0777, true);
mkdir($vendorDir.'/myfakevendor/myfakepackage2');
touch($vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php');
touch($vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php');
touch($vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php');
}

private static function getVendorDir(): string
{
$reflection = new \ReflectionClass(ClassLoader::class);

return \dirname($reflection->getFileName(), 2);
}

public function testItCanDetermineTheClassWhereTheDeprecationHappened()
{
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
Expand Down Expand Up @@ -118,12 +138,135 @@ public function testItTakesMutesDeprecationFromPhpUnitFiles()
$this->assertTrue($deprecation->isMuted());
}

public function providerGetTypeDetectsSelf(): array
{
foreach (get_declared_classes() as $class) {
if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) {
$r = new \ReflectionClass($class);
$v = \dirname(\dirname($r->getFileName()));
if (file_exists($v.'/composer/installed.json')) {
$loader = require $v.'/autoload.php';
$reflection = new \ReflectionClass($loader);
$prop = $reflection->getProperty('prefixDirsPsr4');
$prop->setAccessible(true);
$currentValue = $prop->getValue($loader);
$currentValue['Symfony\\Bridge\\PhpUnit\\'] = [realpath(__DIR__.'/../..')];
$prop->setValue($loader, $currentValue);
}
}
}

return [
'not_from_vendors_file' => [Deprecation::TYPE_SELF, '', 'MyClass1', ''],
Copy link
Contributor

@l-vo l-vo Oct 20, 2019 •
edited

Choose a reason for hiding this comment

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

Don't pass anymore. A way to test it:

foreach (get_declared_classes() as $class) {
            if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) {
                $r = new \ReflectionClass($class);
                $v = \dirname(\dirname($r->getFileName()));
                if (file_exists($v.'/composer/installed.json')) {
                    $loader = require $v.'/autoload.php';
                    $reflection = new \ReflectionClass($loader);
                    $prop = $reflection->getProperty('prefixDirsPsr4');
                    $prop->setAccessible(true);
                    $currentValue = $prop->getValue($loader);
                    $currentValue['Symfony\\Bridge\\PhpUnit\\'] = [realpath(__DIR__.'/../..')];
                    $prop->setValue($loader, $currentValue);
                }
            }
        }

return [
            'not_from_vendors_file' => [Deprecation::TYPE_SELF, '', 'MyClass1', __FILE__],
            //...
];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cannot say I understand but I applied this change 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just change composer loader psr4 paths which are used to determine "self" type :)

'nonexistent_file' => [Deprecation::TYPE_UNDETERMINED, '', 'MyClass1', 'dummy_vendor_path'],
'serialized_trace_with_nonexistent_triggering_file' => [
Deprecation::TYPE_UNDETERMINED,
serialize([
'class' => '',
'method' => '',
'deprecation' => '',
'triggering_file' => 'dummy_vendor_path',
'files_stack' => [],
]),
SymfonyTestsListenerForV5::class,
'',
],
];
}

/**
* @dataProvider providerGetTypeDetectsSelf
*/
public function testGetTypeDetectsSelf(string $expectedType, string $message, string $traceClass, string $file): void
{
$trace = [
['class' => 'MyClass1', 'function' => 'myMethod'],
['class' => $traceClass, 'function' => 'myMethod'],
];
$deprecation = new Deprecation($message, $trace, $file);
$this->assertEquals($expectedType, $deprecation->getType());
}

public function providerGetTypeUsesRightTrace(): array
{
$vendorDir = self::getVendorDir();

return [
'no_file_in_stack' => [Deprecation::TYPE_DIRECT, '', [['function' => 'myfunc1'], ['function' => 'myfunc2']]],
'files_in_stack_from_various_packages' => [
Deprecation::TYPE_INDIRECT,
'',
[
['function' => 'myfunc1', 'file' => $vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php'],
['function' => 'myfunc2', 'file' => $vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php'],
],
],
'serialized_stack_files_from_same_package' => [
Deprecation::TYPE_DIRECT,
serialize([
'deprecation' => 'My deprecation message',
'class' => 'MyClass',
'method' => 'myMethod',
'files_stack' => [
$vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php',
$vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php',
],
]),
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
],
'serialized_stack_files_from_various_packages' => [
Deprecation::TYPE_INDIRECT,
serialize([
'deprecation' => 'My deprecation message',
'class' => 'MyClass',
'method' => 'myMethod',
'files_stack' => [
$vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php',
$vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php',
],
]),
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
],
];
}

/**
* @dataProvider providerGetTypeUsesRightTrace
*/
public function testGetTypeUsesRightTrace(string $expectedType, string $message, array $trace): void
{
$deprecation = new Deprecation(
$message,
$trace,
self::getVendorDir().'/myfakevendor/myfakepackage2/MyFakeFile.php'
);
$this->assertEquals($expectedType, $deprecation->getType());
}

/**
* This method is here to simulate the extra level from the piece of code
* triggering an error to the error handler
* triggering an error to the error handler.
*/
public function debugBacktrace(): array
{
return debug_backtrace();
}

private static function removeDir($dir): void
{
$files = glob($dir.'/*');
foreach ($files as $file) {
if (is_file($file)) {
unlink($file);
} else {
self::removeDir($file);
}
}
rmdir($dir);
}

public static function tearDownAfterClass(): void
{
self::removeDir(self::getVendorDir().'/myfakevendor');
}
}