Skip to content

[PhpUnitBridge] Add ExpectUserDeprecationMessageTrait #54593

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

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Apr 13, 2024

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.

@derrabus derrabus requested review from xabbuh and yceruto as code owners April 13, 2024 12:26
@carsonbot carsonbot added this to the 7.1 milestone Apr 13, 2024
@derrabus derrabus force-pushed the feature/expect-deprecation-polyfill branch 3 times, most recently from 5c769cc to 24ff1b6 Compare April 17, 2024 20:41
@derrabus derrabus force-pushed the feature/expect-deprecation-polyfill branch from 24ff1b6 to 66430f3 Compare May 3, 2024 13:06
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@derrabus derrabus force-pushed the feature/expect-deprecation-polyfill branch 5 times, most recently from c5093c3 to 4fd7256 Compare May 21, 2024 16:23
@derrabus derrabus force-pushed the feature/expect-deprecation-polyfill branch from 4fd7256 to 2485e15 Compare July 22, 2024 10:44
@OskarStark OskarStark changed the title [PhpUnitBridge] Add ExpectUserDeprecationMessageTrait [PhpUnitBridge] Add ExpectUserDeprecationMessageTrait Jul 22, 2024
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is surprisingly simple. Well done.

Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Thanks Alexander.

@fabpot
Copy link
Member

fabpot commented Jul 25, 2024

Thank you @derrabus.

@fabpot fabpot merged commit a980a46 into symfony:7.2 Jul 25, 2024
3 of 11 checks passed
@derrabus derrabus deleted the feature/expect-deprecation-polyfill branch July 25, 2024 06:47

final protected function expectUserDeprecationMessage(string $expectedUserDeprecationMessage): void
{
$this->expectDeprecation($expectedUserDeprecationMessage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful. The PHPUnit 11 API performs an exact match of the deprecation message. expectDeprecation uses assertStringMatchesFormat. So this is not a good polyfill (tests that rely on format placeholders would break when using PHPUnit 11 which performs exact match)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to escape those placeholders?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can escape by doubling % (like in sprintf patterns)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I replace all % with %% and I should be good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should, yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fabpot added a commit to twigphp/Twig that referenced this pull request Sep 2, 2024
This PR was merged into the 4.x branch.

Discussion
----------

Switch to expectUserDeprecationMessage()

This PR leverages symfony/symfony#54593. This will make it easier to switch to a vanilla PHPUnit 11 in the future.

Commits
-------

0ab26b0 Switch to expectUserDeprecationMessage()
derrabus added a commit that referenced this pull request Sep 19, 2024
This PR was merged into the 7.2 branch.

Discussion
----------

Switch to ExpectUserDeprecationMessageTrait

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | N/A
| License       | MIT

This PR switches some tests added after #54593 to our new trait, allowing us to run those under PHPUnit 11 as well.

Commits
-------

d923c28 Switch to ExpectUserDeprecationMessageTrait
@fabpot fabpot mentioned this pull request Oct 27, 2024
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.

7 participants