-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PhpUnitBridge] Add the ability to expect a deprecation inside a test #35192
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
[PhpUnitBridge] Add the ability to expect a deprecation inside a test #35192
Conversation
fabbot failure must remain for 5.3 compat. |
3037bdc
to
c84e1c8
Compare
19e3d1f
to
4dd759e
Compare
4dd759e
to
295fdc2
Compare
295fdc2
to
532a477
Compare
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.
LGTM, just minor things.
src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/PhpUnit/Tests/SymfonyExpectDeprecationTraitTest.php
Outdated
Show resolved
Hide resolved
0a59633
to
b953982
Compare
src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
Outdated
Show resolved
Hide resolved
b953982
to
a5cb78a
Compare
a5cb78a
to
a3a9280
Compare
Thank you @fancyweb. |
…n inside a test (fancyweb) This PR was merged into the 5.1-dev branch. Discussion ---------- [PhpUnitBridge] Add the ability to expect a deprecation inside a test | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | todo Replaces #25757 Proposed implementation uses a dedicated trait for a better DX. Using `$this->expectDeprecation()` feels natural. Unfortunately it is not currently possible to test the cases that should produce errors or risky, so there are some things that are not testable here. I plan to propose another feature for the PhpUnitBridge to be able to test those kind of things. If it's accepted, we will then be able to strenghten the tests of this one. Commits ------- a3a9280 [PhpUnitBridge] Add the ability to expect a deprecation inside a test
public function testMany() | ||
{ | ||
$this->expectDeprecation('foo'); | ||
$this->expectDeprecation('bar'); |
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.
To actually test that the feature is working, we would also need to write tests where expectDeprecation
is called but not deprecation is triggered, to show that the test fail in such case.
Of course, this must not be part of the main Symfony testsuite run (here, you are writing tests as if they were testing trigger_error
, not as if they were testing the bridge), but as phpt tests which can assert that running these tests report a failure.
|
good catch. I think this is fine: when ppl use the trait, they want the behavior of the bridge. |
…ion (hkdobrev) This PR was squashed before being merged into the 5.1-dev branch. Discussion ---------- [PhpUnitBridge] Deprecate @expectedDeprecation annotation | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | yes<!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | N/A | License | MIT | Doc PR | N/A Addresses https://github.com/orgs/symfony/projects/1#card-32934769 as a follow-up to #35192. Deprecating `@expectedDeprecation` annotation on tests in favour of the `expectDeprecation()` method similar to other PHPUnit deprecations of annotations in favour of methods. Commits ------- 36a57cc [PhpUnitBridge] Deprecate @expectedDeprecation annotation
Replaces #25757
Proposed implementation uses a dedicated trait for a better DX. Using
$this->expectDeprecation()
feels natural.Unfortunately it is not currently possible to test the cases that should produce errors or risky, so there are some things that are not testable here. I plan to propose another feature for the PhpUnitBridge to be able to test those kind of things. If it's accepted, we will then be able to strenghten the tests of this one.