phpunit icon indicating copy to clipboard operation
phpunit copied to clipboard

Use `shortenedRecursiveExport()` for data-providers

Open staabm opened this issue 1 year ago • 7 comments

I just realized we already have a shortenedRecursiveExport version of the exporter, we just need to use

so instead of https://github.com/sebastianbergmann/exporter/pull/55 we could just do this single line change

tested this change on roave/BetterReflection:

phpunit 11.0.8

Time: 01:21.576, Memory: 3.79 GB

OK, but some tests were skipped!
Tests: 10383, Assertions: 65007, Skipped: 7.

this PR:

Time: 00:20.042, Memory: 1.04 GB

OK, but some tests were skipped!
Tests: 10383, Assertions: 65007, Skipped: 7.

maybe I did not yet understand that you actually try to make this type of exporter configurable via https://github.com/sebastianbergmann/phpunit/pull/5773 ?

staabm avatar Mar 28 '24 13:03 staabm

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.42%. Comparing base (1b5b302) to head (92227b5).

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           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 28 '24 13:03 codecov[bot]

@sebastianbergmann does this PR depend on the exporter object PR or can it land independently for 11.x ?

staabm avatar Apr 05 '24 17:04 staabm

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.

sebastianbergmann avatar Apr 06 '24 05:04 sebastianbergmann

btw: I am cooking up a alternative to this PR which consists of a fix in the exporter instead

staabm avatar Jun 17 '24 11:06 staabm

@sebastianbergmann I just saw you rebased this PR. do you have any new insights to share?

staabm avatar Jun 17 '24 11:06 staabm

@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.

sebastianbergmann avatar Jun 17 '24 11:06 sebastianbergmann

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 https://github.com/sebastianbergmann/exporter/pull/55

(so looking into ways to shorten the export, when a shortenedRecursiveExport is requested)

e.g. https://github.com/sebastianbergmann/exporter/pull/59

staabm avatar Jun 17 '24 12:06 staabm