Make export of objects customizable

[BladeMF]

The Exporter that provides text representation of arguments and assertions. There are, however, really big objects which are not practical to be displayed (e.g. Doctrine entities, Phpstan test case arguments). Since the actual Exporter class is not injected and is not an interface, its behaviour cannot be modified. If I have a test that is asserting equality between two Doctrine entities and it fails, the displayed error is hundreds of screens long and not practical.

It should be possible to decorate the default export and modify its behaviour without that affecting anything in Phpunit. That way one can create plugins that print certain large objects properly - doctrine and phpstan included.

[sebastianbergmann]

Configurable on the test suite level (in the XML configuration file) or on the test level (using an attribute)?

[BladeMF]

Ideally it should be on project level, e.g. install a plugin to phpunit.

[BladeMF]

My current problems, to be precise are these:

1. I cannot compare Doctirne entities, because if the test fails, their textual representation is huge and unreadable.
2. The test cases for my Phpstan extension, mentioned here are now taking 5 minutes or longer, because of the exporting of the arguments of the data providers before the test runs.

[ondrejmirtes]

Similar problem that happened previously: #5524

[sebastianbergmann]

Correct me if I'm wrong, but what you are asking for can already be achieved using a custom comparator:

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;
use SebastianBergmann\Comparator\Comparator;
use SebastianBergmann\Comparator\ComparisonFailure;

final class C
{
    private array $data;
    
    public function __construct(array $data)
    {
        $this->data = $data;
    }

    public function largeDataStructure(): array
    {
        return $this->data;
    }
}

final class Test extends TestCase
{
    public function testOne(): void
    {
        $this->registerComparator(
            new class extends Comparator
            {
                public function accepts(mixed $expected, mixed $actual): bool
                {
                    return true;
                }

                public function assertEquals(mixed $expected, mixed $actual, float $delta = 0.0, bool $canonicalize = false, bool $ignoreCase = false): void
                {
                    $expectedAsString = 'foo';
                    $actualAsString   = 'bar';

                    throw new ComparisonFailure(
                        $expected,
                        $actual,
                        $expectedAsString,
                        $actualAsString,
                        'Failed asserting that ...',
                    );
                }
            }
        );

        $a = new C([1, 2, 3]);
        $b = new C([4, 5, 6]);

        $this->assertEquals($a, $b);
    }
}

Of course, you can also "simplify" $expected and $actual before passing them to ComparisonFailure (from where PHPUnit will pass them to Exporter, IIRC).

[BladeMF]

Correct me if I'm wrong, but what you are asking for can already be achieved using a custom comparator:

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;
use SebastianBergmann\Comparator\Comparator;
use SebastianBergmann\Comparator\ComparisonFailure;

final class C
{
    private array $data;
    
    public function __construct(array $data)
    {
        $this->data = $data;
    }

    public function largeDataStructure(): array
    {
        return $this->data;
    }
}

final class Test extends TestCase
{
    public function testOne(): void
    {
        $this->registerComparator(
            new class extends Comparator
            {
                public function accepts(mixed $expected, mixed $actual): bool
                {
                    return true;
                }

                public function assertEquals(mixed $expected, mixed $actual, float $delta = 0.0, bool $canonicalize = false, bool $ignoreCase = false): void
                {
                    $expectedAsString = 'foo';
                    $actualAsString   = 'bar';

                    throw new ComparisonFailure(
                        $expected,
                        $actual,
                        $expectedAsString,
                        $actualAsString,
                        'Failed asserting that ...',
                    );
                }
            }
        );

        $a = new C([1, 2, 3]);
        $b = new C([4, 5, 6]);

        $this->assertEquals($a, $b);
    }
}

Of course, you can also "simplify" $expected and $actual before passing them to ComparisonFailure (from where PHPUnit will pass them to Exporter, IIRC).

Actually I think I have considered that solution (I've looked extensively through the source code since 9.5), but found something lacking.

What if I wan't to compare, throughout my project, complex data structures (arrays, objects, etc.) which might or might not contain things I'd want to simplify on various levels thoughout the given structure. This means I need to a) recreate the original equality comparer from scratch, or b) somehow use it for everything but the complicated objects, which gets incredibly difficult. Am I making any sense to you?

[sebastianbergmann]

Am I making any sense to you?

Not really, sorry.

Maybe this helps: registerComparator() only registers an additional comparator, it does not replace any of the existing ones. assertEquals() will try the registered comparators, custom and built-in ones, one after the other. The first one that says "I can compare this!" (by returning true from accepts() will be used to perform the assertion (by calling its assertEquals() method).

[BladeMF]

I get your point. Lets consider an example.

Say that I want a custom string representation of the class Apple, because the default one is too big. Then, I can register a comparator that expects two instances of the Apple class. So far, so good. This means that this code is fine and dandy:

$a = new Apple(3);
$b = new Apple(5);
self::assertEquals($a, $b);

Imagine then I have this code:

self::assertEquals(
    [
        'key1' => 10,
        'key2' => new Apple(5),
        'key3' => [,
             'key3.1' => 'string 1',
             'key3.2' => new Apple(7),
        ],
    ],
    getDataStructure(),
);

Will then my comparer be called for keys key2 and key3.2?

[sebastianbergmann]

No.

[BladeMF]

See my point? If I compare data structures that contain the objects I need to use my comparer on, it won't be called and the default textual representation is used. So using a custom comparer has a very limited application even if it is a workaround in these cases.

But this gets us back to the original point - we don't really care about the comparing. What we care about is the textual representation.

P.S. It would be nice to be able to have a custom comparer that works in all cases though. Say, for example, if the equality comparer works on type level - for every value compared get the relevant comparer and when there are containers (arrays/colletions of values or objects with properties), the relevant container is called for each property/key type. But that's a different subject.

P.P.S. I'd be happy to contribute when time permits if you're open to the idea we're discussing.

[sebastianbergmann]

This is a complex topic that I am uncomfortable with delegating and I will look into this when I find the time.

[BladeMF]

I completely understand that. Given that you probably don't have enough time, what if I told you it would be completely okay if you decided to completely throw away my work at the end? I can explain my idea of doing it, probably do some prototyping and I won't mind if you decide my work sucks at the end. (Imagine how bad I need this 🤣😂🤣)

[staabm]

as I stumbled over this today when running the test-suite of roave/BetterReflection:

Exporter->resursiveExporter consumes a huge portion of the required memory and the same is true for CPU time:

[sebastianbergmann]

I know, @staabm. It is not easy to tackle, though.

As far as I understand it, though, the most problematic Exporter usage we have is that for data coming from a data provider into a test. And this is best addressed outside of PHPUnit by not generating large object graphs that are expensive to export inside data providers in the first place. IMO, data providers should, especially when the resulting object graphs are expensive, provide scalar input to the test method where that input is used to create the expensive data structures.

That being said, I am willing to make Exporter configurable, but improving performance is not my primary motivation for that. It's a nice bonus for sure, though.

[staabm]

As far as I understand it, though, the most problematic Exporter usage we have is that for data coming from a data provider into a test.

I also think thats the main problem. is this data only used for showing it to the human looking at the phpunit result output?
if so, we could think about e.g. limiting the depth how far this data is exported, because a deeply nested structure is also not very useful for a human sitting infront of the screen

[sebastianbergmann]

I am working on a proof-of-concept that may already be "good enough", hope to have it in a shareable state soon.

[ondrejmirtes]

At least I fixed the performance problem in PHPStan by passing around plain strings instead: phpstan/phpstan-src@da87a65

[sebastianbergmann]

I think this could be as simple as #5773.

[staabm]

@BladeMF was the primary motivation for this issue really that you want to implement exporter methods for certain objects or is your actual primary concern only in the inefficient performance of the out-of-the-box export?

I am wondering whether we really need the configurable export, or whether just a more efficient export would already cover all your concerns.

[BladeMF]

@staabm I can try and outline my problems chronologically:

1. When a test fails when comparing two large objects (e.g. Doctirne entities + comparing if they are the same object) and the test fails, then the printout of the error is 1.3G and is practically unreadable. You can't even know what test failed because it is lost in the endless printing. That is solved either by:
   1.1. Making a smaller output. The drawback being that what if I need 4 levels deep, but not all properties (e.g. ignore Doctrine's entity manager, otherwise I want all properties and all levels because I am comparing the whole data structure and what if the difference is on level 4)? This solution though is a very good default setting.
   1.2. Making a configurable exporter. That way Doctrine can provide their own meaningful exporter. Currently they have the same provider for the Symfony's dump function.
2. Providing very large objects from a data provider. Here I am inclined to agree that there should be a better test design, alghough I have the suspicion that there exist cases where that cannot be avoided.
   2.1. Exporting the text representation in this case should not really be more than a level or two. Even better, do not export at all, but rely on the entry's description.

[sebastianbergmann]

@BladeMF Thank you for explaining your use case(s).

What do you think about sebastianbergmann/exporter#56? The idea is similar to #5773, but instead of completely swapping out SebastianBergmann\Exporter\Exporter this would be limited to customizing how SebastianBergmann\Exporter\Exporter handles objects. One or more custom implementations of SebastianBergmann\Exporter\ObjectExporter could be configured in PHPUnit's XML configuration file.

[BladeMF]

Yes, I really like that implementation. I left just one comment explaining what an example implementation of my exporter would look like.
I did not look at the shortened version though.

[BladeMF]

@sebastianbergmann BTW, thank you for your work. If not for Phpunit we who want to write proper code in PHP would not have any tools.