Make exporter configurable

[sebastianbergmann]

PHPUnit uses SebastianBergmann\Exporter\Exporter from sebastian/exporter to export variables as strings, for instance when generating comparison failure messages or when creating immutable value objects representing tests and their data (from data providers) that are passed through the event system to internal as well as third-party subscribers.

In order to allow for the customization of the textual representation of comparison failures, to improve performance (using SebastianBergmann\Exporter\Exporter is expensive when large data structures are involved), and for other use cases it would be nice if SebastianBergmann\Exporter\Exporter could be swapped for a different implementation.

• Introduce PHPUnit\Util\Exporter interface
• Implement PHPUnit\Util\DefaultExporter as the default implementation of PHPUnit\Util\Exporter that simply wraps SebastianBergmann\Exporter\Exporter
• Implement PHPUnit\Util\ExporterFacade that uses PHPUnit\Util\DefaultExporter by default, but can be configured to use an alternative implementation of PHPUnit\Util\Exporter
• Implement PHPUnit\Util\ExporterChain for chaining PHPUnit\Util\Exporter implementations
• Use ExporterFacade::instance()->export() instead of SebastianBergmann\Exporter\Exporter::export()
• Make alternative implementations of PHPUnit\Util\Exporter configurable via PHPUnit's XML configuration file
• Explore whether SebastianBergmann\Exporter\Exporter::shortenedRecursiveExport() and SebastianBergmann\Exporter\Exporter::shortenedExport should also be wrapped
• Explore whether the usage of SebastianBergmann\Exporter\Exporter in sebastian/comparator should also be configurable

[codecov]

Codecov Report

Attention: Patch coverage is 65.38462% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 90.06%. Comparing base (1303379) to head (aeb0007).

Files	Patch %	Lines
src/Util/Exporter/ExporterChain.php	0.00%	13 Missing ⚠️
src/Util/Exporter/DefaultExporter.php	66.66%	2 Missing ⚠️
src/Util/Exporter/ExporterFacade.php	75.00%	2 Missing ⚠️
src/Framework/Constraint/Constraint.php	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5773      +/-   ##
============================================
- Coverage     90.14%   90.06%   -0.08%     
- Complexity     6585     6598      +13     
============================================
  Files           693      696       +3     
  Lines         19943    19966      +23     
============================================
+ Hits          17977    17983       +6     
- Misses         1966     1983      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[staabm]

while I agree that a configurable exporter can have benefit for specific use-cases, I would expect the "default exporter" to be fast enough, so I don't need to structure my unit test arround a "limitation" of phpunit, that objects/arrays in data-providers are not efficient enough.

no offense on this end - just my point of view.

[sebastianbergmann]

no offense on this end - just my point of view

None taken.

I would expect the "default exporter" to be fast enough

Sure!

As I said before, I am not doing this for performance. sebastianbergmann/exporter#55 is likely a more appropriate solution for performance issues.

[BladeMF]

Isn't it better architecture here to have a Factory where one can add multiple exporters (in a config file, probably? or a bootstrap file?) that are called for each type? E.g.

class ExporterFactory 
{
  public function addExporter(ExporterInterface $exporter) { ... }
  
  public function getExporterForValue(mixed $value) {
    foreach($this->exporters as $exporter) {
      if($exporter->supports($value)) return $exporter;
    }
    return $this->defaultExporter;
  }
}

That way doctrine can create their own exporter, I can make my own, etc.

[sebastianbergmann]

@BladeMF I wanted to keep this as simple as possible, but will ponder your suggestion.

[BladeMF]

In this implementation I have two questions:

• How will the use method be called? A configuration value (seems best, no)?
• Will the custom exporter have access to the defaul timplementation to defer some of the work to?

[sebastianbergmann]

How will the use method be called? A configuration value (seems best, no)?

That is what I mean by "Make alternative implementation of PHPUnit\Util\Exporter configurable via PHPUnit's XML configuration file" in #5773 (comment).

• Will the custom exporter have access to the defaul timplementation to defer some of the work to?

Currently not, but I will ponder that.

[BladeMF]

Thanks for looking into it.

[sebastianbergmann]

Will the custom exporter have access to the defaul timplementation to defer some of the work to?

I think the best way would be for custom implementations of PHPUnit\Util\Exporter to create their own instance of SebastianBergmann\Exporter\Exporter when needed.

[sebastianbergmann]

where one can add multiple exporters

I just created PHPUnit\Util\ExporterChain as an implementation of PHPUnit\Util\Exporter to allow for multiple implementations to be used. The first one to return true for PHPUnit\Util\ExporterChain::handles() will be used. The default implementation of PHPUnit\Util\Exporter will be used if no other implementation handles the value to be exported.

[sebastianbergmann]

I slept on it and now think that SebastianBergmann\Exporter\Exporter should be changed to allow for customizable (and chainable) handling of objects. I do not think that the exporting of scalar values needs to be customizable and the same goes for the recursive processing of arrays.

[staabm]

Agree. I also wonder whether all callsites of ->export should be configurable in the same way..? Do all call-sites work if the export shortens the information ? Or do some callsites exist which would break and always need the full export?

[sebastianbergmann]

Here is a proof-of-concept change for sebastian/exporter: sebastianbergmann/exporter#56

[BladeMF]

I do not think that the exporting of scalar values needs to be customizable and the same goes for the recursive processing of arrays.

Sounds reasonable.