Skip to content

[PhpUnitBridge] Added a CoverageListener to enhance the code coverage report #23149

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.

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 10 commits into from
5 changes: 5 additions & 0 deletions src/Symfony/Bridge/PhpUnit/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

3.4.0
-----

* added a `CoverageListener` to enhance the code coverage report

3.3.0
-----

Expand Down
44 changes: 44 additions & 0 deletions src/Symfony/Bridge/PhpUnit/CoverageListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\PhpUnit;

use PHPUnit\Framework\BaseTestListener;
use PHPUnit\Framework\Test;
use Symfony\Bridge\PhpUnit\Legacy\CoverageListenerTrait;

if (class_exists('PHPUnit_Runner_Version') && version_compare(\PHPUnit_Runner_Version::id(), '6.0.0', '<')) {
class_alias('Symfony\Bridge\PhpUnit\Legacy\CoverageListener', 'Symfony\Bridge\PhpUnit\CoverageListener');
// Using an early return instead of a else does not work when using the PHPUnit
// phar due to some weird PHP behavior (the class gets defined without executing
// the code before it and so the definition is not properly conditional)
Copy link
Member

Choose a reason for hiding this comment

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

fabbot does not seem to be happy with the comments not being indented

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a false positive. I ignored it.

Copy link
Member

Choose a reason for hiding this comment

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

How is that a false positive?

} else {
/**
* CoverageListener adds `@covers <className>` on each test suite when possible
* to make the code coverage more accurate.
*
* @author Grégoire Pineau <lyrixx@lyrixx.info>
*/
class CoverageListener extends BaseTestListener
{
private $trait;

public function __construct(callable $sutFqcnResolver = null, $warningOnSutNotFound = false)
{
$this->trait = new CoverageListenerTrait($sutFqcnResolver, $warningOnSutNotFound);
}

public function startTest(Test $test)
{
$this->trait->startTest($test);
}
}
}
35 changes: 35 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Legacy/CoverageListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\PhpUnit\Legacy;

/**
* CoverageListener adds `@covers <className>` on each test suite when possible
* to make the code coverage more accurate.
*
* @author Grégoire Pineau <lyrixx@lyrixx.info>
*
* @internal
*/
class CoverageListener extends \PHPUnit_Framework_BaseTestListener
{
private $trait;

public function __construct(callable $sutFqcnResolver = null, $warningOnSutNotFound = false)
{
$this->trait = new CoverageListenerTrait($sutFqcnResolver, $warningOnSutNotFound);
}

public function startTest(\PHPUnit_Framework_Test $test)
{
$this->trait->startTest($test);
}
}
113 changes: 113 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Legacy/CoverageListenerTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\PhpUnit\Legacy;

use PHPUnit\Framework\Test;
use PHPUnit\Framework\Warning;

/**
* PHP 5.3 compatible trait-like shared implementation.
*
* @author Grégoire Pineau <lyrixx@lyrixx.info>
*
* @internal
*/
class CoverageListenerTrait
{
private $sutFqcnResolver;
Copy link
Member

Choose a reason for hiding this comment

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

could we please add typehints here and in other places?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's useless, it's already typehinted in the constructor.

And about startTest method: I did not find a wat because of PHPUnit namespace change. We have to support many version.

Copy link
Member

Choose a reason for hiding this comment

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

It's useless, it's already typehinted in the constructor.

constructor param is typehinted which is great, yet class property is not, thus when I want to use property in some method, I need to remember property method by looking at constructor, not from typehinting

Copy link
Member Author

Choose a reason for hiding this comment

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

If you are using an IDE, The IDE do it for you.
If you don't (like me) the naming does everything.

private $warningOnSutNotFound;
private $warnings;

public function __construct(callable $sutFqcnResolver = null, $warningOnSutNotFound = false)
{
$this->sutFqcnResolver = $sutFqcnResolver;
$this->warningOnSutNotFound = $warningOnSutNotFound;
$this->warnings = array();
}

public function startTest($test)
{
$annotations = $test->getAnnotations();

$ignoredAnnotations = array('covers', 'coversDefaultClass', 'coversNothing');

foreach ($ignoredAnnotations as $annotation) {
if (isset($annotations['class'][$annotation]) || isset($annotations['method'][$annotation])) {
return;
}
}

$sutFqcn = $this->findSutFqcn($test);
if (!$sutFqcn) {
if ($this->warningOnSutNotFound) {
$message = 'Could not find the tested class.';
// addWarning does not exist on old PHPUnit version
if (method_exists($test->getTestResultObject(), 'addWarning') && class_exists(Warning::class)) {
$test->getTestResultObject()->addWarning($test, new Warning($message), 0);
} else {
$this->warnings[] = sprintf("%s::%s\n%s", get_class($test), $test->getName(), $message);
}
}

return;
}

$testClass = \PHPUnit\Util\Test::class;
if (!class_exists($testClass, false)) {
$testClass = \PHPUnit_Util_Test::class;
}

$r = new \ReflectionProperty($testClass, 'annotationCache');
$r->setAccessible(true);

$cache = $r->getValue();
$cache = array_replace_recursive($cache, array(
get_class($test) => array(
'covers' => array($sutFqcn),
),
));
$r->setValue($testClass, $cache);
}

private function findSutFqcn($test)
{
if ($this->sutFqcnResolver) {
$resolver = $this->sutFqcnResolver;

return $resolver($test);
}

$class = get_class($test);

$sutFqcn = str_replace('\\Tests\\', '\\', $class);
$sutFqcn = preg_replace('{Test$}', '', $sutFqcn);

if (!class_exists($sutFqcn)) {
return;
}

return $sutFqcn;
}

public function __destruct()
{
if (!$this->warnings) {
return;
}

echo "\n";

foreach ($this->warnings as $key => $warning) {
echo sprintf("%d) %s\n", ++$key, $warning);
}
}
}
35 changes: 35 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Tests/CoverageListenerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Symfony\Bridge\PhpUnit\Tests;

use PHPUnit\Framework\TestCase;

class CoverageListenerTest extends TestCase
{
public function test()
{
if ("\n" !== PHP_EOL) {
$this->markTestSkipped('This test cannot be run on Windows.');
}

if (defined('HHVM_VERSION')) {
$this->markTestSkipped('This test cannot be run on HHVM.');
}

$dir = __DIR__.'/../Tests/Fixtures/coverage';
$php = PHP_BINARY;
$phpunit = $_SERVER['argv'][0];

exec("$php -d zend_extension=xdebug.so $phpunit -c $dir/phpunit-without-listener.xml.dist $dir/tests/ --coverage-text", $output);
$output = implode("\n", $output);
$this->assertContains('Foo', $output);

exec("$php -d zend_extension=xdebug.so $phpunit -c $dir/phpunit-with-listener.xml.dist $dir/tests/ --coverage-text", $output);
$output = implode("\n", $output);
$this->assertNotContains('Foo', $output);
$this->assertContains("SutNotFoundTest::test\nCould not find the tested class.", $output);
$this->assertNotContains("CoversTest::test\nCould not find the tested class.", $output);
$this->assertNotContains("CoversDefaultClassTest::test\nCould not find the tested class.", $output);
$this->assertNotContains("CoversNothingTest::test\nCould not find the tested class.", $output);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.1/phpunit.xsd"
backupGlobals="false"
colors="true"
bootstrap="tests/bootstrap.php"
failOnRisky="true"
failOnWarning="true"
>

<testsuites>
<testsuite name="Fixtures/coverage Test Suite">
<directory>tests</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory>src</directory>
</whitelist>
</filter>

<listeners>
<listener class="Symfony\Bridge\PhpUnit\CoverageListener">
<arguments>
<null/>
<boolean>true</boolean>
</arguments>
</listener>
</listeners>
</phpunit>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.1/phpunit.xsd"
backupGlobals="false"
colors="true"
bootstrap="tests/bootstrap.php"
failOnRisky="true"
failOnWarning="true"
>

<testsuites>
<testsuite name="Fixtures/coverage Test Suite">
<directory>tests</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory>src</directory>
</whitelist>
</filter>
</phpunit>
29 changes: 29 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Tests/Fixtures/coverage/src/Bar.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace PhpUnitCoverageTest;

class Bar
{
private $foo;

public function __construct(Foo $foo)
{
$this->foo = $foo;
}

public function barZ()
{
$this->foo->fooZ();

return 'bar';
}
}
20 changes: 20 additions & 0 deletions src/Symfony/Bridge/PhpUnit/Tests/Fixtures/coverage/src/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace PhpUnitCoverageTest;

class Foo
{
public function fooZ()
{
return 'foo';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace PhpUnitCoverageTest\Tests;

use PHPUnit\Framework\TestCase;

class BarTest extends TestCase
{
public function testBar()
{
if (!class_exists('PhpUnitCoverageTest\Foo')) {
$this->markTestSkipped('This test is not part of the main Symfony test suite. It\'s here to test the CoverageListener.');
}

$foo = new \PhpUnitCoverageTest\Foo();
$bar = new \PhpUnitCoverageTest\Bar($foo);

$this->assertSame('bar', $bar->barZ());
}
}
Loading