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
#2 3274066-2.patch10.95 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 3274066-2.patch10.95 KB

As anticipated, node was a little more complicated than most other modules. Some important notes for reviewers:

1. NodeDisplayConfigurableTest still has Classy in a data provider. I added a todo to remove it. The bigger problem with NodeDisplayConfigurableTest is that it has a huge reliance on RDF. I think it probably makes more sense to deal with that as part of #3273976: [Meta] Tasks to remove RDF from core and move to contrib.

2. PagePreviewTest still uses Classy as the base theme. I added a todo with the reason and opened the followup: #3274077: Update PagePreviewTest to use starterkit

3. In FrontPageTest I essential changed a selector to find the views-element-container instead of the views-frontpage class. I don't believe this effects the robustness of the test. This particular coverage was added in #2191011: PHP Warnings in views_preprocess_page() caused by frontpage view. It's a bug about views_preprocess_page, but there's nothing special about it being the front page. So just showing ANY view appears should be just fine, and even that is likely overkill. The test would fail before that assertion if there were a regression for that particular bug.

UPDATE: Re: Item 3: On closer inspection views_preprocess_page doesn't even exist anymore! I think that test method FrontPageTest::testAdminFrontPage is stale code that could be removed entirely.

danflanagan8’s picture

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

tagging

larowlan’s picture

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

Let's put item 3 into a followup to not halt progress here.

This looks good to me

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs followup

Committed and pushed 4fa09fb85c to 10.0.x and 532250841e to 9.4.x. Thanks!

Tagging with needs follow-up for #2.3

  • alexpott committed 4fa09fb on 10.0.x 
    Issue #3274066 by danflanagan8: Node tests should not rely on Classy

  • alexpott committed 5322508 on 9.4.x 
    Issue #3274066 by danflanagan8: Node tests should not rely on Classy

(...
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Issue tags: -Needs followup

Thanks, @alexpott!

I created a followup, so I'm removing the tag. Cheers!

Status: Fixed » Closed (fixed)

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