Follow up from #3128746: Replace assertions involving calls to strpos() with more accurate string assertions. There are a number of cases where we are using strstr() or stristr() to assert whether a string contains another string. We should replace these with assertStringContainsString()/assertStringNotContainsString() instead.

Found 8 occurrences:

grep -nri '\->assert.*stri\?str' core
core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php:108:    $this->assertTrue(stristr($top_form_elements[0]->getText(), 'Title field is required.') !== FALSE);
core/modules/image/tests/src/Functional/ImageFieldDisplayTest.php:153:      $this->assertTrue(strstr($this->drupalGetHeader('Cache-Control'), 'private') !== FALSE, 'Cache-Control header was sent.');
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:61:        $this->assertTrue(strstr($style, 'width: 555px;') !== FALSE, 'Dialog width respected.');
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:65:        $this->assertTrue(strstr($style, 'width: 555px;') === FALSE, 'Dialog width reset to default.');
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:80:        $this->assertTrue((bool) strstr($style, 'height: auto;'));
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:83:        $this->assertTrue((bool) strstr($style, 'height: 421px;'));
core/modules/settings_tray/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php:154:    $this->assertTrue(strstr($href, "/admin/structure/block/manage/custom/settings-tray?destination=$destination") !== FALSE);
core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:265:      $this->assertTrue(strstr($this->drupalGetHeader('Cache-Control'), 'private') !== FALSE, 'Cache-Control header was sent.');
CommentFileSizeAuthor
#5 3132778-5.patch839 bytesquietone
#2 3132778-2.patch5.34 KBkim.pepper

Comments

kim.pepper created an issue. See original summary.

kim.pepper’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new5.34 KB

Patch attached.

jungle’s picture

@kim.pepper thank you!

Per @mondrake's comment here this was partially covered by #3131186: Replace assertions involving calls to drupalGetHeader() with session-based assertions, where possible, let' postpone this as well. :p

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

Status: Postponed » Needs review
StatusFileSize
new839 bytes

There is one instance left.

$ grep -nri '\->assert.*stri\?str' core
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:80:        $this->assertTrue((bool) strstr($style, 'height: auto;'));
core/modules/system/tests/src/FunctionalJavascript/OffCanvasTest.php:83:        $this->assertTrue((bool) strstr($style, 'height: 421px;'));
$

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

The changes in the patch look good.
I did not find any more occurrences of "strstr(" or "stristr(" in assertions.
For me it is RTBC.

  • catch committed db54bf0 on 9.3.x
    Issue #3132778 by kim.pepper, quietone, jungle, daffie: Replace usages...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

  • catch committed 4e9fc29 on 9.2.x
    Issue #3132778 by kim.pepper, quietone, jungle, daffie: Replace usages...

Status: Fixed » Closed (fixed)

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