Add an automated way for slow tests to fail drupalci builds

This would help with 📌 [PP-1] Reduce test pipeline times to around 18 minutes and always report back unit tests Closed: duplicate although we might need to think about distribution of the slow tests when running in parallel.

IMHO we should really really try to avoid implementing listeners at this point… they’re removed in PHPUnit 10. We would be adding an obstacle here.

Might be ready for review here. Current threshold set at 60s.

Once this is green I'll lower the threshold to 30s to see how many tests are affected. We should probably tag any tests that are up to 15s faster than the threshold to allow for variation, so if we leave it at 60s, add #slow to all tests 45s and over. Now that slow tests are distributed between runners a 45s threshold might be OK even.

See #62. IMHO we should not implement listeners any more. They are deprecated since PHPUnit 8 IIRC. Can the same be achieved in tearDown or in the test runner?

For example, in 🌱 [meta] Support PHPUnit 10 in Drupal 11 Active 's MR I try to use Symfony Process component to run PHPUnit, where you can set set a timeout

    $process = new Process($command, \Drupal::root() . "/core", $process_environment_variables);
    $process->setTimeout(300);
    $process->run();

🤩🤩🤩

LOVE THIS! Automating the finding of slow tests. Perfect. That's how it should be. No human should spend time on this.

Once this is green I'll lower the threshold to 30s to see how many tests are affected. We should probably tag any tests that are up to 25% or so faster than the threshold to allow for variation, so if we leave it at 60s, add #slow to all tests 45s and over. Now that slow tests are distributed between runners a 45s threshold might be OK even.

🤯🤩

In today's Drupal core world, this sounds like dark magic.

In 2024's Drupal core world, we'll have forgotten it was ever any different. (I hope.)

For now, just one question: shouldn't we only enforce this limit on GitLab CI, and not by default on local development? Because many local development environments won't be able to match GitLab CI's speed.

For now, just one question: shouldn't we only enforce this limit on GitLab CI, and not by default on local development? Because many local development environments won't be able to match GitLab CI's speed.

See the phpunit.xml.dist hunks - it's configurable in there.

@mondrake that makes sense. I'll split out the run-tests.sh and test changes to a separate issue, then we can continue working on the enforcement here.

Here's the spin-off 📌 Distribute @group #slow tests between test runners and mark more tests RTBC .

If it's possible, the ideal place to do this would be in run-tests.sh itself, based on an environment variable(s) - the advantage of this is that run-tests.sh --all with concurrency runs file-at-a-time so a test with multiple methods or using a dataprovider which wouldn't fail a per-method timer can be several minutes in total. i.e. 12 29 second methods is a six minute test that will be below a 30 second method threshold.

Is it possible to use timings from .phpunit.result.cache? Then we don't need our own listener.

We can run phpunit --cache-result --cache-result-file=.phpunit.result.cache --testsuite=functional. This gives us a json document with the timings of each test method. The major difference here is that it is per method rather than the whole class, so we'd have to sum up the timings for a whole class.

Agree with #74. PHPUnit already caches timing information. We just need a way to read it.

We are building a prototype symfony command that reads this info to do test splitting by file based on timings.

What does `phpunit --order-by=duration` yield?

Re #77:

I ran the following in order to generate the result cache:

./vendor/bin/phpunit -c core/phpunit.xml.dist --testsuite=unit --cache-result --cache-result-file=.phpunit.result.cache

Then order by duration:

$ ./vendor/bin/phpunit -c core/phpunit.xml.dist --testsuite=unit --list-tests --order-by=duration | head -n 10
PHPUnit 9.6.8 by Sebastian Bergmann and contributors.

Available test(s):
 - Drupal\Tests\CSpell\SortTest::testFileExists
 - Drupal\Tests\CSpell\SortTest::testSorted
 - Drupal\Tests\Core\DrupalKernel\DrupalKernelTest::testTrustedHosts#0
 - Drupal\Tests\Core\DrupalKernel\DrupalKernelTest::testTrustedHosts#1
 - Drupal\Tests\Core\DrupalKernel\DrupalKernelTest::testTrustedHosts#2
 - Drupal\Tests\Core\DrupalKernel\DrupalKernelTest::testTrustedHosts#3
 - Drupal\Tests\Core\DrupalKernel\DrupalKernelTest::testTrustedHosts#4

Note that this is the same order with or without the cache result file, maybe I'm missing something. Either way, we'd need to use reflection or something else to determine the file path from the class name.

https://git.drupalcode.org/project/drupal/-/pipelines/23348/test_report (which I've just found this morning) orders test by slowest descending so is good to compare.

Re-titling this one for the result cache ordering idea.

As mentioned in #76 we wrote a little experimental library to read the result cache timing info. See the readme for more info: https://github.com/previousnext/phpunit-splitter

After working on 📌 Try to store the phpstan result cache as an artifact Active , it feels like reliably getting the result cache from parallel jobs will be relatively tricky and hard to debug, it was bad enough with one artifact from one job, but thinking about this issue again gave me an idea for a simpler version which still automates most of what we use @group #slow for: 📌 Order tests by number of public methods to optimize gitlab job times Fixed .

📌 Deprecate TestDiscovery test file scanning, use PHPUnit API instead Active improves the ordering/distribution from run-tests.sh further, so I think this is probably not worth the additional complexity.