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.

CommentFileSizeAuthor
#3 3272543-3.patch2.42 KBdanflanagan8
#2 3272543-2.patch2.31 KBdanflanagan8

Comments

danflanagan8 created an issue. See original summary.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Assigned: danflanagan8 » Unassigned
Status: Active » Needs review
StatusFileSize
new 3272543-2.patch2.31 KB

Here's a small one that is a novel scenario. It's a Kernel test that uses classy because the mark.html.twig template is easy to target. The mark template in the system module has absolutely zero markup, which makes selecting...uh...problematic.

So in this case I went he route of creating a test theme in history that provides a copy of the mark.html.twig template from classy. Seems to me to be the smoothest path forward.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
StatusFileSize
new 3272543-3.patch2.42 KB

Oops. That had a booboo. My new test theme wasn't in its own directory. This one is better.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented 
+++ b/core/modules/history/tests/src/Kernel/Views/HistoryTimestampTest.php
@@ -40,10 +40,10 @@ protected function setUp($import_test_views = TRUE): void {
+    // Use history_test_theme because its marker is wrapped in a span so it can
+    // be easily targeted with xpath.
+    \Drupal::service('theme_installer')->install(['history_test_theme']);
+    \Drupal::theme()->setActiveTheme(\Drupal::service('theme.initialization')->initTheme('history_test_theme'));

Should this test just set $defaultTheme = 'history_test_theme'?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

Ah nevermind, its a kernel test

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 3272543-3.patch, failed testing. View results

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed e58aed3e18 to 10.0.x and eabf471d9f to 9.5.x and e095d1ac6c to 9.4.x and 9772963474 to 9.3.x. Thanks!

Backported to 9.3.x to keep tests aligned.

  • alexpott committed e58aed3 on 10.0.x 
    Issue #3272543 by danflanagan8, larowlan: History tests should not rely...

  • alexpott committed eabf471 on 9.5.x 
    Issue #3272543 by danflanagan8, larowlan: History tests should not rely...

  • alexpott committed e095d1a on 9.4.x 
    Issue #3272543 by danflanagan8, larowlan: History tests should not rely...

  • alexpott committed 9772963 on 9.3.x 
    Issue #3272543 by danflanagan8, larowlan: History tests should not rely...

Status: Fixed » Closed (fixed)

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