Problem/Motivation
See https://git.drupalcode.org/project/drupal/-/jobs/3373087
Drupal\Tests\package_manager\Kernel\SupportedReleaseValidato 10 passes 190s
Drupal\Tests\package_manager\Kernel\ComposerPluginsValidator 33 passes 388s
Also
https://git.drupalcode.org/project/drupal/-/jobs/3373088
Drupal\Tests\package_manager\Kernel\StageBaseTest 42 passes 271s
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3487826
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:
Comments
Comment #2
catchComment #4
catchThis was already flagged as a problem in #3345222: Optimize Composer calls in FixtureManipulator but the issue got won't fixed. I'm taking a different approach to the performance issue here, so might as well leave that issue where it is.
Two approaches we can take:
1. Try to optimize the test itself by reducing the amount of work that needs to be done in setUp/each method.
2. Split the test into multiple classes and rely on gitlab parallelism to reduce overall test run time.
Obviously #1 is preferable but not always easy. Pushed a couple of commits that at least result in a green ComposerPluginsValidatorTest but didn't run the rest of the kernel or functional tests yet.
Comment #5
catchBefore:
After
https://git.drupalcode.org/project/drupal/-/pipelines/350984 is green and finished in 7 minutes and 50 seconds, which is still about 2m50s than our fastest test runs, but also a solid couple of minutes faster than test runs against HEAD at the moment.
Comment #6
catchI think we also need to split the test up further.
We can move the test methods (renamed to doWhatever and protected) and the data providers to a trait, then we can have 2 or 3 test classes which define test methods calling the doWhatever helpers with the appropriate @dataProvider annotations. That will let us have two 140 second tests instead of one 180 second tests and bring things back down to around 5 minutes hopefully.
Also need to add @group #slow. But there's enough changes here already that it would be good to get reviews.
Comment #7
catchNearly there I think, what the MR does now:
1. composer update --lock is slow. There are a few places in the FixtureManipulator that called it. Instead of doing that, added explicit opt-in calls, and then call those from the tests that actually need it. This saved tens of seconds from the slowest tests.
2. Split the three very slowests tests (4-6 minutes per test class) into 2-3 classes each with a subset of the test methods. This lets run-tests.sh/gitlab parallel runners do their job.
3. Adds an extra kernel test job in the pipeline to spread the load a bit more.
We might need to add @group #slow to some tests to balance them between the jobs better, but that needs some good runs on the faster test runners to know where to apply this, and haven't seen one of those in this issue yet,not blocking.
Comment #8
catchWe seem to be getting slower AWS instances at the moment so it's hard to compare against previous best times which were around 5-7 minutes. However, we seem to be down to around 8-9 minutes from 11-13 minutes which also matches what I'm seeing running the individual package_manager tests locally. Bumping to major since it's looking like 10-20% off each pipeline. There might be more to do, but we could do that in another issue if any of these tests continue to be persisently the slowest.
Comment #9
nicxvan commentedLooks good to me. But I think someone from package manager might want to look too.
Comment #10
nicxvan commentedI can't tell if those new tests are relevant, if you can rerun them I reviewed the code more closely and I can RTBC as long as the failure are random.
I'm running into the same issue where I cannot rerun them.
Comment #11
catchRebased on 11.x to get a new run, had to re-run a couple of functional tests, but all green now.
Kernel tests are running very slow in general on gitlab,
even relative to functional tests- wondering if we have an overall performance regression affecting them, however these are particularly long running tests regardless.edit: functional tests were also running at about 5 minutes instead of about 3 minutes for most of the jobs, so not specific to kernel tests. Could be the instances, could be a code change.
Comment #12
nicxvan commentedComment #13
eiriksmJust randomly jumping in here, but from a quick glance it seems `update --lock' (and all other relevant commands) will run with audit enabled? We should probably also pass "--no-audit" or simply disable it altogether (especially for tests): https://getcomposer.org/doc/03-cli.md#composer-no-audit
I believe that could potentially also shave off some CI time
Forgive me if I missed it by glancing on my phone 🤓
Comment #16
alexpottCommitted and pushed fa75872de78 to 11.x and 3cf296532f1 to 11.1.x. Thanks!
@eiriksm let's open an separate issue to have a look into the no audit option.
Comment #17
eiriksm👌👍Opened #3491268: Look into skipping audit of composer operations in package manager tests
Comment #18
catchhttps://git.drupalcode.org/project/drupal/-/pipelines/357428 just finished in 7 minutes 5 seconds so we approaching the old status quo again. package_manager kernel tests are still the slowest kernel tests in that run, let's see where we get to with #3491268: Look into skipping audit of composer operations in package manager tests.