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][HttpKernel]Always display container deprecations in simple phpunit#29519
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
nicolas-grekas commentedDec 8, 2018
This should be considered as a new feature IMHO. |
478240a to7dbde2bComparel-vo commentedDec 10, 2018
@nicolas-grekas I'm not sure I fully understand. Do you mean that KernelTestCase should catch container compile deprecation and display it (although other deprecations will be displayed by PHPUnit bridge) ? |
nicolas-grekas commentedDec 10, 2018
KernelTestCase could eg open that file and @trigger_error() them - or call the deprecation handler if possible? |
l-vo commentedDec 10, 2018
I can't use trigger_error since I would lose original line number information. But maybe I can deal with registered error handler. I'm going to try that, thank you :) |
l-vo commentedDec 11, 2018
status: needs work |
7dbde2b to09feeaeComparel-vo commentedDec 13, 2018
status: needs work |
4763a3e to9197928Comparef819427 to14db14fComparefabpot commentedMar 31, 2019
@l-vo What's the status of your PR? |
l-vo commentedMar 31, 2019
@fabpot there is more work than I originally thought. I recently didn't have the time to continue this PR but I didn't forget it :) I'm going back to work on it very soon :) |
fabpot commentedMar 31, 2019
@l-vo thanks for the feedback. |
8071a01 tod2749e4Compare268e856 tocadca3aCompare… the container is already built
cadca3a tobf17514Comparel-vo commentedMay 16, 2019
status: needs review |
nicolas-grekas commentedJul 8, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry I didn't look close enough before. There is no reason why the phpunit-bridge should know anything about KernelTestCase. Let's find a proper way that would decouple things. There is already some code infrastructure that deals with serialized deprecations passed via messages. Can't we improve this to achieve what is needed here? |
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.
I think this shouldn't be on phpunit-bridge's side.
Let's have some logic in a base test-case (KernelTestCase?) that triggers those to load the file and trigger the deprecations there.
l-vo commentedAug 3, 2019
@nicolas-grekas I'm not sure to understand. If we put the logic into KernelTestCase (without coupling with phpunit-bridge), we'll lose the logic of theDeprecation class of phpunit-bridge ? |
nicolas-grekas commentedAug 3, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
You mean: being able to differentiate direct vs indirect deprecations, right? But can't we trigger a serialized message asallowed by the bridge to fix that? |
l-vo commentedAug 9, 2019
@nicolas-grekas sorry if I miss something... We can trigger a serialized message, but we must determine in the bridge the message source. It's what is donehere, but it's also what introduces the coupling with KernelTestCase. |
nicolas-grekas commentedFeb 3, 2020
I'm closing because this would apparently add a lot of complexity for a case that happens to be solvable in other ways. There are other strategies to get the deprecations. It would be nice to achieve the goal here, but not at all costs. |
Uh oh!
There was an error while loading.Please reload this page.
When the container is not built or not fresh, simple-phpunit may trigger deprecations which occured during container built. If simple-phpunit is launched again, the deprecations are not displayed since the container hasn't be built this time.
Deprecations can make tests fail. So the behavior is currently inconsistent. With the same code base, the tests may pass if the container has been built before and may fail if the container is built during the process.
The suggested fix is to retrieve container compile deprecations from a file created by Symfony. It's the file which is used to display container compile deprecations in Logger data collector.
Some deprecations are generated during cache warmup. Without cache warmup, these deprecations can be triggered from anywhere. For this reason, without cache warmup, we can't memorize them to display them on all test launches.So to keep test launches consistent, it could be a good practice to warmup the cache before.
Note: since phpunit-bridge create a new phpunit project with a phpunit-bridge dependency, the phpunit-bridge version used for the CI tests hasn't the modification of this PR. So the tests fail. Reporting the
DeprecationErrorHandlermodifications in the created phpunit project allows tests to be successful.Some very minor bugs has been detected after#29211; it could be a good think to fix the bugs treated by#31478 before merging this PR.