-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PhpUnitBridge] Introduce AssertDeprecationTrait #54538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
86f19c6
to
1a39d70
Compare
1a39d70
to
f834b43
Compare
{ | ||
$matched = false; | ||
$observed = []; | ||
$previousErrorHandler = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this initialization necessary? AFAICS it should work without it.
* @return TReturn | ||
* @template TReturn | ||
*/ | ||
private static function assertDeprecation(string $expectedMessage, callable $callable): mixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if dealing with \Closure
instead would be enough and faster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think narrowing to Closure will give uns any benefit in this particular case.
PHPUnit 11 (not sure about which 10.x version introduced it but our decision was to not care about 10.x anyway as it does not have the full replacement features) also provides an API to expect deprecations. Why would we introduce a new trait to define our own API instead of relying on the core PHPUnit feature ? |
Thanks for pointing that out. You are absolutely right. The idea should be to leverage native features instead of reinventing them. I'll look into that API. If it works for us, I'll look into polyfilling it for PHPUnit 9 so that we can gradually migrate our codebase. |
Closing in favor of #54593. |
…` (derrabus) This PR was merged into the 7.2 branch. Discussion ---------- [PhpUnitBridge] Add `ExpectUserDeprecationMessageTrait` | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Part of #49069, replaces #54538 | License | MIT PHPUnit 11 introduces a method `expectUserDeprecationMessage()` which lets us define which deprecation messages we expect the tested code to raise. This new method can replace our own `expectDeprecation()` method once we upgrade to PHPUnit 11. This PR introduces a `ExpectUserDeprecationMessageTrait` that polyfills this method for older PHPUnit versions. This allowed me to run all tests that I've migrated to `expectUserDeprecationMessage()` with PHPUnit 11. Commits ------- 2485e15 [PhpUnitBridge] Add ExpectUserDeprecationMessageTrait
PHPUnit made some progress with regards to error handling which renders our current way of dealing with deprecations incompatible. I'd like to leverage PHPUnit's error handler instead of having our error handler collect deprecations.
As a first step, I'd like to propose a lightweight replacement for
ExpectDeprecationTrait
. It comed with a methodassertDeprecation()
that takes an expected deprecation message and a callable that is supposed to trigger that deprecation. I'm registering a temporary error handler that filters out the expected deprecation while letting everything else bubble up.An interesting side effect is that tests which only trigger expected deprecations don't have to be added to the
legacy
group anymore. This is especially useful for testing tooling around deprecations in the Config and DependencyInjection components.With this new trait, I was able to execute the affected tests with PHPUnit 11.1 locally.