Tests with data providers in parent class don't work anymore

[l-x]

Q	A
PHPUnit version	8.4.0
PHP version	7.3.9
Installation Method	Composer

Summary

Tests classes that contain a test method utilizing a data provider defined in a parent class will produce a warning.

Previous Behavior

No warnings will be shown

Current Behavior

Warning:

The data provider specified for FuTest::testFu is invalid.
Cannot instantiate abstract class AbstractFuTestCase

How to reproduce

abstract class AbstractFuTestCase extends \PHPUnit\Framework\TestCase
{
    public function dataProvider(): array
    {
        return [
            ['fu'],
            ['bar'],
        ];
    }

    /**
     * @dataProvider dataProvider
     */
    public function testFu(string $bar): void
    {
        $this->assertTrue(true);
    }
}

class FuTest extends AbstractFuTestCase
{

}

Just run the FuTest

[pavbis]

Can also confirm the issue. Just updated to 8.4.0.

[sebastianbergmann]

@Ocramius This looks like it could be related to #3836. PHPUnit tries to create an instance of AbstractFuTestCase instead of FuTest to call the data provider method on.

[Ocramius]

@sebastianbergmann basically the patch takes the parent class instead of the child class when considering an annotation, right?

[panvid]

The problem also appear on simple abstraction, see: #3881 (comment)

[sebastianbergmann]

I am not sure whether this is related to annotation parsing. Might as well be related to #3381 (or something else). I cannot look into this right now, will do so over the weekend.

[l-x]

To clarify the problem I reported: It is not necessary to put the two classes in a single file to reproduce this behaviour.

[sebastianbergmann]

@Ocramius According to git bisect, 9309062 is the first "bad commit":

$ cat run.sh               
#!/bin/sh
composer update
./phpunit --fail-on-warning /tmp/FuTest.php

$ git bisect start
$ git bisect good 8.3.5
$ git bisect bad 8.4.0
$ git bisect run ./run.sh

The contents of /tmp/FuTest.php is the example from #3879 (comment).

[gleb-svitelskiy]

Here is the bug 9309062
This is all about reflection. For example:

class Foo
{
    public function baz() {}
}

class Bar extends Foo {}

$reflectionMethod = new \ReflectionMethod('Bar', 'baz');
var_dump($reflectionMethod->getDeclaringClass()->getName());

It will be string(3) "Foo"
So it's not a good idea to get class name from declaring class.
It works before because class name passed to getDataFromDataProviderAnnotation() was received in TestUtil::getProvidedData(), not from ReflectionMethod

[wgevaert]

Should be fixed with the following adjustments:

index 027737c8d..c96971bee 100644
--- a/src/Util/Annotation/DocBlock.php
+++ b/src/Util/Annotation/DocBlock.php
@@ -101,7 +101,7 @@ public static function ofClass(\ReflectionClass $class): self
         );
     }
 
-    public static function ofMethod(\ReflectionMethod $method): self
+    public static function ofMethod(\ReflectionMethod $method, string $className = null): self
     {
         return new self(
             (string) $method->getDocComment(),
@@ -111,7 +111,7 @@ public static function ofMethod(\ReflectionMethod $method): self
             $method->getEndLine(),
             $method->getFileName(),
             $method->getName(),
-            $method->getDeclaringClass()->getName()
+            $className ?? $method->getDeclaringClass()->getName()
         );
     }
 
diff --git a/src/Util/Annotation/Registry.php b/src/Util/Annotation/Registry.php
index 795dbf6a9..fa6cf3641 100644
--- a/src/Util/Annotation/Registry.php
+++ b/src/Util/Annotation/Registry.php
@@ -80,6 +80,6 @@ public function forMethod(string $class, string $method): DocBlock
             );
         }
 
-        return $this->methodDocBlocks[$class][$method] = DocBlock::ofMethod($reflection);
+        return $this->methodDocBlocks[$class][$method] = DocBlock::ofMethod($reflection, $class);
     }
 }

I am quite new to github, and trying to make a pull request gave me remote: Permission to sebastianbergmann/phpunit.git denied to wgevaert.. How can I do this?

All I've done so far was clone the repo, make a local branch and do changes on this branch. I do know how git works, but I am new to github.

[pavbis]

@wgevaert you should fork the repository first.
See https://guides.github.com/activities/forking/ .

[wgevaert]

Thanks! I've made a pull request that should solve this issue

[Ocramius]

Can you re-generate the baseline (see `psalm --help | grep baseline`)? That would suppress the error for that file, and update baseline.xml with known issues.

…

On Mon, Oct 7, 2019, 12:39 wgevaert ***@***.***> wrote: Currently having problems with the following: Adding the class-string to the classNames for psalm gave problems in Framework/DataProviderTestSuite, in the following snippet:``` [$className, $methodName] = \explode('::', $this->getName()); return TestUtil::getSize($className, $methodName); So I made this ``` [$className, $methodName] = \explode('::', $this->getName()); + /** @psalm-suppress ArgumentTypeCoercion */ return TestUtil::getSize($className, $methodName); Now php-Cs-fixer wants it to look like ``` [$className, $methodName] = \explode('::', $this->getName()); - /** @psalm-suppress ArgumentTypeCoercion */ - /* @psalm-suppress ArgumentTypeCoercion */ return TestUtil::getSize($className, $methodName); but then psalm does not recognize the psalm-annotation. What should I do? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3879?email_source=notifications&email_token=AABFVEG65LPX2L7UGWFHZLTQNMGVJA5CNFSM4I5LDSA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAP2F2Y#issuecomment-538944235>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABFVEBWCCW5KO3RKCHAEODQNMGVJANCNFSM4I5LDSAQ> .