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

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because Drupal works without this change.
Unfrozen changes Unfrozen because it only changes tests.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii’s picture

Status: Active » Needs review
FileSize
2.67 KB

Start for this patch

Status: Needs review » Needs work

The last submitted patch, 1: use_the_classy_theme_as-2409811-1.patch, failed testing.

Wim Leers’s picture

I 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.

davidhernandez’s picture

what would still be the use of Stark once we use Classy for the purpose Stark was designed for?

Stark would never go away, because it will always be the way to receive core module templates that are not being overridden by Classy.

Wim Leers’s picture

#4: right, right, stupid me! I think the rest of my comment still stands though.

alexpott’s picture

Status: Needs work » Needs review
FileSize
829 bytes

So 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.

cilefen’s picture

Title: Use the Classy theme as a theme for Kernel tests » Don't use the Classy theme as a theme for Kernel tests

I think this is the correct title.

cilefen’s picture

Status: Needs review » Reviewed & tested by the community

I agree with this issue 100%. It doesn't interfere with tests that extend KernelTestBase but also enable themes, such as CKEditorTest, which enables bartik.

tim.plunkett’s picture

Title: Don't use the Classy theme as a theme for Kernel tests » Kernel tests should explicitly install themes
Issue tags: +Needs issue summary update

The issue summary and title were describing the old approach, I think this new title better explains things?

humansky’s picture

Issue summary: View changes
cilefen’s picture

Category: Bug report » Task
Issue summary: View changes
almaudoh’s picture

Component: theme system » simpletest.module

Based on patch and title, this is really a simpletest issue.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed a34f0db on 8.0.x
    Issue #2409811 by lauriii, alexpott: Kernel tests should explicitly...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.