Skip data provider build when requirements are not satisfied#5957
Conversation
039cc5c to
f872c06
Compare
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.
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.
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.