Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PhpUnitBridge] testing for deprecations is not risky#21786
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedFeb 27, 2017
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
bb8fd0f to864e459Compare864e459 to044cc8fCompare| publicfunctionstartTest($test) | ||
| { | ||
| if (-2 <$this->state && ($testinstanceof \PHPUnit_Framework_TestCase ||$testinstanceof TestCase)) { | ||
| if (null !==$test->getTestResultObject()) { |
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.
getTestResultObject() can returnnull, for example, when the test case leads to a PHP warning. In this case,$test is an instance ofPHPUnit\Framework\WarningTestCase which does not emit a test result object.
nicolas-grekas left a comment
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.
👍
fabpot commentedFeb 28, 2017
Thank you@xabbuh. |
…(xabbuh)This PR was merged into the 3.3-dev branch.Discussion----------[PhpUnitBridge] testing for deprecations is not risky| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------044cc8f testing for deprecations is not risky
stof commentedFeb 28, 2017
couldn't we ensure that checks for |
nicolas-grekas commentedFeb 28, 2017
Good idea! Is that the case for |
xabbuh commentedFeb 28, 2017
It's the case for |
nicolas-grekas commentedFeb 28, 2017
Trigger a dummy |
stof commentedFeb 28, 2017
better: use |
stof commentedFeb 28, 2017
and this even allows to add the proper number of assertions (one per expected deprecation) |
xabbuh commentedMar 1, 2017
see#21828 |
…tion counter (xabbuh)This PR was merged into the 3.3-dev branch.Discussion----------[PhpUnitBridge] include expected deprecations in assertion counter| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#21786 (comment)| License | MIT| Doc PR |We still need to include the changes from #21786 as we cannot increment the number of assertions in the `startTest()` method (the PHPUnit test runner resets the counter after the listeners have been executed).Commits-------cdcd5ae include expected deprecations in assertion counter
…tion counter (xabbuh)This PR was merged into the 3.3-dev branch.Discussion----------[PhpUnitBridge] include expected deprecations in assertion counter| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21786 (comment)| License | MIT| Doc PR |We still need to include the changes from#21786 as we cannot increment the number of assertions in the `startTest()` method (the PHPUnit test runner resets the counter after the listeners have been executed).Commits-------cdcd5ae include expected deprecations in assertion counter
…nneeded code (fancyweb)This PR was merged into the 3.4 branch.Discussion----------[PhpUnitBridge][SymfonyTestsListenerTrait] Remove some unneeded code| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Risky errors when there are no assertions are added before the test end listeners are called (ie, before the code in endTest is executed) so forcing beStrictAboutTestsThatDoNotTestAnything to false when there is a expectedDeprecation annotation is enough.If the goal is to reset the value to the original value, then I think we should not do it since we basically "lie" to the next listeners. Let's assume that when a test expect a deprecation, it can have 0 assertions. Also this flag is not used anymore by PHPUnit after we reset it.Ref#21786 btwCommits-------fb48bbc [PhpUnitBridge][SymfonyTestsListenerTrait] Remove some unneeded code