- ๐ฌ๐งUnited Kingdom catch
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.
- last update
over 1 year ago Custom Commands Failed - @catch opened merge request.
- last update
over 1 year ago 30,167 pass - last update
over 1 year ago 30,175 pass - ๐ฎ๐นItaly mondrake ๐ฎ๐น
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.
- last update
about 1 year ago 30,152 pass, 12 fail - last update
about 1 year ago 30,176 pass - last update
about 1 year ago 30,176 pass - last update
about 1 year ago 30,176 pass - last update
about 1 year ago 30,152 pass, 12 fail - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 30,161 pass, 11 fail - last update
about 1 year ago 30,170 pass, 4 fail - last update
about 1 year ago 30,171 pass, 3 fail - Status changed to Needs review
about 1 year ago 2:10pm 19 September 2023 - ๐ฌ๐งUnited Kingdom catch
Might be ready for review here. Current threshold set at 60s.
- ๐ฌ๐งUnited Kingdom catch
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.
- last update
about 1 year ago 30,135 pass, 16 fail - Status changed to Needs work
about 1 year ago 2:31pm 19 September 2023 - ๐ฎ๐นItaly mondrake ๐ฎ๐น
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();
- last update
about 1 year ago 30,143 pass, 11 fail - last update
about 1 year ago 30,145 pass, 9 fail - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐คฉ๐คฉ๐คฉ
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.
- last update
about 1 year ago 30,138 pass, 7 fail - ๐ฌ๐งUnited Kingdom catch
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.
- last update
about 1 year ago 29,974 pass, 53 fail - last update
about 1 year ago 30,092 pass, 17 fail - ๐ฌ๐งUnited Kingdom catch
@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.
- last update
about 1 year ago 30,100 pass, 13 fail - ๐ฌ๐งUnited Kingdom catch
Here's the spin-off ๐ Distribute @group #slow tests between test runners and mark more tests RTBC .
- ๐ฌ๐งUnited Kingdom catch
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.
- ๐ฆ๐บAustralia mstrelan
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. - last update
about 1 year ago 30,168 pass - @catch opened merge request.
- last update
about 1 year ago 30,168 pass - ๐ฆ๐บAustralia kim.pepper ๐โโ๏ธ๐ฆ๐บSydney, Australia
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.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
What does `phpunit --order-by=duration` yield?
- ๐ฆ๐บAustralia mstrelan
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.
- ๐ฌ๐งUnited Kingdom catch
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.
- last update
about 1 year ago Patch Failed to Apply - ๐ฌ๐งUnited Kingdom catch
Re-titling this one for the result cache ordering idea.
- ๐ฆ๐บAustralia kim.pepper ๐โโ๏ธ๐ฆ๐บSydney, Australia
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
- ๐ฌ๐งUnited Kingdom catch
After working on ๐ Use artifacts to share the phpstan result and cspell caches from core to MRs Downport , 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 .