As we found out in #3263618-21: Deprecate HAL module, the tests in core/tests/Drupal/KernelTests/Config/DefaultConfigTest.php trigger a deprecation notice whenever a Core module with config is deprecated.
This happens since in this test all (optional) config for core modules and themes are installed and re-saved to check for differences.
This requires installing/enabling the modules/themes to which those config belong, which in turn triggers the deprecation notice when a module/theme has lifecycle: deprecated in its info.yml file.
Do this by marking the test skipped before the install/enable call.
- Enable a deprecated Core module/theme
- Run the tests in core/tests/Drupal/KernelTests/Config/DefaultConfigTest.php
- Curl up in fetal position and sob endlessly.
Exclude modules/themes with lifecycle: deprecated in their info.yml file from being installed in Drupal\KernelTests\Config\DefaultConfigTest::assertExtensionConfig().
The down side of this is that config belonging to a deprecated Core module/theme won't be tested for correctness any more.
This is mitigated by the fact that, since it being a Core module/theme, the config belonging to it would be tested on every run until the Core module/theme is marked as deprecated.
The likely-hood of changes on config belonging to an already deprecated module/theme (and thus not being tested any more after this change) seems very unlikely.
#FamousLastWords
- MR
- Review
- Commit
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3265546-fail-8.patch | 3.64 KB | spokje |
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
Comments
Comment #2
spokjeMarked this as a Major priority, since it will block deprecating Core Modules, which in turn blocks removing Core modules, which (at least to my knowledge) have Priority Major by default.
Comment #3
spokjeComment #4
spokjeThis is also valid for Core themes.
Comment #5
catchI think this is the right approach. If we really want to keep all the generic test coverage for modules/themes after they're deprecated, I think we'd need to revert the deprecation error when they're enabled - the whole point is to find tests with dependencies, and core has several of these catch-all tests. So better to exclude, and if it comes around to bite us at some point, we can revisit.
Comment #6
spokjeThanks @catch.
Failing test patch to get the party started (and to see if I can get away with _not_ creating Yet Another Test Theme/Module).
Comment #7
spokje*cough*
Failing test patch to get the party started (and to see if I can get away with _not_ creating Yet Another Test Theme/Module).
*cough*
Comment #8
spokjeHmmm, when it fails it pours...
Let's try that (yet) again
Comment #10
spokjeComment #11
spokjeComment #12
quietone commentedOh and what happens in the extension is obsolete, will a deprecation error also occur?
Comment #13
quietone commentedAlso a blocker
Comment #14
spokjeThanks @quietone for the review.
Made changes/left comments in the MR, resolved all threads, back to NR.
Comment #15
bbrala@Spokje you missed #12 I think.
I was gonna just see how it behaves, but my d10 install script is broken today, sorry.
Comment #16
catchIf an extension is obsolete, it can't be installed because we throw an exception. But also an obsolete module can never have any configuration (if it did it wouldn't be obsolete), and this test is only installing modules with configuration, so it should be fine due to that.
Comment #17
bbralaLooking good! Nice work.
Comment #20
catchCommitted/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!
Comment #21
catch