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.The class assumes in many places that the assert methods return values, but they don't, not in phpunit. So, since we've converted that test to phpunit, only the first assertion actually runs in these helper assertions.
I've also removed the @return, but that's not an API change, or to be more exact, that change already happend back in 8.2 or something, when we updated kernel tests, I'm just updating the documentation.
Noticed in #2806009: Installing a module causes translations to be overwritten. With this, I now have two failing test methods in the LocaleConfigSubscriberForeignTest subclass, before only one.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3079330-language-config-subscriber-test-assertions-2.patch | 5.11 KB | Berdir |
Comments
Comment #2
BerdirComment #3
BerdirNote: There are more problematic tests like this one, search for
if \(\$this->assert.+\(to find conditional asserts,return $this->assert.. is also interesting, but mostly in legacy traits. The "assert() && secondAssert" is a bit harder to grep for, as there are a lot of false positives with things like assertTrue(A && B).Comment #5
Ghost of Drupal PastThe patch is fine as is and we would be ahead by it, thanks!
It would be nice to convert to the non legacy asserts while at it but whether this needs to be done in this issue is something I leave to decide to the committers.
Comment #6
alexpottCommitted 54d12c1 and pushed to 9.0.x. Thanks!
As a test only fix I backported to 8.9.x and 8.8.x too.
We can fix up the legacy asserts when we properly deprecate then,