Skip to content

[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

Closed
wants to merge 1 commit into from

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Apr 9, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues Part of #49069
License MIT

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 method assertDeprecation() 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.

public function testDeprecatedMethod()
{
    $someObject = new SomeClass();

    $returnValue = self::assertDeprecation(
        'Since symfony/some-component 7.1: This is deprecated, do something else instead.',
        static fn () => $someObject->someDeprecatedMethod(47, 11),
    );

    self::assertSame('foo', $returnValue);
}

@derrabus derrabus requested review from xabbuh and yceruto as code owners April 9, 2024 13:42
@carsonbot carsonbot added this to the 7.1 milestone Apr 9, 2024
@derrabus derrabus force-pushed the feature/assert-deprecation branch 3 times, most recently from 86f19c6 to 1a39d70 Compare April 9, 2024 13:52
@derrabus derrabus force-pushed the feature/assert-deprecation branch from 1a39d70 to f834b43 Compare April 9, 2024 13:55
{
$matched = false;
$observed = [];
$previousErrorHandler = null;
Copy link
Member

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
Copy link
Member

@yceruto yceruto Apr 9, 2024

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?

Copy link
Member Author

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.

@stof
Copy link
Member

stof commented Apr 9, 2024

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 ?

@derrabus
Copy link
Member Author

derrabus commented Apr 9, 2024

PHPUnit 11 […] also provides an API to expect deprecations.

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.

@derrabus
Copy link
Member Author

Closing in favor of #54593.

@derrabus derrabus closed this Apr 13, 2024
fabpot added a commit that referenced this pull request Jul 25, 2024
…` (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
@derrabus derrabus deleted the feature/assert-deprecation branch September 20, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants