-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use shortenedRecursiveExport()
for data-providers
#5774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5774 +/- ##
=========================================
Coverage 92.42% 92.42%
Complexity 6557 6557
=========================================
Files 699 699
Lines 19767 19768 +1
=========================================
+ Hits 18270 18271 +1
Misses 1497 1497 ☔ View full report in Codecov by Sentry. |
@sebastianbergmann does this PR depend on the exporter object PR or can it land independently for 11.x ? |
Not completely exporting data provided by data providers is a BC break and should be opt-in. I am still not sure, though, what the right course of action is here "in the big picture". I hope to find time soon to think about this. |
cccba66
to
92227b5
Compare
btw: I am cooking up a alternative to this PR which consists of a fix in the exporter instead |
@sebastianbergmann I just saw you rebased this PR. do you have any new insights to share? |
I just noticed it's out-of-date and updated it. |
I see thanks. if we are still at a point in which we don't know whether this PR here is acceptable, it might make sense to instead look more into things similar to sebastianbergmann/exporter#55 (so looking into ways to shorten the export, when a |
da21665
to
4dddc8c
Compare
1bb9097
to
5af6639
Compare
ondrej started upgrading PHPUnit to 11.x in PHPStan which leads to us facing this problem again. find details in phpstan/phpstan-src#4057 Changing $testData[] = DataFromDataProvider::from(
$dataSetName,
- Exporter::export($testCase->providedData()),
+ Exporter::shortenedRecursiveExport($testCase->providedData()),
$testCase->dataSetAsStringWithData(),
); brings us back to PHPUnit 10 levels. Is there anything I can do to bring this topic onto the agenda again? |
$testData[] = DataFromDataProvider::from( | ||
$dataSetName, | ||
$exporter->export($testCase->providedData()), | ||
$exporter->shortenedRecursiveExport($providedData), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the exported correctly handle references/recursive arrays?
Thank you 🙏 |
I just realized we already have a
shortenedRecursiveExport
version of the exporter, we just need to useso instead of sebastianbergmann/exporter#55 we could just do this single line change
tested this change on roave/BetterReflection:
phpunit 11.0.8
this PR:
maybe I did not yet understand that you actually try to make this type of exporter configurable via #5773 ?