sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.69k stars 2.2k forks source link

Unable to chain a @dataProvider in method a with a @depends in method b #3093

Closed CC007 closed 6 years ago

CC007 commented 6 years ago
Q A
PHPUnit version 7.0.2
PHP version 7.1.10
Installation Method Composer

Here is a code snippet of what I mean in the title

/**
 * @return array
 */
public function demoDP(): array 
{
    return [[0], [1], [2], [3]];
}

/**
 * @dataProvider demoDP
 * @param int $nr
 * @return int
 */
public function testDemoA(int $nr): int
{
    self::assertTrue($nr >= 0);
    return $nr + 1;
}

/**
 * @depends testDemoA
 * @param int $nr
 */
public function testDemoB(int $nr)
{
    self::assertTrue($nr > 0);
}

I would expect testDemoA to run 4 times: with $nr equal to 0, 1, 2 and 3. I also expect testDemoB to run 4 times: with $nr equal to 1, 2, 3 and 4.

testDemoA works fine, but testDemoB gives the error `Argument 1 passed to ::testDemoB() must be of the type integer, null given

If I don't use the dataprovider for testDemoA, but a hardcoded value for $nr instead, testDemoB works fine, so the problem lies with using @depends in one method that refers to a method that uses a dataprovider.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

CC007 commented 6 years ago

Any chance that this issue will be addressed?

epdenouden commented 6 years ago

@CC007 thanks for submitting this detailed issue! Confirmed by reproducing it in branch https://github.com/epdenouden/phpunit/tree/github-3093-chaining-dataprovider-via-depends

To reproduce:

phpunit tests/Regression/GitHub/3093/Issue3093Test.php

output:

Runtime:       PHP 7.2.2 with Xdebug 2.6.0
Configuration: /Users/ewout/proj/phpunit-new-order/phpunit.xml

....E                                                               5 / 5 (100%)

Time: 92 ms, Memory: 4.00MB

There was 1 error:

1) Issue3093Test::testTwoThatDependsOnTestOne
TypeError: Argument 1 passed to Issue3093Test::testTwoThatDependsOnTestOne() must be of the type integer, null given, called in /Users/ewout/proj/phpunit-new-order/src/Framework/TestCase.php on line 1142

/Users/ewout/proj/phpunit-new-order/tests/Regression/GitHub/3093/Issue3093Test.php:30
/Users/ewout/proj/phpunit-new-order/src/Framework/TestCase.php:1142
/Users/ewout/proj/phpunit-new-order/src/Framework/TestCase.php:840
/Users/ewout/proj/phpunit-new-order/src/Framework/TestResult.php:645
/Users/ewout/proj/phpunit-new-order/src/Framework/TestCase.php:797
/Users/ewout/proj/phpunit-new-order/src/Framework/TestSuite.php:765
/Users/ewout/proj/phpunit-new-order/src/TextUI/TestRunner.php:566
/Users/ewout/proj/phpunit-new-order/src/TextUI/Command.php:200
/Users/ewout/proj/phpunit-new-order/src/TextUI/Command.php:155

ERRORS!
Tests: 5, Assertions: 4, Errors: 1.
epdenouden commented 6 years ago

@CC007 I am currently working on a large pull request for new features for 7.3 and it will take me a few days before I can dive into this.

If possible, do you have a link to the specific part of the PHPUnit manual that details that this should work? I know about chaining @depends and mixing @dataprovider with @depends but I never tried this. Is this a bug or a feature request? :)

CC007 commented 6 years ago

A producer is a test method that yields its unit under test as return value.

A consumer is a test method that depends on one or more producers and their return values.

https://phpunit.de/manual/6.5/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.test-dependencies

I believe that it is stated unconditionally over there. That's why I would expect when the producer returns an int, that the consumer does not receive a null value.

If its not fixed soon, maybe it would be nice to add a notice in all documentation places that mention @dataProvider that its return value cannot be used as input for a test using @depends.

epdenouden commented 6 years ago

@sebastianbergmann Can you clarify how PHPUnit is supposed to work? Going from there I'm sure you have a preference for what to do with this issue.

epdenouden commented 6 years ago
bcremer commented 6 years ago

I stubled upon the same issue. I added a minimal testcase here: https://github.com/sebastianbergmann/phpunit/issues/3168.

epdenouden commented 6 years ago

@bcremer Thank you! I am going to investigate this in depth this weekend.

epdenouden commented 6 years ago

I have done some digging! @bcremer I have found the root cause of your issue and will have a fix end of this weekend. Without boring you too much with the internals of PHPUnit, it has to do with how test names are handled: testName, TestCaseClass::testMethodName, with @dataprovider-details or even just a filename for PHPT tests.

The dependency resolver needs the names it gets to match exactly otherwise I will not see the dependencies. I will fix the name handling and provide extra test coverage.

epdenouden commented 6 years ago

@CC007 Your issue I can only take a proper look at after the dependency resolver matches all current naming schemes.

That said, it looks like this issue is fundamentally different: it concerns how the execution plan for tests is handled. In your example testDemoB depends on testDemoA which uses a dataprovider. If I read you correctly, you expect the final execution order to be:

  1. testDemoB(testDemoA(0)), other words: run testDemoA with data set #0 and feed the result to testDemoB
  2. testDemoB(testDemoA(1))
  3. testDemoB(testDemoA(2))
  4. testDemoB(testDemoA(3))

The main point here is that the iteration over the list comes from the @dataprovider from testDemoA but still influences how often testDemoB is run. When the issue from @bcremer is fixed I can have a deeper look, however I do not think this is trivial to get working.

epdenouden commented 6 years ago

@bcremer and @CC007: thank you both for spending some of your time on these issues with regression tests. Unfortunately there are two separate things going on.

@bcremer: the issue as reported by you was a small bug that was solved by improving the naming scheme used for resolving dependencies. I have openend #3169 with this fix.

@CC007: changing the behaviour you reported would take more work. I am not even sure if PHPUnit was meant to work like that. Perhaps you can get what you want using a different pattern or perhaps make a feature request with a bit more background on the use case.

epdenouden commented 6 years ago

@bcremer The fix has been merged into branch 7.2. Thanks again and hope this helps you out.

vladfilimon commented 2 years ago

Hello, I think there's a mistake about..

The fix that has been merged here is for the related issue, in which there was a problem combining @dataProvider and @depends in the same test, but this issue is not a duplicate of that.

The case mentioned by @CC007 still does not work, where a test testDemoB is dependent on another test with dataProvider testDemoA

CC007 commented 2 years ago

Agreed, but this issue seems to have been reopened in #3498