Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In #2350823: Use the Classy theme in the Testing profile we made tests use Classy as their theme. Kernel tests should not expect a theme to be set. If a theme is needed, the test should set a default theme.
Proposed resolution
Remove the default theme from KernelTestBase
Remaining tasks
Test patch
User interface changes
None
API changes
None
Beta phase evaluation
Issue category | Task because Drupal works without this change. |
---|---|
Unfrozen changes | Unfrozen because it only changes tests. |
Comment | File | Size | Author |
---|---|---|---|
#6 | 2409811.6.patch | 829 bytes | alexpott |
#1 | use_the_classy_theme_as-2409811-1.patch | 2.67 KB | lauriii |
Comments
Comment #1
lauriiiStart for this patch
Comment #3
Wim LeersI understand we want to be able to test Classy. But let us make sure this is the right solution; it could be argued that if you want to test Classy-based markup, you have to write Kernel tests using Classy.
The guideline has been to test with Stark for a good reason: to test everything in an as bare/minimal as possible form. But I think it was to test with minimal *CSS*, not with minimal *markup*, right? If it was about testing with minimal markup, then the change to Classy would be okay, I think.
That being said: what would still be the use of Stark once we use Classy for the purpose Stark was designed for? It sounds to me like Stark should go away, because Classy supersedes it?
Ideally, we'd get sun's input on this.
Comment #4
davidhernandezStark would never go away, because it will always be the way to receive core module templates that are not being overridden by Classy.
Comment #5
Wim Leers#4: right, right, stupid me! I think the rest of my comment still stands though.
Comment #6
alexpottSo I think the reason for the theme hack in KernelTestBase was because of how drupal_render used to work. It works different now and so is unnecessary.
Any KernelTestBase test that needs something specific from a theme should just install the theme in that test's setUp and set the default theme.
Comment #7
cilefen CreditAttribution: cilefen commentedI think this is the correct title.
Comment #8
cilefen CreditAttribution: cilefen commentedI agree with this issue 100%. It doesn't interfere with tests that extend KernelTestBase but also enable themes, such as CKEditorTest, which enables bartik.
Comment #9
tim.plunkettThe issue summary and title were describing the old approach, I think this new title better explains things?
Comment #10
humansky CreditAttribution: humansky commentedComment #11
cilefen CreditAttribution: cilefen commentedComment #12
almaudoh CreditAttribution: almaudoh commentedBased on patch and title, this is really a simpletest issue.
Comment #13
catchCommitted/pushed to 8.0.x, thanks!