8000 [PhpUnitBridge] More accurate grouping by greg0ire · Pull Request #31730 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[PhpUnitBridge] More accurate grouping #31730

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

Merged
merged 1 commit into from
Jun 26, 2019
Merged

[PhpUnitBridge] More accurate grouping #31730

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,13 @@ public function handleError($type, $msg, $file, $line, $context = [])
$group = 'unsilenced';
} elseif ($deprecation->isLegacy(self::$utilPrefix)) {
$group = 'legacy';
} elseif (!$deprecation->isSelf()) {
$group = $deprecation->isIndirect() ? 'remaining indirect' : 'remaining direct';
} else {
$group = 'remaining self';
$group = [
Deprecation::TYPE_SELF => 'remaining self',
Deprecation::TYPE_DIRECT => 'remaining direct',
Deprecation::TYPE_INDIRECT => 'remaining indirect',
Deprecation::TYPE_UNDETERMINED => 'other',
][$deprecation->getType()];
}

if ($this->getConfiguration()->shouldDisplayStackTrace($msg)) {
Expand Down
110 changes: 70 additions & 40 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php
View file
Open in desktop
6D40
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@
*/
class Deprecation
{
private const PATH_TYPE_VENDOR = 'path_type_vendor';
private const PATH_TYPE_SELF = 'path_type_internal';
private const PATH_TYPE_UNDETERMINED = 'path_type_undetermined';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have used an exception instead of 3-value return, but it did not feel "exceptional" enough. This is going to happen whenever a deprecation comes from the cache.

Copy link
Member

Choose a reason for hiding this comment

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

hello PHP 5.5, we cannot use these consts

Copy link
Member

Choose a reason for hiding this comment

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

fixed in 0c9b3c0


public const TYPE_SELF = 'type_self';
public const TYPE_DIRECT = 'type_direct';
public const TYPE_INDIRECT = 'type_indirect';
public const TYPE_UNDETERMINED = 'type_undetermined';

/**
* @var array
*/
Expand All @@ -39,13 +48,21 @@ class Deprecation
private $originMethod;

/**
* @var bool
* @var string one of the PATH_TYPE_* constants
*/
private $self;
private $triggeringFilePathType;

/** @var string[] absolute paths to vendor directories */
private static $vendors;

/**
* @var string[] absolute paths to source or tests of the project. This
* excludes cache directories, because it is based on
* autoloading rules and cache systems typically do not use
* those.
*/
private static $internalPaths;

/**
* @param string $message
* @param string $file
Expand All @@ -59,7 +76,7 @@ public function __construct($message, array $trace, $file)
// No-op
}
$line = $trace[$i];
$this->self = !$this->pathOriginatesFromVendor($file);
$this->trigerringFilePathType = $this->getPathType($file);
if (isset($line['object']) || isset($line['class'])) {
if (isset($line['class']) && 0 === strpos($line['class'], SymfonyTestsListenerFor::class)) {
$parsedMsg = unserialize($this->message);
Expand All @@ -70,8 +87,9 @@ public function __construct($message, array $trace, $file)
// \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest()
// then we need to use the serialized information to determine
// if the error has been triggered from vendor code.
$this->self = isset($parsedMsg['triggering_file'])
&& $this->pathOriginatesFromVendor($parsedMsg['triggering_file']);
if (isset($parsedMsg['triggering_file'])) {
$this->trigerringFilePathType = $this->getPathType($parsedMsg['triggering_file']);
}

return;
}
Expand Down Expand Up @@ -101,14 +119,6 @@ public function originatesFromAnObject()
return isset($this->originClass);
}

/**
* @return bool
*/
public function isSelf()
{
return $this->self;
}

/**
* @return string
*/
Expand Down Expand Up @@ -163,10 +173,16 @@ public function isLegacy($utilPrefix)
* Tells whether both the calling package and the called package are vendor
* packages.
*
* @return bool
* @return string
*/
public function isIndirect()
public function getType()
{
if (self::PATH_TYPE_SELF === $this->trigerringFilePathType) {
return self::TYPE_SELF;
}
if (self::PATH_TYPE_UNDETERMINED === $this->trigerringFilePathType) {
return self::TYPE_UNDETERMINED;
}
$erroringFile = $erroringPackage = null;
foreach ($this->trace as $line) {
if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
Expand All @@ -179,25 +195,28 @@ public function isIndirect()
if ('-' === $file || 'Standard input code' === $file || !realpath($file)) {
continue;
}
if (!$this->pathOriginatesFromVendor($file)) {
return false;
if (self::PATH_TYPE_SELF === $this->getPathType($file)) {
return self::TYPE_DIRECT;
}
if (self::PATH_TYPE_UNDETERMINED === $this->getPathType($file)) {
return self::TYPE_UNDETERMINED;
}
if (null !== $erroringFile && null !== $erroringPackage) {
$package = $this->getPackage($file);
if ('composer' !== $package && $package !== $erroringPackage) {
return true;
return self::TYPE_INDIRECT;
}
continue;
}
$erroringFile = $file;
$erroringPackage = $this->getPackage($file);
}

return false;
return self::TYPE_DIRECT;
}

/**
* pathOriginatesFromVendor() should always be called prior to calling this method.
* getPathType() should always be called prior to calling this method.
*
* @param string $path
*
Expand Down Expand Up @@ -237,6 +256,15 @@ private static function getVendors()
$v = \dirname(\dirname($r->getFileName()));
if (file_exists($v.'/composer/installed.json')) {
self::$vendors[] = $v;
$loader = require $v.'/autoload.php';
$paths = self::getSourcePathsFromPrefixes(array_merge($loader->getPrefixes(), $loader->getPrefixesPsr4()));
}
}
}
foreach ($paths as $path) {
foreach (self::$vendors as $vendor) {
if (0 !== strpos($path, $vendor)) {
self::$internalPaths[] = $path;
}
}
}
Expand All @@ -245,24 +273,41 @@ private static function getVendors()
return self::$vendors;
}

private static function getSourcePathsFromPrefixes(array $prefixesByNamespace)
{
foreach ($prefixesByNamespace as $prefixes) {
foreach ($prefixes as $prefix) {
if (false !== realpath($prefix)) {
yield realpath($prefix);
}
}
}
}

/**
* @param string $path
*
* @return bool
* @return string
*/
private function pathOriginatesFromVendor($path)
private function getPathType($path)
{
$realPath = realpath($path);
if (false === $realPath && '-' !== $path && 'Standard input code' !== $path) {
return true;
return self::PATH_TYPE_UNDETERMINED;
}
foreach (self::getVendors() as $vendor) {
if (0 === strpos($realPath, $vendor) && false !== strpbrk(substr($realPath, \strlen($vendor), 1), '/'.\DIRECTORY_SEPARATOR)) {
return true;
return self::PATH_TYPE_VENDOR;
}
}

return false;
foreach (self::$internalPaths as $internalPath) {
if (0 === strpos($realPath, $internalPath)) {
return self::PATH_TYPE_SELF;
}
}

return self::PATH_TYPE_UNDETERMINED;
}

/**
Expand All @@ -281,19 +326,4 @@ public function toString()
"\n".str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString()).
"\n";
}

private function getPackageFromLine(array $line)
{
if (!isset($line['file'])) {
return 'internal function';
}
if (!$this->pathOriginatesFromVendor($line['file'])) {
return 'source code';
}
try {
return $this->getPackage($line['file']);
} catch (\RuntimeException $e) {
return 'unknown';
}
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function testItCanDetermineTheClassWhereTheDeprecationHappened()
public function testItCanTellWhetherItIsInternal()
{
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
$this->assertTrue($deprecation->isSelf());
$this->assertSame(Deprecation::TYPE_SELF, $deprecation->getType());
}

public function testLegacyTestMethodIsDetectedAsSuch()
Expand All @@ -46,7 +46,7 @@ public function testItCanBeConvertedToAString()
public function testItRulesOutFilesOutsideVendorsAsIndirect()
{
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
$this->assertFalse($deprecation->isIndirect());
$this->assertNotSame(Deprecation::TYPE_INDIRECT, $deprecation->getType());
}

/**
Expand Down
10 changes: 4 additions & 6 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/default.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,13 @@ Unsilenced deprecation notices (3)
1x: unsilenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Remaining self deprecation notices (1)

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Legacy deprecation notices (1)

Other deprecation notices (1)
Other deprecation notices (2)

1x: root deprecation

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

I get precedence over any exit statements inside the deprecation error handler.
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/fake_vendor/autoload.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<?php

require_once __DIR__.'/composer/autoload_real.php';

return ComposerAutoloaderInitFake::getLoader();
17 changes: 17 additions & 0 deletions ...mfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/fake_vendor/composer/autoload_real.php
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
<?php

class ComposerLoaderFake
{
public function getPrefixes()
{
return [];
}

public function getPrefixesPsr4()
{
return [];
}
}

class ComposerAutoloaderInitFake
{
public static function getLoader()
{
return new ComposerLoaderFake();
}
}
10 changes: 4 additions & 6 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/self_on_non_vendor.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ Unsilenced deprecation notices (3)
1x: unsilenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Remaining self deprecation notices (1)

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Legacy deprecation notices (1)

Other deprecation notices (1)
Other deprecation notices (2)

1x: root deprecation

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

10 changes: 4 additions & 6 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/shutdown_deprecations.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,15 @@ Unsilenced deprecation notices (3)
1x: unsilenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Remaining self deprecation notices (1)

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Legacy deprecation notices (1)

Other deprecation notices (1)
Other deprecation notices (2)

1x: root deprecation

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Shutdown-time deprecations:

Other deprecation notices (1)
Expand Down
11 changes: 5 additions & 6 deletions src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/weak_vendors_on_non_vendor.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,13 @@ Unsilenced deprecation notices (3)
1x: unsilenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Remaining self deprecation notices (1)

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Legacy deprecation notices (1)

Other deprecation notices (1)
Other deprecation notices (2)

1x: root deprecation

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar


0