Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The optional $ignoreCase parameter of assertContains() is deprecated and will be removed in PHPUnit 9.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#11 | 3126848-10.patch | 4.62 KB | mondrake |
#9 | 3126848-9.patch | 4.62 KB | mondrake |
Comments
Comment #2
mondrakeAt least in one test, the parameter seems just redundant given the usage of strlower.
Comment #4
mondrakeComment #5
daffie CreditAttribution: daffie commentedAll test code changes look good to me.
By removing the the 2 lines from the ignored warnings that are thrown by PHPUnit 8, we have testing that all cases of the use of the parameter are removed.
For me it is RTBC.
Comment #6
longwaveThese are functionally the same except one is inverted, so I think we should pick one way and use it consistently - using assertStringContainsStringIgnoringCase() in the first seems cleaner as there is an explicit method for it.
Comment #7
mondrake#6 actually there is a subtle difference - we want to check positively there is a 'cookie' substring in the main string, but also that is bound by commas, or first or last part of the string. So we explode by ',' and check there is one token which equals 'cookie'.
On the negative check, that is irrelevant since it's enough no instance of the 'cookie' subtring exists.
Comment #8
alexpottNeeds a reroll... and it's more than just core/tests/Drupal/Tests/Traits/PHPUnit8Warnings.php
Comment #9
mondrakeRerolled. The assertStringContainsString patch missed to figure out these were to be case insensitive. Probably because
the string under test is just emtpy, anyway.
Comment #10
mondrakeComment #11
mondrakeNeeded another reroll.
Comment #12
longwaveRe #7 I agree there is a subtle difference but if the exact comma separated string "cookie" is the thing under test shouldn't it be tested the same in both cases? It is probably irrelevant for the test in question and also perhaps out of scope to change here though!
Comment #13
alexpottCommitted and pushed 125b33ab76 to 9.1.x and c58a76feaa to 9.0.x. Thanks!