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
Because of #3083275: [meta] Update tests that rely on Classy to not rely on it anymore and Classy being deprecated in Drupal 9 + removed in Drupal 10,: Tests that aren't specifically testing Classy yet declare $defaultTheme = 'classy'; should be refactored to use Stark as the default theme instead.
Proposed resolution
Change all tests in this module to use Stark as the default theme, and refactor the tests where needed so they continue to function properly.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff_2-10.txt | 1.57 KB | danflanagan8 |
#10 | 3268443-10.patch | 6.79 KB | danflanagan8 |
#2 | 3268443-2.patch | 6.67 KB | danflanagan8 |
Comments
Comment #2
danflanagan8Here's a go at it. This module was interesting in that there were a number of test themes and test modules that use classy. None of them appear to have used classy for any good reason though.
ConfigImportInstallProfileTest
is an interesting case too. Stark was always the base theme, but classy is installed as part of the test because it's testing installing themes. There's nothing special about classy markup that it uses. So I just changed it to Stable.There are at least some tests outside of the config module that use the config test modules/themes though. So let's see if everything is green elsewhere...
Comment #3
danflanagan8Comment #4
danflanagan8Updating priority to Major just like @xjm did for #3248295: Taxonomy tests should not rely on Classy and adding D10 tag.
Comment #5
dwwBefore
After
Review
This seems better than what we had before. Not just any 'odd' row, but the 1st row.
😢.
stable
should be going away, too. Can this bestark
instead? Or at leaststable9
?stark
would definitely be better if that can work.And here.
NW for removing 'stable' from this patch, if possible...
Comment #6
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedTried to address #5.
Comment #7
dwwGreat, thanks! If the testbot is happy, I am. 😉
Comment #9
danflanagan8@ravi.shankar, thanks for jumping in here, but it's important to take a closer look at changes you are making, even if those changes have been suggested by another reviewer.
After applying the patch, there are consecutive lines in a test that read:
Certainly one of these assertion is going to fail since stark cannot be simultaneously installed and not installed. Perhaps Schrödinger's Theme could be, but not Stark. :)
It is important to run tests locally after you have made changes.
Here are the official docs on running phpunit tests locally: https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/running-...
Comment #10
danflanagan8Here's an updated patch that changes from
stable
totest_theme_theme
. Essentially the only important thing is to use a theme that is notstark
.test_theme_theme
is a test theme with no dependencies, so it's a good candidate for something like this.The interdiff is relative to patch #2, since #6 was a step in the wrong direction.
Comment #11
longwaveThe changes all look good - as minimal as can be, the XPath changes look sensible, the changes to ConfigImportInstallProfileTest are as discussed above, and there are no remaining instances of "classy" in
core/modules/config
after applying this patch.Comment #14
lauriiiCommitted 58b15e1 and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!