-
Since #2188661: Extension System, Part II: ExtensionDiscovery, test extensions are no longer discovered at runtime.
-
#2198713: Add a settings.php variable to allow to discover/enable test extensions at regular runtime adds a settings.php setting to allow developers to discover test extensions at regular runtime (for debugging).
-
→ We can safely remove the
hidden: trueproperty from all test extension.info.ymlfiles, which in turn allows developers to simply enable a test module on the regular Modules page when the setting is enabled.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 2209337-17-remove-hidden.patch | 70.97 KB | tstoeckler |
Comments
Comment #1
sunComment #2
tstoecklerHmm... if we want to do this, I'm wondering whether we shouldn't expose the setting introduced in #2198713: Add a settings.php variable to allow to discover/enable test extensions at regular runtime in the UI, i.e. turn it into config instead?! I would personally find that very convenient, but I don't know how others feel.
In any case, +1.
Comment #3
tstoecklerHere we go. The only hidden:true that remains in core is testing.info.yml from Testing profile. I verified that indeed no test modules are shown on admin/modules.
Comment #5
tstoecklerThe locale_test_translation module needs to be hidden for the test to work.
Comment #7
tstoecklerlocale_test itself needs to be hidden, as well. That's a bit weird, but hey...
Comment #8
tstoecklerRolling patches is hard... :-) (Interdiff is correct, though)
Comment #9
tstoecklerThis needed a re-roll. Anyone want to RTBC?
The interdiff is for all the test modules that have been added in the meantime.
Comment #11
tstoecklerOops I inadvertantly reverted the interdiffs from 5 and 7. This should be green again.
Comment #12
sunFWIW:
This would allow us to make
InstallerTestBaseuse the Testing profile by default instead of Minimal profile, which would speed up the core test suite.InstallerTestBasetests the interactive installer. Because the Testing profile is hidden, it cannot be selected in the UI. Therefore, it has to select Minimal profile.Comment #13
tstoecklerRe @sun: Is that an RTBC? :-)
Comment #14
sun11: 2209337-11-remove-hidden.patch queued for re-testing.
Comment #15
sunComment #17
tstoecklerHmm... that one passes locally.
Here's a re-roll with two more modules that have been introduced in the meantime.
Comment #18
tstoecklerPer #12 marking normal.
Comment #19
tstoecklerRTBC per #15.
Comment #20
alexpottJust pondering if anyone has use hidden for anything other than test modules?
Comment #21
berdirYep, I do, but as I understand it we're not removing the ability to use hidden but just remove it from test modules? I think we should keep the ability to use hidden.
One example use case is that we started providing demo/set-up modules for simplytest.me for contrib modules, which provides support for enabling modules automatically. We don't want those to be visible because they mess with your site configuration and you shouldn't enable them on a normal site.
Comment #22
alexpottSo now I'm wondering if we need to add a test :)
Comment #23
sunThere is indeed no explicit test coverage for the (purely UI-facing) 'hidden' property in System module's tests yet.
Note: The property really has effects on the Extend/Modules page only. The extension system, module system,
ModuleHandlerdoesn't care.However, that's off-topic/out-of-scope for this issue. Adding test coverage for the expected 'hidden' behavior sounds like a good idea to me, but this issue/patch does not touch any code that would be related to that behavior.
Comment #24
sunFWIW, I was mistaken in #12 — since this patch retains the hidden property for the Testing profile, the interactive installer tests still won't be able to use it (because they cannot select it in the UI). But we can address that differently.
Regardless of that, I still think this change makes sense and simplifies things. There's no need for testing modules to be hidden, if they're not discovered in the first place. And when the system is told to discover them, it would be useful if they'd actually appear in the UI, too.
Double-DX-win in my book.
Comment #25
sunCome to think of it, created #2260321: Use Testing profile instead of Minimal profile in interactive installer tests by default
Comment #26
catchCommitted/pushed to 8.x, thanks!