Blacklisting of object details for some classes/interfaces

[stof]

Currently, Prophecy contains a (outdated) copy of the exporter, altered to avoid dumping the internals of Prophecy objects, to make the export much more readable.
It would be great to support such feature in this library directly, allowing Prophecy to use it instead of copying it (making maintenance much easier). See phpspec/prophecy#296 (comment) for the discussion about it.

See https://github.com/phpspec/prophecy/blob/5c324f4951aca87a2de61b7903b75126516b6386/src/Prophecy/Util/ExportUtil.php#L184 for the current implementation.

[sebastianbergmann]

I would, of course, accept a pull request that implements this in a generic way.

[usox]

Hi

First of all sorry for bringing out this old ticket again.

Since PHPUnit 10 we experience the described issue also when using Mockery as a provider for mock objects (triggered by the event system, the exact reason is described here mockery/mockery#1215).
I invested a little time and checked various ways to selectively prevent/control serialization of objects and have the following suggestion:

The exporter could provide a registry where custom serializers for objects can be stored. Instead of serializing all properties of the object, the exporter would first check if the object implements one of the interfaces stored in the registry. If yes, the serialization would be executed by the stored callable. If not, the usual way would take effect.

Definition of the registry:

/** @var array<class-string, callable(object):array<mixed>> */
private static $typeExporter = [];

/**
 * @param class-string $className
 * @param callable(object):array<mixed> $typeHandler
 */
public static function registerTypeExporter(
  string $className,
  callable $typeHandler
): void {
  self::$typeExporter[$className] = $typeHandler;
}

Lookup within Exporter::toArray()

foreach (self::$typeExporter as $className => $exporter) {
  if ($value instanceof $className) {
    return $exporter($value);
  }
}

Before I set a PR now, I wanted to clarify first whether this way would be accepted in principle.

I also thought about variants with an attribute or interface provided by the exporter, but rejected them because it did not make sense to me to introduce any magic interfaces/attributes that only fulfill a control function.

Thanks

[sebastianbergmann]

I do not really understand what you are trying to report as you talk about performance, exporting objects, the new event system, and Mockery. And you expect me to read into mockery/mockery#1215, forcing me to switch context from "dependency of PHPUnit" (exporter) to third-party test double library for PHPUnit. In mockery/mockery#1215 you write

Mockery's mock objects are serialized and stored in memory

PHPUnit by itself does not know about Mockery and will not do the above for test doubles created by Mockery.

Unless these objects are passed to an assertion method provided by PHPUnit and that assertion fails. Then PHPUnit will export such an object so that it can be part of the "assertion failed" event that is emitted. Early versions of PHPUnit also did that for "assertion succeeded" events, but that was a performance issue (sebastianbergmann/phpunit#5183) and is no longer done.

That being said, my focus is on PHPUnit and its own test double system, for instance. A test double created using PHPUnit's own createStub() or createMock() methods is usually not passed to an assertion method. If that is a common use case for Mockery then I can see how that would be a problem. But only when the assertion fails. When the assertion succeeds, the object passed to the assertion is not exported.

While I am willing to spend time to work on features such as sebastianbergmann/phpunit#5201 to make it easier for the developers of third-party test double libraries to integrate with PHPUnit, I do have to prioritize where I spend time. And right now I do not think that this is something that should be addressed in PHPUnit (or one of its dependencies such as this library). I am sorry, but I simply do not have the time to research how Mockery works, for instance.

I will close this ticket now as I do not think that customization of the export, for instance in the form of excluding details, as proposed by @stof back in 2016 and now by you is outside the scope of this library.

[usox]

Unless these objects are passed to an assertion method provided by PHPUnit and that assertion fails. Then PHPUnit will export such an object so that it can be part of the "assertion failed" event that is emitted. Early versions of PHPUnit also did that for "assertion succeeded" events, but that was a performance issue (sebastianbergmann/phpunit#5183) and is no longer done.

That being said, my focus is on PHPUnit and its own test double system, for instance. A test double created using PHPUnit's own createStub() or createMock() methods is usually not passed to an assertion method. If that is a common use case for Mockery then I can see how that would be a problem. But only when the assertion fails. When the assertion succeeds, the object passed to the assertion is not exported.

That's exactly the point - the export on assertion-success still happens (just through a different mechanism in particular cases) and I assumed that this is exactly how this behavior is desired - so I didn't (re)open the issues in the PHPUnit repository but look for alternative solutions.

[sebastianbergmann]

Can you elaborate on how this happens?

[usox]

Can you elaborate on how this happens?

This happens when you compare not only single objects but lists of objects e.g. with assertSame.

IsIdentical::toString() is called by DispatchingEmitter::testAssertionSucceeded() and - in case of object lists - an export is executed.
If comparing object lists is considered an antipattern - ok, fair enough. One simply has to know that then.

$foo = new class() {
  private string $foo = 'bar';
);

static::assertSame(
  [$foo],
  [$foo]
);

And that's the exporter output:

"is identical to Array &0 (
    0 => class@anonymous�/home/wolverine/src/github/mockery-performance-issue/tests/SomeClassTest.php:9$9b Object #401 (
        'foo' => 'bar'
    )
)"

[sebastianbergmann]

You are right: expected values are exported even for "assertion succeeded" events. But why would you assert that something is identical, for instance, to a test double? This makes no sense to me, sorry.

[davedevelopment]

I think the use of Mockery clouds the problem a little. You don't really need to read the Mockery issue, it mostly provides some context to @usox's specific issue. In general:

• The Exporter::export method can be expensive for complex types
• Previously, PHPUnit did not use the exporter, unless an assertion failed.
• Now PHPUnit calls the exporter even for successful assertions

For larger test suites like @usox's, 100k plus assertions, I'm sure this will cause some issues. I suggested @usox came here because of this comment:

I would, of course, accept a pull request that implements this in a generic way.

Sorry for wasting everyone's time.

[sebastianbergmann]

@davedevelopment No need to apologize, and thank you for your explanation.

I am exploring a solution for this issue in sebastianbergmann/phpunit#5261.