The patch in #3227033-52: Remove Quick Edit from core where we deprecated Core module quickedit showed that enabling deprecated and experimental modules at the same time leads to a test failure in Drupal\Tests\system\Functional\Module\InstallUninstallTest .
- Make a core module deprecated by adding to appropriated lifecyle parameters in its .info.yml file.
- Weep whilst looking at the test failure:
There was 1 error:
1) Drupal\Tests\system\Functional\Module\InstallUninstallTest::testInstallUninstall
Behat\Mink\Exception\ResponseTextException: The text "Are you sure you wish to enable experimental modules?" was not found anywhere in the text of the current page.
/var/www/html/vendor/behat/mink/src/WebAssert.php:785
/var/www/html/vendor/behat/mink/src/WebAssert.php:262
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:898
/var/www/html/core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php:234
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726
Create MR
Review
Commit
Show commandsStart 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, plain diff MR !1701
Comments
Comment #3
spokjeComment #4
spokjeComment #5
spokjeI speak English, I learned it from a book
Comment #6
spokjeComment #7
spokjeComment #8
andypostIs it ok to use the trait in testing class?
Comment #9
spokjeSurely not when it's not working as planned...
Comment #10
daffie commentedAll the code changes look, just 1 question.
Comment #11
spokjeThanks @daffie, tried to give an answer in the MR comments
Comment #12
daffie commentedThe code changes to the test look good to me.
For me it is RTBC.
Comment #13
spokjeThanks @daffie.
Although this is RTBC now, he got me thinking: What we're doing here is making the test very explicit (looking for very specific page text/titles for very specific cases). Since those specific cases are already handled in
NonStableModulesTestwe could go the other way: MakingInstallUninstallTesta lot more general, so more like a catch-all "If there's a confirm page for any lifecycle warning, click the continue button".This would make this test a bit more robust so it might handle the next "new thing" (for example another confirm screen for a specific combo of experimental/deprecated modules) instead of failing straight away when that would appear.
We won't loose any test-coverage because of each and every combo being (hopefully) handled in
NonStableModulesTest.Personally I like that approach, but since this is already RTBC and currently a blocker for deprecating Core modules in D9.4 for removing in D10, this might be for a follow-up or even just my ramblings before having enough caffeine in the morning. (My doctor ordered me to stay away from any keyboard in that case... ;)
I'll leave decisions on this idea to Bigger Brains.
Drops EUR 0.02
Comment #16
catchAgreed with #13, the idea behind the test is 'can we do this without anything blowing up', so it'd be fine to remove the specifics.
However, also agreed that's good follow-up material.
Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!
Comment #18
spokje