Closed (duplicate)
Project:
Drupal core
Version:
11.x-dev
Component:
phpunit
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
11 Sep 2023 at 10:40 UTC
Updated:
26 Sep 2023 at 08:31 UTC
Jump to comment: Most recent
Problem/Motivation
InstallUninstallTest is a general smoke test that enables then uninstalls every module one by one via the UI, then enables every module via the UI a second time.
It has caught some bugs in the past, but per #2975163: InstallUninstallTest is taking 20 minutes to run it is very slow.
On gitlabCI we have reduced the time taken for a full tes run to 15-17 minutes, I estimate that at least two minutes of that are exclusively due to InstallUninstallTest which appears to take around 12 minutes or so. Because linting and unit tests run first and take about 4-5 minutes, this creates a hard floor for total test run times.
Additionally, this means that any other test optimisations we do can never improve full test run times because InstallUninstallTest will still be running even if everything else was to be done in 10 minutes.
Any bugs this test might find in the future are outweighed by it adding 20-30% to test run times. Because it it is a 'hit and hope' test that is designed to test every possible module in sequence then all together, there is not really a way to split it up either - it would just duplicate other tests then.
Steps to reproduce
Proposed resolution
We have a lot more test coverage since this was added, and modules tend to do less on install than they used to, because a lot is handled by shipped config, so just delete the test. Move the hook help checking to help module which already has a test which install every core module.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3386448
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:
changes
Comments
Comment #2
catchComment #3
catchHelp topics test already installs and uninstalls every module. I have moved the hook help verification from InstallUninstallTest to help module so we don't lose that coverage.
That means the only coverage we lose here is trying to reinstall all modules at once after they've previously been uninstalled.
Comment #4
catchComment #6
andypostComment #7
catchComment #8
smustgrave commentedSeem test failure could be legit.
Comment #9
catchNeed to ignore hidden modules.
Also I opened #3386458: Add GenericModuleTestBase and use it to test general module things which would handle one more bit of lost test coverage - if we add that base class, then every core module could test itself being installed, uninstalled, then installed again. No way to do that efficiently without making it a per-module test though so a bit of a bigger change.
Comment #10
smustgrave commentedAll green!
Comment #11
longwaveAs the finder of a reinstall bug in the past - #2534532: Cannot reinstall Forum after it was previously installed - I am kinda sad to see this go, but if we can replace it with a generic test that can be used by all modules, that would be even better. I suppose it's a shame that data providers cannot be parallelised.
Comment #12
alexpottShould we not add the generic coverage suggested to replace this before / at the same time as removing it?
Comment #13
catchWe could probably merge this with #3386458: Add GenericModuleTestBase and use it to test general module things and remove HelpTopicsSyntaxTest at the same time. However having this MR open made #3386474: [omnibus] Speed up gitlab ci runs easier to work on, so let's instead postpone it on that issue, and close it once it gets done.
Comment #14
kim.pepper#3386458: Add GenericModuleTestBase and use it to test general module things is in so this can go back to active.
Comment #15
catch#3386458: Add GenericModuleTestBase and use it to test general module things removed InstallUninstallTest too in the end, marking this as duplicate.