Collect deprecations triggered by autoloading while loading/building the test suite#6165
Conversation
|
|
||
| EventFacade::instance()->seal(); | ||
|
|
||
| $depreciations = []; |
There was a problem hiding this comment.
The name of this variable should be $deprecations.
| return $buffer; | ||
| } | ||
|
|
||
| public function collectGlobalDepreciations(array $deprecations): void |
There was a problem hiding this comment.
The name of this method should be collectGlobalDeprecations.
|
I think this idea is worthwhile to pursue. Please fix the existing tests and add new tests to cover the changed/added functionality. I do not consider this to be a bug fix, so this pull request should target the |
d02f1ab to
10515dd
Compare
#5844 linkI updated the code, interestingly this conflicts with the resolution of #5844 (https://github.com/sebastianbergmann/phpunit/commit/8e8776083c71277fa90403b183530c956b110b60). To mitigate this I need to use something like:phpunit/src/Framework/TestCase.php Lines 1598 to 1610 in dd3bb4b The idea would be to restore only the handler that we set to avoid restoring another handler registered by third party (bootstrap or test suite configuration). The handler that's created in the |
3a94e60 to
af0dc23
Compare
| @@ -0,0 +1,12 @@ | |||
| <?php declare(strict_types=1); | |||
|
|
|||
| spl_autoload_register(function ($class) { | |||
There was a problem hiding this comment.
This reflects what happens when loading classes usually, if we do the same as in #5844 we'll trigger the deprecation during the loadBootstrapScript.
987f371 to
452fedf
Compare
|
Did you test this with OPcache? I came across this PR on my phone and php/php-src#17422 sounds relevant. |
|
I don't use opcache |
1bb9097 to
5af6639
Compare
|
@sebastianbergmann tests added and code cleaned, let me know if I should do something else. Thanks! |
|
I've added a new commit as I had an issue with the baseline trace, the CI failures looks unrelated though. |
|
The tests for OTR logging use information from the Git checkout and apparently Git checkouts for pull requests are "different". I do not get these failures locally. However, locally I get this (after rebasing your branch against |
|
Please rebase after #6225. Thank you! |
|
rebased, this one works well on our project now. Side note, this patch reports global deprecations only when using In my opinion we should document that global deprecations are reported only when using the autoload mechanism, as that triggers the error only when we use the class, which is what maintainers want to report. |
|
I am sorry, but running PHPUnit's own test suite with the changes proposed here triggers a lot of deprecations: https://github.com/sebastianbergmann/phpunit/actions/runs/15436395576/job/43443720453?pr=6165 |
|
nice catch, indeed I forgot to clean up the ErrorHandler instance in the unit test. |
086064b
into
sebastianbergmann:main
This was forgotten in sebastianbergmann#6165 This makes a difference with today's new "sleep" deprecation, which is not correctly gathered.
This was forgotten in #6165 This makes a difference with today's new "sleep" deprecation, which is not correctly gathered.
|
@soyuka I am trying to investigate a potentially related issue, but I cannot get PHPUnit to autoload classes before the tests actually start. What did you do to get there, what does your PHPUnit configuration look like for this case? |
|
It may be a hard one to debug (see also #6165 (comment)) but please share your use case and I can help (in another issue). |
When one triggers a deprecation in a class directly like this:
This technique is notably used in many Symfony components, for example:
https://github.com/symfony/symfony/blob/82cf90aefb7e9117ebd981fec83d7546e310e52b/src/Symfony/Component/PropertyInfo/Type.php#L12-L15
The deprecation will be triggered when autoloading the class, which happens in:
phpunit/src/TextUI/Application.php
Line 181 in 4b6a4ee
At that point, the ErrorHandler is not registered and we loose the deprecation.
This pull request gives an idea on how it may be fixed, indeed the
ErrorHandlerneeds aTestCasetherefore I'm collecting these deprecations and triggering them once we callErrorHandler::enable. I'm open to suggestions on how to fix this differently. Obviously I'll add tests if/once a solution is found that can be merged.Thanks!