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
#3126333: Replace usage of the optional $canonicalize parameter of assertEquals(), that is deprecated removes usages of this function from core in D9. It might be useful for contrib projects trying to maintain both 8.x and 9.x compatibility to have the method available in 8.9.x, or even 8.8.x.
This is a D8 only issue.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#15 | 3126695-15.d88.patch | 3.02 KB | jonathan1055 |
Comments
Comment #2
mondrakeHere's a concept. If this works, could also be extended to other FC needs (thinking of assertString(Not)ContainsString, assertIsString, etc)
Comment #3
mondrakeComment #4
mondrakeComment #5
longwaveCan we just call static::assertEquals() here? Both methods are functionally identical in the final release of PHPUnit 7.5, there will never be a deprecation warning, and so using reflection here seems like over engineering.
Reference: https://github.com/sebastianbergmann/phpunit/blob/7.5.20/src/Framework/A...
Comment #6
mondrake#5 I agree, but then we should make clear that we only support PHPUnit 7.5.0 and above in D8, whereas in the composer.json we currently have
"phpunit/phpunit": "^6.5 || ^7"
.And if we do that, then we do not even need to have ANY method in the PHPUnit7 version of the trait.
Comment #7
longwaveWhy does this change the version of PHPUnit?
parent::assertEquals()
will work the same in 7.0.0 up to 7.5.20 unless I am missing something?edit: in both cases I wrote
parent::assertEquals()
but really I should saystatic::assertEquals()
!Comment #8
mondrake@longwave you're right, thanks
from https://www.php.net/manual/en/language.oop5.traits.php :
Comment #9
mondrakeHere we go.
Comment #10
longwaveLooks great, as simple as it can be.
Comment #12
catchCommitted f24bce3 and pushed to 8.9.x. Thanks!
Thinking about an 8.8 backport here too - it is a small API addition, but it's test-only and enables modules to stay compatible with all three branches easier. But since it's a patch release will get a +1 from xjm too.
Comment #13
catchDoesn't apply to 8.8.x though, so could use a reroll.
Comment #14
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedOK I'll do the re-roll
Comment #15
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedRe-rolled patch #9 for core 8.8
Comment #16
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedremoving re-roll tag
Comment #17
longwaveReroll looks good.
Comment #19
catchOriginally tagged this for @xjm to look at too, but since this is useful for 8.8 -> 9.0 compatibility and it's test only, I think it's a straightforward decision to backport it after all.
Committed ebf37dd and pushed to 8.8.x. Thanks!
Comment #21
xjmWe should put a brief note in the 8.9.0 release notes that a forward-compatibility layer for PHPUnit 9 is provided, and mention it in the 8.8.6 release notes since technically backporting this API is outside our allowed changes policy and there's a small risk it might be disruptive with method name collisions.
Comment #22
xjmOops, this issue is missing a CR. We could add the issue to the CR for #3126787: [D8 only] Add forwards-compatibility shim for assertInternalType() replacements in phpunit 6&7 and cover all the BC layer methods in one place.
Comment #23
daffie CreditAttribution: daffie commentedAdded a CR.
The CR is not jet published, therefor putting the status to RTBC.
Comment #24
xjmThanks @daffie -- I guess a separate CR is OK too. Published the CR.
Comment #26
xjm