Problem/Motivation
On my machine, no ramdisk for mysql so a lot slower than gitlabci, but also some of the slowest functional js tests on gitlab.
time ../vendor/bin/phpunit modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php
real 15m21.824s
user 3m31.420s
sys 0m36.982s
time ../vendor/bin/phpunit modules/ckeditor5/tests/src/FunctionalJavascript/SourceEditingTest.php
real 6m46.141s
user 1m57.289s
sys 0m19.853s
Steps to reproduce
Proposed resolution
gitlabci / run-tests.sh concurrency does concurrent test files, not concurrent methods, so splitting these into a base class + individual test classes per method would help a lot.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3388505
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3388505-ckeditor-5s-functional
changes, plain diff MR !4874
Comments
Comment #2
catchComment #3
wim leersYep, functional JS tests are slow.
There's a lot of waiting for things to happen.
The only proposal I have is a very simple one, somewhat reminiscent of #3386458: Add GenericModuleTestBase and use it to test general module things: require all functional JS tests (in core at least) to have only a single test method. That way, they can be maximally parallellized.
Comment #4
wim leersComment #5
catchI think we need to figure out if we want to do #2781123: [experiment] Explore paratest to run our phpunit tests in parallel since that runs by-method I think. If we don't, then we should do one-test-method-per-class for the very slowest tests per #3.
Comment #6
wim leersSounds like a plan!
Comment #7
catchHad a look at paratest, running by method will take a lot of refactoring and it also supports running by class, so even if we were to replace run-tests.sh with it we might not switch to per-method.
I think we should go ahead here, even if we switch to per-method concurrency eventually, it won't do any harm, it'll be just slightly unnecessary work.
I don't think we need to split these into method-per-class, a test class with 10-12 methods each or whatever logical split makes sense should be enough for now - i.e. splitting in two is ~half the overall test time and that might be enough at least to start with.
Comment #9
catchComment #10
smustgrave commentedTHe breakup seems fine. Naming seems good too.
Comment #13
lauriiiCommitted c669e40 and pushed to 11.x. Thanks!