- 🇬🇧United Kingdom catch
Double checked the performance pipeline on 11.x and it's down to 8m30 now with the combination of this and 📌 Consolidate Umami performance tests Fixed , was previously closer to 13 minutes.
https://git.drupalcode.org/project/drupal/-/pipelines/235402
- 🇬🇧United Kingdom catch
OK the commit conflicts are because we haven't done 📌 Resync .gitlab-ci.yml and .gitignore following Yarn 4 in 11.x Needs review yet. Put up an MR there. Once that's in, this should be a clean(er) cherry-pick.
- 🇫🇷France nod_ Lille
- 🇫🇷France nod_ Lille
Committed and pushed f82e35fb1f to 11.x and 62a37e84de to 11.0.x and 3ec2fa91d5 to 10.4.x and 0d664bc8b4 to 10.3.x. Thanks!
- 🇳🇱Netherlands Spokje
Thanks, tests are now green, as expected.
Code changes make sense.RTBC for me.
- 🇬🇧United Kingdom catch
Shouldn't affect the nightwatch test at all but re-ran it just in case.
- 🇳🇱Netherlands Spokje
The nightwatch job failed, not sure if fluke or bad, and can't rerun since MR was created by a core committer.
- 🇬🇧United Kingdom catch
Since this takes a full minute off every core MR pipeline, bumping to major.
Combining this with some other changes, was able to get the minimum time for a full MR pipeline run down to 5m37s https://git.drupalcode.org/project/drupal/-/pipelines/234735
- 🇬🇧United Kingdom catch
This will also help the performance test run finish quicker. At the moment we're building composer and yarn to use the cache in performance tests, but it's much quicker to just run performance install in the actual test job before running the tests. Should save ~90s on those jobs.
- @catch opened merge request.
- 🇬🇧United Kingdom catch
Tried removing the data provider but there's a lot of field/entity state to keep track of if we do that.
So trying the brute force approach of splitting this into two classes - one for the data provider test, one for the other test methods.
- Issue created by @catch
- 🇫🇷France nod_ Lille
Committed and pushed c0e94e3446 to 11.x and 87b8c59a49 to 11.0.x and 9e73b2b33f to 10.4.x and 2328738ae7 to 10.3.x. Thanks!
- 🇫🇷France nod_ Lille
Committed and pushed 27fcad90d9 to 11.x and fd1bcbe5aa to 10.4.x. Thanks!
- 🇬🇧United Kingdom catch
Latest pipeline on the MR finished in 6 minutes and 52 seconds: https://git.drupalcode.org/project/drupal/-/pipelines/233855
- 🇬🇧United Kingdom catch
Just a straightforward rebase.
However I noticed since we added when: manual to the performance job, the job is blocked - needs to allow fail too it looks like.
The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇬🇧United Kingdom catch
Only realised this after working on 📌 Use artifacts to share the phpstan result cache from core to MRs Needs work and seeing just how much time we were spending on spinning up vs doing things. With the combination of the two issues, the newly combined lint stage finishes in around a minute. Once we add cspell/phpcs/eslint cache support, it should be consistently under a minute hopefully.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Yeah this came from how we test on circle ci where spin up is faster, agree on current gitlab setup it's hurting us in overall resection time
-
alexpott →
committed a01c36cc on 11.x
Issue #3463544 by catch: Three more slow functional tests
-
alexpott →
committed a01c36cc on 11.x
-
alexpott →
committed 277352a4 on 11.0.x
Issue #3463544 by catch: Three more slow functional tests (cherry...
-
alexpott →
committed 277352a4 on 11.0.x
-
alexpott →
committed 9e55a96b on 10.4.x
Issue #3463544 by catch: Three more slow functional tests (cherry...
-
alexpott →
committed 9e55a96b on 10.4.x
-
alexpott →
committed b6d1fffe on 10.3.x
Issue #3463544 by catch: Three more slow functional tests (cherry...
-
alexpott →
committed b6d1fffe on 10.3.x
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
Committed and pushed a01c36ccaf to 11.x and 277352a450 to 11.0.x and 9e55a96be2 to 10.4.x and b6d1fffef3 to 10.3.x. Thanks!
- 🇺🇸United States smustgrave
Seems straight forward, nightwatch appears unrelated
Changes looks good: used FilterFormat() API instead of hit the page & update using form submission, verify test changes on local, its output on the basis of time & memory usage as follow:
Before applied MR changes test takes : Time: 00:21.007, Memory: 6.00 MB
After applied MR changes test takes: Time: 00:18.433, Memory: 6.00 MB
observed test resources usage improved & attached screenshot as well, moving to RTBC
- @catch opened merge request.
- Issue created by @catch
- 🇬🇧United Kingdom catch
Self explanatory. Pretty sure this is a very old test from the early days of simpletest.
- @catch opened merge request.
- Issue created by @catch
- @catch opened merge request.
- Issue created by @catch
- @catch opened merge request.
- Issue created by @catch
- 🇬🇧United Kingdom catch
https://git.drupalcode.org/project/drupal/-/jobs/2211630 down to 3m26 seconds from around 4m10s now.
- 🇬🇧United Kingdom catch
SourceEditingTest
completing in about 1/6 the time with a foreach loop instead of a data provider.Before:
../vendor/bin/phpunit modules/ckeditor5/tests/src/FunctionalJavascript/SourceEditingTest.php PHPUnit 10.5.26 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.8 Configuration: /var/www/html/core/phpunit.xml ...................... 22 / 22 (100%) Time: 02:24.161, Memory: 6.00 MB
After:
../vendor/bin/phpunit modules/ckeditor5/tests/src/FunctionalJavascript/SourceEditingTest.php PHPUnit 10.5.26 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.8 Configuration: /var/www/html/core/phpunit.xml .. 2 / 2 (100%) Time: 00:23.510, Memory: 6.00 MB
- 🇬🇧United Kingdom catch
Doesn't reduce the overall job time yet, but finished seventh from last on https://git.drupalcode.org/project/drupal/-/jobs/2210968
- @catch opened merge request.
- 🇬🇧United Kingdom catch
This moves around half the methods in the test to protected methods - mostly the ones that don't change any configuration. On my local ddev it reduces runtime from just under 3 minutes to around 2m 30s, but it's very different on gitlab with concurrency - hopefully can get an idea from the test run.
- Issue created by @catch
- 🇬🇧United Kingdom catch
This gets the second of two FunctionalJavascript jobs down to ~3m20s from about ~4m10s usually and will allow the special performance test runner job to finish a fair bit quicker too - although won't be able to see by how much until after it's committed.
- 🇬🇧United Kingdom catch
Had test failures that only happened on gitlab and not locally, tried various attempts at sleep() to see if it was a race condition all of which failed. What worked was changing a full ::rebuildAll() to just emptying cache bins (i.e. skipping router, twig and container rebuild mostly). Given we have a successful cold cache test where it really does start just after a full rebuilt, it's a pretty minimal change to the test to get the timings down.
- 🇬🇧United Kingdom catch
An example of it not working is in 📌 Consolidate Umami performance tests Fixed which I am currrently struggling with. Passes locally but fails with what looks like a race condition in the chrome driver on gitlab.
- 🇬🇧United Kingdom longwave UK
I don't think there is one, it's kinda case by case - it's suitable when the individual tests don't do much work but also we must ensure that combining them doesn't cause side effects where data from an earlier test leaks into a later one.
- 🇺🇸United States smustgrave
Can I ask a good rule of thumb on when this would be preferred
-
longwave →
committed 24b7c3ff on 11.x
Issue #3463288 by catch: Consolidate test methods in...
-
longwave →
committed 24b7c3ff on 11.x
- 🇬🇧United Kingdom longwave UK
Makes sense to me, as long as it doesn't affect the timings why reinstall over and over. Backported down to 10.3.x as a test-only change.
Committed and pushed 24b7c3ffa3 to 11.x and 64e80c8d84 to 11.0.x and 12448d24bc to 10.4.x and 816ac01be9 to 10.3.x. Thanks!
-
longwave →
committed 64e80c8d on 11.0.x
Issue #3463288 by catch: Consolidate test methods in...
-
longwave →
committed 64e80c8d on 11.0.x
-
longwave →
committed 12448d24 on 10.4.x
Issue #3463288 by catch: Consolidate test methods in...
-
longwave →
committed 12448d24 on 10.4.x
-
longwave →
committed 816ac01b on 10.3.x
Issue #3463288 by catch: Consolidate test methods in...
-
longwave →
committed 816ac01b on 10.3.x
- 🇺🇸United States smustgrave
Oh nice, I like tests like this that call the functions. Wonder if we could make that more standard?
Change here looks good though, no loss of coverage.
- @catch opened merge request.
- Issue created by @catch
- @catch opened merge request.
- 🇬🇧United Kingdom catch
Local ddev timings. Down from nearly 9 minutes to 1m43. On gitlab it currently finishes after about 4m10s so would expect it to finish in somewhere around 1 minute give or take.
Before:
../vendor/bin/phpunit profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php PHPUnit 10.5.26 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.8 Configuration: /var/www/html/core/phpunit.xml ... 3 / 3 (100%) Time: 08:55.340, Memory: 6.00 MB
After:
../vendor/bin/phpunit profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php PHPUnit 10.5.26 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.8 Configuration: /var/www/html/core/phpunit.xml . 1 / 1 (100%) Time: 01:43.987, Memory: 6.00 MB
- Issue created by @catch
- 🇬🇧United Kingdom longwave UK
Committed and pushed afce6da313 to 11.x and e8d874f4db to 11.0.x and b9c8018c12 to 10.4.x and 32075e1a5f to 10.3.x. Thanks!
-
longwave →
committed afce6da3 on 11.x
Issue #3462759 by catch: Try to rebalance kernel tests between gitlab...
-
longwave →
committed afce6da3 on 11.x
-
longwave →
committed e8d874f4 on 11.0.x
Issue #3462759 by catch: Try to rebalance kernel tests between gitlab...
-
longwave →
committed e8d874f4 on 11.0.x
-
longwave →
committed b9c8018c on 10.4.x
Issue #3462759 by catch: Try to rebalance kernel tests between gitlab...
-
longwave →
committed b9c8018c on 10.4.x
-
longwave →
committed 32075e1a on 10.3.x
Issue #3462759 by catch: Try to rebalance kernel tests between gitlab...
-
longwave →
committed 32075e1a on 10.3.x
- 🇬🇧United Kingdom catch
Add 'after' numbers to the issue summary from the latest run: https://git.drupalcode.org/project/drupal/-/pipelines/230965
- 🇬🇧United Kingdom catch
Ah yeah that might have been it, added as a child issue of this one since this includes gitlab tweaks and other things too.
- 🇬🇧United Kingdom joachim
🌱 [META] Convert some tests into Kernel or Unit tests Active is maybe the meta-issue you were thinking of?
- 🇮🇳India ankitv18
Thanks @catch
Looks perfect after the refractor, pipelines are all executed properly ~~ hence marking this one RTBC.