Php-cs-fixer: php_unit_test_class_requires_covers should only affect methods, not classes

[jnvsor]

You can have coverage marked on every test method in your class, and this fixer will come along and mark the entire class as @coversNothing blowing away your coverage.

You also can't mark the class yourself since that will blow away per-test granularity.

This should only affect methods without @covers in classes without @covers

The PHP version you are using ($ php -v):

7.2.4-1

PHP CS Fixer version you are using ($ php-cs-fixer -V):

2.11.1 Grey Devil

The command you use to run PHP CS Fixer:

php-cs-fixer fix --rules="php_unit_test_class_requires_covers" t

The configuration file you are using, if any:

=> ....................................

If applicable, please provide minimum samples of PHP code (as plain text, not screenshots):

• before running PHP CS Fixer (no changes):

<?php

class RandomTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers \RandomClass::randomMethod
     */
    public function testRandomMethod()
    {
        // ...
    }
}

• with unexpected changes applied when running PHP CS Fixer:

<?php

/**
 * @coversNothing
 */
class RandomTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers \RandomClass::randomMethod
     */
    public function testRandomMethod()
    {
        // ...
    }
}

• with the changes you expected instead:

<?php

class RandomTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers \RandomClass::randomMethod
     */
    public function testRandomMethod()
    {
        // ...
    }
}

[keradus]

Hi, thanks for report !

I do agree current situation may be unexpected, but I would say it's a bug on PHPUnit.

Consider this:

<?php

/**
 * @covers \RandomClass
 */
class RandomTest extends PHPUnit_Framework_TestCase
{
    /**
     * @coversNothing
     */
    public function testRandomMethod()
    {
        // ...
    }

    public function testBar() { /*...*/ }
}

testRandomMethod will not collect coverage, as it's annotated with coversNothing and annotation on method level, as it is considering more precise piece of tests, is taking as prio.
At the same time, testBar will collect coverage, as it will use less precise annotation of whole class.

---

in revert situation of

<?php

/**
 * @coversNothing
 */
class RandomTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers \RandomClass::randomMethod
     */
    public function testRandomMethod()
    {
        // ...
    }

    public function testBar() { /*...*/ }
}

testBar, as there is no method-level annotation, yet there is class-level annotation - shall follow class-level annotation and not collect coverage.
same time testRandomMethod shall treat it's own annotation as prio and collect coverage.

do you want to raise an issue at PHPUnit repo?

[jnvsor]

While I agree that PHPUnit is doing the wrong thing here, I'm pretty sure it's not going to change. PHPUnit has been doing it this way for a 'long long time' (tm) so I doubt they'd break BC on all coverage notations.

Their market share is also enough that this is essentially the 'default' behavior of coverage annotations.

Perhaps a config option is called for, but I think it should still default to affecting methods not classes

[keradus]

big :-1: for affecting all the methods, it gonna be super messy to add it to every single method of test class

Why assuming ahead that PHPUnit will ignore this issue? Simply raise it.
I would say it's kind of big edge cases nobody thoughts before, as covers annotations are designed to not be used together like that (only combination designed to be used together is @coversDefaultClass on class-level + @covers on method-level

[SpacePossum]

I wouldn't bet to much on the long time thingy on PHPUnit, if you make a good case people there will adopt the new and better, we've some examples of that from the past :)

In the meantime, tweaking the fixer is still a nice improvement (IMHO)

[keradus]

Closing, as it was fixed on PHPUnit !
https://github.com/sebastianbergmann/phpunit/pull/3115