sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.71k stars 2.2k forks source link

Values used in assertions are exported for Test\AssertionSucceeded events #5183

Closed muzcb closed 1 year ago

muzcb commented 1 year ago

Since PHPUnit 10, all assertion results will be handed to DispatchingEmitter, which in turn will call

(new Exporter)->export($value)

each time, including for succeeded assertions. If the values can be complex object graphs (similar to those described in issue #4965) that will cause the memory usage and runtime to multiply.

Slamdunk commented 1 year ago

I can confirm this can have a huge impact: on my biggest project exporting $value makes the test suite run 3x times slower than stripping away that single line of code from PHPUnit.

sebastianbergmann commented 1 year ago

I have to say that I am surprised that this has such a huge impact, simply because I have never used assertions on object graphs so large that exporting them became noticably expensive.

Slamdunk commented 1 year ago

The most recurring use case for me is where you have a variable that might be an object or null, so the first assertion on the variable is assertNotNull:

$myBigObject = getMyBigObjectOrNull();
self::assertNotNull($myBigObject);
self::assertTrue($myBigObject->active()); // And so on

I have a hundred of snippets like this in my test folder.

sebastianbergmann commented 1 year ago

Thank you, @muzcb and @Slamdunk for bringing this to my attention!

With the changes in c2fcb8f1bec7773ac1d745078ba9eb4570868e11, we no longer export values used in assertions to emit the Test\AssertionSucceeded event. The Test\AssertionSucceeded::value() message has been marked as deprecated, it will be removed in PHPUnit 11. Until then, this method will always return ''.

Slamdunk commented 1 year ago

Thank you very much for your support @sebastianbergmann

sakarikl commented 1 year ago

After this change PhpUnit 10 is actually faster than 9.5 or 9.6 for us.

Before this change test suite took around 8 minutes with PhpUnit 10, now after this it took only under 2 minutes.

with 9.6/9.5 it takes about 3 minutes.

sebastianbergmann commented 1 year ago

@sakarikl That is good to hear, thanks! It is also surprising to hear, as PHPUnit 10 "does more" (event system) than PHPUnit 9, so I have to wonder where this improvement is coming from.

sakarikl commented 1 year ago

Only change for us was using attributes instead of phpdoc (in covers and dataprovider) and changing data providers to statics.

sakarikl commented 1 year ago

oh. and paratest was also updated from 6 to 7. Those were all run with WrapperRunner with 4 processes.