Skip data provider build when requirements are not satisfied #5957
Conversation
MauricioFauth
force-pushed
the
data-provider-requires-phpunit
branch
2 times, most recently
from
039cc5c
to
f872c06
Compare
| 1) Method PHPUnit\TestFixture\InvalidRequirementsTest::testInvalidVersionConstraint is annotated using an invalid version requirement: Version constraint ~~9.0 is not supported. | ||
| 1) Class PHPUnit\TestFixture\InvalidRequirementsTest is annotated using an invalid version requirement: Version constraint ~~9.0 is not supported. | ||
|
|
||
| 2) Class PHPUnit\TestFixture\InvalidRequirementsTest is annotated using an invalid version requirement: Version constraint ~~9.0 is not supported. | ||
| 2) Method PHPUnit\TestFixture\InvalidRequirementsTest::testInvalidVersionConstraint is annotated using an invalid version requirement: Version constraint ~~9.0 is not supported. |
There was a problem hiding this comment.
I'm not sure, but I think this changed because the class metadata are now cached before the method metadata as PHPUnit\Metadata\Api\Requirements::requirementsNotSatisfiedFor() are called before PHPUnit\Metadata\Api\DataProvider::providedData().
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 10.5 #5957 +/- ##
=========================================
Coverage 94.14% 94.14%
- Complexity 6446 6447 +1
=========================================
Files 678 678
Lines 19455 19455
=========================================
Hits 18316 18316
Misses 1139 1139 ☔ View full report in Codecov by Sentry. |
| ); | ||
| $data = null; | ||
|
|
||
| if ((new Requirements)->requirementsNotSatisfiedFor($className, $methodName) === []) { |
There was a problem hiding this comment.
I wonder whether it would make sense to not build the whole test and e.g. return early (or handle this case further up the call chain), in case requirements will not be satisfied
atm this only prevents building the dataprovider
There was a problem hiding this comment.
and would probably solve #4391 as well
There was a problem hiding this comment.
It's not possible to skip the test here, because the data provider build happens earlier when building the test suite, since a test with a data provider is a test suite itself. What this fix does is ignore the data provider and treat the test as a single test, and not a test suite.
The test will be skipped later when running the test suite. Creating a separate process happens just before running the test, but after the test suite build.
MauricioFauth
force-pushed
the
data-provider-requires-phpunit
branch
from
961ef5a
to
29dc63d
Compare
|
btw: we recently reworked some of our testing in PHPStan because some data-provider building was super slow, as we did a lot of stuff there. having them only built when the tests really ran, might have a positive perf impact. (just wanted to leave another argument why this PR is a good thing) |
There is no reason to build the data provider if the test will be skipped anyway.
This also makes it easier to support a wide range of PHPUnit versions, as a data provider method that is incompatible with newer PHPUnit versions will not cause an error.