- 🇬🇧United Kingdom joachim
This should be resurrected - but should be split into separate issues for Kernel / Browser / JS tests.
- 🇷🇺Russia Chi
It feels if we had fixed this issue in 2012 the Drupal Association could save tens of thousands of dollars on CI environments.
- 🇬🇧United Kingdom joachim
I discussed this with @Alex Pott at DrupalCon Lille a few months ago.
His comment IIRC was along the lines of 'you can usually just comment parts out of the code'.
Here's an example where AFAICT, you can't. I want to test all the possible combinations of 4 factors. So I'm using a cross product setup. To do so with a @dataProvider would mean a kernel test setup for each combination, which is needlessly expensive.
So I'm doing the cross product manually in the test method like this:
foreach ([FALSE, TRUE] as $permission_access) { foreach ([FALSE, TRUE] as $operand_access) { foreach ([FALSE, TRUE] as $operability) { foreach ([FALSE, TRUE] as $reachable) { $this->state->set('test_mocked_control:permission_access', $permission_access ? AccessResult::allowed() : AccessResult::forbidden()); $this->state->set('test_mocked_control:operand_access', $operand_access ? AccessResult::allowed() : AccessResult::forbidden()); $this->state->set('test_mocked_control:operability', $operability); $this->state->set('test_mocked_control:next_state', $reachable ? 'cake' : NULL); // Generate the links. $links = $action_link->getStateActionPlugin()->buildLinkArray($action_link, $user_no_access); // No access always means no links. if (!$permission_access || !$operand_access()) { $this->assertEmpty($links, implode(':', [$permission_access, $operand_access, $operability, $reachable])); }
But this has a number of drawbacks:
- I have to ensure each assertion has a custom message otherwise I will have no idea which set of values caused a failure
- furthermore, I have to use cross product values which are easily stringable, so the message can be easily made without complex code. This harms DX, as it would have been easier to read if the values for the access factors were AccessResult objects -- the checks further down would be easier to read than the boolean check for instance
- I can't run just a single combination for debugging. Or well, I can if I rewrite the cross product code like this so that values can be commented out, but it still requires a lot of faffing about to isolate a single case:$permission_access_values = [ FALSE, TRUE, ]; $operand_access_values = [ FALSE, TRUE, ]; $operability_values = [ FALSE, TRUE, ]; $reachable_values = [ FALSE, TRUE, ]; foreach ($permission_access_values as $permission_access) { foreach ($operand_access_values as $operand_access) { foreach ($operability_values as $operability) { foreach ($reachable_values as $reachable) {
All of this is reinventing the wheel which PHPUnit provides with the data provider system.
- 🇨🇦Canada mgifford Ottawa, Ontario
How often are we running these tests? Seems like a good investment of time to cut the execution time in half, even if it's a matter of allowing us to know earlier that the patch works.
- 🇨🇦Canada mgifford Ottawa, Ontario
It has been over a year since someone looked at it. If @chi's perception of this is right, there are thousands more in savings that have been lost (let alone the environmental impact of running millions of processes on servers as part of this).
@joachim’s 2023 suggestion was to split the problem into separate issues for each test type. I tried to summarize would this would look like using ChatGPT to highlight what this would look like, along with justification and next steps. I'm mostly doing this to nudge this ahead.
Do these new issues sound like good places to begin? Has there been other work to do this already?
⸻
1. Kernel Tests: Optimize setUp() Execution
Goal: Avoid redundant setUp() execution for each method in Kernel tests.
Justification:
• Kernel tests do not boot the full stack and have fewer side effects.
• A shared setUp() could save significant time across large test classes.Suggested New Issue:
“Optimize PHPUnit Kernel test execution by avoiding repeated setUp() calls per method.”
Action Items:
• Benchmark time saved when sharing setUp() across methods.
• Propose an attribute or annotation to allow opting in/out per class.
• Ensure tearDown cleanup logic remains reliable.⸻
2. Browser Tests (Functional + FunctionalJavascript): Improve Test Efficiency
Goal: Investigate safe ways to cache installed sites between test methods in browser-based test classes.
Justification:
• Browser tests are the slowest due to full Drupal installs and HTTP interactions.
• Current design reinstalls Drupal for every method—even within the same class.Suggested New Issue:
“Enable shared Drupal site installations across Browser test methods to reduce test run time.”
Action Items:
• Explore integration with #2900208 for site install caching.
• Identify classes that could safely share a site environment.
• Flag test classes with dependencies on state or side effects to avoid breaking isolation.⸻
3. JavaScript Tests: Evaluate Efficiency Gains from Shared setUp()
Goal: Identify whether JS tests (Nightwatch or others) could benefit from setup sharing, or if they already do.
Justification:
• JS test infrastructure is separate (Node/Nightwatch/WebDriver), but setup redundancy may still exist.
• Even if not using PHPUnit, parallels in redundant test bootstrapping may exist.Suggested New Issue:
“Audit and optimize Nightwatch JS tests for redundant setup executions across test methods.”
Action Items:
• Confirm whether test fixtures or browser states are rebuilt between methods.
• Identify opportunities for fixture reuse or browser session persistence.⸻
Cross-Issue Meta Task: Central Coordination and Tracking
Create a meta issue to link all three above:
“Optimize test performance by limiting redundant setUp() execution across test types.”
⸻
Justification for Splitting
Joachim’s 2023 comment was correct: different test types have different performance bottlenecks, state requirements, and risk profiles. Attempting to solve them all in one issue creates blockers and lack of clarity. Splitting enables:
• Focused benchmarking
• Different contributors to work in parallel
• Faster consensus and review
• Targeted test infrastructure changes - 🇬🇧United Kingdom catch
The chatgpt output isn't helpful.
📌 Improve performance of functional tests by caching Drupal installations Needs work had more recent work on it, but the performance gains there aren't clear.
📌 Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller Active significantly reduced setup costs for both functional and functional js tests (possibly kernel tests too although it would be a smaller proportion of the test time).
The bigger problem here is that a lot of tests rely on starting off with a fresh environment, so individual test methods would have to be rewritten to account for changes made in other methods. This means the new attribute would have to be opt-in, which means no performance gain until it's applied individually to each test and those tests are potentially refactored - it's a lot of work and it's possible to optimize individual tests to have less methods individually without these APIs by doing more in one method.
I did some work on trying to optimize some of the very slowest tests over the past couple of years, and you can see how much work it is per test:
📌 Consolidate two test methods in NumberFieldTest Fixed
📌 Merge test methods in FieldUIRouteTest for better performance Fixed
📌 Consolidate Umami performance tests Fixed
📌 Consolidate ckeditor5's FunctionalJavascript tests Needs review
There are other similar ones around that stalled. So finding the slowest tests and speeding them up is for me a more efficient use of time. The ability to do setup/teardown once for a test could be helpful for some of those, but it's only part of the work - and you can just make test methods protected and call them from one public method to get pretty much the same effect.✨ Add drupalGet() to KernelTestBase Active is likely to have much more impact too.
🌱 [meta] Core test run performance Active isn't up-to-date but has more issue references.
- 🇨🇦Canada mgifford Ottawa, Ontario
Thanks @catch I appreciate the better update!
- 🇷🇺Russia Chi
There are a few critical points that I believe deserve attention.
1. Need for fast tests across all kind of modules
Fast tests are essential not only for Drupal core but also for contributed and custom modules. In my projects, I often avoid using data providers in kernel tests with large datasets because they become prohibitively slow. This limitation is frustrating.
2. Outdated testing System
The current testing framework has seen little improvement since the PHPUnit migration. It has become a tangled mess of gigantic base classes, traits, and poorly documented environment variables. A deep refactoring is necessary, and, I think, we should consider building a new testing framework from scratch. This framework should provide:
• Factories for generating demo content.
• Comprehensive assertion helpers.
• A helper to install Drupal.
• Strict separation between tests and the testing framework. See #3067979: Exclude test files from release packages → .3. A streamlined way to install Drupal with specified parameters.
Obsolete install system blocks progress. While not directly related to testing performance, the outdated installation subsystem is a blocker for improving the testing framework. It still relies on global variables, legacy functions like
install_tasks()
, and patterns dating back to Drupal 6 (or earlier). The new testing framework needs a clean way to spin up isolated Drupal instances. - 🇬🇧United Kingdom catch
@chi yeah I also stopped using data providers for several tests in core like 📌 Stop using a data provider in UserPasswordResetTest Fixed , agreed it's a pain and if you don't pay attention to test runtimes it can add minutes to runtimes.
I don't really see how e.g. KernelTestBase::setUp() can be implemented once per test class though, because it's setting a load of class properties. But if we were going to try to do this, then kernel tests are probably the right place to start because there is often less custom logic in setUp().
For me though I think I would consider making it a new base class like SingleEnvironmentKernelTestBase or similar so we don't have loads of logic branching.
- 🇬🇧United Kingdom catch
To expand on #63. Actual unit tests are generally fine with dataproviders because there's limited things you can do in ::setUp().
Functional and functional js tests there are much less use cases for data providers, and at least for functional tests, they can be converted to kernel tests once ✨ Add drupalGet() to KernelTestBase Active is available.
So if we have a SingleEnvironmentKernelTestBase, and also do ✨ Add drupalGet() to KernelTestBase Active , and then convert quite a lot of core and contrib tests to use those two things, that should give a good idea if there are any gaps left.
- 🇬🇧United Kingdom joachim
> In my projects, I often avoid using data providers in kernel tests with large datasets because they become prohibitively slow.
Agreed, and for me data providers are also the key use case for this issue's functionality.
> It has become a tangled mess of gigantic base classes, traits, and poorly documented environment variables. A deep refactoring is necessary
Yes!!
But I think rebuilding from scratch would be insanely complex - there would be lots of functionality to discover and ensure we don't lose. I think a steady refactoring can work too -- there is an issue for standardizing which folders we put traits in, for example. Standardizing naming and locations of things would be a big first step.
- 🇬🇧United Kingdom catch
Moving classes and traits around should be straightforward now we have 📌 Add a classloader that can handle class moves Active but also that discussion needs its own issue. Probably as a child/related issue of #3067979: Exclude test files from release packages → .
- First commit to issue fork.