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

CommentFileSizeAuthor
#4 interdiff_2-4.txt767 bytesdanflanagan8
#4 3269267-4.patch2.4 KBdanflanagan8
#2 3269267-2.patch2.5 KBdanflanagan8

Comments

danflanagan8 created an issue. See original summary.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Active » Needs review
StatusFileSize
new 3269267-2.patch2.5 KB

Just two tests here, but they both take ages to run on my local machine! About 5 minutes each.

There's a nice win here getting to refactor such that we use the AssertBreadcrumbsTrait, which was already made to work with stark in #3248309: AssertBreadcrumbTrait should not rely on Classy.

Status: Needs review » Needs work

The last submitted patch, 2: 3269267-2.patch, failed testing. View results

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Needs work » Needs review
StatusFileSize
new 3269267-4.patch2.4 KB
new interdiff_2-4.txt767 bytes

Hmmm, that passed locally. Looks like maybe I need to remove the backslashes when building the trail when asserting the breadcrumbs. Here's that. Better pass!

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Assigned: danflanagan8 » Unassigned
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Priority: Normal » Major
Issue tags: +Drupal 10

Updating priority to Major just like @xjm did for #3248295: Taxonomy tests should not rely on Classy and adding D10 tag.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

Before

% egrep -ir classy core/modules/dblog/tests
core/modules/dblog/tests/src/Functional/DbLogTest.php:  protected $defaultTheme = 'classy';
core/modules/dblog/tests/src/Functional/DbLogViewsTest.php:  protected $defaultTheme = 'classy';

After

% egrep -ir classy core/modules/dblog/tests
%

Review

Patch looks great:

  1. +++ b/core/modules/dblog/tests/src/Functional/DbLogTest.php
@@ -366,9 +368,13 @@ private function verifyBreadcrumbs() {
+    $this->assertBreadcrumb('admin/reports/dblog/event/' . $wid, $trail);

    Love it! Way better. Nice trait! Thanks to whoever(s) added that. 😉

  2. +++ b/core/modules/dblog/tests/src/Functional/DbLogViewsTest.php
@@ -31,13 +31,13 @@ class DbLogViewsTest extends DbLogTest {
+    return $this->xpath('.//div[contains(@class, "views-element-container")]//table/tbody/tr');

    ✔️ looks good.

Whereas:

  1. No "classy" in any dblog tests remaining.
  2. Testbot is happy.
  3. Nothing to complain about in the patch.

Therefore be it resolved: RTBC. 😉

Thanks again @danflanagan8!

  • catch committed f67f0cb on 10.0.x 
    Issue #3269267 by danflanagan8, dww: dblog tests should not rely on...

  • catch committed 4fb5cf2 on 9.4.x 
    Issue #3269267 by danflanagan8, dww: dblog tests should not rely on...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

Status: Fixed » Closed (fixed)

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