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 3272581-2.patch19.76 KBdanflanagan8

Comments

danflanagan8 created an issue. See original summary.

danflanagan8’s picture

Assigned: danflanagan8 » Unassigned
Status: Active » Needs review
StatusFileSize
new19.76 KB

Here's a patch. The ones that needed the class->stark change were

Functional/ImageStyles/ConvertTest
Functional/ImageDimensionsTest
Functional/ImageFieldDisplayTest

The rest of the changes are status message assertions I'm trying to refactor as I go through these classy issues. Hopefully they are not too distracting.

It's noteworthy that there is a Functional JS test with the following code:

    $this->assertSession()->waitForElement('css', '.messages--error');

    // Verify that Ajax validation messages are displayed only once.
    $this->assertSession()->elementsCount('xpath', '//div[contains(@class, "messages--error")]', 1);

First, the JsWebAssert status message methods can't count, which is what this assertion is about. Second, the assertion works because the message is added by js rather than by the theme. Probably file.js, but I'm not 100%.

Also worth calling out there's a @todo in one of the updated files that I considered handling here:

    // @todo Uncomment this once
    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
    // $this->assertEquals('<img src="' . $url . '" width="41" height="41" alt="" class="image-style-test" />', $this->getImageTag($variables));

This will need to be modified after it's uncommented in order to remove the class. The problem is there are two more assertions a few lines down that are unrelated to classy/stark but fail when I uncomment them. So it's probably better to make the issue to act on the todo and not worry about it here.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs followup

Thanks, looks good to me.

Tagging for this

So it's probably better to make the issue to act on the todo and not worry about it here

Status: Reviewed & tested by the community » Needs work

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

danflanagan8’s picture

Status: Needs work » Reviewed & tested by the community

failure in Drupal\Tests\ckeditor\FunctionalJavascript\CKEditorIntegrationTest. Resetting status.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs followup

Committed and pushed b2c5ada364 to 10.0.x and 1c89f62b4c to 9.5.x and a07de32c7f to 9.4.x. Thanks!

I don't think this issue needs a follow-up - if anything it is #2670966: Warn users of old PHP versions - but that issue is closed.

  • alexpott committed b2c5ada on 10.0.x
    Issue #3272581 by danflanagan8: Image tests should not rely on Classy
    

  • alexpott committed 1c89f62 on 9.5.x
    Issue #3272581 by danflanagan8: Image tests should not rely on Classy
    
    (...

  • alexpott committed a07de32 on 9.4.x
    Issue #3272581 by danflanagan8: Image tests should not rely on Classy
    
    (...

Status: Fixed » Closed (fixed)

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