php / php-src

The PHP Interpreter
https://www.php.net
Other
38.28k stars 7.76k forks source link

PHPT test runner improvements #16623

Open staabm opened 3 weeks ago

staabm commented 3 weeks ago

Description

Recently I have added a optimization to the PHPUnit PHPT testrunner (which was initially copied from run-tests.php) to avoid subprocess overhead when evaluating side-effect free —SKIPIF— conditions.

the implementation is in https://github.com/sebastianbergmann/phpunit/pull/5998 and I blogged about it (and other optimizations) in https://staabm.github.io/2024/10/19/phpunit-codesprint-munich.html

symfony reported a 9% perf improvement of their PHPT tests in https://github.com/symfony/symfony/pull/58680#issuecomment-2441035323

is this something which would be worth adding into run-tests.php ? (It would require some code of my tiny lib https://github.com/staabm/side-effects-detector)

cmb69 commented 3 weeks ago

Very interesting!

In the future try to prevent use of die() or exit() in --SKIPIF-- sections of PHPT tests, and use echo or similar instead, so PHPUnit can run your --SKIPIF-- code in the main process.

That's a problem. We're dying all the time.

I'm also somewhat concerned about --INI-- sections (I think these would be required for --SKIPIF--s), and that the side effect detector might have some false negatives, causing possibly passing tests which should not, or even causing intermittent test failures.

But certainly something worth to investigate more closely.

staabm commented 3 weeks ago

That's a problem. We're dying all the time.

Yeah, thats easy to adjust. See e.g. https://github.com/symfony/symfony/pull/58680

I'm also somewhat concerned about --INI-- sections

In phpunit we always run --SKIPIF-- in a subprocess when --INI-- is defined

cmb69 commented 3 weeks ago

See e.g. symfony/symfony#58680

Yeah, but wouldn't all skipif conditions be processed this way? E.g.

https://github.com/symfony/symfony/pull/58680/files#diff-1db3a2b96846d95d5bef6f8edd8ad7e4dced7b66866a6b206e8f30cb1a1f1c3c.

In this case not a problem, but I think we have a bunch of tests where bailing out after the first failing condition makes sense, e.g.

https://github.com/php/php-src/blob/50a3f019dcb9574342488a75b1cf3681449ce93d/ext/pdo/tests/pdo_001.phpt#L5-L11

If no driver is available, there's no point in including a helper script, and trying to connect.

Anyhow, maybe go ahead and provide a PR which focuses on a single test suite where skipif performance matters, and only run this test suite. Then we could compare the performance.

staabm commented 3 weeks ago

thanks. I created a POC in https://github.com/php/php-src/pull/16664