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
For example:
- $this->assertTrue(array_search($item, $referenceables[$this->bundle]) !== FALSE);
+ $this->assertContains($item, $referenceables[$this->bundle]);
Proposed resolution
Recommended regex: assert.+array_search
for searching.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#18 | interdiff-8-18.txt | 1.39 KB | jungle |
#18 | 3131474-18.patch | 3.63 KB | jungle |
#8 | 3131474-8.9.x-8.patch | 5.02 KB | jungle |
#8 | raw-interdiff-4-8.txt | 2.07 KB | jungle |
#4 | interdiff-3-4.txt | 515 bytes | jungle |
Comments
Comment #2
jungleComment #3
jungleComment #4
jungleRemoved one unused use statement
Comment #5
mondrakegrep -Er '>assert.*array_search' .
yields two more results,
but those have their own reason.
LGTM
Comment #8
jungle@catch, thanks for committing, a patch for 8.9.x. checked against 8.8.x as well, it applies, and no more assertion to replace, to me.
Comment #10
alexpottWoah the original code is very confusing lol.
These conversions don't look right. I don't think they should be changed.
Comment #11
alexpottI don't think this is worth doing this task on Drupal 8. There's enough other tasks to get in and now 8.9.x is beta officially we don't do tasks on 8.x - of course we can make exceptions for tests but this one does not feel worth it.
Fortunately the patch for D9 didn't have the incorrect conversion I spotted in #10.
Comment #12
xjmI actually would prefer to backport all these changes to 8.9.x once the BC shim is in, for parity between 8.9 and 9.0 and easier backports.
Comment #13
xjmPostponing explicitly on the backport of #3126787: [D8 only] Add forwards-compatibility shim for assertInternalType() replacements in phpunit 6&7.
Comment #14
xjmOh,
assertContains()
doesn't appear to be in the backport set -- probably would need another patch on top of that. Which I think is worth doing. We have to maintain 8.9.x for the next year and a half so let's make it easy.Comment #15
xjmThis (and all the issues like it) would be postponed on a followup from #3126787: [D8 only] Add forwards-compatibility shim for assertInternalType() replacements in phpunit 6&7 that provides the shim.
Comment #16
mondrakeassertContains is a native PHPUnit method since at least version 4, so no need of fc shims here. NW though for #10 for the backport.
Comment #17
xjmlol, thanks @mondrake for sorting that out. :)
Comment #18
jungleAddressing #10.2
Thanks!
Comment #19
jungleRemoving tag as @mondrake said assertContains() is a native PHPUnit method.
Comment #20
mondrakeLGTM now
Comment #23
xjmOK, those remaining few seem fine.
Had to read the original code like three times. This is much better.
Committed the backport to 8.9.x and 8.8.x. Thanks! Sorry for the brief confusion; I was looking at too many different issues at once.