Problem/Motivation
\Drupal\FunctionalTests\Installer\InstallerTestBase tests currently enable the update module. This causes tests to fail - see https://www.drupal.org/pift-ci-job/2409126 - the fail message is:
Exception: Warning: Undefined array key "date"
update_calculate_project_update_status()() (Line: 528)
BUT the update module should not be installed according to \Drupal\Core\Test\FunctionalTestSetupTrait::installParameters(). So what's going on here?
The code in \Drupal\Core\Test\FunctionalTestSetupTrait::installParameters() is:
// form_type_checkboxes_value() requires NULL instead of FALSE values
// for programmatic form submissions to disable a checkbox.
'enable_update_status_module' => NULL,
'enable_update_status_emails' => NULL,
and while this works for a non-interactive install, it does not work in an interactive install. We need to set it to FALSE there. This is due to how we process the form input using \Drupal\Tests\BrowserTestBase::translatePostValues() - it removes NULL values completely :) - it's a feature of http_build_query() - see https://3v4l.org/7NHaH.
So how come this suddenly broke? It appears that this is due to the recent 10.1.x release. It is not a valid release because it could not be packaged - because it only contains the first commit from drupal. See https://www.drupal.org/project/drupal/releases/10.1.x-dev - so the update module can't work out the available release.
Steps to reproduce
Run \Drupal\FunctionalTests\Installer\InstallerTest
on 10.0.x - it will fail - like lots of other tests.
Proposed resolution
Don't install the update module as part of a regular InstallerTestBase test. Override the install parameters to work for an interactive install.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#5 | 3291877-5.patch | 3.26 KB | alexpott |
#5 | 2-5-interdiff.txt | 1.83 KB | alexpott |
#2 | 3291877-2.patch | 2.34 KB | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
alexpottRelatedly some of our docs are out-of-date - see #3291887: Fix outdated references to form_type_checkboxes_value()
Comment #5
alexpottFixing the config tests. The efforts to remove update module status checkbox there where wrong... this is down to
http_build_query()
- see issue summary.We're still going to have test fails but I think we should commit this to have way less noise.
Comment #7
longwaveNo real need to bother with custom assertion messages given that PHPUnit is usually quite clear already.
Wonder if we should assert existence of the things we are removing first, to catch changes to the form in the future?
Comment #8
alexpottRe #7.1 - I think assertTrue / assertFalse should always have messages. If this is TRUE PHPUnit will just say TRUE is not FALSE.
Re #7.2 - The form will error if we try to set a none existing field - this was all due to http_build_query() unexpectedly removing stuff. I don't think we need to worry about that.
Comment #10
alexpottThis will still fail on 10.0.x but it reduces the fails from 70 to 6 so I think this is good approach irrespective of the fix in #3291893: Stub release for 10.1.x breaks 10.0.x tests in HEAD. which will fix the last 6 fails, as well as the other 64.
Comment #11
longwave70 fails down to 6 is a vast improvement.
#8.1 is fine by me, for #7.2 I meant that previously we were unsetting a non-existent
update_status_module
instead ofenable_update_status_module
but the fix here is good enough.Comment #15
catchCommitted/pushed to 10.0.x, cherry-picked to 9.5.x and 9.4.x, thanks!