`#[IgnoreDeprecations]` attribute to ignore `E_(USER_)DEPRECATED` issues on test class and test method level
OskarStark opened this issue · 9 comments
https://packagist.org/packages/symfony/phpunit-bridge
Today at the SymfonyLive Berlin, @sebastianbergmann, @derrabus, @nicolas-grekas and me talked about the possibility of dropping the need for most features in symfony/phpunit-bridge. Many things are already implemented out of the box in PHPUnit 10.4, while not all, some features like ClockMock and class_exists mocks are still a valid use case for a 3rd party bridge.
Right now one can filter the deprecations and get them as grouped output via restrictDeprecations option 👍
In symfony/phpunit-bridge we use @group legacy annotation. Tests/classes marked with this annotation ignore all raised deprecations to be able to test this code properly without notifying about the deprecations. More infos here...
An idea would be to contribute a new #[IgnoreDeprecations] attribute, to use this as a replacement. Open for better naming ideas 🙏
Any thoughts?
I have this working locally already, just need to implement more end-to-end tests for it. Should be ready for me to push to main next week.
There's one more thing on the topic that's important to the use case : being able to exclude legacy tests.
On the CI of Symfony, we run tests with --exclude-group legacy when testing a component at eg v6 while loading its deps at v7. Legacy tests cannot work by definition in this situation, so we have to skip them, and the fact we're using a group makes it trivial.
I do not want to conflate #[IgnoreDeprecations] with #[Group].
Sure, I'm not saying using a group should be the solution, I'm just sharing the use case so we can think about the best way to make it covered.
I think a test for legacy functionality should have both #[Group('legacy')] and #[IgnoreDeprecations].
Ah, one more thing just to be 100% sure this would work: a test case with #[IgnoreDeprecations] should still be able to assert that deprecations are triggered. It's actually those test cases that test deprecations.
I do not know how you test deprecations, so I cannot comment on that. At this point, I would like to ask you to move Symfony-specific discussions to your issue tracker, if that is okay with you. Testing (Symfony) deprecations is out of scope for PHPUnit, sorry.
What @nicolas-grekas means is not Symfony related, but test framework related.
Let's say we introduce a deprecation because of a new code path, we want to have 2 tests (for simplicity) afterwards. One testing the old code, that it still works as expected, while asserting our new deprecation is thrown in the right place with the expected message. The other (new) test should just do regular assertion.
TLDR
an "expectException()" method. If such a method is not welcome, are we able todo such assertion on our own by leveraging the new event system?
We had something similar in the past, it did not work reliably and caused problems, so we removed it. I am very reluctant to add support for "assert that an E_* was triggered". If you really want to test that then my suggestion would be to disable PHPUnit's error handler for such a test, register your own, collect issues, and verify that the issue you're expecting was triggered.