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
#2 3268708-2.patch4.62 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 3268708-2.patch4.62 KB

This one was surprisingly tricky considering only one class ended up needing any substantive changes

For one, it asserted the presence of an element in a very strange way:

$this->assertSession()->responseContains('class="field field--name-message field--type-string-long field--label-above');

That escaped my initial search for css and xpath. And obviously that assertion relies strongly on Classy.

The bigger thing though was that I felt compelled to channel @xjm and really make sure that the assertions I updated are truly asserting what they claim. So there's more to the diff than just changing selectors. But I think my changes here are very much in line with @xjm's feedback on the sibling Taxonomy issue.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

I like that the test is easier to read and clarifies exactly what it is looking for now, so +1 to the changes.

  • lauriii committed 801e645 on 10.0.x 
    Issue #3268708 by danflanagan8, longwave: Contact tests should not rely...

  • lauriii committed 2fefddb on 9.4.x 
    Issue #3268708 by danflanagan8, longwave: Contact tests should not rely...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Nice improvements to the assertions. Feels like this should a lot less brittle than the original assertions.

Committed 801e645 and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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